From 8c06a19db2c6989bc834a3cedc5e81f1fc1ae629 Mon Sep 17 00:00:00 2001 From: Gor Nishanov Date: Sat, 27 May 2017 19:41:09 +0000 Subject: [PATCH] Cloning: Fix debug info cloning Summary: I believe https://reviews.llvm.org/rL302576 introduced two bugs: 1) it produces duplicate distinct variables for every: dbg.value describing the same variable. To fix the problme I switched form getDistinct() to get() in DebugLoc.cpp: auto reparentVar = [&](DILocalVariable *Var) { return DILocalVariable::getDistinct( 2) It passes NewFunction plain name as a linkagename parameter to Subprogram constructor. Breaks assert in: || DeclLinkageName.empty()) || LinkageName == DeclLinkageName) && "decl has a linkage name and it is different"' failed. #9 0x00007f5010261b75 llvm::DwarfUnit::applySubprogramDefinitionAttributes(llvm::DISubprogram const*, llvm::DIE&) /home/gor/llvm/lib/CodeGen/AsmPrinter/DwarfUnit.cpp:1173:3 # (Edit: reproducer added) Here how https://reviews.llvm.org/rL302576 broke coroutine debug info. Coroutine body of the original function is split into several parts by cloning and removing unneeded code. All parts describe the original function and variables present in the original function. For a simple case, prior to Split, original function has these two blocks: ``` PostSpill: ; preds = %AllocaSpillBB call void @llvm.dbg.value(metadata i32 %x, i64 0, metadata !14, metadata !15), !dbg !13 store i32 %x, i32* %x.addr, align 4 ... and sw.epilog: ; preds = %sw.bb %x.addr.reload.addr = getelementptr inbounds %f.Frame, %f.Frame* %FramePtr, i32 0, i32 4, !dbg !20 %4 = load i32, i32* %x.addr.reload.addr, align 4, !dbg !20 call void @llvm.dbg.value(metadata i32 %4, i64 0, metadata !14, metadata !15), !dbg !13 !14 = !DILocalVariable(name: "x", arg: 1, scope: !6, file: !7, line: 55, type: !11) ``` Note that in two blocks different expression represent the same original user variable X. Before rL302576, for every cloned function there was exactly one cloned DILocalVariable(name: "x" as in: ``` define i8* @f(i32 %x) #0 !dbg !6 { ... !6 = distinct !DISubprogram(name: "f", scope: !7, file: !7, line: 55, type: !8, isLocal: false, isDefinition: true, scopeLine: 55, flags: DIFlagPrototyped, ... !14 = !DILocalVariable(name: "x", arg: 1, scope: !6, file: !7, line: 55, type: !11) define internal fastcc void @f.resume(%f.Frame* %FramePtr) #0 !dbg !25 { ... !25 = distinct !DISubprogram(name: "f", scope: !7, file: !7, line: 55, type: !8, isLocal: false, isDefinition: true, scopeLine: 55, flags: DIFlagPrototyped, isOptimized: false, unit: !0, variables: !2) !28 = !DILocalVariable(name: "x", arg: 1, scope: !25, file: !7, line: 55, type: !11) ``` After rL302576, for every cloned function there were as many DILocalVariable(name: "x" as there were "call void @llvm.dbg.value" for that variable. This was causing asserts in VerifyDebugInfo and AssemblyPrinter. Example: ``` !27 = distinct !DISubprogram(name: "f", linkageName: "f.resume", scope: !7, file: !7, line: 55, type: !8, isLocal: false, isDefinition: true, scopeLine: 55, !29 = distinct !DILocalVariable(name: "x", arg: 1, scope: !27, file: !7, line: 55, type: !11) !39 = distinct !DILocalVariable(name: "x", arg: 1, scope: !27, file: !7, line: 55, type: !11) !41 = distinct !DILocalVariable(name: "x", arg: 1, scope: !27, file: !7, line: 55, type: !11) ``` Second problem: Prior to rL302576, all clones were described by DISubprogram referring to original function. ``` define i8* @f(i32 %x) #0 !dbg !6 { ... !6 = distinct !DISubprogram(name: "f", scope: !7, file: !7, line: 55, type: !8, isLocal: false, isDefinition: true, scopeLine: 55, flags: DIFlagPrototyped, define internal fastcc void @f.resume(%f.Frame* %FramePtr) #0 !dbg !25 { ... !25 = distinct !DISubprogram(name: "f", scope: !7, file: !7, line: 55, type: !8, isLocal: false, isDefinition: true, scopeLine: 55, flags: DIFlagPrototyped, ``` After rL302576, DISubprogram for clones is of two minds, plain name refers to the original name, linkageName refers to plain name of the clone. ``` !27 = distinct !DISubprogram(name: "f", linkageName: "f.resume", scope: !7, file: !7, line: 55, type: !8, isLocal: false, isDefinition: true, scopeLine: 55, ``` I think the assumption in AsmPrinter is that both name and linkageName should refer to the same entity. It asserts here when they are not: ``` || DeclLinkageName.empty()) || LinkageName == DeclLinkageName) && "decl has a linkage name and it is different"' failed. #9 0x00007f5010261b75 llvm::DwarfUnit::applySubprogramDefinitionAttributes(llvm::DISubprogram const*, llvm::DIE&) /home/gor/llvm/lib/CodeGen/AsmPrinter/DwarfUnit.cpp:1173:3 ``` After this fix, behavior (with respect to coroutines) reverts to exactly as it was before and therefore making them debuggable again, or even more importantly, compilable, with "-g" Reviewers: dblaikie, echristo, aprantl Reviewed By: dblaikie Subscribers: llvm-commits Differential Revision: https://reviews.llvm.org/D33614 llvm-svn: 304079 --- lib/IR/DebugLoc.cpp | 2 +- lib/Transforms/Coroutines/CoroSplit.cpp | 10 +- lib/Transforms/Utils/CloneFunction.cpp | 2 +- test/Transforms/Coroutines/coro-debug.ll | 142 +++++++++++++++++++++++ 4 files changed, 145 insertions(+), 11 deletions(-) create mode 100644 test/Transforms/Coroutines/coro-debug.ll diff --git a/lib/IR/DebugLoc.cpp b/lib/IR/DebugLoc.cpp index 3168ec6944a..b7e3f0c6779 100644 --- a/lib/IR/DebugLoc.cpp +++ b/lib/IR/DebugLoc.cpp @@ -163,7 +163,7 @@ void DebugLoc::reparentDebugInfo(Instruction &I, DISubprogram *OrigSP, // Fix up debug variables to point to NewSP. auto reparentVar = [&](DILocalVariable *Var) { - return DILocalVariable::getDistinct( + return DILocalVariable::get( Ctx, cast( reparentScope(Ctx, Var->getScope(), OrigSP, NewSP, Cache)), diff --git a/lib/Transforms/Coroutines/CoroSplit.cpp b/lib/Transforms/Coroutines/CoroSplit.cpp index 78e3c806d80..cd549e4be28 100644 --- a/lib/Transforms/Coroutines/CoroSplit.cpp +++ b/lib/Transforms/Coroutines/CoroSplit.cpp @@ -228,15 +228,7 @@ static Function *createClone(Function &F, Twine Suffix, coro::Shape &Shape, SmallVector Returns; - if (DISubprogram *SP = F.getSubprogram()) { - // If we have debug info, add mapping for the metadata nodes that should not - // be cloned by CloneFunctionInfo. - auto &MD = VMap.MD(); - MD[SP->getUnit()].reset(SP->getUnit()); - MD[SP->getType()].reset(SP->getType()); - MD[SP->getFile()].reset(SP->getFile()); - } - CloneFunctionInto(NewF, &F, VMap, /*ModuleLevelChanges=*/true, Returns); + CloneFunctionInto(NewF, &F, VMap, /*ModuleLevelChanges=*/false, Returns); // Remove old returns. for (ReturnInst *Return : Returns) diff --git a/lib/Transforms/Utils/CloneFunction.cpp b/lib/Transforms/Utils/CloneFunction.cpp index bf2ab7c55be..1ec3d0d4963 100644 --- a/lib/Transforms/Utils/CloneFunction.cpp +++ b/lib/Transforms/Utils/CloneFunction.cpp @@ -133,7 +133,7 @@ void llvm::CloneFunctionInto(Function *NewFunc, const Function *OldFunc, auto *SP = cast(MD.second); NewMD = DISubprogram::getDistinct( NewFunc->getContext(), SP->getScope(), SP->getName(), - NewFunc->getName(), SP->getFile(), SP->getLine(), SP->getType(), + SP->getLinkageName(), SP->getFile(), SP->getLine(), SP->getType(), SP->isLocalToUnit(), SP->isDefinition(), SP->getScopeLine(), SP->getContainingType(), SP->getVirtuality(), SP->getVirtualIndex(), SP->getThisAdjustment(), SP->getFlags(), SP->isOptimized(), diff --git a/test/Transforms/Coroutines/coro-debug.ll b/test/Transforms/Coroutines/coro-debug.ll new file mode 100644 index 00000000000..4da545499f9 --- /dev/null +++ b/test/Transforms/Coroutines/coro-debug.ll @@ -0,0 +1,142 @@ +; Tests that debug information is sane after coro-split +; RUN: opt < %s -coro-split -S | FileCheck %s + +source_filename = "simple-repro.c" +target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128" +target triple = "x86_64-unknown-linux-gnu" + +; Function Attrs: noinline nounwind +define i8* @f(i32 %x) #0 !dbg !6 { +entry: + %x.addr = alloca i32, align 4 + %coro_hdl = alloca i8*, align 8 + store i32 %x, i32* %x.addr, align 4 + call void @llvm.dbg.declare(metadata i32* %x.addr, metadata !12, metadata !13), !dbg !14 + call void @llvm.dbg.declare(metadata i8** %coro_hdl, metadata !15, metadata !13), !dbg !16 + %0 = call token @llvm.coro.id(i32 0, i8* null, i8* bitcast (i8* (i32)* @f to i8*), i8* null), !dbg !16 + %1 = call i64 @llvm.coro.size.i64(), !dbg !16 + %call = call i8* @malloc(i64 %1), !dbg !16 + %2 = call i8* @llvm.coro.begin(token %0, i8* %call) #7, !dbg !16 + store i8* %2, i8** %coro_hdl, align 8, !dbg !16 + %3 = call i8 @llvm.coro.suspend(token none, i1 false), !dbg !17 + %conv = sext i8 %3 to i32, !dbg !17 + call void @coro.devirt.trigger(i8* null) + switch i32 %conv, label %sw.default [ + i32 0, label %sw.bb + i32 1, label %sw.bb1 + ], !dbg !17 + +sw.bb: ; preds = %entry + br label %sw.epilog, !dbg !18 + +sw.bb1: ; preds = %entry + br label %coro_Cleanup, !dbg !18 + +sw.default: ; preds = %entry + br label %coro_Suspend, !dbg !18 + +sw.epilog: ; preds = %sw.bb + %4 = load i32, i32* %x.addr, align 4, !dbg !20 + %add = add nsw i32 %4, 1, !dbg !21 + store i32 %add, i32* %x.addr, align 4, !dbg !22 + br label %coro_Cleanup, !dbg !23 + +coro_Cleanup: ; preds = %sw.epilog, %sw.bb1 + %5 = load i8*, i8** %coro_hdl, align 8, !dbg !24 + %6 = call i8* @llvm.coro.free(token %0, i8* %5), !dbg !24 + call void @free(i8* %6), !dbg !24 + br label %coro_Suspend, !dbg !24 + +coro_Suspend: ; preds = %coro_Cleanup, %sw.default + %7 = call i1 @llvm.coro.end(i8* null, i1 false) #7, !dbg !24 + %8 = load i8*, i8** %coro_hdl, align 8, !dbg !24 + ret i8* %8, !dbg !24 +} + +; Function Attrs: nounwind readnone speculatable +declare void @llvm.dbg.declare(metadata, metadata, metadata) #1 + +; Function Attrs: argmemonly nounwind readonly +declare token @llvm.coro.id(i32, i8* readnone, i8* nocapture readonly, i8*) #2 + +declare i8* @malloc(i64) #3 + +; Function Attrs: nounwind readnone +declare i64 @llvm.coro.size.i64() #4 + +; Function Attrs: nounwind +declare i8* @llvm.coro.begin(token, i8* writeonly) #5 + +; Function Attrs: nounwind +declare i8 @llvm.coro.suspend(token, i1) #5 + +declare void @free(i8*) #3 + +; Function Attrs: argmemonly nounwind readonly +declare i8* @llvm.coro.free(token, i8* nocapture readonly) #2 + +; Function Attrs: nounwind +declare i1 @llvm.coro.end(i8*, i1) #5 + +; Function Attrs: alwaysinline +define private void @coro.devirt.trigger(i8*) #6 { +entry: + ret void +} + +; Function Attrs: argmemonly nounwind readonly +declare i8* @llvm.coro.subfn.addr(i8* nocapture readonly, i8) #2 + +attributes #0 = { noinline nounwind "coroutine.presplit"="1" "correctly-rounded-divide-sqrt-fp-math"="false" "disable-tail-calls"="false" "less-precise-fpmad"="false" "no-frame-pointer-elim"="false" "no-infs-fp-math"="false" "no-jump-tables"="false" "no-nans-fp-math"="false" "no-signed-zeros-fp-math"="false" "no-trapping-math"="false" "stack-protector-buffer-size"="8" "target-features"="+mmx,+sse,+sse2,+x87" "unsafe-fp-math"="false" "use-soft-float"="false" } +attributes #1 = { nounwind readnone speculatable } +attributes #2 = { argmemonly nounwind readonly } +attributes #3 = { "correctly-rounded-divide-sqrt-fp-math"="false" "disable-tail-calls"="false" "less-precise-fpmad"="false" "no-frame-pointer-elim"="false" "no-infs-fp-math"="false" "no-nans-fp-math"="false" "no-signed-zeros-fp-math"="false" "no-trapping-math"="false" "stack-protector-buffer-size"="8" "target-features"="+mmx,+sse,+sse2,+x87" "unsafe-fp-math"="false" "use-soft-float"="false" } +attributes #4 = { nounwind readnone } +attributes #5 = { nounwind } +attributes #6 = { alwaysinline } +attributes #7 = { noduplicate } + +!llvm.dbg.cu = !{!0} +!llvm.module.flags = !{!3, !4} +!llvm.ident = !{!5} + +!0 = distinct !DICompileUnit(language: DW_LANG_C99, file: !1, producer: "clang version 5.0.0 (http://llvm.org/git/clang.git 97b002238b11ff30d94d0516d6a0515db5725fd8) (http://llvm.org/git/llvm.git 0cb060ba567f1aa5b4b04e86665f88e4632b528a)", isOptimized: false, runtimeVersion: 0, emissionKind: FullDebug, enums: !2) +!1 = !DIFile(filename: "", directory: "C:\5CGitHub\5Cllvm\5Cbuild\5CDebug\5Cbin") +!2 = !{} +!3 = !{i32 2, !"Dwarf Version", i32 4} +!4 = !{i32 2, !"Debug Info Version", i32 3} +!5 = !{!"clang version 5.0.0 (http://llvm.org/git/clang.git 97b002238b11ff30d94d0516d6a0515db5725fd8) (http://llvm.org/git/llvm.git 0cb060ba567f1aa5b4b04e86665f88e4632b528a)"} +!6 = distinct !DISubprogram(name: "f", linkageName: "flink", scope: !7, file: !7, line: 55, type: !8, isLocal: false, isDefinition: true, scopeLine: 55, flags: DIFlagPrototyped, isOptimized: false, unit: !0, variables: !2) +!7 = !DIFile(filename: "simple-repro.c", directory: "C:\5CGitHub\5Cllvm\5Cbuild\5CDebug\5Cbin") +!8 = !DISubroutineType(types: !9) +!9 = !{!10, !11} +!10 = !DIDerivedType(tag: DW_TAG_pointer_type, baseType: null, size: 64) +!11 = !DIBasicType(name: "int", size: 32, encoding: DW_ATE_signed) +!12 = !DILocalVariable(name: "x", arg: 1, scope: !6, file: !7, line: 55, type: !11) +!13 = !DIExpression() +!14 = !DILocation(line: 55, column: 13, scope: !6) +!15 = !DILocalVariable(name: "coro_hdl", scope: !6, file: !7, line: 56, type: !10) +!16 = !DILocation(line: 56, column: 3, scope: !6) +!17 = !DILocation(line: 58, column: 5, scope: !6) +!18 = !DILocation(line: 58, column: 5, scope: !19) +!19 = distinct !DILexicalBlock(scope: !6, file: !7, line: 58, column: 5) +!20 = !DILocation(line: 59, column: 9, scope: !6) +!21 = !DILocation(line: 59, column: 10, scope: !6) +!22 = !DILocation(line: 59, column: 7, scope: !6) +!23 = !DILocation(line: 59, column: 5, scope: !6) +!24 = !DILocation(line: 62, column: 3, scope: !6) + +; CHECK: define i8* @f(i32 %x) #0 !dbg ![[ORIG:[0-9]+]] +; CHECK: define internal fastcc void @f.resume(%f.Frame* %FramePtr) #0 !dbg ![[RESUME:[0-9]+]] +; CHECK: define internal fastcc void @f.destroy(%f.Frame* %FramePtr) #0 !dbg ![[DESTROY:[0-9]+]] +; CHECK: define internal fastcc void @f.cleanup(%f.Frame* %FramePtr) #0 !dbg ![[CLEANUP:[0-9]+]] + +; CHECK: ![[ORIG]] = distinct !DISubprogram(name: "f", linkageName: "flink" +; CHECK: !DILocalVariable(name: "x", arg: 1, scope: ![[ORIG]] + +; CHECK: ![[RESUME]] = distinct !DISubprogram(name: "f", linkageName: "flink" +; CHECK: !DILocalVariable(name: "x", arg: 1, scope: ![[RESUME]] + +; CHECK: ![[DESTROY]] = distinct !DISubprogram(name: "f", linkageName: "flink" + +; CHECK: ![[CLEANUP]] = distinct !DISubprogram(name: "f", linkageName: "flink"