This is an archive of the discontinued LLVM Phabricator instance.

Hardware-assisted AddressSanitizer (llvm part).
ClosedPublic

Authored by eugenis on Dec 6 2017, 4:57 PM.

Details

Summary

This is LLVM instrumentation for the new HWASan tool. It is basically
a stripped down copy of ASan at this point, w/o stack or global
support. Instrumenation adds a global constructor + runtime callbacks
for every load and store.

HWASan comes with its own IR attribute.

A brief design document can be found in
clang/docs/HardwareAssistedAddressSanitizerDesign.rst (submitted earlier).

Diff Detail

Repository
rL LLVM

Event Timeline

eugenis created this revision.Dec 6 2017, 4:57 PM
kcc edited edge metadata.Dec 6 2017, 5:06 PM

Please document the new attribute and explain why the old attribute doesn't work for us (there are cases when we need one, but not the other, in both directions)

llvm/lib/Transforms/Instrumentation/HWAddressSanitizer.cpp
15 ↗(On Diff #125849)

Do you really need all these includes?

102 ↗(On Diff #125849)

update comment

eugenis updated this revision to Diff 125866.Dec 6 2017, 5:54 PM
eugenis marked an inline comment as done.

Documented the new attribute.

eugenis added inline comments.Dec 6 2017, 5:54 PM
llvm/lib/Transforms/Instrumentation/HWAddressSanitizer.cpp
15 ↗(On Diff #125849)

I'm not sure how to find the ones that are unused.

kcc added inline comments.Dec 6 2017, 6:11 PM
llvm/lib/Transforms/Instrumentation/HWAddressSanitizer.cpp
15 ↗(On Diff #125849)

I don't think we have a tool for that.
I've done it like this:

given e.g. #include "llvm/IR/MDBuilder.h" search for MDBuilder in the code. 
If none found, remove the include and rebuild. If build passes, you don't need the header.

There is clearly a risk that some other file actually includes MDBuilder and this code will break
with further refactoring of header files. But that's tolerable IMHO.

226 ↗(On Diff #125866)

do you need this section of code at all?
If not, I'd suggest to remove it and reinstate if needed, with a test.

256 ↗(On Diff #125866)

Do you need this? Do you have a test?
I'd prefer if we don't add code from asan/msan that we might not need in hwasan

336 ↗(On Diff #125866)

no {}

eugenis updated this revision to Diff 125867.Dec 6 2017, 6:27 PM
eugenis marked 2 inline comments as done.

removed some extra code and includes

eugenis marked an inline comment as done.Dec 6 2017, 6:27 PM
eugenis added inline comments.
llvm/lib/Transforms/Instrumentation/HWAddressSanitizer.cpp
256 ↗(On Diff #125866)

added a test

kcc added a comment.Dec 7 2017, 11:21 AM

LGTM, please wait for (at least) Aleksey's review.

alekseyshl accepted this revision.Dec 7 2017, 4:22 PM
This revision is now accepted and ready to land.Dec 7 2017, 4:22 PM
This revision was automatically updated to reflect the committed changes.