-
Notifications
You must be signed in to change notification settings - Fork 15.2k
release/21.x: [sancov] Fix stack-depth tracking to use debug locations (#162428) #162697
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: release/21.x
Are you sure you want to change the base?
Conversation
As fixed in commits llvm/llvm-project@913f7e9, llvm/llvm-project@4a8b124, and llvm/llvm-project@4eef2e3, also fix the stack-depth tracking code to use InstrumentationIRBuilder, and set the Call's Debug location to EntryLoc. ClangBuiltLinux/linux#2125 cc @nathanchance @melver @JustinStitt @bwendling (cherry picked from commit 28b7f66)
@melver What do you think about merging this PR to the release branch? |
@llvm/pr-subscribers-compiler-rt-sanitizer @llvm/pr-subscribers-llvm-transforms Author: None (llvmbot) ChangesBackport 28b7f66 Requested by: @nathanchance Full diff: https://github.com/llvm/llvm-project/pull/162697.diff 2 Files Affected:
diff --git a/llvm/lib/Transforms/Instrumentation/SanitizerCoverage.cpp b/llvm/lib/Transforms/Instrumentation/SanitizerCoverage.cpp
index 5b8ea1547ca2f..b74a0708b67ae 100644
--- a/llvm/lib/Transforms/Instrumentation/SanitizerCoverage.cpp
+++ b/llvm/lib/Transforms/Instrumentation/SanitizerCoverage.cpp
@@ -1084,8 +1084,10 @@ void ModuleSanitizerCoverage::InjectCoverageAtBlock(Function &F, BasicBlock &BB,
auto ThenTerm = SplitBlockAndInsertIfThen(
IRB.CreateIsNull(Load), &*IP, false,
MDBuilder(IRB.getContext()).createUnlikelyBranchWeights());
- IRBuilder<> ThenIRB(ThenTerm);
+ InstrumentationIRBuilder ThenIRB(ThenTerm);
auto Store = ThenIRB.CreateStore(ConstantInt::getTrue(Int1Ty), FlagPtr);
+ if (EntryLoc)
+ Store->setDebugLoc(EntryLoc);
Load->setNoSanitizeMetadata();
Store->setNoSanitizeMetadata();
}
@@ -1131,7 +1133,10 @@ void ModuleSanitizerCoverage::InjectCoverageAtBlock(Function &F, BasicBlock &BB,
EstimatedStackSize >= Options.StackDepthCallbackMin) {
if (InsertBefore)
IRB.SetInsertPoint(InsertBefore);
- IRB.CreateCall(SanCovStackDepthCallback)->setCannotMerge();
+ auto Call = IRB.CreateCall(SanCovStackDepthCallback);
+ if (EntryLoc)
+ Call->setDebugLoc(EntryLoc);
+ Call->setCannotMerge();
}
} else {
// Check stack depth. If it's the deepest so far, record it.
@@ -1144,8 +1149,10 @@ void ModuleSanitizerCoverage::InjectCoverageAtBlock(Function &F, BasicBlock &BB,
auto ThenTerm = SplitBlockAndInsertIfThen(
IsStackLower, &*IP, false,
MDBuilder(IRB.getContext()).createUnlikelyBranchWeights());
- IRBuilder<> ThenIRB(ThenTerm);
+ InstrumentationIRBuilder ThenIRB(ThenTerm);
auto Store = ThenIRB.CreateStore(FrameAddrInt, SanCovLowestStack);
+ if (EntryLoc)
+ Store->setDebugLoc(EntryLoc);
LowestStack->setNoSanitizeMetadata();
Store->setNoSanitizeMetadata();
}
diff --git a/llvm/test/Instrumentation/SanitizerCoverage/missing_dbg.ll b/llvm/test/Instrumentation/SanitizerCoverage/missing_dbg.ll
index 35684346c4d5a..07b9a1ce496d9 100644
--- a/llvm/test/Instrumentation/SanitizerCoverage/missing_dbg.ll
+++ b/llvm/test/Instrumentation/SanitizerCoverage/missing_dbg.ll
@@ -1,5 +1,7 @@
; NOTE: Assertions have been autogenerated by utils/update_test_checks.py UTC_ARGS: --version 5
; RUN: opt < %s -passes='module(sancov-module)' -sanitizer-coverage-level=2 -S | FileCheck %s
+; RUN: opt < %s -passes='module(sancov-module)' -sanitizer-coverage-level=1 -sanitizer-coverage-stack-depth -sanitizer-coverage-stack-depth-callback-min=1 -S | FileCheck %s --check-prefix=CHECK-STACK-CALLBACK
+; RUN: opt < %s -passes='module(sancov-module)' -sanitizer-coverage-level=1 -sanitizer-coverage-stack-depth -S | FileCheck %s --check-prefix=CHECK-STACK-DEPTH
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"
@@ -55,6 +57,86 @@ entry:
ret i32 %t
}
+define i32 @with_dbg_stack_callback(ptr %a) !dbg !8 {
+; CHECK-STACK-CALLBACK-LABEL: define i32 @with_dbg_stack_callback(
+; CHECK-STACK-CALLBACK-SAME: ptr [[A:%.*]]) !dbg [[DBG8:![0-9]+]] {
+; CHECK-STACK-CALLBACK-NEXT: entry:
+; CHECK-STACK-CALLBACK-NEXT: [[BUF:%.*]] = alloca [64 x i8], align 1
+; CHECK-STACK-CALLBACK-NEXT: call void @__sanitizer_cov_stack_depth() #[[ATTR1:[0-9]+]], !dbg [[DBG9:![0-9]+]]
+; CHECK-STACK-CALLBACK-NEXT: %t = load i32, ptr [[A]], align 4
+; CHECK-STACK-CALLBACK-NEXT: call void @external_func()
+; CHECK-STACK-CALLBACK-NEXT: ret i32 %t
+;
+entry:
+ %buf = alloca [64 x i8], align 1
+ %t = load i32, ptr %a, align 4
+ call void @external_func()
+ ret i32 %t
+}
+
+define i32 @with_dbg_stack_depth(ptr %a) !dbg !10 {
+; CHECK-STACK-DEPTH-LABEL: define i32 @with_dbg_stack_depth(
+; CHECK-STACK-DEPTH-SAME: ptr [[A:%.*]]) !dbg [[DBG10:![0-9]+]] {
+; CHECK-STACK-DEPTH-NEXT: entry:
+; CHECK-STACK-DEPTH-NEXT: [[BUF:%.*]] = alloca [64 x i8], align 1
+; CHECK-STACK-DEPTH-NEXT: [[TMP1:%.*]] = call ptr @llvm.frameaddress.p0(i32 0)
+; CHECK-STACK-DEPTH-NEXT: [[TMP2:%.*]] = ptrtoint ptr [[TMP1]] to i64
+; CHECK-STACK-DEPTH-NEXT: [[TMP3:%.*]] = load i64, ptr @__sancov_lowest_stack, align 8
+; CHECK-STACK-DEPTH-NEXT: [[TMP4:%.*]] = icmp ult i64 [[TMP2]], [[TMP3]]
+; CHECK-STACK-DEPTH-NEXT: br i1 [[TMP4]], label {{%.*}}, label {{%.*}}
+; CHECK-STACK-DEPTH: store i64 [[TMP2]], ptr @__sancov_lowest_stack, align 8, !dbg [[DBG11:![0-9]+]], {{.*}}!nosanitize
+; CHECK-STACK-DEPTH: %t = load i32, ptr [[A]], align 4
+; CHECK-STACK-DEPTH-NEXT: call void @external_func()
+; CHECK-STACK-DEPTH-NEXT: ret i32 %t
+;
+entry:
+ %buf = alloca [64 x i8], align 1
+ %t = load i32, ptr %a, align 4
+ call void @external_func()
+ ret i32 %t
+}
+
+define i32 @without_dbg_stack_callback(ptr %a) {
+; CHECK-STACK-CALLBACK-LABEL: define i32 @without_dbg_stack_callback(
+; CHECK-STACK-CALLBACK-SAME: ptr [[A:%.*]]) {
+; CHECK-STACK-CALLBACK-NEXT: entry:
+; CHECK-STACK-CALLBACK-NEXT: [[BUF:%.*]] = alloca [64 x i8], align 1
+; CHECK-STACK-CALLBACK-NEXT: call void @__sanitizer_cov_stack_depth() #[[ATTR1]]
+; CHECK-STACK-CALLBACK-NEXT: %t = load i32, ptr [[A]], align 4
+; CHECK-STACK-CALLBACK-NEXT: call void @external_func()
+; CHECK-STACK-CALLBACK-NEXT: ret i32 %t
+;
+entry:
+ %buf = alloca [64 x i8], align 1
+ %t = load i32, ptr %a, align 4
+ call void @external_func()
+ ret i32 %t
+}
+
+define i32 @without_dbg_stack_depth(ptr %a) {
+; CHECK-STACK-DEPTH-LABEL: define i32 @without_dbg_stack_depth(
+; CHECK-STACK-DEPTH-SAME: ptr [[A:%.*]]) {
+; CHECK-STACK-DEPTH-NEXT: entry:
+; CHECK-STACK-DEPTH-NEXT: [[BUF:%.*]] = alloca [64 x i8], align 1
+; CHECK-STACK-DEPTH-NEXT: [[TMP1:%.*]] = call ptr @llvm.frameaddress.p0(i32 0)
+; CHECK-STACK-DEPTH-NEXT: [[TMP2:%.*]] = ptrtoint ptr [[TMP1]] to i64
+; CHECK-STACK-DEPTH-NEXT: [[TMP3:%.*]] = load i64, ptr @__sancov_lowest_stack, align 8
+; CHECK-STACK-DEPTH-NEXT: [[TMP4:%.*]] = icmp ult i64 [[TMP2]], [[TMP3]]
+; CHECK-STACK-DEPTH-NEXT: br i1 [[TMP4]], label {{%.*}}, label {{%.*}}
+; CHECK-STACK-DEPTH: store i64 [[TMP2]], ptr @__sancov_lowest_stack, align 8, {{.*}}!nosanitize
+; CHECK-STACK-DEPTH: %t = load i32, ptr [[A]], align 4
+; CHECK-STACK-DEPTH-NEXT: call void @external_func()
+; CHECK-STACK-DEPTH-NEXT: ret i32 %t
+;
+entry:
+ %buf = alloca [64 x i8], align 1
+ %t = load i32, ptr %a, align 4
+ call void @external_func()
+ ret i32 %t
+}
+
+declare void @external_func()
+
!llvm.dbg.cu = !{!0}
!llvm.module.flags = !{!2}
@@ -66,6 +148,10 @@ entry:
!5 = !{}
!6 = !DILocation(line: 192, scope: !3)
!7 = !DILocation(line: 0, scope: !3)
+!8 = distinct !DISubprogram(name: "with_dbg_stack_callback", scope: !1, file: !1, line: 200, type: !4, scopeLine: 200, flags: DIFlagPrototyped | DIFlagAllCallsDescribed, spFlags: DISPFlagLocalToUnit | DISPFlagDefinition | DISPFlagOptimized, unit: !0)
+!9 = !DILocation(line: 200, scope: !8)
+!10 = distinct !DISubprogram(name: "with_dbg_stack_depth", scope: !1, file: !1, line: 210, type: !4, scopeLine: 210, flags: DIFlagPrototyped | DIFlagAllCallsDescribed, spFlags: DISPFlagLocalToUnit | DISPFlagDefinition | DISPFlagOptimized, unit: !0)
+!11 = !DILocation(line: 210, scope: !10)
;.
; CHECK: [[META0:![0-9]+]] = distinct !DICompileUnit(language: DW_LANG_C89, file: [[META1:![0-9]+]], isOptimized: true, runtimeVersion: 0, emissionKind: LineTablesOnly, splitDebugInlining: false, nameTableKind: None)
@@ -76,3 +162,9 @@ entry:
; CHECK: [[DBG6]] = !DILocation(line: 192, scope: [[DBG3]])
; CHECK: [[DBG7]] = !DILocation(line: 0, scope: [[DBG3]])
;.
+; CHECK-STACK-CALLBACK: [[DBG8]] = distinct !DISubprogram(name: "with_dbg_stack_callback", scope: {{.*}}, file: {{.*}}, line: 200
+; CHECK-STACK-CALLBACK: [[DBG9]] = !DILocation(line: 200, scope: [[DBG8]])
+;.
+; CHECK-STACK-DEPTH: [[DBG10]] = distinct !DISubprogram(name: "with_dbg_stack_depth", scope: {{.*}}, file: {{.*}}, line: 210
+; CHECK-STACK-DEPTH: [[DBG11]] = !DILocation(line: 210, scope: [[DBG10]])
+;.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
This is safe, and should strictly be a bug fix. AFAIK without it using that feature might result in compiler error/crash otherwise.
Backport 28b7f66
Requested by: @nathanchance