[PATCH] D87375: [compiler-rt] Support glibc's non-standard I printf flag character

[Hubert Tong via Phabricator via llvm-commits]

hubert.reinterpretcast added inline comments.


================
Comment at: compiler-rt/lib/sanitizer_common/sanitizer_common_interceptors_format.inc:410
     // Flags
-    while (char_is_one_of(*p, "'-+ #0")) {
+    while (char_is_one_of(*p, "'-+ #0I")) {
       ++p;
----------------
sberg wrote:
> hubert.reinterpretcast wrote:
> > Note that Clang favours the MSVCRT interpretation of `printf("%I64d\n", 42LL);`.
> But then again all of this code is guarded by
> ```
> #if SANITIZER_INTERCEPT_PRINTF
> ```
> and the only place to define `SANITIZER_INTERCEPT_PRINTF` appears to be
> ```
> # define SANITIZER_INTERCEPT_PRINTF SI_POSIX
> ```
> in `compiler-rt/lib/sanitizer_common/sanitizer_platform_interceptors.h`, so it looks like this code is only used for Posix environments, at least for now?
> 
> But maybe it would be better to only include `I` in the above string literal conditionally on `SI_POSIX` (or `SANITIZER_POSIX`, which appears to be the same thing, cf. `llvm-project/compiler-rt/lib/sanitizer_common/sanitizer_platform_interceptors.h`)?
The more specific guarding here sounds like a good idea to me (but I have very little exposure to/experience in the sanitizer code). `SI_POSIX` would at least take out the Windows side of things in terms of considerations.

Would using `__GLIBC_PREREQ` make sense here? Or is that too specific?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87375