Skip to content

Commit 906d348

Browse files
authored
Make late gc lower handle insertelement of alloca use. (JuliaLang#58637)
This was in DAECompiler.jl code found by @serenity4. He also mentioned that writing up how one might go and fix a bug like this so i'll give a quick writeup (this was a very simple bug so it might not be too interesting) The original crash which looked something like > %19 = alloca [10 x i64], align 8 %155 = insertelement <4 x ptr> poison, ptr %19, i32 0 Unexpected instruction > [898844] signal 6 (-6): Aborted in expression starting at /home/gbaraldi/DAECompiler.jl/test/reflection.jl:28 pthread_kill at /lib/x86_64-linux-gnu/libc.so.6 (unknown line) gsignal at /lib/x86_64-linux-gnu/libc.so.6 (unknown line) abort at /lib/x86_64-linux-gnu/libc.so.6 (unknown line) RecursivelyVisit<llvm::IntrinsicInst, LateLowerGCFrame::PlaceRootsAndUpdateCalls(llvm::ArrayRef<int>, int, State&, std::map<llvm::Value*, std::pair<int, int> >)::<lambda(llvm::AllocaInst*&)>::<lambda(llvm::Use&)> > at /home/gbaraldi/julia4/src/llvm-late-gc-lowering.cpp:803 operator() at /home/gbaraldi/julia4/src/llvm-late-gc-lowering.cpp:2560 [inlined] PlaceRootsAndUpdateCalls at /home/gbaraldi/julia4/src/llvm-late-gc-lowering.cpp:2576 runOnFunction at /home/gbaraldi/julia4/src/llvm-late-gc-lowering.cpp:2638 run at /home/gbaraldi/julia4/src/llvm-late-gc-lowering.cpp:2675 run at /home/gbaraldi/julia4/usr/include/llvm/IR/PassManagerInternal.h:91 which means it was crashing inside of late-gc-lowering, so the first thing I did was ran julia and the same test with LLVM_ASSERTIONS=1 and FORCE_ASSERTIONS=1 to see if LLVM complained about a malformed module, and both were fine. Next step was trying to get the failing code out for inspection. Easiest way is to do `export JULIA_LLVM_ARGS="--print-before=LateLowerGCFrame --print-module-scope"` and pipe the output to a file. The file is huge, but since it's a crash in LLVM we know that the last thing is what we want, and that gave me the IR I wanted. To verify that this is failing I did `make -C src install-analysis-deps` to install the LLVM machinery (opt...). That gets put in the `tools` directory of a julia build. Then I checked if this crashed outside of julia by doing `./opt -load-pass-plugin=../lib/libjulia-codegen.dylib --passes=LateLowerGCFrame -S test.ll -o tmp3.ll `. This is run from inside the tools dir so your paths might vary (the -S is so LLVM doesn't generate bitcode) and my code did crash, however it was over 500 lines of IR which makes it harder to debug and to write a test. Next step then is to minimize the crash by doing [`llvm-reduce`](https://llvm.org/docs/CommandGuide/llvm-reduce.html) over it (it's basically creduce but optimized for LLVM IR) which gave me a 2 line reproducer (in this case apparently just having the insertelement was enough for the pass to fail). One thing to be wary is that llvm-reduce will usually make very weird code, so it might be useful to modify the code slightly so it doesn't look odd (it will have unreachable basic-blocks and such). After the cleanup fixing the bug here wasn't interesting but this doesn't apply generally. And also always transform your reduced IR into a test to put in llvmpasses.
1 parent fe7dc7d commit 906d348

File tree

2 files changed

+15
-1
lines changed

2 files changed

+15
-1
lines changed

src/llvm-late-gc-lowering.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -790,7 +790,7 @@ void RecursivelyVisit(callback f, Value *V) {
790790
if (isa<CallInst>(TheUser) || isa<LoadInst>(TheUser) ||
791791
isa<SelectInst>(TheUser) || isa<PHINode>(TheUser) || // TODO: should these be removed from this list?
792792
isa<StoreInst>(TheUser) || isa<PtrToIntInst>(TheUser) ||
793-
isa<ICmpInst>(TheUser) || // ICmpEQ/ICmpNE can be used with ptr types
793+
isa<ICmpInst>(TheUser) || isa<InsertElementInst>(TheUser)|| // ICmpEQ/ICmpNE can be used with ptr types
794794
isa<AtomicCmpXchgInst>(TheUser) || isa<AtomicRMWInst>(TheUser))
795795
continue;
796796
if (isa<GetElementPtrInst>(TheUser) || isa<BitCastInst>(TheUser) || isa<AddrSpaceCastInst>(TheUser)) {

test/llvmpasses/late-lower-gc.ll

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -199,6 +199,20 @@ define void @decayar([2 x {} addrspace(10)* addrspace(11)*] %ar) {
199199
; CHECK: %r = call i32 @callee_root(ptr addrspace(10) %l0, ptr addrspace(10) %l1)
200200
; CHECK: call void @julia.pop_gc_frame(ptr %gcframe)
201201

202+
define swiftcc ptr addrspace(10) @insert_element(ptr swiftself %0) {
203+
; CHECK-LABEL: @insert_element
204+
%2 = alloca [10 x i64], i32 1, align 8
205+
; CHECK: %gcframe = call ptr @julia.new_gc_frame(i32 10)
206+
; CHECK: [[gc_slot_addr_:%.*]] = call ptr @julia.get_gc_frame_slot(ptr %gcframe, i32 0)
207+
; CHECK: call void @julia.push_gc_frame(ptr %gcframe, i32 10)
208+
call void null(ptr sret([2 x [5 x ptr addrspace(10)]]) %2, ptr null, ptr addrspace(11) null, ptr null)
209+
%4 = insertelement <4 x ptr> zeroinitializer, ptr %2, i32 0
210+
; CHECK: [[gc_slot_addr_:%.*]] = insertelement <4 x ptr> zeroinitializer, ptr [[gc_slot_addr_:%.*]], i32 0
211+
; CHECK: call void @julia.pop_gc_frame(ptr %gcframe)
212+
ret ptr addrspace(10) null
213+
}
214+
215+
202216
!0 = !{i64 0, i64 23}
203217
!1 = !{!1}
204218
!2 = !{!7} ; scope list

0 commit comments

Comments
 (0)