From 9d4f1a9eff27716069dc6a2d991baa228c197b85 Mon Sep 17 00:00:00 2001 From: Dmitry Vyukov Date: Fri, 20 Jan 2023 10:26:20 +0100 Subject: [PATCH] sanmd: refine selection of functions for UAR checking There are no intrinsic functions that leak arguments. If the called function does not return, the current function does not return as well, so no possibility of use-after-return. Sanitizer function also don't leak or don't return. It's safe to both pass pointers to local variables to them and to tail-call them. Reviewed By: melver Differential Revision: https://reviews.llvm.org/D142190 --- compiler-rt/test/metadata/CMakeLists.txt | 7 ++++++- compiler-rt/test/metadata/uar.cpp | 19 +++++++++++++++++++ .../Instrumentation/SanitizerBinaryMetadata.cpp | 22 +++++++++++++++++++++- 3 files changed, 46 insertions(+), 2 deletions(-) diff --git a/compiler-rt/test/metadata/CMakeLists.txt b/compiler-rt/test/metadata/CMakeLists.txt index 3391a89..4a17b0e 100644 --- a/compiler-rt/test/metadata/CMakeLists.txt +++ b/compiler-rt/test/metadata/CMakeLists.txt @@ -4,6 +4,11 @@ if(CAN_TARGET_x86_64) set(METADATA_LIT_SOURCE_DIR ${CMAKE_CURRENT_SOURCE_DIR}) set(METADATA_LIT_BINARY_DIR ${CMAKE_CURRENT_BINARY_DIR}) + set(METADATA_TEST_DEPS ${SANITIZER_COMMON_LIT_TEST_DEPS}) + if (NOT COMPILER_RT_STANDALONE_BUILD) + list(APPEND METADATA_TEST_DEPS asan ubsan) + endif() + set(SANITIZER_COMMON_TEST_TARGET_ARCH ${X86_64}) get_test_cc_for_arch(x86_64 METADATA_TEST_TARGET_CC METADATA_TEST_TARGET_CFLAGS) configure_lit_site_cfg( @@ -12,6 +17,6 @@ if(CAN_TARGET_x86_64) add_lit_testsuite(check-sanmd "Running the SanitizerBinaryMetadata tests" ${CMAKE_CURRENT_BINARY_DIR} - DEPENDS ${SANITIZER_COMMON_LIT_TEST_DEPS}) + DEPENDS ${METADATA_TEST_DEPS}) set_target_properties(check-sanmd PROPERTIES FOLDER "Compiler-RT Misc") endif() diff --git a/compiler-rt/test/metadata/uar.cpp b/compiler-rt/test/metadata/uar.cpp index f4e6430..ed7f7dd8 100644 --- a/compiler-rt/test/metadata/uar.cpp +++ b/compiler-rt/test/metadata/uar.cpp @@ -1,4 +1,5 @@ // RUN: %clangxx %s -O1 -o %t -fexperimental-sanitize-metadata=covered,uar && %t | FileCheck %s +// RUN: %clangxx %s -O1 -o %t -fexperimental-sanitize-metadata=covered,uar -fsanitize=address,signed-integer-overflow && %t | FileCheck %s // CHECK: metadata add version 1 @@ -15,6 +16,16 @@ __attribute__((noinline, not_tail_called)) void use(int x) { // CHECK: empty: features=0 stack_args=0 void empty() {} +// CHECK: simple: features=0 stack_args=0 +int simple(int *data, int index) { return data[index + 1]; } + +// CHECK: builtins: features=0 stack_args=0 +int builtins() { + int x = 0; + __builtin_prefetch(&x); + return x; +} + // CHECK: ellipsis: features=0 stack_args=0 void ellipsis(const char *fmt, ...) { int x; @@ -60,6 +71,11 @@ __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); } +__attribute__((noinline, noreturn)) int noreturn(int x) { __builtin_trap(); } + +// CHECK: with_noreturn_tail_call: features=0 +int with_noreturn_tail_call(int x) { return noreturn(x); } + // CHECK: local_array: features=0 void local_array(int x) { int data[10]; @@ -81,6 +97,8 @@ void escaping_alloca(int size, int i) { #define FUNCTIONS \ FN(empty); \ + FN(simple); \ + FN(builtins); \ FN(ellipsis); \ FN(non_empty_function); \ FN(no_stack_args); \ @@ -88,6 +106,7 @@ void escaping_alloca(int size, int i) { FN(more_stack_args); \ FN(struct_stack_args); \ FN(with_tail_call); \ + FN(with_noreturn_tail_call); \ FN(local_array); \ FN(local_alloca); \ FN(escaping_alloca); \ diff --git a/llvm/lib/Transforms/Instrumentation/SanitizerBinaryMetadata.cpp b/llvm/lib/Transforms/Instrumentation/SanitizerBinaryMetadata.cpp index 0a71df9..80eab77 100644 --- a/llvm/lib/Transforms/Instrumentation/SanitizerBinaryMetadata.cpp +++ b/llvm/lib/Transforms/Instrumentation/SanitizerBinaryMetadata.cpp @@ -271,11 +271,31 @@ void SanitizerBinaryMetadata::runOn(Function &F, MetadataInfoSet &MIS) { } } +bool isUARSafeCall(CallInst *CI) { + auto *F = CI->getCalledFunction(); + // There are no intrinsic functions that leak arguments. + // If the called function does not return, the current function + // does not return as well, so no possibility of use-after-return. + // Sanitizer function also don't leak or don't return. + // It's safe to both pass pointers to local variables to them + // and to tail-call them. + return F && (F->isIntrinsic() || F->doesNotReturn() || + F->getName().startswith("__asan_") || + F->getName().startswith("__hwsan_") || + F->getName().startswith("__ubsan_") || + F->getName().startswith("__msan_") || + F->getName().startswith("__tsan_")); +} + bool hasUseAfterReturnUnsafeUses(Value &V) { for (User *U : V.users()) { if (auto *I = dyn_cast(U)) { if (I->isLifetimeStartOrEnd() || I->isDroppable()) continue; + if (auto *CI = dyn_cast(U)) { + if (isUARSafeCall(CI)) + continue; + } if (isa(U)) continue; if (auto *SI = dyn_cast(U)) { @@ -303,7 +323,7 @@ bool useAfterReturnUnsafe(Instruction &I) { // 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 CI->isTailCall() && !isUARSafeCall(CI); return false; } -- 2.7.4