From 75dba048f49ddb4281c625ed4d12ffbab3e3c426 Mon Sep 17 00:00:00 2001 From: Jameson Nash Date: Thu, 13 Feb 2025 23:02:09 -0500 Subject: [PATCH] [LateLowerGCFrame] fix PlaceGCFrameReset for returns_twice (#57392) Using the right variable here should help quite a bit with the random GC segfaults we have seen. We already have the tests for this, but it is quite hard to make them just complex enough to trigger reliably. Fixes #57333 --- src/codegen.cpp | 5 ++--- src/llvm-late-gc-lowering.cpp | 2 +- 2 files changed, 3 insertions(+), 4 deletions(-) diff --git a/src/codegen.cpp b/src/codegen.cpp index 26f34d63e578f..b99a9eff473fd 100644 --- a/src/codegen.cpp +++ b/src/codegen.cpp @@ -3232,7 +3232,6 @@ static bool slot_eq(jl_value_t *e, int sl) // --- find volatile variables --- // assigned in a try block and used outside that try block - static bool local_var_occurs(jl_value_t *e, int sl) { if (slot_eq(e, sl)) { @@ -3272,13 +3271,13 @@ static bool have_try_block(jl_array_t *stmts) return 0; } -// conservative marking of all variables potentially used after a catch block that were assigned before it +// conservative marking of all variables potentially used after a catch block that were assigned after the try static void mark_volatile_vars(jl_array_t *stmts, SmallVectorImpl &slots, const std::set &bbstarts) { if (!have_try_block(stmts)) return; size_t slength = jl_array_dim0(stmts); - BitVector assigned_in_block(slots.size()); // conservatively only ignore slots assigned in the same basic block + BitVector assigned_in_block(slots.size()); // since we don't have domtree access, conservatively only ignore slots assigned in the same basic block for (int j = 0; j < (int)slength; j++) { if (bbstarts.count(j + 1)) assigned_in_block.reset(); diff --git a/src/llvm-late-gc-lowering.cpp b/src/llvm-late-gc-lowering.cpp index 7d6fba65a79e7..2b9b918c4bd53 100644 --- a/src/llvm-late-gc-lowering.cpp +++ b/src/llvm-late-gc-lowering.cpp @@ -2291,7 +2291,7 @@ void LateLowerGCFrame::PlaceGCFrameStores(State &S, unsigned MinColorRoot, const LargeSparseBitVector &NowLive = S.LiveSets[*rit]; // reset slots which are no longer alive for (int Idx : *LastLive) { - if (Idx >= PreAssignedColors && !HasBitSet(NowLive, Idx)) { + if (Colors[Idx] >= PreAssignedColors && !HasBitSet(NowLive, Idx)) { PlaceGCFrameReset(S, Idx, MinColorRoot, Colors, GCFrame, S.ReverseSafepointNumbering[*rit]); }