r352690 - [Sanitizers] UBSan unreachable incompatible with ASan in the presence of `noreturn` calls
Julian Lettner via cfe-commits
cfe-commits at lists.llvm.org
Thu Jan 31 14:34:39 PST 2019
Thank you, Eric, for the reproducer and instructions. Actionable and much appreciated.
Can you test my updated patch on your build before I try again?
> On Jan 31, 2019, at 6:19 AM, Eric Liu <ioeric at google.com> wrote:
>
> And the stack trace is:
> ```
> 1. <eof> parser at end of file [31/1788]
> 2. Code generation
> 3. Running pass 'Function Pass Manager' on module 'absl/base/internal/throw_delegate.cc'.
> 4. Running pass 'X86 DAG->DAG Instruction Selection' on function '@_ZN4absl13base_internal18ThrowStdLogicErrorERKNSt7__cxx1112basic_stringIcSt11char_traitsIcESaIcEEE'
> #0 0x000055c42e7bce9d SignalHandler(int) (bin/clang+0x1aabe9d)
> #1 0x00007f41b11309a0 __restore_rt (/usr/grte/v4/lib64/libpthread.so.0+0xf9a0)
> #2 0x000055c42fe10b5b findUnwindDestinations(llvm::FunctionLoweringInfo&, llvm::BasicBlock const*, llvm::BranchProbability, llvm::SmallVectorImpl<std::__g::pair<llvm::MachineBasicBlock*, llvm::BranchProbability
> > >&) (bin/clang+0x30ffb5b)
> #3 0x000055c42fe0b49a llvm::SelectionDAGBuilder::visitInvoke(llvm::InvokeInst const&) (bin/clang+0x30fa49a)
> #4 0x000055c4308a8a2f llvm::SelectionDAGBuilder::visit(llvm::Instruction const&) (bin/clang+0x3b97a2f)
> #5 0x000055c430878aa5 llvm::SelectionDAGISel::SelectBasicBlock(llvm::ilist_iterator<llvm::ilist_detail::node_options<llvm::Instruction, false, false, void>, false, true>, llvm::ilist_iterator<llvm::ilist_detail
> ::node_options<llvm::Instruction, false, false, void>, false, true>, bool&) (bin/clang+0x3b67aa5)
> #6 0x000055c4308768ed llvm::SelectionDAGISel::SelectAllBasicBlocks(llvm::Function const&) (bin/clang+0x3b658
> ed)
> #7 0x000055c43087354a llvm::SelectionDAGISel::runOnMachineFunction(llvm::MachineFunction&) (bin/clang+0x3b62
> 54a)
> #8 0x000055c43086d1da (anonymous namespace)::X86DAGToDAGISel::runOnMachineFunction(llvm::MachineFunction&) (
> bin/clang+0x3b5c1da)
> #9 0x000055c4309eb833 llvm::MachineFunctionPass::runOnFunction(llvm::Function&) (bin/clang+0x3cda833)
> #10 0x000055c43070edbe llvm::FPPassManager::runOnFunction(llvm::Function&) (bin/clang+0x39fddbe)
> #11 0x000055c430711521 llvm::FPPassManager::runOnModule(llvm::Module&) (bin/clang+0x3a00521)
> #12 0x000055c42e4a6f7f llvm::legacy::PassManagerImpl::run(llvm::Module&) (bin/clang+0x1795f7f)
> #13 0x000055c42e7d9594 clang::EmitBackendOutput(clang::DiagnosticsEngine&, clang::HeaderSearchOptions const&, clang::CodeGenOptions const&, clang::TargetOptions const&, clang::LangOptions const&, llvm::DataLayou
> t const&, llvm::Module*, clang::BackendAction, std::__g::unique_ptr<llvm::raw_pwrite_stream, std::__g::default_delete<llvm::raw_pwrite_stream> >) (bin/clang+0x1ac8594)
> #14 0x000055c42e7a4d8c clang::BackendConsumer::HandleTranslationUnit(clang::ASTContext&) (bin/clang+0x1a93d8c
> )
> #15 0x000055c42e6499b6 clang::ParseAST(clang::Sema&, bool, bool) (bin/clang+0x19389b6)
> #16 0x000055c42e7a8b37 clang::FrontendAction::Execute() (bin/clang+0x1a97b37)
> #17 0x000055c42e7ae75f clang::CompilerInstance::ExecuteAction(clang::FrontendAction&) (bin/clang+0x1a9d75f)
> #18 0x000055c42e796cfb clang::ExecuteCompilerInvocation(clang::CompilerInstance*) (bin/clang+0x1a85cfb)
> #19 0x000055c42e791071 cc1_main(llvm::ArrayRef<char const*>, char const*, void*) (bin/clang+0x1a80071)
> #20 0x000055c42e5c019e main (bin/clang+0x18af19e)
> #21 0x00007f41b0e9ebbd __libc_start_main (/usr/grte/v4/lib64/libc.so.6+0x38bbd)
> #22 0x000055c42e6d44a9 _start (bin/clang+0x19c34a9)
> clang: error: unable to execute command: Segmentation fault
> clang: error: clang frontend command failed due to signal (use -v to see invocation)
> ```
>
> On Thu, Jan 31, 2019 at 3:11 PM Eric Liu <ioeric at google.com <mailto:ioeric at google.com>> wrote:
> I managed to get a reproducer (attached) from absl:
> ```
> clang++ -std=c++17 -fsanitize=address,unreachable throw_delegate.pic.ii
> ```
>
> You could regenerate the preprocessed code:
> ```
> git clone https://github.com/abseil/abseil-cpp.git <https://github.com/abseil/abseil-cpp.git>
> cd abseil-cpp/absl
> bazel build --compilation_mode=fastbuild --save_temps --compile_one_dependency base/internal/throw_delegate.cc
> ```
>
> I'll revert the commit to unblock our integration process. Let us know if you need more information.
>
> - Eric
>
> On Thu, Jan 31, 2019 at 9:01 AM Eric Christopher <echristo at gmail.com <mailto:echristo at gmail.com>> wrote:
> Looks like this broke optimized asan builds via an assert in SCCP. I'll see what I can do about a testcase (or Eric will), however, would you mind reverting in the meantime?
>
> Thanks!
>
> -eric
>
> On Wed, Jan 30, 2019 at 4:41 PM Julian Lettner via cfe-commits <cfe-commits at lists.llvm.org <mailto:cfe-commits at lists.llvm.org>> wrote:
> Author: yln
> Date: Wed Jan 30 15:42:13 2019
> New Revision: 352690
>
> URL: http://llvm.org/viewvc/llvm-project?rev=352690&view=rev <http://llvm.org/viewvc/llvm-project?rev=352690&view=rev>
> Log:
> [Sanitizers] UBSan unreachable incompatible with ASan in the presence of `noreturn` calls
>
> Summary:
> UBSan wants to detect when unreachable code is actually reached, so it
> adds instrumentation before every unreachable instruction. However, the
> optimizer will remove code after calls to functions marked with
> noreturn. To avoid this UBSan removes noreturn from both the call
> instruction as well as from the function itself. Unfortunately, ASan
> relies on this annotation to unpoison the stack by inserting calls to
> _asan_handle_no_return before noreturn functions. This is important for
> functions that do not return but access the the stack memory, e.g.,
> unwinder functions *like* longjmp (longjmp itself is actually
> "double-proofed" via its interceptor). The result is that when ASan and
> UBSan are combined, the noreturn attributes are missing and ASan cannot
> unpoison the stack, so it has false positives when stack unwinding is
> used.
>
> Changes:
> Clang-CodeGen now directly insert calls to `__asan_handle_no_return`
> when a call to a noreturn function is encountered and both
> UBsan-unreachable and ASan are enabled. This allows UBSan to continue
> removing the noreturn attribute from functions without any changes to
> the ASan pass.
>
> Previously generated code:
> ```
> call void @longjmp
> call void @__asan_handle_no_return
> call void @__ubsan_handle_builtin_unreachable
> ```
>
> Generated code (for now):
> ```
> call void @__asan_handle_no_return
> call void @longjmp
> call void @__asan_handle_no_return
> call void @__ubsan_handle_builtin_unreachable
> ```
>
> rdar://problem/40723397
>
> Reviewers: delcypher, eugenis, vsk
>
> Differential Revision: https://reviews.llvm.org/D57278 <https://reviews.llvm.org/D57278>
>
> Added:
> cfe/trunk/test/CodeGen/ubsan-asan-noreturn.c
> Modified:
> cfe/trunk/lib/CodeGen/CGCall.cpp
> cfe/trunk/lib/CodeGen/CodeGenFunction.h
> cfe/trunk/test/CodeGenCXX/ubsan-unreachable.cpp
>
> Modified: cfe/trunk/lib/CodeGen/CGCall.cpp
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGCall.cpp?rev=352690&r1=352689&r2=352690&view=diff <http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGCall.cpp?rev=352690&r1=352689&r2=352690&view=diff>
> ==============================================================================
> --- cfe/trunk/lib/CodeGen/CGCall.cpp (original)
> +++ cfe/trunk/lib/CodeGen/CGCall.cpp Wed Jan 30 15:42:13 2019
> @@ -4398,10 +4398,23 @@ RValue CodeGenFunction::EmitCall(const C
>
> // Strip away the noreturn attribute to better diagnose unreachable UB.
> if (SanOpts.has(SanitizerKind::Unreachable)) {
> + // Also remove from function since CI->hasFnAttr(..) also checks attributes
> + // of the called function.
> if (auto *F = CI->getCalledFunction())
> F->removeFnAttr(llvm::Attribute::NoReturn);
> CI->removeAttribute(llvm::AttributeList::FunctionIndex,
> llvm::Attribute::NoReturn);
> +
> + // Avoid incompatibility with ASan which relies on the `noreturn`
> + // attribute to insert handler calls.
> + if (SanOpts.has(SanitizerKind::Address)) {
> + SanitizerScope SanScope(this);
> + Builder.SetInsertPoint(CI);
> + auto *FnType = llvm::FunctionType::get(CGM.VoidTy, /*isVarArg=*/false);
> + auto *Fn = CGM.CreateRuntimeFunction(FnType, "__asan_handle_no_return");
> + EmitNounwindRuntimeCall(Fn);
> + Builder.SetInsertPoint(CI->getParent());
> + }
> }
>
> EmitUnreachable(Loc);
>
> Modified: cfe/trunk/lib/CodeGen/CodeGenFunction.h
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CodeGenFunction.h?rev=352690&r1=352689&r2=352690&view=diff <http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CodeGenFunction.h?rev=352690&r1=352689&r2=352690&view=diff>
> ==============================================================================
> --- cfe/trunk/lib/CodeGen/CodeGenFunction.h (original)
> +++ cfe/trunk/lib/CodeGen/CodeGenFunction.h Wed Jan 30 15:42:13 2019
> @@ -4084,8 +4084,8 @@ public:
> /// passing to a runtime sanitizer handler.
> llvm::Constant *EmitCheckSourceLocation(SourceLocation Loc);
>
> - /// Create a basic block that will call a handler function in a
> - /// sanitizer runtime with the provided arguments, and create a conditional
> + /// Create a basic block that will either trap or call a handler function in
> + /// the UBSan runtime with the provided arguments, and create a conditional
> /// branch to it.
> void EmitCheck(ArrayRef<std::pair<llvm::Value *, SanitizerMask>> Checked,
> SanitizerHandler Check, ArrayRef<llvm::Constant *> StaticArgs,
>
> Added: cfe/trunk/test/CodeGen/ubsan-asan-noreturn.c
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGen/ubsan-asan-noreturn.c?rev=352690&view=auto <http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGen/ubsan-asan-noreturn.c?rev=352690&view=auto>
> ==============================================================================
> --- cfe/trunk/test/CodeGen/ubsan-asan-noreturn.c (added)
> +++ cfe/trunk/test/CodeGen/ubsan-asan-noreturn.c Wed Jan 30 15:42:13 2019
> @@ -0,0 +1,21 @@
> +// Ensure compatiblity of UBSan unreachable with ASan in the presence of
> +// noreturn functions.
> +// RUN: %clang_cc1 -fsanitize=unreachable,address -triple x86_64-linux -emit-llvm -o - %s | FileCheck %s
> +
> +void my_longjmp(void) __attribute__((noreturn));
> +
> +// CHECK-LABEL: define void @calls_noreturn()
> +void calls_noreturn() {
> + my_longjmp();
> + // CHECK: @__asan_handle_no_return{{.*}} !nosanitize
> + // CHECK-NEXT: @my_longjmp(){{[^#]*}}
> + // CHECK: @__asan_handle_no_return()
> + // CHECK-NEXT: @__ubsan_handle_builtin_unreachable{{.*}} !nosanitize
> + // CHECK-NEXT: unreachable
> +}
> +
> +// CHECK: declare void @my_longjmp() [[FN_ATTR:#[0-9]+]]
> +// CHECK: declare void @__asan_handle_no_return()
> +
> +// CHECK-LABEL: attributes
> +// CHECK-NOT: [[FN_ATTR]] = { {{.*noreturn.*}} }
>
> Modified: cfe/trunk/test/CodeGenCXX/ubsan-unreachable.cpp
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenCXX/ubsan-unreachable.cpp?rev=352690&r1=352689&r2=352690&view=diff <http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenCXX/ubsan-unreachable.cpp?rev=352690&r1=352689&r2=352690&view=diff>
> ==============================================================================
> --- cfe/trunk/test/CodeGenCXX/ubsan-unreachable.cpp (original)
> +++ cfe/trunk/test/CodeGenCXX/ubsan-unreachable.cpp Wed Jan 30 15:42:13 2019
> @@ -1,39 +1,37 @@
> // RUN: %clang_cc1 -triple x86_64-apple-darwin10 -emit-llvm -o - %s -fsanitize=unreachable | FileCheck %s
>
> -extern void __attribute__((noreturn)) abort();
> +void abort() __attribute__((noreturn));
>
> -// CHECK-LABEL: define void @_Z14calls_noreturnv
> +// CHECK-LABEL: define void @_Z14calls_noreturnv()
> void calls_noreturn() {
> + // Check absence ([^#]*) of call site attributes (including noreturn)
> + // CHECK: call void @_Z5abortv(){{[^#]*}}
> abort();
>
> - // Check that there are no attributes on the call site.
> - // CHECK-NOT: call void @_Z5abortv{{.*}}#
> -
> // CHECK: __ubsan_handle_builtin_unreachable
> // CHECK: unreachable
> }
>
> struct A {
> - // CHECK: declare void @_Z5abortv{{.*}} [[ABORT_ATTR:#[0-9]+]]
> + // CHECK: declare void @_Z5abortv() [[EXTERN_FN_ATTR:#[0-9]+]]
>
> // CHECK-LABEL: define linkonce_odr void @_ZN1A5call1Ev
> void call1() {
> - // CHECK-NOT: call void @_ZN1A16does_not_return2Ev{{.*}}#
> + // CHECK: call void @_ZN1A16does_not_return2Ev({{.*}}){{[^#]*}}
> does_not_return2();
>
> // CHECK: __ubsan_handle_builtin_unreachable
> // CHECK: unreachable
> }
>
> - // Test static members.
> - static void __attribute__((noreturn)) does_not_return1() {
> - // CHECK-NOT: call void @_Z5abortv{{.*}}#
> + // Test static members. Checks are below after `struct A` scope ends.
> + static void does_not_return1() __attribute__((noreturn)) {
> abort();
> }
>
> // CHECK-LABEL: define linkonce_odr void @_ZN1A5call2Ev
> void call2() {
> - // CHECK-NOT: call void @_ZN1A16does_not_return1Ev{{.*}}#
> + // CHECK: call void @_ZN1A16does_not_return1Ev(){{[^#]*}}
> does_not_return1();
>
> // CHECK: __ubsan_handle_builtin_unreachable
> @@ -41,23 +39,23 @@ struct A {
> }
>
> // Test calls through pointers to non-static member functions.
> - typedef void __attribute__((noreturn)) (A::*MemFn)();
> + typedef void (A::*MemFn)() __attribute__((noreturn));
>
> // CHECK-LABEL: define linkonce_odr void @_ZN1A5call3Ev
> void call3() {
> MemFn MF = &A::does_not_return2;
> + // CHECK: call void %{{[0-9]+\(.*}}){{[^#]*}}
> (this->*MF)();
>
> - // CHECK-NOT: call void %{{.*}}#
> // CHECK: __ubsan_handle_builtin_unreachable
> // CHECK: unreachable
> }
>
> // Test regular members.
> // CHECK-LABEL: define linkonce_odr void @_ZN1A16does_not_return2Ev({{.*}})
> - // CHECK-SAME: [[DOES_NOT_RETURN_ATTR:#[0-9]+]]
> - void __attribute__((noreturn)) does_not_return2() {
> - // CHECK-NOT: call void @_Z5abortv(){{.*}}#
> + // CHECK-SAME: [[USER_FN_ATTR:#[0-9]+]]
> + void does_not_return2() __attribute__((noreturn)) {
> + // CHECK: call void @_Z5abortv(){{[^#]*}}
> abort();
>
> // CHECK: call void @__ubsan_handle_builtin_unreachable
> @@ -68,7 +66,9 @@ struct A {
> }
> };
>
> -// CHECK: define linkonce_odr void @_ZN1A16does_not_return1Ev() [[DOES_NOT_RETURN_ATTR]]
> +// CHECK-LABEL: define linkonce_odr void @_ZN1A16does_not_return1Ev()
> +// CHECK-SAME: [[USER_FN_ATTR]]
> +// CHECK: call void @_Z5abortv(){{[^#]*}}
>
> void force_irgen() {
> A a;
> @@ -77,5 +77,7 @@ void force_irgen() {
> a.call3();
> }
>
> -// CHECK-NOT: [[ABORT_ATTR]] = {{[^}]+}}noreturn
> -// CHECK-NOT: [[DOES_NOT_RETURN_ATTR]] = {{[^}]+}}noreturn
> +// `noreturn` should be removed from functions and call sites
> +// CHECK-LABEL: attributes
> +// CHECK-NOT: [[USER_FN_ATTR]] = { {{.*noreturn.*}} }
> +// CHECK-NOT: [[EXTERN_FN_ATTR]] = { {{.*noreturn.*}} }
>
>
> _______________________________________________
> cfe-commits mailing list
> cfe-commits at lists.llvm.org <mailto:cfe-commits at lists.llvm.org>
> https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits <https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20190131/694c58e0/attachment-0003.html>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: updated_patch.patch
Type: application/octet-stream
Size: 9021 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20190131/694c58e0/attachment-0002.obj>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20190131/694c58e0/attachment-0004.html>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: delta.diff
Type: application/octet-stream
Size: 774 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20190131/694c58e0/attachment-0003.obj>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20190131/694c58e0/attachment-0005.html>
More information about the cfe-commits
mailing list