This is an archive of the discontinued LLVM Phabricator instance.

sanitizer_common: deduplicate CheckFailed
ClosedPublic

Authored by dvyukov on May 11 2021, 1:22 AM.

Details

Summary

We have some significant amount of duplication around
CheckFailed functionality. Each sanitizer copy-pasted
a chunk of code. Some got random improvements like
dealing with recursive failures better. These improvements
could benefit all sanitizers, but they don't.

Deduplicate CheckFailed logic across sanitizers and let each
sanitizer only print the current stack trace.
I've tried to dedup stack printing as well,
but this got me into cmake hell. So let's keep this part
duplicated in each sanitizer for now.

Diff Detail

Event Timeline

dvyukov created this revision.May 11 2021, 1:22 AM
dvyukov requested review of this revision.May 11 2021, 1:22 AM
Herald added a project: Restricted Project. · View Herald TranscriptMay 11 2021, 1:22 AM
Herald added a subscriber: Restricted Project. · View Herald Transcript
vitalybuka accepted this revision.May 11 2021, 7:36 PM
vitalybuka added inline comments.
compiler-rt/lib/sanitizer_common/sanitizer_termination.cpp
67–90

maybe?

This revision is now accepted and ready to land.May 11 2021, 7:36 PM
dvyukov updated this revision to Diff 344667.May 11 2021, 10:48 PM

Simplify CheckFailed logic.

dvyukov added inline comments.May 11 2021, 10:52 PM
compiler-rt/lib/sanitizer_common/sanitizer_termination.cpp
67–90

Good point.
I see you removed num_calls, but we will still print the second CHECK failure as we recurse, right? I think that's enough.
PTAL

For future: for the diff you posted is there a way to look at it as side-by-side diff, or apply locally, or download raw diff with +/- marks?

vitalybuka accepted this revision.May 11 2021, 11:09 PM
vitalybuka added inline comments.
compiler-rt/lib/sanitizer_common/sanitizer_termination.cpp
67–90

I can't find anything useful we can do with such comments. I suspect that maybe pasting code as-is in comment is more convenient:

void f();
This revision was landed with ongoing or failed builds.May 11 2021, 11:51 PM
This revision was automatically updated to reflect the committed changes.