This is an archive of the discontinued LLVM Phabricator instance.

[Sanitizer] Reland "Cleanup INTERCEPT_FUNCTION macro"
ClosedPublic

Authored by yln on Apr 30 2019, 5:54 PM.

Details

Summary

On Linux both version of the INTERCEPT_FUNCTION macro now return true
when interception was successful. Adapt and cleanup some usages.

Also note that &(func) == &WRAP(func) is a link-time property, but we
do a runtime check.

Tested on Linux and macOS.

Previous attempt reverted by: 5642c3feb03d020dc06a62e3dc54f3206a97a391

This attempt to bring order to the interceptor macro goes the other
direction and aligns the Linux implementation with the way things are
done on Windows.

Diff Detail

Event Timeline

yln created this revision.Apr 30 2019, 5:54 PM
Herald added projects: Restricted Project, Restricted Project. · View Herald TranscriptApr 30 2019, 5:54 PM
Herald added subscribers: llvm-commits, Restricted Project, kubamracek. · View Herald Transcript

Please don't submit without my review even if @rnk accept this.
I've noticed report that original patch breaks compiler-rt/test/tsan/Linux/user_malloc.cc on our internal llvm build.
I don't yet understand why it's there and not on our public bots or my local checkout. May be a problem with the patch or our miss-configuration. I'll debug this morning.

Please don't submit without my review even if @rnk accept this.
I've noticed report that original patch breaks compiler-rt/test/tsan/Linux/user_malloc.cc on our internal llvm build.
I don't yet understand why it's there and not on our public bots or my local checkout. May be a problem with the patch or our miss-configuration. I'll debug this morning.

Oh, the test is "// UNSUPPORTED: linux" and it didn't work before patches.

LGTM then

yln added a comment.May 1 2019, 10:31 AM

Cool, thanks Vitaly! Waiting for sign-off from Reid.

rnk accepted this revision.May 1 2019, 10:49 AM

lgtm, thanks

compiler-rt/lib/asan/asan_interceptors.h
125 ↗(On Diff #197493)

This could be a change in behavior, but I think I'm OK with it. I see that the check is redundant on the Linux side, but it's not immediately clear that the Windows side interception maintains the invariant that it returns false if it can't fill in REAL(name). But, ultimately this condition only affects logging here, and it would help uncover bugs in the Windows side code, so I think this is a good change.

This revision is now accepted and ready to land.May 1 2019, 10:49 AM
This revision was automatically updated to reflect the committed changes.