diff --git a/compiler-rt/test/metadata/covered.cpp b/compiler-rt/test/metadata/covered.cpp --- 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 @@ // 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 @@ // 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 @@ // 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 @@ 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 --- 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 @@ 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(__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(__builtin_alloca(size)); + escape(&p[i]); } #define FUNCTIONS \ @@ -54,6 +87,10 @@ 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 --- a/llvm/lib/Transforms/Instrumentation/SanitizerBinaryMetadata.cpp +++ b/llvm/lib/Transforms/Instrumentation/SanitizerBinaryMetadata.cpp @@ -250,8 +250,10 @@ 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 @@ } } +bool hasUseAfterReturnUnsafeUses(Value &V) { + for (User *U : V.users()) { + if (auto *I = dyn_cast(U)) { + if (I->isLifetimeStartOrEnd() || I->isDroppable()) + continue; + if (isa(U)) + continue; + if (auto *SI = dyn_cast(U)) { + // If storing TO the alloca, then the address isn't taken. + if (SI->getOperand(1) == &V) + continue; + } + if (auto *GEPI = dyn_cast(U)) { + if (!hasUseAfterReturnUnsafeUses(*GEPI)) + continue; + } else if (auto *BCI = dyn_cast(U)) { + if (!hasUseAfterReturnUnsafeUses(*BCI)) + continue; + } + } + return true; + } + return false; +} + +bool useAfterReturnUnsafe(Instruction &I) { + if (isa(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(&I)) + return CI->isTailCall(); + return false; +} + bool SanitizerBinaryMetadata::runOn(Instruction &I, MetadataInfoSet &MIS, MDBuilder &MDB, uint32_t &FeatureMask) { SmallVector 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()) {