This is an archive of the discontinued LLVM Phabricator instance.

[sanitizer] Move mmap interceptors into sanitizer_common
ClosedPublic

Authored by vitalybuka on Mar 5 2018, 4:07 PM.

Event Timeline

vitalybuka created this revision.Mar 5 2018, 4:07 PM
vitalybuka updated this revision to Diff 137102.Mar 5 2018, 4:09 PM

clang-format

Fine by me.

General question of curiosity, do we need to be admins to be able to launch a build based on a diff ?

General question of curiosity, do we need to be admins to be able to launch a build based on a diff ?

What build are you talking about?

General question of curiosity, do we need to be admins to be able to launch a build based on a diff ?

What build are you talking about?

Is it a builedbot ?
...
Build Status
Buildable 15701
Build 15701: arc lint + arc unit

vitalybuka added a comment.EditedMar 6 2018, 10:54 AM

General question of curiosity, do we need to be admins to be able to launch a build based on a diff ?

What build are you talking about?

Is it a builedbot ?
...
Build Status
Buildable 15701
Build 15701: arc lint + arc unit

Usually you don't need any login to trigger build on buildbot. Just put SVN revision into "Revision to build" and click build.
Also there is http://green.lab.llvm.org/green/, not sure if you can rebuild anything there.

morehouse added inline comments.Mar 6 2018, 1:35 PM
compiler-rt/lib/hwasan/hwasan_interceptors.cc
272

Aren't the above 3 lines handled by COMMON_INTERCEPTOR_ENTER?

472

Why does INTERCEPT_FUNCTION(mmap) stay here but not for other sanitizers?

compiler-rt/lib/msan/msan_interceptors.cc
979

Above 3 lines handled by COMMON_INTERCEPTOR_ENTER?

compiler-rt/lib/sanitizer_common/sanitizer_common_interceptors.inc
276

Purpose of if (0 && ... )?

compiler-rt/lib/tsan/rtl/tsan_interceptors.cc
2292

Looks like this is already called by COMMON_INTERCEPTOR_ENTER.

vitalybuka updated this revision to Diff 137268.Mar 6 2018, 1:56 PM
vitalybuka marked 4 inline comments as done.

addressed morehouse@ comments

vitalybuka updated this revision to Diff 137270.Mar 6 2018, 1:59 PM
vitalybuka marked an inline comment as done.

remove SCOPED_TSAN_INTERCEPTOR

morehouse added inline comments.Mar 6 2018, 2:42 PM
compiler-rt/lib/sanitizer_common/sanitizer_common_interceptors.inc
276

Does it make sense to condition this on common_flags()->intercept_intrin? Isn't that flag intended to affect only memset/memcpy/memmove?

277

Does it make sense to call COMMON_INTERCEPTOR_WRITE_RANGE at all? Doesn't mmap just create a mapping rather than writing to memory?

vitalybuka updated this revision to Diff 137278.Mar 6 2018, 2:58 PM
vitalybuka marked 2 inline comments as done.

No COMMON_INTERCEPTOR_WRITE_RANGE

compiler-rt/lib/sanitizer_common/sanitizer_common_interceptors.inc
277

sure, we don't need this.

This revision is now accepted and ready to land.Mar 6 2018, 3:06 PM
This revision was automatically updated to reflect the committed changes.
Herald added a subscriber: Restricted Project. · View Herald TranscriptMar 6 2018, 4:16 PM

Hi, this broke a Darwin bot: http://green.lab.llvm.org/green/job/clang-stage1-configure-RA/43370/console. Can you fix this so that we don't try to intercept mmap64 on Darwin?

Ok, I landed a fix in r326864.

eugenis added inline comments.Mar 14 2018, 4:48 PM
lib/sanitizer_common/sanitizer_common_interceptors.inc
6818 ↗(On Diff #137290)

This should use OFF64_T.

OFF_T is 32-bit and we pick garbage from the stack as the higher half of the 64-bit offset. This affects linux-i386 with FILE_OFFSET_BITS set to 64.

vitalybuka marked an inline comment as done.Mar 14 2018, 6:16 PM
vitalybuka added inline comments.
lib/sanitizer_common/sanitizer_common_interceptors.inc
6818 ↗(On Diff #137290)

Thanks, fixed in r327596