[PATCH] D48446: [ubsan] Add support for reporting diagnostics to a monitor process

[Kuba (Brecka) Mracek via Phabricator via llvm-commits]

kubamracek added inline comments.


================
Comment at: lib/ubsan/ubsan_monitor.h:36
+/// that a UB report is available.
+extern "C" SANITIZER_INTERFACE_ATTRIBUTE void __ubsan_on_report(void);
+
----------------
vitalybuka wrote:
> vsk wrote:
> > vitalybuka wrote:
> > > vsk wrote:
> > > > vitalybuka wrote:
> > > > > I'd prefer interface consistent with similar one from TSAN:
> > > > > __tsan_on_report(void *report) {
> > > > >   __tsan_get_report_loc(report.....
> > > > >   __tsan_get_report_loc_object_type(report.....
> > > > > }
> > > > > 
> > > > > Also this will avoid having state set by RegisterUndefinedBehaviorReport
> > > > Point taken, it would be a bit simpler this way. However, we can't make this change because it would break debuggability of code compiled with Xcode 9.
> > > Does Xcode 9 works for tsan?
> > Are you asking if IDE diagnostics for tsan issues found in a binary compiled with Xcode 9, work when running under Xcode (9+K)? I'm not sure. Generally we try to make it possible to debug old binaries.
> I am confused. Could you please elaborate why change in non existing yet API will break code compiled with Xcode 9?
> 
> To clarify, I propose to have:
> ```
> __ubsan_on_report(void *report) {
>     __ubsan_get_report_loc(report.....
>     __ubsan_get_report_other_stuff(report.....
> }
> ...
> similar to tsan
Yes, Xcode 9 supports TSan and UBSan. I think we really need to keep the interface as it is, at least for this initial commit. Changes in the interface should then try to be as compatible as possible. Every time TSan's debugging interface changed, we did that in a backwards-compatible manner (debugging old binaries works), and forwards-compatible way as well (worst case is you'll miss some data in the report).


https://reviews.llvm.org/D48446