Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove non-deterministically failing tests from CI. #34

Merged
merged 1 commit into from
Aug 4, 2023

Conversation

vext01
Copy link
Contributor

@vext01 vext01 commented Jul 31, 2023

No description provided.

@ltratt
Copy link
Contributor

ltratt commented Jul 31, 2023

Can we raise an issue pointing out that these tests are problematic?

bors r+

@vext01
Copy link
Contributor Author

vext01 commented Jul 31, 2023

okie dokie: #35

@ltratt
Copy link
Contributor

ltratt commented Aug 3, 2023

Hmm this didn't merge.

bors r+

bors bot added a commit that referenced this pull request Aug 3, 2023
34: Remove non-deterministically failing tests from CI. r=ltratt a=vext01



Co-authored-by: Edd Barrett <[email protected]>
@bors
Copy link
Contributor

bors bot commented Aug 3, 2023

Build failed:

@ltratt
Copy link
Contributor

ltratt commented Aug 3, 2023

bors r+

bors bot added a commit that referenced this pull request Aug 3, 2023
34: Remove non-deterministically failing tests from CI. r=ltratt a=vext01



Co-authored-by: Edd Barrett <[email protected]>
@bors
Copy link
Contributor

bors bot commented Aug 3, 2023

Build failed:

@vext01
Copy link
Contributor Author

vext01 commented Aug 3, 2023

We should (tm) have more luck with this after the "pipeline revert" PR is merged.

@vext01
Copy link
Contributor Author

vext01 commented Aug 3, 2023

It just merged after I sent my last comment :)

@ltratt
Copy link
Contributor

ltratt commented Aug 3, 2023

bors r+

bors bot added a commit that referenced this pull request Aug 3, 2023
34: Remove non-deterministically failing tests from CI. r=ltratt a=vext01



Co-authored-by: Edd Barrett <[email protected]>
@bors
Copy link
Contributor

bors bot commented Aug 3, 2023

Build failed:

@vext01
Copy link
Contributor Author

vext01 commented Aug 3, 2023

I think this PR will need more investigation...

@vext01
Copy link
Contributor Author

vext01 commented Aug 3, 2023

bors try

bors bot added a commit that referenced this pull request Aug 3, 2023
@bors
Copy link
Contributor

bors bot commented Aug 3, 2023

try

Build succeeded!

The publicly hosted instance of bors-ng is deprecated and will go away soon.

If you want to self-host your own instance, instructions are here.
For more help, visit the forum.

If you want to switch to GitHub's built-in merge queue, visit their help page.

@vext01
Copy link
Contributor Author

vext01 commented Aug 3, 2023

There's our problem. The files.lua test always crashes if YKD_SERIALISE_COMPILATION=0.

