Re: r352690 - [Sanitizers] UBSan unreachable incompatible with ASan in the presence of `noreturn` calls

[Julian Lettner via cfe-commits]

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?

updated_patch.patch
Description: Binary data

delta.diff
Description: Binary data

On Jan 31, 2019, at 6:19 AM, Eric Liu <ioe...@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 <ioe...@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 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 <echri...@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@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 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 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 ============================================================================== --- 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 ============================================================================== --- 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 ============================================================================== --- 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 ============================================================================== --- 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@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits