Skip to content

Commit

Permalink
[LateLowerGCFrame] fix PlaceGCFrameReset for returns_twice (#57392)
Browse files Browse the repository at this point in the history
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
  • Loading branch information
vtjnash authored Feb 14, 2025
1 parent 864d2bf commit 75dba04
Show file tree
Hide file tree
Showing 2 changed files with 3 additions and 4 deletions.
5 changes: 2 additions & 3 deletions src/codegen.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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)) {
Expand Down Expand Up @@ -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<jl_varinfo_t> &slots, const std::set<int> &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();
Expand Down
2 changes: 1 addition & 1 deletion src/llvm-late-gc-lowering.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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]);
}
Expand Down

0 comments on commit 75dba04

Please sign in to comment.