[PATCH] D149002: [compiler-rt][interception][asan][win] Improve error reporting

Alvin Wong via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Tue May 2 00:44:50 PDT 2023


alvinhochun marked 2 inline comments as not done.
alvinhochun added inline comments.


================
Comment at: compiler-rt/lib/interception/interception_win.cpp:157-161
+#  define Report(...)                     \
+    do {                                  \
+      if (ErrorReportCallback)            \
+        ErrorReportCallback(__VA_ARGS__); \
+    } while (0)
----------------
vitalybuka wrote:
> alvinhochun wrote:
> > vitalybuka wrote:
> > > Can we avoid a macro?
> > > static inline should work?
> > > 
> > > Can you call it ReportError, to avoid confusion for Report in common
> > The problem is calling a variadic function, there is no way that I know of to just forward the arguments without using `va_list`. There is currently not a version of `__sanitizer::Report` that takes `va_list`, so I would have to add one, which is more work than just using a macro here.
> > 
> > I can rename the macro to `ReportError`, yes.
> This should work https://en.cppreference.com/w/cpp/language/parameter_pack
> 
> ```
> template<class... Ts>
> void ReportError(Ts... args) {
>       if (ErrorReportCallback)            
>         ErrorReportCallback(args...); 
> }
> ```
Ah , didn't know we can do this. However, if I take this approach then we will lose the printf format check warnings provided by `__attribute__((format))` (doesn't work when applied to the template function with parameter pack). I think these checks are quite worth having.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D149002/new/

https://reviews.llvm.org/D149002



More information about the llvm-commits mailing list