repress tail call optimization when performing use-after-dtor sanitization

[eugenis]

lib/CodeGen/CGClass.cpp

1373 ↗

(On Diff #30964)

It's not the callback that is being optimized away. The optimization avoids allocating a new frame for the callback function, making the destructor frame to disappear from the stack trace.

1389 ↗

(On Diff #30964)

Why not SetTailCall() on the new call instruction?

test/CodeGenCXX/sanitize-dtor-tail-call.cpp

2 ↗

(On Diff #30964)

try adding -disable-llvm-optzns flag to avoid running any optimizations and testing the IR right out of the frontend

19 ↗

(On Diff #30964)

You can check the lack of a tail on an instruction like this: CHECK: {{^ *}}call ...

[nmusgrave]

lib/CodeGen/CGClass.cpp

1389 ↗

(On Diff #30964)

When I did that, it was still emitting tail calls

[nmusgrave]

• Updated test regex and flags

[nmusgrave]

• updated test to be more explicit

[nmusgrave]

• Testing to verify function attribute disable-tail-calls applied to destructor

[nmusgrave]

• removed hardcoding for attribute associated with dtor in test

[eugenis]

LGTM

test/CodeGenCXX/sanitize-dtor-tail-call.cpp

17 ↗

(On Diff #31046)

#[0-9]+
Just in case.

19 ↗

(On Diff #31046)

Remove "tail".
That way this will check that there is no other call to the dtor callback, tail or not.

[nmusgrave]

• simplified test case

[jroelofs]

cfe/trunk/test/CodeGenCXX/sanitize-dtor-tail-call.cpp

17

I don't think this check verifies what you want it to verify... you need to order it like this:

// CHECK-NOT: {{\s*}}tail call void @__sanitizer_dtor_callback
// CHECK: {{\s*}}call void @__sanitizer_dtor_callback
// CHECK-NOT: {{\s*}}call void @__sanitizer_dtor_callback

or better yet:

// CHECK: {{^ +}}call void @__sanitizer_dtor_callback
// CHECK-NOT: call void @__sanitizer_dtor_callback

[nmusgrave]

cfe/trunk/test/CodeGenCXX/sanitize-dtor-tail-call.cpp

17

The final committed version reflects your suggested changes. I'm not sure why they're not showing up here- I had some difficulty squashing my local commits into a single commit for the remote.

[jroelofs]

ah, thanks!

cfe/trunk/lib/CodeGen/CGClass.cpp

for (const auto *Field : ClassDecl->fields())
     if (!FieldHasTrivialDestructorBody(Context, Field))
       return false;
 
   return true;
 }
 
 // Generates function call for handling object poisoning, passing in
 // references to 'this' and its size as arguments.
+// Disables tail call elimination, to save emitted callback from
+// being optimized away.
 static void EmitDtorSanitizerCallback(CodeGenFunction &CGF,
                                       const CXXDestructorDecl *Dtor) {
   const ASTRecordLayout &Layout =
       CGF.getContext().getASTRecordLayout(Dtor->getParent());
 
   llvm::Value *Args[] = {
       CGF.Builder.CreateBitCast(CGF.LoadCXXThis(), CGF.VoidPtrTy),
       llvm::ConstantInt::get(CGF.SizeTy, Layout.getSize().getQuantity())};
   llvm::Type *ArgTypes[] = {CGF.VoidPtrTy, CGF.SizeTy};
 
   llvm::FunctionType *FnType =
       llvm::FunctionType::get(CGF.VoidTy, ArgTypes, false);
   llvm::Value *Fn =
       CGF.CGM.CreateRuntimeFunction(FnType, "__sanitizer_dtor_callback");
+
+  CGF.CurFn->addFnAttr("disable-tail-calls", "true");
   CGF.EmitNounwindRuntimeCall(Fn, Args);
 }
 
 /// EmitDestructorBody - Emits the body of the current destructor.
 void CodeGenFunction::EmitDestructorBody(FunctionArgList &Args) {
   const CXXDestructorDecl *Dtor = cast<CXXDestructorDecl>(CurGD.getDecl());
   CXXDtorType DtorType = CurGD.getDtorType();
 

cfe/trunk/test/CodeGenCXX/sanitize-dtor-tail-call.cpp

+// Test -fsanitize-memory-use-after-dtor
+// RUN: %clang_cc1 -O1 -fsanitize=memory -fsanitize-memory-use-after-dtor -std=c++11 -triple=x86_64-pc-linux -emit-llvm -o - %s | FileCheck %s
+
+struct Simple {
+  int x_;
+  Simple() {
+    x_ = 5;
+  }
+  ~Simple() {
+    x_ += 1;
+  }
+};
+
+Simple s;
+// Simple internal member is poisoned by compiler-generated dtor
+// CHECK-LABEL: define {{.*}}SimpleD2Ev
+// CHECK: {{\s*}}call void @__sanitizer_dtor_callback
+// CHECK-NOT: {{\s*}}call void @__sanitizer_dtor_callback
+// CHECK-NOT: {{\s*}}tail call void @__sanitizer_dtor_callback
+// CHECK: ret void