This is an archive of the discontinued LLVM Phabricator instance.

[ASan] Enable optional ASan recovery (LLVM core part)
ClosedPublic

Authored by ygribov on Nov 2 2015, 9:07 AM.

Details

Summary

This patch adds support for recoverable ASan in LLVM. This is an addition to http://reviews.llvm.org/D12318

Diff Detail

Event Timeline

ygribov updated this revision to Diff 38933.Nov 2 2015, 9:07 AM
ygribov retitled this revision from to [ASan] Enable optional ASan recovery (LLVM core part).
ygribov updated this object.
ygribov added reviewers: eugenis, samsonov, kcc.
ygribov added subscribers: llvm-commits, kubamracek, filcab and 2 others.
eugenis edited edge metadata.Nov 5 2015, 11:39 AM

Please add an IR test for the _noabort calls.

lib/Transforms/Instrumentation/AddressSanitizer.cpp
143

I'd remove these WARNINGS here and in the compiler-rt patch too. The whole "ASan" thing is use-at-your-own-risk, and this feature is not that much scarier than the rest.

1097–1100

{}
I think we never do "if" w/o {} followed by "else" with {}.

ygribov added inline comments.Nov 5 2015, 11:44 AM
lib/Transforms/Instrumentation/AddressSanitizer.cpp
143

Ok, although Kostya may disagree here)

1097–1100

Sorry, I remember you mentioning this last time.

ygribov updated this revision to Diff 39520.Nov 6 2015, 6:51 AM
ygribov edited edge metadata.

Minor fixes after review.

ygribov marked 4 inline comments as done.Nov 6 2015, 6:51 AM

Guys, is this one ok?

eugenis requested changes to this revision.Nov 9 2015, 9:22 AM
eugenis edited edge metadata.

Needs a test.

This revision now requires changes to proceed.Nov 9 2015, 9:22 AM
ygribov updated this revision to Diff 39714.Nov 9 2015, 9:55 AM
ygribov edited edge metadata.

Added a test.

eugenis accepted this revision.Nov 9 2015, 10:30 AM
eugenis edited edge metadata.

LGTM

test/Instrumentation/AddressSanitizer/keep_going.ll
9

Maybe also check that it is followed by a branch to the exit block, or that the basic block does not end with unreachable.

This revision is now accepted and ready to land.Nov 9 2015, 10:30 AM
This revision was automatically updated to reflect the committed changes.