This is an archive of the discontinued LLVM Phabricator instance.

[esan] EfficiencySanitizer libc interceptors
ClosedPublic

Authored by bruening on Apr 22 2016, 7:57 AM.

Details

Summary

Adds libc interceptors to the runtime library for the new
EfficiencySanitizer ("esan") family of tools. The interceptors cover
the memory operations in most common library calls and will be shared
among all esan tools.

Diff Detail

Repository
rL LLVM

Event Timeline

bruening updated this revision to Diff 54653.Apr 22 2016, 7:57 AM
bruening retitled this revision from to [esan] EfficiencySanitizer libc interceptors.
bruening updated this object.
bruening added a reviewer: aizatsky.
bruening added subscribers: kcc, eugenis, vitalybuka, llvm-commits.
aizatsky added inline comments.Apr 22 2016, 11:01 AM
lib/esan/esan_interceptors.cpp
130 ↗(On Diff #54653)

Use COMMON_INTERCEPTOR_WRITE_RANGE & COMMON_INTERCEPTOR_READ_RANGE
throughout these methods. You have defined them anyway. It will make moving this code easier.

137 ↗(On Diff #54653)

Is this "n" correct or should it be the same expression as next line?

143 ↗(On Diff #54653)

msan_interceptor does this differently:

(pseudocode)

#if FREEBSD

INTERCEPTOR(stat)

#else

INTERCEPTOR(__xstat)

#endif.

Is there a reason you always add 2 intercepts?

bruening marked 3 inline comments as done.Apr 22 2016, 1:15 PM
bruening added inline comments.
lib/esan/esan_interceptors.cpp
137 ↗(On Diff #54653)

Both asan and tsan consider it to write the full passed-in size but read the min value, as I had it here (modeled after tsan). The full passed-in size should be addressable, but it should not be marked as written, so they both seem incorrect. It should be the next line, for purposes of what's actually written. I will send a separate CL to change tsan and asan (unless you know of reasons why they want the capacity and not the actual written value: for syscalls, DrMemory treats the capacity and written amount differently, but I'm not sure whether these sanitizers do something like that on libc calls and I don't see code for such separate handling at first glance).

143 ↗(On Diff #54653)

This is precisely what tsan does. It looks incorrect. I will fix here and and have a separate CL for fixing tsan. It sounds like tsan wasn't the best one to use as a model, or that perhaps these should have been moved to the common pool first where the process would have compared to other sanitizers. There are several confusing differences between the tsan and msan interceptors though which will take effort to sort out: later.

bruening updated this revision to Diff 54708.Apr 22 2016, 1:17 PM
bruening marked 2 inline comments as done.

Normalize interceptors

Switches the esan interceptors to use the COMMON_INTERCEPTOR_ENTER and the
common read and write range routines with contexts, to match the common
interceptors and make it easier to move later.

Updates the stat interceptor to match msan instead of the
seemingly-incorrect tsan.

aizatsky added inline comments.Apr 22 2016, 1:32 PM
lib/esan/esan_interceptors.cpp
177 ↗(On Diff #54708)

should xstat64, lstat, lstat64 interceptors be structured the same way as stat?

bruening added inline comments.Apr 22 2016, 2:05 PM
lib/esan/esan_interceptors.cpp
177 ↗(On Diff #54708)

Not for xstat64 or lstat64, no: they are simply not present on FreeBSD or Android so there is no #else. As to the !FreeBSD check of msan vs the !Android check of tsan, I suppose both should be there.

For lstat + lxstat, yes, that does look like a similar thing to stat + xstat.

bruening updated this revision to Diff 54722.Apr 22 2016, 2:07 PM

Add FreeBSD checks to *stat64 and split lstat in a cleaner way.

Adds further improvements to the *stat interceptors, combining the
incomplete msan and tsan interceptors.

aizatsky accepted this revision.Apr 22 2016, 4:07 PM
aizatsky edited edge metadata.
This revision is now accepted and ready to land.Apr 22 2016, 4:07 PM
This revision was automatically updated to reflect the committed changes.