[PATCH] D74369: [AddressSanitizer] Ensure only AllocaInst is passed to dbg.declare

Mon Feb 10 16:51:08 PST 2020

vsk created this revision.
vsk added reviewers: aprantl, eugenis, kubamracek.
Herald added a subscriber: hiraditya.
Herald added a project: LLVM.

Various parts of the LLVM code generator assume that the address
argument of a dbg.declare is an alloca. ASan breaks this assumption.
Consequently, local variables are sometimes unavailable at -O0 with
ASan.

GlobalISel, SelectionDAG, and FastISel all basically drop non-alloca
dbg.declares. This means the usual MachineFunction side table for frame-
index backed variables goes unused. In some cases LLVM tries to muddle
through by lowering the dbg.declare to a DBG_VALUE, but this usually
/also/ results in dropped variable locations because replaceDbgDeclare
has moved the dbg.declare to the entry block. /That/ breaks the lexical
dominance check in LiveDebugValues.

To address this, I propose:

1. Have ASan (always) pass an alloca to dbg.declares (this patch). This is a narrow bugfix which immediately makes debugging better at -O0.

2. Make replaceDbgDeclare not move dbg.declares around. This should be a generic improvement for optimized debug info, as it makes the lexical dominance check in LiveDebugValues kill fewer variables.

  Essentially, this means reverting llvm/r227544, which was introduced to fix an assertion failure (llvm.org/PR22386). The underlying issue there needs to be addressed a different way (if it's not already fixed).

3. (veeery tentative) Ban non-alloca dbg.declares in the Verfier. We can eliminate a class of bugs this way. If this is too aggressive, let's at least document the assumptions made about dbg.declare a bit more.

rdar://54688991


https://reviews.llvm.org/D74369

Files:
  llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp
  llvm/test/DebugInfo/X86/asan_debug_info.ll
  llvm/test/Instrumentation/AddressSanitizer/debug_info.ll


Index: llvm/test/Instrumentation/AddressSanitizer/debug_info.ll
===================================================================

--- llvm/test/Instrumentation/AddressSanitizer/debug_info.ll
+++ llvm/test/Instrumentation/AddressSanitizer/debug_info.ll
@@ -23,10 +23,11 @@
 ;   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))
+;   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
 
Index: llvm/test/DebugInfo/X86/asan_debug_info.ll
===================================================================
--- /dev/null
+++ llvm/test/DebugInfo/X86/asan_debug_info.ll
@@ -0,0 +1,50 @@
+; RUN: opt < %s -asan -asan-module -asan-use-after-return=0 -S | \
+; RUN:   llc -O0 -filetype=obj - -o - | \
+; RUN:   llvm-dwarfdump - | FileCheck %s
+
+; CHECK: DW_TAG_formal_parameter
+; CHECK-NEXT: DW_AT_location        (DW_OP_breg7 RSP+64, DW_OP_plus_uconst 0x20)
+; CHECK-NEXT: DW_AT_name    ("p")
+
+; CHECK: DW_TAG_variable
+; CHECK-NEXT: DW_AT_location      (DW_OP_breg7 RSP+64, DW_OP_plus_uconst 0x30)
+; CHECK-NEXT: DW_AT_name  ("r")
+
+
+target datalayout = "e-p:64:64:64-i1:8:8-i8:8:8-i16:16:16-i32:32:32-i64:64:64-f32:32:32-f64:64:64-v64:64:64-v128:128:128-a0:0:64-s0:64:64-f80:128:128-n8:16:32:64-S128"
+target triple = "x86_64-unknown-linux-gnu"
+
+define i32 @_Z3zzzi(i32 %p) nounwind uwtable sanitize_address !dbg !5 {
+entry:
+  %p.addr = alloca i32, align 4
+  %r = alloca i32, align 4
+  store volatile i32 %p, i32* %p.addr, align 4
+  call void @llvm.dbg.declare(metadata i32* %p.addr, metadata !10, metadata !DIExpression()), !dbg !11
+  call void @llvm.dbg.declare(metadata i32* %r, metadata !12, metadata !DIExpression()), !dbg !14
+  %0 = load i32, i32* %p.addr, align 4, !dbg !14
+  %add = add nsw i32 %0, 1, !dbg !14
+  store volatile i32 %add, i32* %r, align 4, !dbg !14
+  %1 = load i32, i32* %r, align 4, !dbg !15
+  ret i32 %1, !dbg !15
+}
+
+declare void @llvm.dbg.declare(metadata, metadata, metadata) nounwind readnone
+
+!llvm.dbg.cu = !{!0}
+!llvm.module.flags = !{!17}
+
+!0 = distinct !DICompileUnit(language: DW_LANG_C_plus_plus, producer: "clang version 3.3 (trunk 169314)", isOptimized: true, emissionKind: FullDebug, file: !16, enums: !1, retainedTypes: !1, globals: !1)
+!1 = !{}
+!5 = distinct !DISubprogram(name: "zzz", linkageName: "_Z3zzzi", line: 1, isLocal: false, isDefinition: true, virtualIndex: 6, flags: DIFlagPrototyped, isOptimized: false, unit: !0, scopeLine: 1, file: !16, scope: !6, type: !7, retainedNodes: !1)
+!6 = !DIFile(filename: "a.cc", directory: "/usr/local/google/llvm_cmake_clang/tmp/debuginfo")
+!7 = !DISubroutineType(types: !8)
+!8 = !{!9, !9}
+!9 = !DIBasicType(tag: DW_TAG_base_type, name: "int", size: 32, align: 32, encoding: DW_ATE_signed)
+!10 = !DILocalVariable(name: "p", line: 1, arg: 1, scope: !5, file: !6, type: !9)
+!11 = !DILocation(line: 1, scope: !5)
+!12 = !DILocalVariable(name: "r", line: 2, scope: !13, file: !6, type: !9)
+!13 = distinct !DILexicalBlock(line: 1, column: 0, file: !16, scope: !5)
+!14 = !DILocation(line: 2, scope: !13)
+!15 = !DILocation(line: 3, scope: !13)
+!16 = !DIFile(filename: "a.cc", directory: "/usr/local/google/llvm_cmake_clang/tmp/debuginfo")
+!17 = !{i32 1, !"Debug Info Version", i32 3}
Index: llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp
===================================================================
--- llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp
+++ llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp
@@ -3120,9 +3120,17 @@
   }
 
   // Replace Alloca instructions with base+offset.
+  Value *LocalStackBaseAllocaPtr =
+      isa<PtrToIntInst>(LocalStackBaseAlloca)
+          ? cast<PtrToIntInst>(LocalStackBaseAlloca)->getPointerOperand()
+          : LocalStackBaseAlloca;
+  assert(isa<AllocaInst>(LocalStackBaseAllocaPtr) &&
+         "Variable descriptions relative to ASan stack base will be dropped");
   for (const auto &Desc : SVD) {
     AllocaInst *AI = Desc.AI;
-    replaceDbgDeclareForAlloca(AI, LocalStackBaseAlloca, DIB, DIExprFlags,
+    // The instruction selector assumes that a dbg.declare has an alloca as its
+    // first argument, and may discard the dbg.declare otherwise.
+    replaceDbgDeclareForAlloca(AI, LocalStackBaseAllocaPtr, DIB, DIExprFlags,
                                Desc.Offset);
     Value *NewAllocaPtr = IRB.CreateIntToPtr(
         IRB.CreateAdd(LocalStackBase, ConstantInt::get(IntptrTy, Desc.Offset)),


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D74369.243701.patch
Type: text/x-patch
Size: 5145 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20200211/fe6bb36b/attachment-0001.bin>
