Page MenuHomePhabricator

[ASAN] Properly deal with musttail calls in ASAN
ClosedPublic

Authored by lxfind on Sep 16 2020, 10:26 AM.

Details

Summary

When address sanitizing a function, stack unpinsoning code is inserted before each ret instruction. However if the ret instruciton is preceded by a musttail call, such transformation broke the musttail call contract and generates invalid IR.
This patch fixes the issue by moving the insertion point prior to the musttail call if there is one.

Diff Detail

Event Timeline

lxfind created this revision.Sep 16 2020, 10:26 AM
Herald added a project: Restricted Project. · View Herald TranscriptSep 16 2020, 10:26 AM
lxfind requested review of this revision.Sep 16 2020, 10:26 AM
Harbormaster completed remote builds in B71900: Diff 292265.
Harbormaster completed remote builds in B71902: Diff 292268.
wenlei accepted this revision.Sep 16 2020, 5:25 PM

LGTM except a minor comment.

llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp
562

Is there a better place for this helper and avoid duplication? We have a few copies of it now, TSAN, ASAN and EntryExitInstrumenter...

This revision is now accepted and ready to land.Sep 16 2020, 5:25 PM
lxfind added inline comments.Sep 16 2020, 5:34 PM
llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp
562

Yeah I have been wondering about this too. There are more duplicates than TSAN and ASAN.
Do you have any suggestions? I couldn't find a good place for this.

wenlei added inline comments.Sep 16 2020, 5:53 PM
llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp
562

Was thinking about something like building this into SetInsertPoint of IRBuilder, perhaps with an extra boolean argument with default value. But doesn't look like good layering there - might be too specific for IRBuilder. I'm fine with the way it is now.

This revision was automatically updated to reflect the committed changes.