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

segfault due to invalid CodeInstance from OpaqueClosure optimization #55035

Open
topolarity opened this issue Jul 5, 2024 · 0 comments
Open
Labels
bug Indicates an unexpected problem or unintended behavior

Comments

@topolarity
Copy link
Member

It looks like it's possible to generate an invalid specptr for an OpaqueClosure, so that Julia segfaults when attempting to call it:

julia> foo(x, ::Val{B}, ::Val{RT}) where {B, RT} = begin
          x = B ? x : Base.compilerbarrier(:type, x)
          return Base.Experimental.@opaque _->RT (a)->x[]
       end
foo (generic function with 1 method)

julia> const oc1 = foo(Ref{Any}(Int32(2)), Val(true), Val(Int32))
julia> const oc2 = foo(Ref{Any}(Int32(2)), Val(false), Val(Any))

julia> bar(f) = f(rand(Int32))
julia> bar(oc2)

Running this gives a segfault:

[1319401] signal 11 (1): Segmentation fault
in expression starting at REPL[5]:1
typekeyvalue_hash at /home/topolarity/repos/dae_julia/src/jltypes.c:1662 [inlined]
lookup_typevalue at /home/topolarity/repos/dae_julia/src/jltypes.c:1088
jl_inst_arg_tuple_type at /home/topolarity/repos/dae_julia/src/jltypes.c:2319
arg_type_tuple at /home/topolarity/repos/dae_julia/src/gf.c:2372 [inlined]
jl_lookup_generic_ at /home/topolarity/repos/dae_julia/src/gf.c:3247 [inlined]
ijl_apply_generic at /home/topolarity/repos/dae_julia/src/gf.c:3294
bar at ./REPL[4]:4
unknown function (ip: 0x7f83ad4b1e32)
_jl_invoke at /home/topolarity/repos/dae_julia/src/gf.c:3121 [inlined]
ijl_apply_generic at /home/topolarity/repos/dae_julia/src/gf.c:3298
jl_apply at /home/topolarity/repos/dae_julia/src/julia.h:2185 [inlined]
do_call at /home/topolarity/repos/dae_julia/src/interpreter.c:127
...

I'm fairly certain the problem here is that this compile-time OpaqueClosure optimization applies when generating oc1, which causes us to store a specptr with the Int32 ABI (as enforced in the requested return type bounds) into a code instance with rettype == Any.

From that point on, we have an invalid CodeInstance in the cache.

When oc2 is constructed, we end up looking up the same CodeInstance in the cache, which claims to have a rettype of ::Any even though its specptr really has a rettype of ::Int32

@topolarity topolarity added the bug Indicates an unexpected problem or unintended behavior label Jul 5, 2024
vtjnash added a commit that referenced this issue Oct 15, 2024
The invalid way of doing this became much harder to express, which
exposes a lot of bugs (hits more assertion errors and causes more crashes).

Mostly fixes #55035, since this bug is just that much harder to express
in the more constrained API.
vtjnash added a commit that referenced this issue Oct 15, 2024
The invalid way of doing this became much harder to express, which
exposes a lot of bugs (hits more assertion errors and causes more crashes).

Mostly fixes #55035, since this bug is just that much harder to express
in the more constrained API.
vtjnash added a commit that referenced this issue Oct 16, 2024
The invalid way of doing this became much harder to express, which
exposes a lot of bugs (hits more assertion errors and causes more crashes).

Mostly fixes #55035, since this bug is just that much harder to express
in the more constrained API.
vtjnash added a commit that referenced this issue Oct 18, 2024
The invalid way of doing this became much harder to express, which
exposes a lot of bugs (hits more assertion errors and causes more crashes).

Mostly fixes #55035, since this bug is just that much harder to express
in the more constrained API.
vtjnash added a commit that referenced this issue Oct 19, 2024
…ncy edge tracking (#56179)

Disjoint content can be LLVM optimized in parallel now, since codegen
no longer has any ability to handle recursion, and compilation should
even be able to run in parallel with the GC also. Removes any remaining
global state, since that is unsafe.

Adds a C++ shim for concurrent gc support in conjunction with
using a `std::unique_lock` to DRY code.

Fix RuntimeDyld implementation:
Since we use the ForwardingMemoryManger instead of making a new
RTDyldMemoryManager object every time, we need to reference count the
finalizeMemory calls so that we only call that at the end of relocating
everything when everything is ready. We already happen to conveniently
have a shared_ptr here, so just use that instead of inventing a
duplicate counter.

Fixes many OC bugs, including mostly fixing #55035, since this bug is
just that much harder to express in the more constrained API.
KristofferC pushed a commit that referenced this issue Oct 21, 2024
…ncy edge tracking (#56179)

Disjoint content can be LLVM optimized in parallel now, since codegen
no longer has any ability to handle recursion, and compilation should
even be able to run in parallel with the GC also. Removes any remaining
global state, since that is unsafe.

Adds a C++ shim for concurrent gc support in conjunction with
using a `std::unique_lock` to DRY code.

Fix RuntimeDyld implementation:
Since we use the ForwardingMemoryManger instead of making a new
RTDyldMemoryManager object every time, we need to reference count the
finalizeMemory calls so that we only call that at the end of relocating
everything when everything is ready. We already happen to conveniently
have a shared_ptr here, so just use that instead of inventing a
duplicate counter.

Fixes many OC bugs, including mostly fixing #55035, since this bug is
just that much harder to express in the more constrained API.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Indicates an unexpected problem or unintended behavior
Projects
None yet
Development

No branches or pull requests

1 participant