(This was getting in the way of what I'm working on too)

7b48e1c disables that test for now.

OK to squash?

@ltratt
Copy link
Contributor

ltratt commented Aug 3, 2023

Please squash.

@vext01
Copy link
Contributor Author

vext01 commented Aug 3, 2023

splat.

@ltratt
Copy link
Contributor

ltratt commented Aug 4, 2023

bors r+

@bors
Copy link
Contributor

bors bot commented Aug 4, 2023

Build succeeded!

The publicly hosted instance of bors-ng is deprecated and will go away soon.

If you want to self-host your own instance, instructions are here.
For more help, visit the forum.

If you want to switch to GitHub's built-in merge queue, visit their help page.

@bors bors bot merged commit c376416 into ykjit:main Aug 4, 2023
bors bot added a commit that referenced this pull request Aug 25, 2023
64: Track initialised yk locations r=ltratt a=Pavel-Durov

Partially fixes (only for serial compilation)  - #62

# Changes

- Remove reallocarray in favour of calloc
- Change YkLocations to be stored as pointers
- Change lyk interface to reflect its "hooks" functionality
- Move tests to a separate test script and enable serialised all.lua test suite
- Added logging in lyk module for ease of debugging
- Updated readme

`test.sh` was tested for resiliency multiple times:
```shell
$ try_repeat -v 100 sh ./test.sh
``` 
# Issues

## Uninitialised locations memory 
`reallocarray` does not initialise memory with default zero bytes locations as `calloc`. 
Moving to `calloc` and using pointers allowed to check for NULL with default values set at initialisation time.

## Tests

When lua tests are executed one by one as single files it produces different results from when it's executed via `all.lua` test suite.

Example:

```shell

$ YKD_SERIALISE_COMPILATION=1 ../src/lua -e"_U=true" ./main.lua 
...
../src/lua: ./main.lua:343: assertion failed!
stack traceback:
        [C]: in function 'assert'
        ./main.lua:343: in main chunk
        [C]: in ?
```
When run through `all.lua` it passes:
```shell
$ YKD_SERIALISE_COMPILATION=1 ../src/lua -e"_U=true" ./all.lua 
...
***** FILE 'main.lua'*****
...
***** FILE 'gc.lua'*****
...
```
Since `all.lua` is what we base the stability of yklua, I think we should include it in the tests as is, currently only serialised compilation works.

Running `all.lua` prior to these changes results in error (`main/56c5787799b876f36babbae24e9afc025806b831`):
```
$ YKD_SERIALISE_COMPILATION=1 gdb -ex 'r' -ex 'bt' --args ../src/lua -e"_U=true" ./all.lua 
...
***** FILE 'main.lua'*****

Program received signal SIGSEGV, Segmentation fault.
core::sync::atomic::AtomicUsize::fetch_sub (self=<optimised out>) at /rustc/ef85656a10657ba5e4f7fe2931a4ca6293138d51/library/core/src/sync/atomic.rs:2575
2575    /rustc/ef85656a10657ba5e4f7fe2931a4ca6293138d51/library/core/src/sync/atomic.rs: No such file or directory.
#0  core::sync::atomic::AtomicUsize::fetch_sub (self=<optimised out>) at /rustc/ef85656a10657ba5e4f7fe2931a4ca6293138d51/library/core/src/sync/atomic.rs:2575
#1  alloc::sync::{impl#33}::drop<lock_api::mutex::Mutex<parking_lot::raw_mutex::RawMutex, ykrt::location::HotLocation>, alloc::alloc::Global> (self=0x7fffffffb410)
    at /rustc/ef85656a10657ba5e4f7fe2931a4ca6293138d51/library/alloc/src/sync.rs:2370
#2  0x00007ffff7b0d4ab in core::ptr::drop_in_place<alloc::sync::Arc<lock_api::mutex::Mutex<parking_lot::raw_mutex::RawMutex, ykrt::location::HotLocation>, alloc::alloc::Global>> () at /rustc/ef85656a10657ba5e4f7fe2931a4ca6293138d51/library/core/src/ptr/mod.rs:497
#3  0x00007ffff7b0067e in core::mem::drop<alloc::sync::Arc<lock_api::mutex::Mutex<parking_lot::raw_mutex::RawMutex, ykrt::location::HotLocation>, alloc::alloc::Global>> (_x=...) at /rustc/ef85656a10657ba5e4f7fe2931a4ca6293138d51/library/core/src/mem/mod.rs:987
#4  0x00007ffff7b121f0 in ykrt::location::{impl#1}::drop (self=0x7fffffffb468) at ykrt/src/location.rs:197
#5  0x00007ffff7affa9b in core::ptr::drop_in_place<ykrt::location::Location> () at /rustc/ef85656a10657ba5e4f7fe2931a4ca6293138d51/library/core/src/ptr/mod.rs:497
#6  0x00007ffff7aff6bd in core::mem::drop<ykrt::location::Location> (_x=...) at /rustc/ef85656a10657ba5e4f7fe2931a4ca6293138d51/library/core/src/mem/mod.rs:987
#7  0x00007ffff7b004bd in ykcapi::yk_location_drop (loc=...) at ykcapi/src/lib.rs:90
#8  0x000000000088aa0e in free_loc (f=<optimised out>, i=<optimised out>, idx=<optimised out>) at lyk.c:66
#9  0x000000000088aba5 in yk_free_locactions (f=0x928cc0) at lyk.c:76
--Type <RET> for more, q to quit, c to continue without paging--
#10 0x0000000000807738 in luaF_freeproto (L=0x916e38, f=0x928cc0) at lfunc.c:276
#11 0x0000000000809fde in freeobj (L=0x916e38, o=0x928cc0) at lgc.c:767
#12 0x0000000000817145 in sweepgen (L=0x916e38, g=<optimised out>, p=<optimised out>, limit=<optimised out>, pfirstold1=<optimised out>) at lgc.c:1106
#13 0x0000000000816713 in youngcollection (L=0x916e38, g=0x916f00) at lgc.c:1239
#14 0x000000000081557d in genstep (L=0x916e38, g=0x916f00) at lgc.c:1434
#15 0x0000000000815044 in luaC_step (L=0x916e38) at lgc.c:1686
#16 0x00000000007e4447 in lua_pushstring (L=0x916e38, s=<optimised out>) at lapi.c:553
#17 0x000000000089e60c in findloader (L=0x916e38, name=0x928df8 "tracegc") at loadlib.c:641
#18 0x000000000089de0a in ll_require (L=0x916e38) at loadlib.c:666
#19 0x00000000007fda75 in precallC (L=0x916e38, func=<optimised out>, nresults=<optimised out>, f=0x89db80 <ll_require>) at ldo.c:506
#20 0x00000000007fe016 in luaD_precall (L=0x916e38, func=0x923780, nresults=1) at ldo.c:569
#21 0x0000000000883f68 in luaV_execute (L=0x916e38, ci=<optimised out>) at lvm.c:1655
#22 0x00000000007feb3b in ccall (L=0x916e38, func=<optimised out>, nResults=<optimised out>, inc=<optimised out>) at ldo.c:609
#23 0x00000000007fec61 in luaD_callnoyield (L=0x916e38, func=0x917730, nResults=-1) at ldo.c:627
#24 0x00000000007eaf03 in f_call (L=0x916e38, ud=<optimised out>) at lapi.c:1041
#25 0x00000000007f8be7 in luaD_rawrunprotected (L=0x916e38, f=0x7eae40 <f_call>, ud=0x7ffff2487308) at ldo.c:144
#26 0x0000000000801406 in luaD_pcall (L=0x916e38, func=0x7eae40 <f_call>, u=0x7ffff2487308, old_top=<optimised out>, ef=<optimised out>) at ldo.c:926
#27 0x00000000007ea9ec in lua_pcallk (L=0x916e38, nargs=<optimised out>, nresults=<optimised out>, errfunc=<optimised out>, ctx=<optimised out>, k=<optimised out>)
    at lapi.c:1067
#28 0x00000000007dc623 in docall (L=0x916e38, narg=0, nres=-1) at lua.c:160
#29 0x00000000007dbd24 in handle_script (L=0x916e38, argv=<optimised out>) at lua.c:255
#30 0x00000000007d9fe3 in pmain (L=0x916e38) at lua.c:634
#31 0x00000000007fda75 in precallC (L=0x916e38, func=<optimised out>, nresults=<optimised out>, f=0x7d97f0 <pmain>) at ldo.c:506
#32 0x00000000007fe0c8 in luaD_precall (L=0x916e38, func=0x9176f0, nresults=1) at ldo.c:572
#33 0x00000000007fea7f in ccall (L=0x916e38, func=0x9176f0, nResults=1, inc=<optimised out>) at ldo.c:607
#34 0x00000000007fec61 in luaD_callnoyield (L=0x916e38, func=0x9176f0, nResults=1) at ldo.c:627
#35 0x00000000007eaf03 in f_call (L=0x916e38, ud=<optimised out>) at lapi.c:1041
#36 0x00000000007f8be7 in luaD_rawrunprotected (L=0x916e38, f=0x7eae40 <f_call>, ud=0x7ffff2487058) at ldo.c:144
#37 0x0000000000801406 in luaD_pcall (L=0x916e38, func=0x7eae40 <f_call>, u=0x7ffff2487058, old_top=<optimised out>, ef=<optimised out>) at ldo.c:926
#38 0x00000000007ea9ec in lua_pcallk (L=0x916e38, nargs=<optimised out>, nresults=<optimised out>, errfunc=<optimised out>, ctx=<optimised out>, k=<optimised out>)
    at lapi.c:1067
#39 0x00000000007d94b0 in main (argc=<optimised out>, argv=<optimised out>) at lua.c:660
```

That's cause we're freeing uninitialised yk locations.


Co-authored-by: Pavel Durov <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants