[llvm] 5addb73 - sanmd: improve precision of UAR analysis
Dmitry Vyukov via llvm-commits
llvm-commits at lists.llvm.org
Mon Dec 12 02:42:25 PST 2022
Author: Dmitry Vyukov
Date: 2022-12-12T11:41:59+01:00
New Revision: 5addb736a9a8fd669c5daa80bd98e8d73b584e73
URL: https://github.com/llvm/llvm-project/commit/5addb736a9a8fd669c5daa80bd98e8d73b584e73
DIFF: https://github.com/llvm/llvm-project/commit/5addb736a9a8fd669c5daa80bd98e8d73b584e73.diff
LOG: sanmd: improve precision of UAR analysis
Only mark functions that have address-taken locals
as requiring UAR checking.
On a large internal app this reduces number of marked functions
from 78441 to 66618. Mostly small, trivial getter/setter-type
functions are unmarked, but also some amount of larger
number-crunching-type functions are unmarked as well.
Reviewed By: melver
Differential Revision: https://reviews.llvm.org/D139811
Added:
Modified:
compiler-rt/test/metadata/covered.cpp
compiler-rt/test/metadata/uar.cpp
llvm/lib/Transforms/Instrumentation/SanitizerBinaryMetadata.cpp
Removed:
################################################################################
diff --git a/compiler-rt/test/metadata/covered.cpp b/compiler-rt/test/metadata/covered.cpp
index d00f970c795c9..6fb4a91f10199 100644
--- a/compiler-rt/test/metadata/covered.cpp
+++ b/compiler-rt/test/metadata/covered.cpp
@@ -7,6 +7,11 @@
// RUN: %clangxx %s -o %t -fexperimental-sanitize-metadata=atomics,uar && %t | FileCheck -check-prefix=CHECK-AU %s
// RUN: %clangxx %s -o %t -fexperimental-sanitize-metadata=covered,atomics,uar && %t | FileCheck -check-prefix=CHECK-CAU %s
+__attribute__((noinline, not_tail_called)) void escape(const volatile void *p) {
+ static const volatile void *sink;
+ sink = p;
+}
+
// CHECK-NOT: metadata add
// CHECK: main
// CHECK-NOT: metadata del
@@ -28,19 +33,31 @@ void empty() {}
// CHECK-AU: normal: features=3
// CHECK-CAU:normal: features=3
void normal() {
- volatile int x;
- x = 0;
+ int x;
+ escape(&x);
}
-// CHECK-C: with_atomic: features=0
-// CHECK-A: with_atomic: features=1
-// CHECK-U: with_atomic: features=2
-// CHECK-CA: with_atomic: features=1
-// CHECK-CU: with_atomic: features=2
-// CHECK-AU: with_atomic: features=3
-// CHECK-CAU: with_atomic: features=3
+// CHECK-C: with_atomic: features=0
+// CHECK-A: with_atomic: features=1
+// CHECK-U-NOT: with_atomic:
+// CHECK-CA: with_atomic: features=1
+// CHECK-CU: with_atomic: features=0
+// CHECK-AU: with_atomic: features=1
+// CHECK-CAU: with_atomic: features=1
int with_atomic(int *p) { return __atomic_load_n(p, __ATOMIC_RELAXED); }
+// CHECK-C: with_atomic_escape: features=0
+// CHECK-A: with_atomic_escape: features=1
+// CHECK-U: with_atomic_escape: features=2
+// CHECK-CA: with_atomic_escape: features=1
+// CHECK-CU: with_atomic_escape: features=2
+// CHECK-AU: with_atomic_escape: features=3
+// CHECK-CAU: with_atomic_escape: features=3
+int with_atomic_escape(int *p) {
+ escape(&p);
+ return __atomic_load_n(p, __ATOMIC_RELAXED);
+}
+
// CHECK-C: ellipsis: features=0
// CHECK-A: ellipsis: features=1
// CHECK-U-NOT: ellipsis:
@@ -49,6 +66,7 @@ int with_atomic(int *p) { return __atomic_load_n(p, __ATOMIC_RELAXED); }
// CHECK-AU: ellipsis: features=1
// CHECK-CAU: ellipsis: features=1
void ellipsis(int *p, ...) {
+ escape(&p);
volatile int x;
x = 0;
}
@@ -61,6 +79,7 @@ void ellipsis(int *p, ...) {
// CHECK-AU: ellipsis_with_atomic: features=1
// CHECK-CAU: ellipsis_with_atomic: features=1
int ellipsis_with_atomic(int *p, ...) {
+ escape(&p);
return __atomic_load_n(p, __ATOMIC_RELAXED);
}
@@ -68,6 +87,7 @@ int ellipsis_with_atomic(int *p, ...) {
FN(empty); \
FN(normal); \
FN(with_atomic); \
+ FN(with_atomic_escape); \
FN(ellipsis); \
FN(ellipsis_with_atomic); \
/**/
diff --git a/compiler-rt/test/metadata/uar.cpp b/compiler-rt/test/metadata/uar.cpp
index dc6abdf63ab45..f4e643069c3ad 100644
--- a/compiler-rt/test/metadata/uar.cpp
+++ b/compiler-rt/test/metadata/uar.cpp
@@ -1,40 +1,49 @@
-// RUN: %clangxx %s -o %t -fexperimental-sanitize-metadata=covered,uar && %t | FileCheck %s
+// RUN: %clangxx %s -O1 -o %t -fexperimental-sanitize-metadata=covered,uar && %t | FileCheck %s
// CHECK: metadata add version 1
+__attribute__((noinline, not_tail_called)) void escape(const volatile void *p) {
+ static const volatile void *sink;
+ sink = p;
+}
+
+__attribute__((noinline, not_tail_called)) void use(int x) {
+ static volatile int sink;
+ sink += x;
+}
+
// CHECK: empty: features=0 stack_args=0
void empty() {}
// CHECK: ellipsis: features=0 stack_args=0
void ellipsis(const char *fmt, ...) {
- volatile int x;
- x = 1;
+ int x;
+ escape(&x);
}
// CHECK: non_empty_function: features=2 stack_args=0
void non_empty_function() {
- // Completely empty functions don't get uar metadata.
- volatile int x;
- x = 1;
+ int x;
+ escape(&x);
}
// CHECK: no_stack_args: features=2 stack_args=0
void no_stack_args(long a0, long a1, long a2, long a3, long a4, long a5) {
- volatile int x;
- x = 1;
+ int x;
+ escape(&x);
}
// CHECK: stack_args: features=2 stack_args=16
void stack_args(long a0, long a1, long a2, long a3, long a4, long a5, long a6) {
- volatile int x;
- x = 1;
+ int x;
+ escape(&x);
}
// CHECK: more_stack_args: features=2 stack_args=32
void more_stack_args(long a0, long a1, long a2, long a3, long a4, long a5,
long a6, long a7, long a8) {
- volatile int x;
- x = 1;
+ int x;
+ escape(&x);
}
// CHECK: struct_stack_args: features=2 stack_args=144
@@ -42,8 +51,32 @@ struct large {
char x[131];
};
void struct_stack_args(large a) {
- volatile int x;
- x = 1;
+ int x;
+ escape(&x);
+}
+
+__attribute__((noinline)) int tail_called(int x) { return x; }
+
+// CHECK: with_tail_call: features=2
+int with_tail_call(int x) { [[clang::musttail]] return tail_called(x); }
+
+// CHECK: local_array: features=0
+void local_array(int x) {
+ int data[10];
+ use(data[x]);
+}
+
+// CHECK: local_alloca: features=0
+void local_alloca(int size, int i, int j) {
+ volatile int *p = static_cast<int *>(__builtin_alloca(size));
+ p[i] = 0;
+ use(p[j]);
+}
+
+// CHECK: escaping_alloca: features=2
+void escaping_alloca(int size, int i) {
+ volatile int *p = static_cast<int *>(__builtin_alloca(size));
+ escape(&p[i]);
}
#define FUNCTIONS \
@@ -54,6 +87,10 @@ void struct_stack_args(large a) {
FN(stack_args); \
FN(more_stack_args); \
FN(struct_stack_args); \
+ FN(with_tail_call); \
+ FN(local_array); \
+ FN(local_alloca); \
+ FN(escaping_alloca); \
/**/
#include "common.h"
diff --git a/llvm/lib/Transforms/Instrumentation/SanitizerBinaryMetadata.cpp b/llvm/lib/Transforms/Instrumentation/SanitizerBinaryMetadata.cpp
index e8eaa9439c474..b438b821eb502 100644
--- a/llvm/lib/Transforms/Instrumentation/SanitizerBinaryMetadata.cpp
+++ b/llvm/lib/Transforms/Instrumentation/SanitizerBinaryMetadata.cpp
@@ -250,8 +250,10 @@ void SanitizerBinaryMetadata::runOn(Function &F, MetadataInfoSet &MIS) {
if (F.isVarArg())
FeatureMask &= ~kSanitizerBinaryMetadataUAR;
- if (FeatureMask & kSanitizerBinaryMetadataUAR)
+ if (FeatureMask & kSanitizerBinaryMetadataUAR) {
+ RequiresCovered = true;
NumMetadataUAR++;
+ }
// Covered metadata is always emitted if explicitly requested, otherwise only
// if some other metadata requires it to unambiguously interpret it for
@@ -269,23 +271,50 @@ void SanitizerBinaryMetadata::runOn(Function &F, MetadataInfoSet &MIS) {
}
}
+bool hasUseAfterReturnUnsafeUses(Value &V) {
+ for (User *U : V.users()) {
+ if (auto *I = dyn_cast<Instruction>(U)) {
+ if (I->isLifetimeStartOrEnd() || I->isDroppable())
+ continue;
+ if (isa<LoadInst>(U))
+ continue;
+ if (auto *SI = dyn_cast<StoreInst>(U)) {
+ // If storing TO the alloca, then the address isn't taken.
+ if (SI->getOperand(1) == &V)
+ continue;
+ }
+ if (auto *GEPI = dyn_cast<GetElementPtrInst>(U)) {
+ if (!hasUseAfterReturnUnsafeUses(*GEPI))
+ continue;
+ } else if (auto *BCI = dyn_cast<BitCastInst>(U)) {
+ if (!hasUseAfterReturnUnsafeUses(*BCI))
+ continue;
+ }
+ }
+ return true;
+ }
+ return false;
+}
+
+bool useAfterReturnUnsafe(Instruction &I) {
+ if (isa<AllocaInst>(I))
+ return hasUseAfterReturnUnsafeUses(I);
+ // Tail-called functions are not necessary intercepted
+ // at runtime because there is no call instruction.
+ // So conservatively mark the caller as requiring checking.
+ else if (auto *CI = dyn_cast<CallInst>(&I))
+ return CI->isTailCall();
+ return false;
+}
+
bool SanitizerBinaryMetadata::runOn(Instruction &I, MetadataInfoSet &MIS,
MDBuilder &MDB, uint32_t &FeatureMask) {
SmallVector<const MetadataInfo *, 1> InstMetadata;
bool RequiresCovered = false;
- if (Options.UAR) {
- for (unsigned i = 0; i < I.getNumOperands(); ++i) {
- const Value *V = I.getOperand(i);
- // TODO(dvyukov): check if V is an address of alloca/function arg.
- // See isSafeAndProfitableToSinkLoad for addr-taken allocas
- // and DeadArgumentEliminationPass::removeDeadStuffFromFunction
- // for iteration over function args.
- if (V) {
- RequiresCovered = true;
- FeatureMask |= kSanitizerBinaryMetadataUAR;
- }
- }
+ if (Options.UAR && !(FeatureMask & kSanitizerBinaryMetadataUAR)) {
+ if (useAfterReturnUnsafe(I))
+ FeatureMask |= kSanitizerBinaryMetadataUAR;
}
if (Options.Atomics && I.mayReadOrWriteMemory()) {
More information about the llvm-commits
mailing list