Skip to content

Fix Julia 1.13 segfault on undef/partially-initialized struct return (#3087)#3264

Open
vchuravy wants to merge 1 commit into
mainfrom
fix-3087-undef-sret-1.13
Open

Fix Julia 1.13 segfault on undef/partially-initialized struct return (#3087)#3264
vchuravy wants to merge 1 commit into
mainfrom
fix-3087-undef-sret-1.13

Conversation

@vchuravy

Copy link
Copy Markdown
Member

Fixes #3087.

Symptom

On Julia 1.13, reverse-mode autodiff over a function that calls an inner @noinline routine returning a small mixed tuple via sret (e.g. Tuple{Float64,Int64}) segfaults at runtime. This is the Undef sret testset in test/core/abi.jl (works on 1.12 and earlier).

Root cause

The runtime trace bottoms out in an augmented_julia_* function whose entire body is unreachable — a miscompilation, traced as follows:

  1. On 1.13 the core emits the augmented-forward pass of the inner function with the active scalar returned directly and the primal struct + its shadow delivered through enzyme_sret output pointers (on ≤1.12 it was a single combined sret struct that the caller reads). The caller never reads the primal cache and passes undef for the shadow sret.
  2. propagate_returned!'s interprocedural constant propagation treats "every caller passes undef" as license to replace the argument with undef inside the body. For an sret-like output pointer the body stores into the argument, so this produced store i64 2, ptr undef.
  3. Downstream InstCombine/SROA treat that store-to-undef as UB and collapse the whole augmented function to ret poison, then strip every argument → define ... @aug() with body unreachable.
  4. At runtime the caller invokes the now-empty function, uses the undefined result, and falls through to garbage → SIGSEGV.

Fix (src/llvm/transforms.jl)

  1. Guard the const-prop in propagate_returned! so it never replaces an sret / enzyme_sret / enzyme_sret_v output pointer with undef when there is no concrete value to propagate (those arguments are stored into; undef-propagation creates store-to-undef UB).
  2. Fix a latent typo in delete_writes_into_removed_args: erasing a store into a removed argument referenced the undefined nphi instead of cur.

No C++ core change required.

Verification

  • Undef sret regression test (test/core/abi.jl) now passes on 1.13; gradient matches 1.12 exactly and is finite-difference verified (dx[1] = -1.60899866723324).
  • core/abi passes identically on 1.12 and 1.13 (144 pass / 1 broken).
  • callconv, rooted_args (sret/ABI-heavy) and basic (295/295) pass on 1.13.

🤖 Generated with Claude Code

…3087)

On Julia 1.13 the augmented-forward pass of an inner function returning a
small mixed tuple (e.g. `Tuple{Float64,Int64}` -> sret `{double,i64}`) is
emitted by the core with the active scalar returned directly and the primal
struct plus its shadow delivered through `enzyme_sret` output pointers. The
caller never reads the primal cache and passes `undef` for the shadow sret.

`propagate_returned!`'s interprocedural constant propagation treated "every
caller passes undef" as license to replace the argument with `undef` inside
the body. For an sret-like *output* pointer the body stores into the argument,
so this produced `store i64 2, ptr undef`. Downstream InstCombine/SROA then
treat that store-to-undef as UB and collapse the whole augmented function to
`ret poison` / `unreachable` and strip every argument. At runtime the caller
invokes the now-empty function, uses the undefined result, and falls through
to garbage, segfaulting.

Guard the constant-propagation so it never replaces an
`sret`/`enzyme_sret`/`enzyme_sret_v` output pointer with `undef` when there is
no concrete value to propagate. Also fix a latent typo in
`delete_writes_into_removed_args` where erasing a store into a removed argument
referenced the undefined `nphi` instead of `cur`.

The `Undef sret` regression test in `test/core/abi.jl` now passes on 1.13;
results match 1.12 (finite-difference verified).

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@github-actions

Copy link
Copy Markdown
Contributor

Your PR requires formatting changes to meet the project's style guidelines.
Please consider running Runic (git runic main) to apply these changes.

Click here to view the suggested changes.
diff --git a/src/llvm/transforms.jl b/src/llvm/transforms.jl
index b6d977e4..795aaba3 100644
--- a/src/llvm/transforms.jl
+++ b/src/llvm/transforms.jl
@@ -1901,11 +1901,13 @@ function remove_readonly_unused_calls!(fn::LLVM.Function, next::Set{String})
 end
 
 function propagate_returned!(mod::LLVM.Module)
-    sretkind = LLVM.kind(if LLVM.version().major >= 12
-        LLVM.TypeAttribute("sret", LLVM.Int32Type())
-    else
-        LLVM.EnumAttribute("sret")
-    end)
+    sretkind = LLVM.kind(
+        if LLVM.version().major >= 12
+            LLVM.TypeAttribute("sret", LLVM.Int32Type())
+        else
+            LLVM.EnumAttribute("sret")
+        end
+    )
     globs = LLVM.GlobalVariable[]
     for g in globals(mod)
         if linkage(g) == LLVM.API.LLVMInternalLinkage ||
@@ -2138,24 +2140,24 @@ function propagate_returned!(mod::LLVM.Module)
                         illegalUse = true
                         break
                     end
-                    # If every caller passes undef/poison for this argument (val === nothing),
-                    # the interprocedural const-prop below would replace the argument with undef
-                    # inside the body. That is only sound for arguments used purely as values.
-                    # For an sret-like output pointer (`sret`/`enzyme_sret`/`enzyme_sret_v`) the
-                    # body *stores into* the argument, so replacing it with undef turns those
-                    # stores into stores-to-undef. Downstream InstCombine/SROA then treat that as
-                    # UB and collapse the whole function to `unreachable`/`poison`, miscompiling
-                    # the augmented forward pass (segfault, issue #3087). Skip the replacement for
-                    # such write-only output pointers when there is no concrete value to propagate.
-                    if !illegalUse && val === nothing && any(
-                        (
-                            kind(attr) == sretkind ||
-                            kind(attr) == kind(StringAttribute("enzyme_sret")) ||
-                            kind(attr) == kind(StringAttribute("enzyme_sret_v"))
-                        ) for attr in collect(parameter_attributes(fn, i))
-                    )
-                        illegalUse = true
-                    end
+                        # If every caller passes undef/poison for this argument (val === nothing),
+                        # the interprocedural const-prop below would replace the argument with undef
+                        # inside the body. That is only sound for arguments used purely as values.
+                        # For an sret-like output pointer (`sret`/`enzyme_sret`/`enzyme_sret_v`) the
+                        # body *stores into* the argument, so replacing it with undef turns those
+                        # stores into stores-to-undef. Downstream InstCombine/SROA then treat that as
+                        # UB and collapse the whole function to `unreachable`/`poison`, miscompiling
+                        # the augmented forward pass (segfault, issue #3087). Skip the replacement for
+                        # such write-only output pointers when there is no concrete value to propagate.
+                        if !illegalUse && val === nothing && any(
+                                (
+                                        kind(attr) == sretkind ||
+                                        kind(attr) == kind(StringAttribute("enzyme_sret")) ||
+                                        kind(attr) == kind(StringAttribute("enzyme_sret_v"))
+                                    ) for attr in collect(parameter_attributes(fn, i))
+                            )
+                            illegalUse = true
+                        end
                     if !illegalUse
                         if val === nothing
                             val = LLVM.UndefValue(value_type(arg))

@vchuravy vchuravy requested a review from wsmoses June 29, 2026 18:07
@vchuravy

Copy link
Copy Markdown
Member Author

@wsmoses speculative fix for #3087 letting Claude find a MWE

Comment thread src/llvm/transforms.jl
cur, cval = pop!(todorep)
if isa(cur, LLVM.StoreInst)
if operands(cur)[2] == cval
LLVM.API.LLVMInstructionEraseFromParent(nphi)

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is no nphi defined so that looks real

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah so this part should be extracted to a separate pr and is useful, other parts more unclear

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.

Julia 1.13: Segfault in undefsret — undef/partially-initialized struct return ABI

2 participants