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

Assertion on upcoming 1.6 when running tests #560

Closed
KristofferC opened this issue Nov 17, 2020 · 10 comments
Closed

Assertion on upcoming 1.6 when running tests #560

KristofferC opened this issue Nov 17, 2020 · 10 comments

Comments

@KristofferC
Copy link

This package asserts on the upcoming 1.6 (https://s3.amazonaws.com/julialang-reports/nanosoldier/pkgeval/by_hash/c3bb6df_vs_599ecd8/GAP.1.6.0-DEV-fe2d71bc7f.build-3d16cc1ee71b7eb9.log)

julia: /workspace/srcdir/src/gc.c:2665: gc_mark_loop: Assertion `layout->nfields > 0 && layout->fielddesc_type != 3 && "opaque types should have been handled specially"' failed.

signal (6): Aborted
in expression starting at /home/pkgeval/.julia/packages/GAP/7w7Vl/test/runtests.jl:1
gsignal at /lib/x86_64-linux-gnu/libc.so.6 (unknown line)
abort at /lib/x86_64-linux-gnu/libc.so.6 (unknown line)
unknown function (ip: 0x7fd17ccef728)
__assert_fail at /lib/x86_64-linux-gnu/libc.so.6 (unknown line)
gc_mark_loop at /workspace/srcdir/src/gc.c:2665
_jl_gc_collect at /workspace/srcdir/src/gc.c:2973
jl_gc_collect at /workspace/srcdir/src/gc.c:3179
maybe_collect at /workspace/srcdir/src/gc.c:828 [inlined]
jl_gc_pool_alloc at /workspace/srcdir/src/gc.c:1143
jl_gc_alloc_ at /workspace/srcdir/src/julia_internal.h:276 [inlined]
jl_gc_alloc at /workspace/srcdir/src/gc.c:3221
NewBag at /home/pkgeval/.julia/artifacts/848f0f318405266896d24d11e4baf5cf7bf3f203/gap-4.11.0/src/julia_gc.c:887
NEW_PLIST at /home/pkgeval/.julia/artifacts/848f0f318405266896d24d11e4baf5cf7bf3f203/gap-4.11.0/src/plist.h:44 [inlined]
NewFunctionT at /home/pkgeval/.julia/artifacts/848f0f318405266896d24d11e4baf5cf7bf3f203/gap-4.11.0/src/calls.c:895
MakeTester at /home/pkgeval/.julia/artifacts/848f0f318405266896d24d11e4baf5cf7bf3f203/gap-4.11.0/src/opers.c:2579
NewAttribute at /home/pkgeval/.julia/artifacts/848f0f318405266896d24d11e4baf5cf7bf3f203/gap-4.11.0/src/opers.c:2623
CALL_1ARGS at /home/pkgeval/.julia/artifacts/848f0f318405266896d24d11e4baf5cf7bf3f203/gap-4.11.0/src/calls.h:320 [inlined]
EvalOrExecCall at /home/pkgeval/.julia/artifacts/848f0f318405266896d24d11e4baf5cf7bf3f203/gap-4.11.0/src/funcs.c:164 [inlined]

I don't know if it is the fault of the package or something regressed in Julia.

@fingolfin
Copy link
Member

Probably caused by our use of external types and the GC extension API, though I am worried that the gcext tests in Julia didn't catch it? Also out tests here on Github Actions pass on Julia nightly (or at least did the last time they run; perhaps it is a recent regression).

Anyway, I'll look into it (and the relevant Julia kernel sources and git history) when I am again at a computer.

Perhaps @rbehrends has an idea what might be going on.

@KristofferC
Copy link
Author

Also out tests here on Github Actions pass on Julia nightly (or at least did the last time they run; perhaps it is a recent regression).

These likely don't run with Julia assertions enabled.

@rbehrends
Copy link
Contributor

I'll have to give this a look. However, I'll note that what looks like a GC issue often is some form of memory corruption that is simply detected because the GC traverses the entire object graph regularly. The mark loop will generally crash if the header word of an object has been somehow corrupted, which can easily happen through e.g. a buffer overflow or a bad index. It can still be a GC issue, but it should not automatically be assumed that this is the case.

@rbehrends
Copy link
Contributor

rbehrends commented Nov 17, 2020

Looking at it more closely, the assertion appears to be wrong. I haven't yet figured out the intent of the assertion, but checking that layout->fielddesc_type != 3 just before there's an if chain that specifically is designed to handle layout->fielddesc_type == 3 cannot be correct. I wonder if the condition was supposed to be something like:

assert((layout->nfields > 0 || layout->fielddesc_type == 3) && "opaque types should have been handled specially");

The code is from this commit.

The assertion itself is specifically triggered by a GAP type, which has nfields == 0 and fielddesc_type == 3.

@KristofferC
Copy link
Author

Perhaps you could open an issue in the julia repo with what you described here?

@rbehrends
Copy link
Contributor

Opening an issue is the plan, but I'm first trying to work out what the overall intent of the commit is. If I can't, I'll just write up what I have, but the more concrete info and context I can provide, the more likely it's going to be dealt with quickly.

@fingolfin
Copy link
Member

The commit comes from PR JuliaLang/julia#33724 by @vtjnash ; I think just asking him what the rationale behind the assertion was might be faster and more accurate? Actually, the relevant commit in that PR was authored by @carnaval it seems, but I am not sure where that came from.

@rbehrends
Copy link
Contributor

rbehrends commented Nov 18, 2020

I have opened an issue for the problem.

@fingolfin
Copy link
Member

@KristofferC there is a fix now at JuliaLang/julia#38510

@fingolfin
Copy link
Member

The fix was merged to Julia.

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

No branches or pull requests

3 participants