This is an archive of the discontinued LLVM Phabricator instance.

[UBSan] Add the ability to print more precise error kind in summary line.
ClosedPublic

Authored by samsonov on Aug 20 2015, 2:40 PM.

Diff Detail

Event Timeline

samsonov updated this revision to Diff 32753.Aug 20 2015, 2:40 PM
samsonov retitled this revision from to [UBSan] Add the ability to print more precise error kind in summary line..
samsonov updated this object.
samsonov added reviewers: rsmith, pcc.
samsonov added a subscriber: cfe-commits.
filcab added a subscriber: filcab.Aug 20 2015, 3:48 PM

Looks good.

lib/ubsan/ubsan_flags.inc
26

Do we care that much about keeping the "always say 'undefined-behavior'" behavior around?

samsonov added inline comments.Aug 20 2015, 3:58 PM
lib/ubsan/ubsan_flags.inc
26

Yes, there are users who grep for "SUMMARY: .* undefined-behavior" lines in their logs. We should stay backwards-compatible for a while.

rsmith added inline comments.Aug 20 2015, 4:05 PM
lib/ubsan/ubsan_handlers.cc
57–58

Can we pass the error type to Diag in the place of DL_Error instead of specifying it separately?

samsonov added inline comments.Aug 20 2015, 4:19 PM
lib/ubsan/ubsan_handlers.cc
57–58

We still need error type somewhere outside Diag, because it will be used later when we print (or not print) summary, after several Diag invocations (for DL_Error and DL_Note).

rsmith added inline comments.Aug 20 2015, 4:43 PM
lib/ubsan/ubsan_handlers.cc
57–58

We would make exactly one invocation to Diag with an error type, wouldn't we? Is the summary information computed separately from diagnostic emission?

samsonov added inline comments.Aug 20 2015, 5:02 PM
lib/ubsan/ubsan_handlers.cc
57–58

The answer to last question is "yes". Diag object prints the diagnostic message in destructor (and there can be several Diag objects). We need to print summary information after all the diagnostics (DL_Error and DL_Note), so we do this in ScopedReport destructor.

There is a different benefit in passing error kind to Diag() object - we can print the actual error kind instead of generic runtime error: line, but that's a different problem, which can probably be addressed afterwards.

This revision was automatically updated to reflect the committed changes.