[llvm] 09667bc - [asan] Remove debug locations from alloca prologue instrumentation
Johannes Altmanninger via llvm-commits
llvm-commits at lists.llvm.org
Tue Dec 3 02:24:23 PST 2019
Author: Johannes Altmanninger
Date: 2019-12-03T11:24:17+01:00
New Revision: 09667bc1920463107684a566c3f2c3cef9b156e7
URL: https://github.com/llvm/llvm-project/commit/09667bc1920463107684a566c3f2c3cef9b156e7
DIFF: https://github.com/llvm/llvm-project/commit/09667bc1920463107684a566c3f2c3cef9b156e7.diff
LOG: [asan] Remove debug locations from alloca prologue instrumentation
Summary:
This fixes https://llvm.org/PR26673
"Wrong debugging information with -fsanitize=address"
where asan instrumentation causes the prologue end to be computed
incorrectly: findPrologueEndLoc, looks for the first instruction
with a debug location to determine the prologue end. Since the asan
instrumentation instructions had debug locations, that prologue end was
at some instruction, where the stack frame is still being set up.
There seems to be no good reason for extra debug locations for the
asan instrumentations that set up the frame; they don't have a natural
source location. In the debugger they are simply located at the start
of the function.
For certain other instrumentations like -fsanitize-coverage=trace-pc-guard
the same problem persists - that might be more work to fix, since it
looks like they rely on locations of the tracee functions.
This partly reverts aaf4bb239487e0a3b20a8eaf94fc641235ba2c29
"[asan] Set debug location in ASan function prologue"
whose motivation was to give debug location info to the coverage callback.
Its test only ensures that the call to @__sanitizer_cov_trace_pc_guard is
given the correct source location; as the debug location is still set in
ModuleSanitizerCoverage::InjectCoverageAtBlock, the test does not break.
So -fsanitize-coverage is hopefully unaffected - I don't think it should
rely on the debug locations of asan-generated allocas.
Related revision: 3c6c14d14b40adfb581940859ede1ac7d8ceae7a
"ASAN: Provide reliable debug info for local variables at -O0."
Below is how the X86 assembly version of the added test case changes.
We get rid of some .loc lines and put prologue_end where the user code starts.
```diff
--- 2.master.s 2019-12-02 12:32:38.982959053 +0100
+++ 2.patch.s 2019-12-02 12:32:41.106246674 +0100
@@ -45,8 +45,6 @@
.cfi_offset %rbx, -24
xorl %eax, %eax
movl %eax, %ecx
- .Ltmp2:
- .loc 1 3 0 prologue_end # 2.c:3:0
cmpl $0, __asan_option_detect_stack_use_after_return
movl %edi, 92(%rbx) # 4-byte Spill
movq %rsi, 80(%rbx) # 8-byte Spill
@@ -57,9 +55,7 @@
callq __asan_stack_malloc_0
movq %rax, 72(%rbx) # 8-byte Spill
.LBB1_2:
- .loc 1 0 0 is_stmt 0 # 2.c:0:0
movq 72(%rbx), %rax # 8-byte Reload
- .loc 1 3 0 # 2.c:3:0
cmpq $0, %rax
movq %rax, %rcx
movq %rax, 64(%rbx) # 8-byte Spill
@@ -72,9 +68,7 @@
movq %rax, %rsp
movq %rax, 56(%rbx) # 8-byte Spill
.LBB1_4:
- .loc 1 0 0 # 2.c:0:0
movq 56(%rbx), %rax # 8-byte Reload
- .loc 1 3 0 # 2.c:3:0
movq %rax, 120(%rbx)
movq %rax, %rcx
addq $32, %rcx
@@ -99,7 +93,6 @@
movb %r8b, 31(%rbx) # 1-byte Spill
je .LBB1_7
# %bb.5:
- .loc 1 0 0 # 2.c:0:0
movq 40(%rbx), %rax # 8-byte Reload
andq $7, %rax
addq $3, %rax
@@ -118,7 +111,8 @@
movl %ecx, (%rax)
movq 80(%rbx), %rdx # 8-byte Reload
movq %rdx, 128(%rbx)
- .loc 1 4 3 is_stmt 1 # 2.c:4:3
+.Ltmp2:
+ .loc 1 4 3 prologue_end # 2.c:4:3
movq %rax, %rdi
callq f
movq 48(%rbx), %rax # 8-byte Reload
```
Reviewers: eugenis, aprantl
Reviewed By: eugenis
Subscribers: ormris, aprantl, hiraditya, llvm-commits
Tags: #llvm
Differential Revision: https://reviews.llvm.org/D70894
Added:
llvm/test/Instrumentation/AddressSanitizer/debug-info-alloca.ll
Modified:
llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp
llvm/test/Instrumentation/AddressSanitizer/local_stack_base.ll
Removed:
################################################################################
diff --git a/llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp b/llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp
index 831fdedfc5e5..c7e708127a41 100644
--- a/llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp
+++ b/llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp
@@ -2996,7 +2996,6 @@ void FunctionStackPoisoner::processStaticAllocas() {
Instruction *InsBefore = AllocaVec[0];
IRBuilder<> IRB(InsBefore);
- IRB.SetCurrentDebugLocation(EntryDebugLocation);
// Make sure non-instrumented allocas stay in the entry block. Otherwise,
// debug info is broken, because only entry-block allocas are treated as
@@ -3091,14 +3090,12 @@ void FunctionStackPoisoner::processStaticAllocas() {
Instruction *Term =
SplitBlockAndInsertIfThen(UseAfterReturnIsEnabled, InsBefore, false);
IRBuilder<> IRBIf(Term);
- IRBIf.SetCurrentDebugLocation(EntryDebugLocation);
StackMallocIdx = StackMallocSizeClass(LocalStackSize);
assert(StackMallocIdx <= kMaxAsanStackMallocSizeClass);
Value *FakeStackValue =
IRBIf.CreateCall(AsanStackMallocFunc[StackMallocIdx],
ConstantInt::get(IntptrTy, LocalStackSize));
IRB.SetInsertPoint(InsBefore);
- IRB.SetCurrentDebugLocation(EntryDebugLocation);
FakeStack = createPHI(IRB, UseAfterReturnIsEnabled, FakeStackValue, Term,
ConstantInt::get(IntptrTy, 0));
@@ -3106,14 +3103,11 @@ void FunctionStackPoisoner::processStaticAllocas() {
IRB.CreateICmpEQ(FakeStack, Constant::getNullValue(IntptrTy));
Term = SplitBlockAndInsertIfThen(NoFakeStack, InsBefore, false);
IRBIf.SetInsertPoint(Term);
- IRBIf.SetCurrentDebugLocation(EntryDebugLocation);
Value *AllocaValue =
DoDynamicAlloca ? createAllocaForLayout(IRBIf, L, true) : StaticAlloca;
IRB.SetInsertPoint(InsBefore);
- IRB.SetCurrentDebugLocation(EntryDebugLocation);
LocalStackBase = createPHI(IRB, NoFakeStack, AllocaValue, Term, FakeStack);
- IRB.SetCurrentDebugLocation(EntryDebugLocation);
IRB.CreateStore(LocalStackBase, LocalStackBaseAlloca);
DIExprFlags |= DIExpression::DerefBefore;
} else {
diff --git a/llvm/test/Instrumentation/AddressSanitizer/debug-info-alloca.ll b/llvm/test/Instrumentation/AddressSanitizer/debug-info-alloca.ll
new file mode 100644
index 000000000000..ba148e8d6e7a
--- /dev/null
+++ b/llvm/test/Instrumentation/AddressSanitizer/debug-info-alloca.ll
@@ -0,0 +1,75 @@
+; Checks that asan prologue does not add debug locations, which would
+; fool findPrologueEndLoc because it sets the end of the prologue to the
+; first instruction. Breaking on the instrumented function in a debugger
+; would then stop at that instruction, before the prologue is finished.
+
+; RUN: opt < %s -asan -asan-module -S | FileCheck %s
+; 1: void f(int *arg) {
+; 2: }
+; 3: int main(int argc, char **argv) {
+; 4: f(&argc);
+; 5: }
+; clang 1.cc -g -S -emit-llvm -o - | sed 's/#0 = {/#0 = { sanitize_address/'
+
+target datalayout = "e-m:e-p270:32:32-p271:32:32-p272:64:64-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-unknown-linux-gnu"
+
+define dso_local i32 @main(i32 %argc, i8** %argv) #0 !dbg !15 {
+entry:
+; No suffix like !dbg !123
+; CHECK: %asan_local_stack_base = alloca i64{{$}}
+; CHECK: %3 = call i64 @__asan_stack_malloc_0(i64 64){{$}}
+ %argc.addr = alloca i32, align 4
+ %argv.addr = alloca i8**, align 8
+ store i32 %argc, i32* %argc.addr, align 4
+ call void @llvm.dbg.declare(metadata i32* %argc.addr, metadata !21, metadata !DIExpression()), !dbg !22
+ store i8** %argv, i8*** %argv.addr, align 8
+ call void @llvm.dbg.declare(metadata i8*** %argv.addr, metadata !23, metadata !DIExpression()), !dbg !24
+ call void @f(i32* %argc.addr), !dbg !25
+ ret i32 0, !dbg !26
+}
+
+define dso_local void @f(i32* %arg) #0 !dbg !7 {
+entry:
+ %arg.addr = alloca i32*, align 8
+ store i32* %arg, i32** %arg.addr, align 8
+ call void @llvm.dbg.declare(metadata i32** %arg.addr, metadata !12, metadata !DIExpression()), !dbg !13
+ ret void, !dbg !14
+}
+
+declare void @llvm.dbg.declare(metadata, metadata, metadata) #1
+
+attributes #0 = { sanitize_address noinline nounwind optnone uwtable "correctly-rounded-divide-sqrt-fp-math"="false" "disable-tail-calls"="false" "frame-pointer"="all" "less-precise-fpmad"="false" "min-legal-vector-width"="0" "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-cpu"="x86-64" "target-features"="+cx8,+fxsr,+mmx,+sse,+sse2,+x87" "unsafe-fp-math"="false" "use-soft-float"="false" }
+attributes #1 = { nounwind readnone speculatable willreturn }
+
+!llvm.dbg.cu = !{!0}
+!llvm.module.flags = !{!3, !4, !5}
+!llvm.ident = !{!6}
+
+!0 = distinct !DICompileUnit(language: DW_LANG_C99, file: !1, producer: "clang version 10.0.0 (git at github.com:llvm/llvm-project 1ac700cdef787383ad49a0e37d9894491ef19480)", isOptimized: false, runtimeVersion: 0, emissionKind: FullDebug, enums: !2, nameTableKind: None)
+!1 = !DIFile(filename: "2.c", directory: "/home/builduser")
+!2 = !{}
+!3 = !{i32 7, !"Dwarf Version", i32 4}
+!4 = !{i32 2, !"Debug Info Version", i32 3}
+!5 = !{i32 1, !"wchar_size", i32 4}
+!6 = !{!"clang version 10.0.0 (git at github.com:llvm/llvm-project 1ac700cdef787383ad49a0e37d9894491ef19480)"}
+!7 = distinct !DISubprogram(name: "f", scope: !1, file: !1, line: 1, type: !8, scopeLine: 1, flags: DIFlagPrototyped, spFlags: DISPFlagDefinition, unit: !0, retainedNodes: !2)
+!8 = !DISubroutineType(types: !9)
+!9 = !{null, !10}
+!10 = !DIDerivedType(tag: DW_TAG_pointer_type, baseType: !11, size: 64)
+!11 = !DIBasicType(name: "int", size: 32, encoding: DW_ATE_signed)
+!12 = !DILocalVariable(name: "arg", arg: 1, scope: !7, file: !1, line: 1, type: !10)
+!13 = !DILocation(line: 1, column: 13, scope: !7)
+!14 = !DILocation(line: 2, column: 1, scope: !7)
+!15 = distinct !DISubprogram(name: "main", scope: !1, file: !1, line: 3, type: !16, scopeLine: 3, flags: DIFlagPrototyped, spFlags: DISPFlagDefinition, unit: !0, retainedNodes: !2)
+!16 = !DISubroutineType(types: !17)
+!17 = !{!11, !11, !18}
+!18 = !DIDerivedType(tag: DW_TAG_pointer_type, baseType: !19, size: 64)
+!19 = !DIDerivedType(tag: DW_TAG_pointer_type, baseType: !20, size: 64)
+!20 = !DIBasicType(name: "char", size: 8, encoding: DW_ATE_signed_char)
+!21 = !DILocalVariable(name: "argc", arg: 1, scope: !15, file: !1, line: 3, type: !11)
+!22 = !DILocation(line: 3, column: 14, scope: !15)
+!23 = !DILocalVariable(name: "argv", arg: 2, scope: !15, file: !1, line: 3, type: !18)
+!24 = !DILocation(line: 3, column: 27, scope: !15)
+!25 = !DILocation(line: 4, column: 3, scope: !15)
+!26 = !DILocation(line: 5, column: 1, scope: !15)
diff --git a/llvm/test/Instrumentation/AddressSanitizer/local_stack_base.ll b/llvm/test/Instrumentation/AddressSanitizer/local_stack_base.ll
index ad3a274c8272..67e13e56414f 100644
--- a/llvm/test/Instrumentation/AddressSanitizer/local_stack_base.ll
+++ b/llvm/test/Instrumentation/AddressSanitizer/local_stack_base.ll
@@ -18,8 +18,8 @@ entry:
; CHECK: %asan_local_stack_base = alloca i64
; CHECK: %[[ALLOCA:.*]] = ptrtoint i8* %MyAlloca to i64
; CHECK: %[[PHI:.*]] = phi i64 {{.*}} %[[ALLOCA]],
- ; CHECK: store i64 %[[PHI]], i64* %asan_local_stack_base, !dbg
- ; CHECK: call void @llvm.dbg.declare(metadata i64* %asan_local_stack_base, metadata !13, metadata !DIExpression(DW_OP_deref, DW_OP_plus_uconst, 32)), !dbg !14
+ ; CHECK: store i64 %[[PHI]], i64* %asan_local_stack_base
+ ; CHECK: call void @llvm.dbg.declare(metadata i64* %asan_local_stack_base, metadata !12, metadata !DIExpression(DW_OP_deref, DW_OP_plus_uconst, 32)), !dbg !13
%0 = load i32, i32* %i.addr, align 4, !dbg !14
%add = add nsw i32 %0, 2, !dbg !15
ret i32 %add, !dbg !16
More information about the llvm-commits
mailing list