This is an archive of the discontinued LLVM Phabricator instance.

[sancov] Leave llvm.localescape in the entry block
ClosedPublic

Authored by rnk on Aug 11 2015, 3:26 PM.

Details

Summary

Similar to the change we applied to ASan. The same test case works.

Diff Detail

Event Timeline

rnk updated this revision to Diff 31873.Aug 11 2015, 3:26 PM
rnk retitled this revision from to [sancov] Leave llvm.localescape in the entry block.
rnk updated this object.
rnk added a reviewer: samsonov.
rnk added a subscriber: llvm-commits.
samsonov edited edge metadata.Aug 12 2015, 4:14 PM

*sigh* looks like anyone who splits basic blocks might stumble to this localescape problem. Probably it's worth to put this logic somewhere under lib/Transforms/Utils/BasicBlockUtils.cpp?

rnk added a comment.Aug 12 2015, 4:32 PM

*sigh* looks like anyone who splits basic blocks might stumble to this localescape problem. Probably it's worth to put this logic somewhere under lib/Transforms/Utils/BasicBlockUtils.cpp?

I mean, what kind of twisted person wants to split the entry block? It's so non-canonical. ;)

I think most instrumentation passes pretty much need this utility, but no optimization pass needs it. Should I create something like lib/Transforms/Instrumentation/Utils.cpp?

In D11961#223344, @rnk wrote:

*sigh* looks like anyone who splits basic blocks might stumble to this localescape problem. Probably it's worth to put this logic somewhere under lib/Transforms/Utils/BasicBlockUtils.cpp?

I mean, what kind of twisted person wants to split the entry block? It's so non-canonical. ;)

I think most instrumentation passes pretty much need this utility, but no optimization pass needs it. Should I create something like lib/Transforms/Instrumentation/Utils.cpp?

Sounds good.

rnk updated this revision to Diff 32075.Aug 13 2015, 10:28 AM
rnk edited edge metadata.
  • Make utility to prepare for entry block splitting
rnk added inline comments.Aug 13 2015, 10:44 AM
lib/Transforms/Instrumentation/AddressSanitizer.cpp
1805–1806 ↗(On Diff #32075)

Hm, this change fails because it moves instrumented static allocas around. Coverage and other instrumentation passes that don't replace static allocas don't have this problem. I think we should leave asan alone.

rnk updated this revision to Diff 32078.Aug 13 2015, 10:45 AM
  • Revert asan changes
This revision was automatically updated to reflect the committed changes.
rnk added a comment.Aug 14 2015, 9:48 AM

Woops, this landed accidentally through the usual git svn confusion. Let me know if it needs changes.

Looks good, thank you!