From d5b4f8f53d85ad25e45daf82a26fe15d2bb2c4c2 Mon Sep 17 00:00:00 2001 From: Keno Fischer Date: Sat, 24 Jun 2017 17:17:47 -0400 Subject: [PATCH] Fix missing GC root in the presence of try/catch Since try/catch (returns_twice at the LLVM level) can have control flow edges not visible to LLVM, we need to be very conservative here. In order to fix correctness, simply make sure that a value that is live across a sigsetjmp gets frozen in its own dedicated GC slot. There's fancier schemes we could attempt, but they would rely on expressing the exception edges in LLVM IR in one way or another. --- src/llvm-late-gc-lowering.cpp | 40 +++++++++++++++++++++++++------ test/llvmpasses/returnstwicegc.ll | 30 +++++++++++++++++++++++ 2 files changed, 63 insertions(+), 7 deletions(-) create mode 100644 test/llvmpasses/returnstwicegc.ll diff --git a/src/llvm-late-gc-lowering.cpp b/src/llvm-late-gc-lowering.cpp index 2859c1850c9b2..e034ddda31f87 100644 --- a/src/llvm-late-gc-lowering.cpp +++ b/src/llvm-late-gc-lowering.cpp @@ -257,6 +257,12 @@ struct State { // are indices into the next three maps which store safepoint properties std::map SafepointNumbering; + // Instructions that can return twice. For now, all values live at these + // instructions will get their own, dedicated GC frame slots, because they + // have unobservable control flow, so we can't be sure where they're + // actually live. All of these are also considered safepoints. + std::vector ReturnsTwice; + // The set of values live at a particular safepoint std::vector LiveSets; // The set of values for which this is the first safepoint along some @@ -687,6 +693,9 @@ State LateLowerGCFrame::LocalScan(Function &F) { continue; } } + if (CI->canReturnTwice()) { + S.ReturnsTwice.push_back(CI); + } int SafepointNumber = NoteSafepoint(S, BBS, CI); BBS.HasSafepoint = true; BBS.TopmostSafepoint = SafepointNumber; @@ -942,12 +951,24 @@ std::vector LateLowerGCFrame::ColorRoots(const State &S) { std::vector Colors; Colors.resize(S.MaxPtrNumber + 1, -1); PEOIterator Ordering(S.Neighbors); + int PreAssignedColors = 0; + /* First assign permanent slots to things that need them due + to returns_twice */ + for (auto it : S.ReturnsTwice) { + int Num = S.SafepointNumbering.at(it); + const BitVector &LS = S.LiveSets[Num]; + for (int Idx = LS.find_first(); Idx >= 0; Idx = LS.find_next(Idx)) { + if (Colors[Idx] == -1) + Colors[Idx] = PreAssignedColors++; + } + } /* Greedy coloring */ - int ActiveElement = 1; int MaxAssignedColor = -1; + int ActiveElement = 1; BitVector UsedColors; while ((ActiveElement = Ordering.next()) != -1) { - assert(Colors[ActiveElement] == -1); + if (Colors[ActiveElement] != -1) + continue; UsedColors.resize(MaxAssignedColor + 2, false); UsedColors.reset(); if (S.Neighbors[ActiveElement].empty()) { @@ -955,13 +976,18 @@ std::vector LateLowerGCFrame::ColorRoots(const State &S) { continue; } for (int Neighbor : S.Neighbors[ActiveElement]) { - if (Colors[Neighbor] == -1) + int NeighborColor = Colors[Neighbor]; + if (NeighborColor == -1) + continue; + if (NeighborColor < PreAssignedColors) continue; - UsedColors[Colors[Neighbor]] = 1; + UsedColors[NeighborColor - PreAssignedColors] = 1; } - Colors[ActiveElement] = UsedColors.flip().find_first(); - if (Colors[ActiveElement] > MaxAssignedColor) - MaxAssignedColor = Colors[ActiveElement]; + int NewColor = UsedColors.flip().find_first(); + if (NewColor > MaxAssignedColor) + MaxAssignedColor = NewColor; + NewColor += PreAssignedColors; + Colors[ActiveElement] = NewColor; } return Colors; } diff --git a/test/llvmpasses/returnstwicegc.ll b/test/llvmpasses/returnstwicegc.ll new file mode 100644 index 0000000000000..4a0229443311b --- /dev/null +++ b/test/llvmpasses/returnstwicegc.ll @@ -0,0 +1,30 @@ +; RUN: opt -load libjulia.so -LateLowerGCFrame -S %s | FileCheck %s + +%jl_value_t = type opaque + +declare void @boxed_simple(%jl_value_t addrspace(10)*, %jl_value_t addrspace(10)*) +declare %jl_value_t addrspace(10)* @jl_box_int64(i64) +declare %jl_value_t*** @jl_get_ptls_states() +declare i32 @sigsetjmp(i8*, i32) returns_twice +declare void @one_arg_boxed(%jl_value_t addrspace(10)*) + +define void @try_catch(i64 %a, i64 %b) +{ +; Because of the returns_twice function, we need to keep aboxed live everywhere +; CHECK: %gcframe = alloca %jl_value_t addrspace(10)*, i32 4 +top: + %sigframe = alloca [208 x i8], align 16 + %sigframe.sub = getelementptr inbounds [208 x i8], [208 x i8]* %sigframe, i64 0, i64 0 + call %jl_value_t*** @jl_get_ptls_states() + %aboxed = call %jl_value_t addrspace(10)* @jl_box_int64(i64 %a) + %val = call i32 @sigsetjmp(i8 *%sigframe.sub, i32 0) returns_twice + %cmp = icmp eq i32 %val, 0 + br i1 %cmp, label %zero, label %not +zero: + %bboxed = call %jl_value_t addrspace(10)* @jl_box_int64(i64 %b) + call void @one_arg_boxed(%jl_value_t addrspace(10)* %bboxed) + unreachable +not: + call void @one_arg_boxed(%jl_value_t addrspace(10)* %aboxed) + ret void +}