Commit 34d9f939 authored by Vedant Kumar's avatar Vedant Kumar
Browse files

[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
parent 32aac256

llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp

+11 −1
Original line number Diff line number Diff line
@@ -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)),

llvm/test/DebugInfo/X86/asan_debug_info.ll

0 → 100644
+77 −0
Original line number Diff line number Diff line 
; 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)

llvm/test/Instrumentation/AddressSanitizer/debug_info.ll

+6 −2
Original line number Diff line number Diff line
@@ -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