[PATCH] D73513: [memtag] Plug in stack safety analysis.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG2a3723ef114d: [memtag] Plug in stack safety analysis.
(authored by eugenis).
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D73513/new/
https://reviews.llvm.org/D73513
Files:
clang/lib/CodeGen/BackendUtil.cpp
clang/test/Driver/memtag.c
llvm/include/llvm/Analysis/StackSafetyAnalysis.h
llvm/lib/Analysis/StackSafetyAnalysis.cpp
llvm/lib/Passes/PassRegistry.def
llvm/lib/Target/AArch64/AArch64StackTagging.cpp
llvm/test/Analysis/StackSafetyAnalysis/ipa-attr.ll
llvm/test/CodeGen/AArch64/stack-tagging.ll
Index: llvm/test/CodeGen/AArch64/stack-tagging.ll
===
--- llvm/test/CodeGen/AArch64/stack-tagging.ll
+++ llvm/test/CodeGen/AArch64/stack-tagging.ll
@@ -33,6 +33,7 @@
%x1 = alloca i32, align 4
%x2 = alloca i8, align 4
%x3 = alloca i32, i32 11, align 4
+ %x4 = alloca i32, align 4, !stack-safe !0
call void @use32(i32* %x1)
call void @use8(i8* %x2)
call void @use32(i32* %x3)
@@ -49,6 +50,9 @@
; CHECK: alloca { [11 x i32], [4 x i8] }, align 16
; CHECK: call { [11 x i32], [4 x i8] }* @llvm.aarch64.tagp.{{.*}}({ [11 x i32], [4 x i8] }* {{.*}}, i64 2)
; CHECK: call void @llvm.aarch64.settag(i8* {{.*}}, i64 48)
+; CHECK: alloca i32, align 4
+; CHECK-NOT: @llvm.aarch64.tagp
+; CHECK-NOT: @llvm.aarch64.settag
; CHECK: call void @use32(
; CHECK: call void @use8(
@@ -185,3 +189,5 @@
; CHECK: call void @llvm.aarch64.settag(
; CHECK: call void @llvm.aarch64.settag(
; CHECK: ret void
+
+!0 = !{}
\ No newline at end of file
Index: llvm/test/Analysis/StackSafetyAnalysis/ipa-attr.ll
===
--- /dev/null
+++ llvm/test/Analysis/StackSafetyAnalysis/ipa-attr.ll
@@ -0,0 +1,34 @@
+; RUN: llvm-as %s -o %t0.bc
+; RUN: llvm-as %S/Inputs/ipa.ll -o %t1.bc
+; RUN: llvm-link -disable-lazy-loading %t0.bc %t1.bc -o %t.combined.bc
+; RUN: opt -S -passes="stack-safety-annotator" %t.combined.bc -o - 2>&1 | FileCheck %s
+
+target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-unknown-linux-gnu"
+
+declare void @Write1(i8* %p)
+declare void @Write8(i8* %p)
+
+; Basic out-of-bounds.
+define void @f1() {
+; CHECK-LABEL: define void @f1() {
+; CHECK: alloca i32, align 4{{$}}
+entry:
+ %x = alloca i32, align 4
+ %x1 = bitcast i32* %x to i8*
+ call void @Write8(i8* %x1)
+ ret void
+}
+
+; Basic in-bounds.
+define void @f2() {
+; CHECK-LABEL: define void @f2() {
+; CHECK: alloca i32, align 4, !stack-safe ![[A:[0-9]+]]{{$}}
+entry:
+ %x = alloca i32, align 4
+ %x1 = bitcast i32* %x to i8*
+ call void @Write1(i8* %x1)
+ ret void
+}
+
+; CHECK: ![[A]] = !{}
Index: llvm/lib/Target/AArch64/AArch64StackTagging.cpp
===
--- llvm/lib/Target/AArch64/AArch64StackTagging.cpp
+++ llvm/lib/Target/AArch64/AArch64StackTagging.cpp
@@ -400,7 +400,9 @@
// dynamic alloca instrumentation for them as well.
!AI.isUsedWithInAlloca() &&
// swifterror allocas are register promoted by ISel
- !AI.isSwiftError();
+ !AI.isSwiftError() &&
+ // safe allocas are not interesting
+ !AI.getMetadata("stack-safe");
return IsInteresting;
}
Index: llvm/lib/Passes/PassRegistry.def
===
--- llvm/lib/Passes/PassRegistry.def
+++ llvm/lib/Passes/PassRegistry.def
@@ -92,6 +92,7 @@
MODULE_PASS("kasan-module", ModuleAddressSanitizerPass(/*CompileKernel=*/true, false, true, false))
MODULE_PASS("sancov-module", ModuleSanitizerCoveragePass())
MODULE_PASS("poison-checking", PoisonCheckingPass())
+MODULE_PASS("stack-safety-annotator", StackSafetyGlobalAnnotatorPass())
#undef MODULE_PASS
#ifndef CGSCC_ANALYSIS
Index: llvm/lib/Analysis/StackSafetyAnalysis.cpp
===
--- llvm/lib/Analysis/StackSafetyAnalysis.cpp
+++ llvm/lib/Analysis/StackSafetyAnalysis.cpp
@@ -99,11 +99,11 @@
}
struct AllocaInfo {
- const AllocaInst *AI = nullptr;
+ AllocaInst *AI = nullptr;
uint64_t Size = 0;
UseInfo Use;
- AllocaInfo(unsigned PointerSize, const AllocaInst *AI, uint64_t Size)
+ AllocaInfo(unsigned PointerSize, AllocaInst *AI, uint64_t Size)
: AI(AI), Size(Size), Use(PointerSize) {}
StringRef getName() const { return AI->getName(); }
@@ -205,7 +205,7 @@
namespace {
class StackSafetyLocalAnalysis {
- const Function
+ Function
const DataLayout
ScalarEvolution
unsigned PointerSize = 0;
@@ -227,7 +227,7 @@
}
public:
- StackSafetyLocalAnalysis(const Function , ScalarEvolution )
+ StackSafetyLocalAnalysis(Function , ScalarEvolution )
: F(F), DL(F.getParent()->getDataLayout()), SE(SE),
PointerSize(DL.getPointerSizeInBits()),
UnknownRange(PointerSize, true) {}
@@
[PATCH] D73513: [memtag] Plug in stack safety analysis.
eugenis updated this revision to Diff 249175.
eugenis added a comment.
Herald added a subscriber: danielkiss.
+ a test
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D73513/new/
https://reviews.llvm.org/D73513
Files:
clang/lib/CodeGen/BackendUtil.cpp
clang/test/Driver/memtag.c
llvm/include/llvm/Analysis/StackSafetyAnalysis.h
llvm/lib/Analysis/StackSafetyAnalysis.cpp
llvm/lib/Passes/PassRegistry.def
llvm/lib/Target/AArch64/AArch64StackTagging.cpp
llvm/test/Analysis/StackSafetyAnalysis/ipa-attr.ll
llvm/test/CodeGen/AArch64/stack-tagging.ll
Index: llvm/test/CodeGen/AArch64/stack-tagging.ll
===
--- llvm/test/CodeGen/AArch64/stack-tagging.ll
+++ llvm/test/CodeGen/AArch64/stack-tagging.ll
@@ -33,6 +33,7 @@
%x1 = alloca i32, align 4
%x2 = alloca i8, align 4
%x3 = alloca i32, i32 11, align 4
+ %x4 = alloca i32, align 4, !stack-safe !0
call void @use32(i32* %x1)
call void @use8(i8* %x2)
call void @use32(i32* %x3)
@@ -49,6 +50,9 @@
; CHECK: alloca { [11 x i32], [4 x i8] }, align 16
; CHECK: call { [11 x i32], [4 x i8] }* @llvm.aarch64.tagp.{{.*}}({ [11 x i32], [4 x i8] }* {{.*}}, i64 2)
; CHECK: call void @llvm.aarch64.settag(i8* {{.*}}, i64 48)
+; CHECK: alloca i32, align 4
+; CHECK-NOT: @llvm.aarch64.tagp
+; CHECK-NOT: @llvm.aarch64.settag
; CHECK: call void @use32(
; CHECK: call void @use8(
@@ -185,3 +189,5 @@
; CHECK: call void @llvm.aarch64.settag(
; CHECK: call void @llvm.aarch64.settag(
; CHECK: ret void
+
+!0 = !{}
\ No newline at end of file
Index: llvm/test/Analysis/StackSafetyAnalysis/ipa-attr.ll
===
--- /dev/null
+++ llvm/test/Analysis/StackSafetyAnalysis/ipa-attr.ll
@@ -0,0 +1,34 @@
+; RUN: llvm-as %s -o %t0.bc
+; RUN: llvm-as %S/Inputs/ipa.ll -o %t1.bc
+; RUN: llvm-link -disable-lazy-loading %t0.bc %t1.bc -o %t.combined.bc
+; RUN: opt -S -passes="stack-safety-annotator" %t.combined.bc -o - 2>&1 | FileCheck %s
+
+target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-unknown-linux-gnu"
+
+declare void @Write1(i8* %p)
+declare void @Write8(i8* %p)
+
+; Basic out-of-bounds.
+define void @f1() {
+; CHECK-LABEL: define void @f1() {
+; CHECK: alloca i32, align 4{{$}}
+entry:
+ %x = alloca i32, align 4
+ %x1 = bitcast i32* %x to i8*
+ call void @Write8(i8* %x1)
+ ret void
+}
+
+; Basic in-bounds.
+define void @f2() {
+; CHECK-LABEL: define void @f2() {
+; CHECK: alloca i32, align 4, !stack-safe ![[A:[0-9]+]]{{$}}
+entry:
+ %x = alloca i32, align 4
+ %x1 = bitcast i32* %x to i8*
+ call void @Write1(i8* %x1)
+ ret void
+}
+
+; CHECK: ![[A]] = !{}
Index: llvm/lib/Target/AArch64/AArch64StackTagging.cpp
===
--- llvm/lib/Target/AArch64/AArch64StackTagging.cpp
+++ llvm/lib/Target/AArch64/AArch64StackTagging.cpp
@@ -400,7 +400,9 @@
// dynamic alloca instrumentation for them as well.
!AI.isUsedWithInAlloca() &&
// swifterror allocas are register promoted by ISel
- !AI.isSwiftError();
+ !AI.isSwiftError() &&
+ // safe allocas are not interesting
+ !AI.getMetadata("stack-safe");
return IsInteresting;
}
Index: llvm/lib/Passes/PassRegistry.def
===
--- llvm/lib/Passes/PassRegistry.def
+++ llvm/lib/Passes/PassRegistry.def
@@ -92,6 +92,7 @@
MODULE_PASS("kasan-module", ModuleAddressSanitizerPass(/*CompileKernel=*/true, false, true, false))
MODULE_PASS("sancov-module", ModuleSanitizerCoveragePass())
MODULE_PASS("poison-checking", PoisonCheckingPass())
+MODULE_PASS("stack-safety-annotator", StackSafetyGlobalAnnotatorPass())
#undef MODULE_PASS
#ifndef CGSCC_ANALYSIS
Index: llvm/lib/Analysis/StackSafetyAnalysis.cpp
===
--- llvm/lib/Analysis/StackSafetyAnalysis.cpp
+++ llvm/lib/Analysis/StackSafetyAnalysis.cpp
@@ -99,11 +99,11 @@
}
struct AllocaInfo {
- const AllocaInst *AI = nullptr;
+ AllocaInst *AI = nullptr;
uint64_t Size = 0;
UseInfo Use;
- AllocaInfo(unsigned PointerSize, const AllocaInst *AI, uint64_t Size)
+ AllocaInfo(unsigned PointerSize, AllocaInst *AI, uint64_t Size)
: AI(AI), Size(Size), Use(PointerSize) {}
StringRef getName() const { return AI->getName(); }
@@ -205,7 +205,7 @@
namespace {
class StackSafetyLocalAnalysis {
- const Function
+ Function
const DataLayout
ScalarEvolution
unsigned PointerSize = 0;
@@ -227,7 +227,7 @@
}
public:
- StackSafetyLocalAnalysis(const Function , ScalarEvolution )
+ StackSafetyLocalAnalysis(Function , ScalarEvolution )
: F(F), DL(F.getParent()->getDataLayout()), SE(SE),
PointerSize(DL.getPointerSizeInBits()),
UnknownRange(PointerSize, true) {}
@@ -653,17 +653,47 @@
return PreservedAnalyses::all();
[PATCH] D73513: [memtag] Plug in stack safety analysis.
vitalybuka accepted this revision.
vitalybuka added inline comments.
This revision is now accepted and ready to land.
Comment at: llvm/lib/Analysis/StackSafetyAnalysis.cpp:670
+ if (AllocaRange.contains(AS.Use.Range)) {
+AS.AI->setMetadata(M.getMDKindID("stack-safe"),
+ MDNode::get(M.getContext(), None));
Could you please add basic test in StackSafetyAnalysis to checks "stack-safe"
in IR?
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D73513/new/
https://reviews.llvm.org/D73513
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D73513: [memtag] Plug in stack safety analysis.
merge_guards_bot added a comment.
{icon check-circle color=green} Unit tests: pass. 62250 tests passed, 0 failed
and 816 were skipped.
{icon times-circle color=red} clang-tidy: fail. clang-tidy found 0 errors and 1
warnings
.
0 of them are added as review comments below (why?
).
{icon times-circle color=red} clang-format: fail. Please format your changes
with clang-format by running `git-clang-format HEAD^` or applying this patch
.
Build artifacts
:
diff.json
,
clang-tidy.txt
,
clang-format.patch
,
CMakeCache.txt
,
console-log.txt
,
test-results.xml
//Pre-merge checks is in beta. Report issue
.
Please join beta .//
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D73513/new/
https://reviews.llvm.org/D73513
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D73513: [memtag] Plug in stack safety analysis.
eugenis added a comment. To explain some design decisions: The analysis is added to the IR pipeline and not to the AArch64 codegen pipeline mainly because we plan to extend it with ThinLTO support in the future. To do that, module summary builder will need to depend on the (function-)local stack safety analysis pass, and the global analysis result will be provided by the combined summary reader, and all of that must happen in the target-independent IR pipeline. Another reason is that the legacy pass manager, which is still used for codegen, is not good at mixing function and module analyses. AFAIK a module pass that depends on function analysis will always produce an on-the-fly pass manager which can not reuse earlier analysis results; and a function pass can not depend on a module analysis at all (at least I could not make it work). For these two reasons I opted not to use analysis manager framework to pass the safety information to AArch64StackTaggingPass, and chose to use alloca metadata instead. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D73513/new/ https://reviews.llvm.org/D73513 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D73513: [memtag] Plug in stack safety analysis.
eugenis created this revision.
eugenis added reviewers: pcc, vitalybuka, ostannard.
Herald added subscribers: cfe-commits, hiraditya, kristof.beyls.
Herald added projects: clang, LLVM.
Run StackSafetyAnalysis at the end of the IR pipeline and annotate
proven safe allocas with !stack-safe metadata. Do not instrument such
allocas in the AArch64StackTagging pass.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D73513
Files:
clang/lib/CodeGen/BackendUtil.cpp
clang/test/Driver/memtag.c
llvm/include/llvm/Analysis/StackSafetyAnalysis.h
llvm/lib/Analysis/StackSafetyAnalysis.cpp
llvm/lib/Target/AArch64/AArch64StackTagging.cpp
llvm/test/CodeGen/AArch64/stack-tagging.ll
Index: llvm/test/CodeGen/AArch64/stack-tagging.ll
===
--- llvm/test/CodeGen/AArch64/stack-tagging.ll
+++ llvm/test/CodeGen/AArch64/stack-tagging.ll
@@ -33,6 +33,7 @@
%x1 = alloca i32, align 4
%x2 = alloca i8, align 4
%x3 = alloca i32, i32 11, align 4
+ %x4 = alloca i32, align 4, !stack-safe !0
call void @use32(i32* %x1)
call void @use8(i8* %x2)
call void @use32(i32* %x3)
@@ -49,6 +50,9 @@
; CHECK: alloca { [11 x i32], [4 x i8] }, align 16
; CHECK: call { [11 x i32], [4 x i8] }* @llvm.aarch64.tagp.{{.*}}({ [11 x i32], [4 x i8] }* {{.*}}, i64 2)
; CHECK: call void @llvm.aarch64.settag(i8* {{.*}}, i64 48)
+; CHECK: alloca i32, align 4
+; CHECK-NOT: @llvm.aarch64.tagp
+; CHECK-NOT: @llvm.aarch64.settag
; CHECK: call void @use32(
; CHECK: call void @use8(
@@ -185,3 +189,5 @@
; CHECK: call void @llvm.aarch64.settag(
; CHECK: call void @llvm.aarch64.settag(
; CHECK: ret void
+
+!0 = !{}
\ No newline at end of file
Index: llvm/lib/Target/AArch64/AArch64StackTagging.cpp
===
--- llvm/lib/Target/AArch64/AArch64StackTagging.cpp
+++ llvm/lib/Target/AArch64/AArch64StackTagging.cpp
@@ -400,7 +400,9 @@
// dynamic alloca instrumentation for them as well.
!AI.isUsedWithInAlloca() &&
// swifterror allocas are register promoted by ISel
- !AI.isSwiftError();
+ !AI.isSwiftError() &&
+ // safe allocas are not interesting
+ !AI.getMetadata("stack-safe");
return IsInteresting;
}
Index: llvm/lib/Analysis/StackSafetyAnalysis.cpp
===
--- llvm/lib/Analysis/StackSafetyAnalysis.cpp
+++ llvm/lib/Analysis/StackSafetyAnalysis.cpp
@@ -99,11 +99,11 @@
}
struct AllocaInfo {
- const AllocaInst *AI = nullptr;
+ AllocaInst *AI = nullptr;
uint64_t Size = 0;
UseInfo Use;
- AllocaInfo(unsigned PointerSize, const AllocaInst *AI, uint64_t Size)
+ AllocaInfo(unsigned PointerSize, AllocaInst *AI, uint64_t Size)
: AI(AI), Size(Size), Use(PointerSize) {}
StringRef getName() const { return AI->getName(); }
@@ -205,7 +205,7 @@
namespace {
class StackSafetyLocalAnalysis {
- const Function
+ Function
const DataLayout
ScalarEvolution
unsigned PointerSize = 0;
@@ -227,7 +227,7 @@
}
public:
- StackSafetyLocalAnalysis(const Function , ScalarEvolution )
+ StackSafetyLocalAnalysis(Function , ScalarEvolution )
: F(F), DL(F.getParent()->getDataLayout()), SE(SE),
PointerSize(DL.getPointerSizeInBits()),
UnknownRange(PointerSize, true) {}
@@ -653,17 +653,47 @@
return PreservedAnalyses::all();
}
+static bool SetStackSafetyMetadata(Module ,
+ const StackSafetyGlobalInfo ) {
+ bool Changed = false;
+ unsigned Width = M.getDataLayout().getPointerSizeInBits();
+ for (auto : M.functions()) {
+if (F.isDeclaration() || F.hasOptNone())
+ continue;
+auto Iter = SSGI.find();
+if (Iter == SSGI.end())
+ continue;
+StackSafetyInfo::FunctionInfo *Summary = Iter->second.getInfo();
+for (auto : Summary->Allocas) {
+ ConstantRange AllocaRange{APInt(Width, 0), APInt(Width, AS.Size)};
+ if (AllocaRange.contains(AS.Use.Range)) {
+AS.AI->setMetadata(M.getMDKindID("stack-safe"),
+ MDNode::get(M.getContext(), None));
+Changed = true;
+ }
+}
+ }
+ return Changed;
+}
+
+PreservedAnalyses
+StackSafetyGlobalAnnotatorPass::run(Module , ModuleAnalysisManager ) {
+ auto = AM.getResult(M);
+ (void)SetStackSafetyMetadata(M, SSGI);
+ return PreservedAnalyses::all();
+}
+
char StackSafetyGlobalInfoWrapperPass::ID = 0;
-StackSafetyGlobalInfoWrapperPass::StackSafetyGlobalInfoWrapperPass()
-: ModulePass(ID) {
+StackSafetyGlobalInfoWrapperPass::StackSafetyGlobalInfoWrapperPass(bool SetMetadata)
+: ModulePass(ID), SetMetadata(SetMetadata) {
initializeStackSafetyGlobalInfoWrapperPassPass(
*PassRegistry::getPassRegistry());
}
void StackSafetyGlobalInfoWrapperPass::print(raw_ostream ,
const Module *M) const {
- ::print(SSI, O, *M);
+ ::print(SSGI, O, *M);
}
