From b4ac732910ee32d004284aaa0632e5bb2fecc5fd Mon Sep 17 00:00:00 2001 From: Peter Collingbourne Date: Fri, 4 Jan 2019 19:27:04 +0000 Subject: [PATCH] hwasan: Implement lazy thread initialization for the interceptor ABI. The problem is similar to D55986 but for threads: a process with the interceptor hwasan library loaded might have some threads started by instrumented libraries and some by uninstrumented libraries, and we need to be able to run instrumented code on the latter. The solution is to perform per-thread initialization lazily. If a function needs to access shadow memory or add itself to the per-thread ring buffer its prologue checks to see whether the value in the sanitizer TLS slot is null, and if so it calls __hwasan_thread_enter and reloads from the TLS slot. The runtime does the same thing if it needs to access this data structure. This change means that the code generator needs to know whether we are targeting the interceptor runtime, since we don't want to pay the cost of lazy initialization when targeting a platform with native hwasan support. A flag -fsanitize-hwaddress-abi={interceptor,platform} has been introduced for selecting the runtime ABI to target. The default ABI is set to interceptor since it's assumed that it will be more common that users will be compiling application code than platform code. Because we can no longer assume that the TLS slot is initialized, the pthread_create interceptor is no longer necessary, so it has been removed. Ideally, lazy initialization should only cost one instruction in the hot path, but at present the call may cause us to spill arguments to the stack, which means more instructions in the hot path (or theoretically in the cold path if the spills are moved with shrink wrapping). With an appropriately chosen calling convention for the per-thread initialization function (TODO) the hot path should always need just one instruction and the cold path should need two instructions with no spilling required. Differential Revision: https://reviews.llvm.org/D56038 git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@350429 91177308-0d34-0410-b5e6-96231b3b80d8 --- include/clang/Basic/CodeGenOptions.h | 2 ++ include/clang/Driver/CC1Options.td | 2 ++ include/clang/Driver/Options.td | 4 ++++ include/clang/Driver/SanitizerArgs.h | 1 + lib/CodeGen/CGCall.cpp | 6 ++++++ lib/Driver/SanitizerArgs.cpp | 17 +++++++++++++++++ lib/Frontend/CompilerInvocation.cpp | 2 ++ test/CodeGen/default-function-attr.c | 6 ++++++ test/Driver/fsanitize.c | 8 ++++++++ 9 files changed, 48 insertions(+) create mode 100644 test/CodeGen/default-function-attr.c diff --git a/include/clang/Basic/CodeGenOptions.h b/include/clang/Basic/CodeGenOptions.h index a12744ee3d..ec6eda7fb7 100644 --- a/include/clang/Basic/CodeGenOptions.h +++ b/include/clang/Basic/CodeGenOptions.h @@ -286,6 +286,8 @@ public: /// Set of XRay instrumentation kinds to emit. XRayInstrSet XRayInstrumentationBundle; + std::vector DefaultFunctionAttrs; + public: // Define accessors/mutators for code generation options of enumeration type. #define CODEGENOPT(Name, Bits, Default) diff --git a/include/clang/Driver/CC1Options.td b/include/clang/Driver/CC1Options.td index c0e73e56d7..07c7688406 100644 --- a/include/clang/Driver/CC1Options.td +++ b/include/clang/Driver/CC1Options.td @@ -163,6 +163,8 @@ let Flags = [CC1Option, CC1AsOption, NoDriverOption] in { def debug_info_kind_EQ : Joined<["-"], "debug-info-kind=">; def debug_info_macro : Flag<["-"], "debug-info-macro">, HelpText<"Emit macro debug information">; +def default_function_attr : Separate<["-"], "default-function-attr">, + HelpText<"Apply given attribute to all functions">; def dwarf_version_EQ : Joined<["-"], "dwarf-version=">; def debugger_tuning_EQ : Joined<["-"], "debugger-tuning=">; def fdebug_compilation_dir : Separate<["-"], "fdebug-compilation-dir">, diff --git a/include/clang/Driver/Options.td b/include/clang/Driver/Options.td index 809b28b9dd..1c5cae683a 100644 --- a/include/clang/Driver/Options.td +++ b/include/clang/Driver/Options.td @@ -998,6 +998,10 @@ def fno_sanitize_address_use_odr_indicator : Flag<["-"], "fno-sanitize-address-use-odr-indicator">, Group, HelpText<"Disable ODR indicator globals">; +def fsanitize_hwaddress_abi_EQ + : Joined<["-"], "fsanitize-hwaddress-abi=">, + Group, + HelpText<"Select the HWAddressSanitizer ABI to target (interceptor or platform, default interceptor)">; def fsanitize_recover : Flag<["-"], "fsanitize-recover">, Group; def fno_sanitize_recover : Flag<["-"], "fno-sanitize-recover">, Flags<[CoreOption, DriverOption]>, diff --git a/include/clang/Driver/SanitizerArgs.h b/include/clang/Driver/SanitizerArgs.h index 55c5826bfb..02338c2216 100644 --- a/include/clang/Driver/SanitizerArgs.h +++ b/include/clang/Driver/SanitizerArgs.h @@ -39,6 +39,7 @@ class SanitizerArgs { bool AsanPoisonCustomArrayCookie = false; bool AsanGlobalsDeadStripping = false; bool AsanUseOdrIndicator = false; + std::string HwasanAbi; bool LinkCXXRuntimes = false; bool NeedPIE = false; bool SafeStackRuntime = false; diff --git a/lib/CodeGen/CGCall.cpp b/lib/CodeGen/CGCall.cpp index 64e18e171e..09116d465e 100644 --- a/lib/CodeGen/CGCall.cpp +++ b/lib/CodeGen/CGCall.cpp @@ -1816,6 +1816,12 @@ void CodeGenModule::ConstructDefaultFnAttrList(StringRef Name, bool HasOptnone, if (CodeGenOpts.FlushDenorm) FuncAttrs.addAttribute("nvptx-f32ftz", "true"); } + + for (StringRef Attr : CodeGenOpts.DefaultFunctionAttrs) { + StringRef Var, Value; + std::tie(Var, Value) = Attr.split('='); + FuncAttrs.addAttribute(Var, Value); + } } void CodeGenModule::AddDefaultFnAttrs(llvm::Function &F) { diff --git a/lib/Driver/SanitizerArgs.cpp b/lib/Driver/SanitizerArgs.cpp index 4e0d7491bb..6667cbb347 100644 --- a/lib/Driver/SanitizerArgs.cpp +++ b/lib/Driver/SanitizerArgs.cpp @@ -741,6 +741,18 @@ SanitizerArgs::SanitizerArgs(const ToolChain &TC, AsanUseAfterScope = false; } + if (AllAddedKinds & HWAddress) { + if (Arg *HwasanAbiArg = + Args.getLastArg(options::OPT_fsanitize_hwaddress_abi_EQ)) { + HwasanAbi = HwasanAbiArg->getValue(); + if (HwasanAbi != "platform" && HwasanAbi != "interceptor") + D.Diag(clang::diag::err_drv_invalid_value) + << HwasanAbiArg->getAsString(Args) << HwasanAbi; + } else { + HwasanAbi = "interceptor"; + } + } + if (AllAddedKinds & SafeStack) { // SafeStack runtime is built into the system on Fuchsia. SafeStackRuntime = !TC.getTriple().isOSFuchsia(); @@ -913,6 +925,11 @@ void SanitizerArgs::addArgs(const ToolChain &TC, const llvm::opt::ArgList &Args, if (AsanUseOdrIndicator) CmdArgs.push_back("-fsanitize-address-use-odr-indicator"); + if (!HwasanAbi.empty()) { + CmdArgs.push_back("-default-function-attr"); + CmdArgs.push_back(Args.MakeArgString("hwasan-abi=" + HwasanAbi)); + } + // MSan: Workaround for PR16386. // ASan: This is mainly to help LSan with cases such as // https://github.com/google/sanitizers/issues/373 diff --git a/lib/Frontend/CompilerInvocation.cpp b/lib/Frontend/CompilerInvocation.cpp index 39152fda99..00083bd622 100644 --- a/lib/Frontend/CompilerInvocation.cpp +++ b/lib/Frontend/CompilerInvocation.cpp @@ -1319,6 +1319,8 @@ static bool ParseCodeGenArgs(CodeGenOptions &Opts, ArgList &Args, InputKind IK, Opts.SpeculativeLoadHardening = Args.hasArg(OPT_mspeculative_load_hardening); + Opts.DefaultFunctionAttrs = Args.getAllArgValues(OPT_default_function_attr); + return Success; } diff --git a/test/CodeGen/default-function-attr.c b/test/CodeGen/default-function-attr.c new file mode 100644 index 0000000000..b0d1398a15 --- /dev/null +++ b/test/CodeGen/default-function-attr.c @@ -0,0 +1,6 @@ +// RUN: %clang_cc1 -default-function-attr foo=bar -emit-llvm %s -o - | FileCheck %s + +// CHECK: define void @foo() #[[X:[0-9]+]] +void foo() {} + +// CHECK: attributes #[[X]] = {{.*}} "foo"="bar" diff --git a/test/Driver/fsanitize.c b/test/Driver/fsanitize.c index 0a82174aaa..de45561714 100644 --- a/test/Driver/fsanitize.c +++ b/test/Driver/fsanitize.c @@ -837,3 +837,11 @@ // // RUN: %clang -target x86_64-linux-gnu -fsanitize=scudo,kernel-memory %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-SCUDO-KMSAN // CHECK-SCUDO-KMSAN: error: invalid argument '-fsanitize=kernel-memory' not allowed with '-fsanitize=scudo' + +// RUN: %clang -target x86_64-linux-gnu -fsanitize=hwaddress %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-HWASAN-INTERCEPTOR-ABI +// RUN: %clang -target x86_64-linux-gnu -fsanitize=hwaddress -fsanitize-hwaddress-abi=interceptor %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-HWASAN-INTERCEPTOR-ABI +// RUN: %clang -target x86_64-linux-gnu -fsanitize=hwaddress -fsanitize-hwaddress-abi=platform %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-HWASAN-PLATFORM-ABI +// RUN: %clang -target x86_64-linux-gnu -fsanitize=hwaddress -fsanitize-hwaddress-abi=foo %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-HWASAN-FOO-ABI +// CHECK-HWASAN-INTERCEPTOR-ABI: "-default-function-attr" "hwasan-abi=interceptor" +// CHECK-HWASAN-PLATFORM-ABI: "-default-function-attr" "hwasan-abi=platform" +// CHECK-HWASAN-FOO-ABI: error: invalid value 'foo' in '-fsanitize-hwaddress-abi=foo' -- 2.40.0