Page MenuHomePhabricator

[scudo] Add verbose failures in place of CHECK(0)
ClosedPublic

Authored by cryptoad on Jun 14 2018, 2:31 PM.

Details

Summary

The current FailureHandler mechanism was fairly opaque with regard to the
failure reason due to using CHECK(0). Scudo is a bit different from the other
Sanitizers as it prefers to avoid spurious processing in its failure path. So
we just dieWithMessage using a somewhat explicit string.

Adapted the tests for the new strings.

While this takes care of the OnBadRequest & OnOOM failures, the next step
is probably to migrate the other Scudo failures in the same failes (header
corruption, invalid state and so on).

Diff Detail

Event Timeline

cryptoad created this revision.Jun 14 2018, 2:31 PM
Herald added subscribers: Restricted Project, delcypher, mgorny. · View Herald TranscriptJun 14 2018, 2:31 PM
alekseyshl accepted this revision.Jun 14 2018, 3:14 PM
This revision is now accepted and ready to land.Jun 14 2018, 3:14 PM
filcab added a subscriber: filcab.Jun 15 2018, 3:30 AM
filcab added inline comments.
lib/scudo/scudo_allocator.cpp
328

Maybe also define it out of line? No use making the class definition harder to read if we don't even want to inline this function.

cryptoad added inline comments.Jun 15 2018, 8:16 AM
lib/scudo/scudo_allocator.cpp
328

Let me remove that from this CL, and in another I can add it and move definitions of isRssLimitExceeded & performSanityChecks out of line.

cryptoad updated this revision to Diff 151514.Jun 15 2018, 8:16 AM

Removing a NOINLINE that doesn't belong to this CL.

cryptoad marked 2 inline comments as done.Jun 15 2018, 8:26 AM
This revision was automatically updated to reflect the committed changes.