This is an archive of the discontinued LLVM Phabricator instance.

Add experimental clang/driver flag -fsanitize-address-field-padding=N
ClosedPublic

Authored by kcc on Oct 8 2014, 11:50 AM.

Details

Summary

This change adds an experimental flag -fsanitize-address-field-padding=N (0, 1, 2)
to clang and driver. With this flag ASAN will be able to detect some cases of
intra-object-overflow bugs,
see https://code.google.com/p/address-sanitizer/wiki/IntraObjectOverflow

There is no actual functionality here yet, just the flag parsing.
The functionality is being reviewed at http://reviews.llvm.org/D5687

Diff Detail

Event Timeline

kcc updated this revision to Diff 14587.Oct 8 2014, 11:50 AM
kcc retitled this revision from to Add experimental clang/driver flag -fsanitize-address-field-padding=N.
kcc updated this object.
kcc edited the test plan for this revision. (Show Details)
kcc added a reviewer: samsonov.
kcc added a subscriber: Unknown Object (MLST).
samsonov edited edge metadata.Oct 8 2014, 4:30 PM

What "level of field padding" is, at all? My best guess is "-fsanitize-address-field-padding=1" is "less aggressive", and "2" is "more aggressive". Can you clarify it somewhere (in LangOptions header?) You may also introduce a enum in LangOptions (smth. ike LangOptions::StackProtectorMode).

Please add a test case to test/Driver/fsanitize.c

include/clang/Basic/LangOptions.h
29

Why not AsanFieldPadding (or at least SanitizeAddressFieldPadding, or AddressSanitizerFieldPadding)?

lib/Driver/SanitizerArgs.cpp
171

Comment doesn't match the code.

kcc updated this revision to Diff 14618.Oct 8 2014, 5:30 PM
kcc edited edge metadata.

addressed all comments, PTAL

samsonov accepted this revision.Oct 8 2014, 5:39 PM
samsonov edited edge metadata.

LGTM.
Please commit this together with http://reviews.llvm.org/D5687, so that we don't have unused flags lying around.

lib/Driver/SanitizerArgs.cpp
30

Put this one line above, to respect the order of declaration.

This revision is now accepted and ready to land.Oct 8 2014, 5:39 PM
kcc updated this revision to Diff 14619.Oct 8 2014, 6:23 PM
kcc edited edge metadata.

I actually wanted to commit this separately to disentangle
the review process. It may actually require some additional changes in flags.

Sure, feel free to proceed with this change, and just include the links to the other review threads in the commit message.

kcc added a comment.Oct 9 2014, 10:44 AM

I deliberately did not document the flags in this CL because I don't know what the end state will be.
Maybe we can end up with just one flag, maybe several levels of aggressiveness, maybe even several independent predicates.
This will remain experimental for several months and should not be exposed to users (except for the brave ones)

kcc updated this object.Oct 9 2014, 11:02 AM
kcc closed this revision.Oct 9 2014, 11:03 AM