Page MenuHomePhabricator

[Patch] UBsan: Type-based blacklisting
ClosedPublic

Authored by byoungyoung on Jul 7 2014, 9:03 AM.

Details

Summary

Added Type-based blacklisting for sanitizers (SanitizerBlacklist::isBlacklistedType). On top of this, also added type-based blacklisting for UBsan's vptr.

Diff Detail

Event Timeline

byoungyoung updated this revision to Diff 11119.Jul 7 2014, 9:03 AM
byoungyoung retitled this revision from to [Patch] UBsan: Type-based blacklisting.
byoungyoung updated this object.
byoungyoung edited the test plan for this revision. (Show Details)
byoungyoung added reviewers: samsonov, rsmith, kcc.
byoungyoung added a subscriber: Unknown Object (MLST).
rsmith edited edge metadata.Jul 8 2014, 5:12 PM

The code change looks fine to me, but needs a testcase.

Thanks for taking a look, and let me add a testcase (soon).

byoungyoung edited edge metadata.

Added a testcase for ubsan's vptr blacklisting.

samsonov added inline comments.Jul 9 2014, 5:55 PM
lib/CodeGen/CGExpr.cpp
539

Nit: please upload the diffs with more context. This can be done with e.g. "arc" commandline tool (see http://llvm.org/docs/Phabricator.html). This change is probably fine, though, as it's local.

544

Now you shouldn't have generic "isIn(StringRef)" machinery in SanitizerBlacklist. Instead, it would be better to extend SanitizerBlacklist (which is now moved to lib/CodeGen/SanitizerBlacklist.h) with function like "isBlacklistedType(StringRef MangledTypeName)" and use it here.

samsonov edited edge metadata.Jul 9 2014, 5:55 PM

That is, LLVM part of your patch shouldn't be required now.

byoungyoung edited edge metadata.

Modified to be compatible with r212643, and thus dropping the dependency with http://reviews.llvm.org/D4406.

Thanks Alex for the comments!

samsonov accepted this revision.Jul 10 2014, 10:35 AM
samsonov edited edge metadata.

LGTM modulo nit below. Let me know if I should commit this for you. Test in Clang is fine, but I would also add an "end-to-end" test case in compiler-rt ubsan lit test-suite. This can of course be done separately. Thanks!

test/CodeGen/ubsan-type-blacklist.cpp
20

I'd use less restrictive pattern for TYPE-NOT, smth. like

TYPE-NOT: @__ubsan_handle_dynamic_type_cache_miss
This revision is now accepted and ready to land.Jul 10 2014, 10:35 AM

Thanks Alex! Yes, please commit this if it is possible.

byoungyoung updated this object.Jul 10 2014, 2:02 PM
byoungyoung edited edge metadata.

Landed with minor modifications as r212770. Thanks!

samsonov closed this revision.Jul 10 2014, 3:43 PM