This is an archive of the discontinued LLVM Phabricator instance.

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

Authored by nmusgrave on Jul 29 2015, 4:11 PM.

Diff Detail

Event Timeline

nmusgrave updated this revision to Diff 30964.Jul 29 2015, 4:11 PM
nmusgrave retitled this revision from to repress tail call optimization when performing use-after-dtor sanitization.
nmusgrave updated this object.
nmusgrave added reviewers: eugenis, kcc.
nmusgrave added a subscriber: cfe-commits.
eugenis added inline comments.Jul 29 2015, 4:23 PM
lib/CodeGen/CGClass.cpp
1373

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

Why not SetTailCall() on the new call instruction?

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

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

20

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

nmusgrave marked 3 inline comments as done.Jul 29 2015, 4:51 PM
nmusgrave added inline comments.
lib/CodeGen/CGClass.cpp
1389

When I did that, it was still emitting tail calls

nmusgrave updated this revision to Diff 30969.Jul 29 2015, 4:54 PM
  • Updated test regex and flags
nmusgrave updated this revision to Diff 30982.Jul 29 2015, 7:00 PM
  • updated test to be more explicit
nmusgrave updated this revision to Diff 31040.Jul 30 2015, 9:39 AM
  • Testing to verify function attribute disable-tail-calls applied to destructor
nmusgrave updated this revision to Diff 31046.Jul 30 2015, 10:28 AM
  • removed hardcoding for attribute associated with dtor in test
eugenis accepted this revision.Jul 30 2015, 10:35 AM
eugenis edited edge metadata.

LGTM

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

#[0-9]+
Just in case.

20

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

This revision is now accepted and ready to land.Jul 30 2015, 10:35 AM
nmusgrave updated this revision to Diff 31051.Jul 30 2015, 10:50 AM
nmusgrave marked 2 inline comments as done.
nmusgrave edited edge metadata.
  • simplified test case
This revision was automatically updated to reflect the committed changes.
jroelofs added inline comments.
cfe/trunk/test/CodeGenCXX/sanitize-dtor-tail-call.cpp
17 ↗(On Diff #31053)

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 marked an inline comment as done.Jul 30 2015, 3:21 PM
nmusgrave added inline comments.
cfe/trunk/test/CodeGenCXX/sanitize-dtor-tail-call.cpp
17 ↗(On Diff #31053)

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.