[llvm] 34d9f93 - [AddressSanitizer] Ensure only AllocaInst is passed to dbg.declare
Vedant Kumar via llvm-commits
llvm-commits at lists.llvm.org
Wed Feb 12 11:24:13 PST 2020
Author: Vedant Kumar
Date: 2020-02-12T11:24:02-08:00
New Revision: 34d9f93977fb33e45f9ac09d0a12f41254ef8845
URL: https://github.com/llvm/llvm-project/commit/34d9f93977fb33e45f9ac09d0a12f41254ef8845
DIFF: https://github.com/llvm/llvm-project/commit/34d9f93977fb33e45f9ac09d0a12f41254ef8845.diff
LOG: [AddressSanitizer] Ensure only AllocaInst is passed to dbg.declare
Various parts of the LLVM code generator assume that the address
argument of a dbg.declare is not a `ptrtoint`-of-alloca. ASan breaks
this assumption, and this results in local variables sometimes being
unavailable at -O0.
GlobalISel, SelectionDAG, and FastISel all do not appear to expect
dbg.declares to have a `ptrtoint` as an operand. This means that they do
not place entry block allocas in the usual side table reserved for local
variables available in the whole function scope. This isn't always a
problem, as LLVM can try to lower the dbg.declare to a DBG_VALUE, but
those DBG_VALUEs can get dropped for all the usual reasons DBG_VALUEs
get dropped. In the ObjC test case I'm looking at, the cause happens to
be that `replaceDbgDeclare` has hoisted dbg.declares into the entry
block, causing LiveDebugValues to "kill" the DBG_VALUEs because the
lexical dominance check fails.
To address this, I propose:
1) Have ASan (always) pass an alloca to dbg.declares (this patch). This
is a narrow bugfix for -O0 debugging.
2) Make replaceDbgDeclare not move dbg.declares around. This should be a
generic improvement for optimized debug info, as it would prevent the
lexical dominance check in LiveDebugValues from killing as many
variables.
This means reverting llvm/r227544, which fixed an assertion failure
(llvm.org/PR22386) but no longer seems to be necessary. I was able to
complete a stage2 build with the revert in place.
rdar://54688991
Differential Revision: https://reviews.llvm.org/D74369
Added:
llvm/test/DebugInfo/X86/asan_debug_info.ll
Modified:
llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp
llvm/test/Instrumentation/AddressSanitizer/debug_info.ll
Removed:
################################################################################
diff --git a/llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp b/llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp
index 75b4c8bffc8a..ae537a83ccb0 100644
--- a/llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp
+++ b/llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp
@@ -3119,10 +3119,20 @@ void FunctionStackPoisoner::processStaticAllocas() {
LocalStackBaseAlloca = LocalStackBase;
}
+ // It shouldn't matter whether we pass an `alloca` or a `ptrtoint` as the
+ // dbg.declare address opereand, but passing a `ptrtoint` seems to confuse
+ // later passes and can result in dropped variable coverage in debug info.
+ Value *LocalStackBaseAllocaPtr =
+ isa<PtrToIntInst>(LocalStackBaseAlloca)
+ ? cast<PtrToIntInst>(LocalStackBaseAlloca)->getPointerOperand()
+ : LocalStackBaseAlloca;
+ assert(isa<AllocaInst>(LocalStackBaseAllocaPtr) &&
+ "Variable descriptions relative to ASan stack base will be dropped");
+
// Replace Alloca instructions with base+offset.
for (const auto &Desc : SVD) {
AllocaInst *AI = Desc.AI;
- replaceDbgDeclareForAlloca(AI, LocalStackBaseAlloca, DIB, DIExprFlags,
+ replaceDbgDeclareForAlloca(AI, LocalStackBaseAllocaPtr, DIB, DIExprFlags,
Desc.Offset);
Value *NewAllocaPtr = IRB.CreateIntToPtr(
IRB.CreateAdd(LocalStackBase, ConstantInt::get(IntptrTy, Desc.Offset)),
diff --git a/llvm/test/DebugInfo/X86/asan_debug_info.ll b/llvm/test/DebugInfo/X86/asan_debug_info.ll
new file mode 100644
index 000000000000..37aee265af70
--- /dev/null
+++ b/llvm/test/DebugInfo/X86/asan_debug_info.ll
@@ -0,0 +1,77 @@
+; RUN: opt < %s -asan -asan-module -asan-use-after-return=0 -S | \
+; RUN: llc -O0 -filetype=obj - -o - | \
+; RUN: llvm-dwarfdump - | FileCheck %s
+
+; For this test case, ASan used to produce IR which resulted in the following
+; DWARF (at -O0):
+;
+; DW_TAG_subprogram
+; DW_AT_low_pc (0x0000000000000000)
+; DW_AT_high_pc (0x00000000000000f1)
+;
+; DW_TAG_variable
+; DW_AT_location (0x00000000
+; [0x0000000000000014, 0x000000000000006d): DW_OP_breg0 RAX+32
+; [0x000000000000006d, 0x00000000000000a4): DW_OP_breg7 RSP+16, DW_OP_deref, DW_OP_plus_uconst 0x20
+; [0x00000000000000a6, 0x00000000000000ef): DW_OP_breg7 RSP+16, DW_OP_deref, DW_OP_plus_uconst 0x20)
+;
+; The DWARF produced for the original ObjC code that motivated this test case
+; was actually not as nice! In that example, the location list ranges didn't
+; intersect with the ranges of the parent lexical scope. But recreating that
+; exactly requires playing tricks to get LiveDebugValue's lexical dominance
+; check to kill a variable range early, and it isn't strictly necessary to show
+; the problem here.
+;
+; The problem is that we shouldn't get a location list at all. The instruction
+; selector should recognize that we have an "alloca" in the entry block, and
+; just make the fixed location available in the whole function. We now produce
+; the correct DWARF, namely:
+
+; CHECK: DW_TAG_variable
+; CHECK-NEXT: DW_AT_location (DW_OP_breg7 RSP+32, DW_OP_plus_uconst 0x20)
+
+target triple = "x86_64-apple-macosx10.10.0"
+
+declare void @escape(i8**)
+
+; Function Attrs: sanitize_address
+define i8* @foo(i1 %cond) #0 !dbg !6 {
+entry:
+ %a1 = alloca i8*, !dbg !12
+ call void @escape(i8** %a1), !dbg !13
+ br i1 %cond, label %l1, label %l2, !dbg !14
+
+l1: ; preds = %entry
+ ret i8* null, !dbg !15
+
+l2: ; preds = %entry
+ call void @llvm.dbg.declare(metadata i8** %a1, metadata !11, metadata !DIExpression()), !dbg !16
+ %p = load i8*, i8** %a1, !dbg !16
+ ret i8* %p, !dbg !17
+}
+
+declare void @llvm.dbg.declare(metadata, metadata, metadata)
+
+attributes #0 = { sanitize_address }
+
+!llvm.dbg.cu = !{!0}
+!llvm.debugify = !{!3, !4}
+!llvm.module.flags = !{!5}
+
+!0 = distinct !DICompileUnit(language: DW_LANG_C, file: !1, producer: "debugify", isOptimized: true, runtimeVersion: 0, emissionKind: FullDebug, enums: !2)
+!1 = !DIFile(filename: "redu.ll", directory: "/")
+!2 = !{}
+!3 = !{i32 6}
+!4 = !{i32 2}
+!5 = !{i32 2, !"Debug Info Version", i32 3}
+!6 = distinct !DISubprogram(name: "foo", linkageName: "foo", scope: null, file: !1, line: 1, type: !7, scopeLine: 1, spFlags: DISPFlagDefinition | DISPFlagOptimized, unit: !0, retainedNodes: !8)
+!7 = !DISubroutineType(types: !2)
+!8 = !{!11}
+!10 = !DIBasicType(name: "ty64", size: 64, encoding: DW_ATE_unsigned)
+!11 = !DILocalVariable(name: "2", scope: !6, file: !1, line: 5, type: !10)
+!12 = !DILocation(line: 1, column: 1, scope: !6)
+!13 = !DILocation(line: 2, column: 1, scope: !6)
+!14 = !DILocation(line: 3, column: 1, scope: !6)
+!15 = !DILocation(line: 4, column: 1, scope: !6)
+!16 = !DILocation(line: 5, column: 1, scope: !6)
+!17 = !DILocation(line: 6, column: 1, scope: !6)
diff --git a/llvm/test/Instrumentation/AddressSanitizer/debug_info.ll b/llvm/test/Instrumentation/AddressSanitizer/debug_info.ll
index fb696ec23dbe..97faa1aff469 100644
--- a/llvm/test/Instrumentation/AddressSanitizer/debug_info.ll
+++ b/llvm/test/Instrumentation/AddressSanitizer/debug_info.ll
@@ -23,10 +23,14 @@ entry:
; CHECK: define i32 @_Z3zzzi
; CHECK: entry:
; Verify that llvm.dbg.declare calls are in the entry basic block.
+; CHECK-NEXT: [[MyAlloca:%.*]] = alloca i8, i64 64
; CHECK-NOT: %entry
-; CHECK: call void @llvm.dbg.declare(metadata {{.*}}, metadata ![[ARG_ID:[0-9]+]], metadata !DIExpression(DW_OP_plus_uconst, 32))
+; Note: these dbg.declares used to contain `ptrtoint` operands. The instruction
+; selector would then decline to put the variable in the MachineFunction side
+; table. Check that the dbg.declares have `alloca` operands.
+; CHECK: call void @llvm.dbg.declare(metadata i8* [[MyAlloca]], metadata ![[ARG_ID:[0-9]+]], metadata !DIExpression(DW_OP_plus_uconst, 32))
; CHECK-NOT: %entry
-; CHECK: call void @llvm.dbg.declare(metadata {{.*}}, metadata ![[VAR_ID:[0-9]+]], metadata !DIExpression(DW_OP_plus_uconst, 48))
+; CHECK: call void @llvm.dbg.declare(metadata i8* [[MyAlloca]], metadata ![[VAR_ID:[0-9]+]], metadata !DIExpression(DW_OP_plus_uconst, 48))
declare void @llvm.dbg.declare(metadata, metadata, metadata) nounwind readnone
More information about the llvm-commits
mailing list