[msan] Fix build on musl

[thesamesam]

Summary

[msan] fix build on musl

In 4e1a6c07052b466a2a1cd0c3ff150e4e89a6d87a, a check for glibc was added,
but this breaks the build on musl, as the __GLIBC_PREREQ() macro isn't
defined.

Bug: https://bugs.gentoo.org/829742
Signed-off-by: Sam James <[e-mail]>

[MaskRay]

compiler-rt/lib/msan/msan_interceptors.cpp

655

Perhaps defined(__GLIBC__) && ((__GLIBC_MAJOR__ == 2 && __GLIBC_MINOR__ >= 33) || __GLIBC_MAJOR__ >= 3))

__GLIBC__ may trigger a -Wundef warning.

[thesamesam]

compiler-rt/lib/msan/msan_interceptors.cpp

655

I was wondering about this and tried it, but I sadly get:

/var/tmp/portage/sys-libs/compiler-rt-sanitizers-13.0.0-r1/work/compiler-rt/lib/msan/msan_interceptors.cpp:660:46: warning: macro expansion producing 'defined' has undefined behavior [-Wexpansion-to-defined]
#if SANITIZER_FREEBSD || SANITIZER_NETBSD || SANITIZER_STAT_LINUX
                                             ^
/var/tmp/portage/sys-libs/compiler-rt-sanitizers-13.0.0-r1/work/compiler-rt/lib/msan/msan_interceptors.cpp:659:50: note: expanded from macro 'SANITIZER_STAT_LINUX'
#define SANITIZER_STAT_LINUX (SANITIZER_LINUX && defined(__GLIBC__) && ((__GLIBC_MAJOR__ == 2 && __GLIBC_MINOR__ >= 33) || __GLIBC_MAJOR__ >= 3))
                                                 ^
/var/tmp/portage/sys-libs/compiler-rt-sanitizers-13.0.0-r1/work/compiler-rt/lib/msan/msan_interceptors.cpp:699:46: warning: macro expansion producing 'defined' has undefined behavior [-Wexpansion-to-defined]
#if SANITIZER_FREEBSD || SANITIZER_NETBSD || SANITIZER_STAT_LINUX
                                             ^
/var/tmp/portage/sys-libs/compiler-rt-sanitizers-13.0.0-r1/work/compiler-rt/lib/msan/msan_interceptors.cpp:659:50: note: expanded from macro 'SANITIZER_STAT_LINUX'
#define SANITIZER_STAT_LINUX (SANITIZER_LINUX && defined(__GLIBC__) && ((__GLIBC_MAJOR__ == 2 && __GLIBC_MINOR__ >= 33) || __GLIBC_MAJOR__ >= 3))
                                                 ^
2 warnings generated.

According to https://stackoverflow.com/questions/42074035/how-to-deal-with-clangs-3-9-wexpansion-to-defined-warning,

I don't see a -Wundef warning on musl, but it is still a valid point..

I could change to the style of change given on SO if needed but wanted to avoid too much more complexity as this area is already somewhat macro heavy.

[vitalybuka]

compiler-rt/lib/msan/msan_interceptors.cpp

655

Can we just drop version check at all?
with version check we can't build runtime on one host and the use on another if glibc missmatch
like here:
https://lab.llvm.org/staging/#/waterfall?tags=sanitizer

this bot uses runtime from chromium project

[thesamesam]

compiler-rt/lib/msan/msan_interceptors.cpp

655

In ad294e572bc5c16f9dc420cc994322de6ca3fbfb, I see that we handle runtime checks differently (which makes sense, for the reason you describe.

I'd be more comfortable dropping the version check entirely in a separate change given I'd like to request this change for backporting though, and this isn't my area of expertise.

[MaskRay]

compiler-rt/lib/msan/msan_interceptors.cpp

655

I may be missing something. Won't removing version checks cause something like
https://github.com/google/oss-fuzz/issues/2836 (there is a __GLIBC_PREREQ usage for getrandom)?

Do sanitizers reliably work with (build with glibc X, it can work with glibc y where x < y`)?

[vitalybuka]

compiler-rt/lib/msan/msan_interceptors.cpp

655

To clarify:
just

#define SANITIZER_STAT_LINUX (SANITIZER_LINUX && __GLIBC_)
...
#endif

655

depends on interceptor.
if version mismatch we may define function which is missing from glibc.
Then program will crash on REAL() call. However program should not call the interceptor on glibc without the function.
But we can't control that. So it's also bad :(

655

this code includes sanitizer_glibc_version.h
it should define __GLIBC_PREREQ there

655

In ad294e572bc5c16f9dc420cc994322de6ca3fbfb, I see that we handle runtime checks differently (which makes sense, for the reason you describe.

I'd be more comfortable dropping the version check entirely in a separate change given I'd like to request this change for backporting though, and this isn't my area of expertise.

I agree

[vitalybuka]

compiler-rt/lib/msan/msan_interceptors.cpp

655

To clarify:
just

#define SANITIZER_STAT_LINUX (SANITIZER_LINUX && __GLIBC_)
...
#endif

Ignore this comment, as we agreed to keep version.

[thesamesam]

compiler-rt/lib/msan/msan_interceptors.cpp

655

That's a good point, let's try find out why.

If I add: #define __GLIBC_PREREQ(x,y) 0 right above the broken line, it works. But removing it fails.

It appears to be related to the order of inclusion?

If I add a #warning and a #error to sanitizer_glibc_version.h like:

#warning "here"
#error "now here"

I get:

/var/tmp/portage/sys-libs/compiler-rt-sanitizers-13.0.0-r1/work/compiler-rt/lib/msan/msan_interceptors.cpp:660:46: error: function-like macro '__GLIBC_PREREQ' is not defined
#if SANITIZER_FREEBSD || SANITIZER_NETBSD || SANITIZER_STAT_LINUX
                                             ^
/var/tmp/portage/sys-libs/compiler-rt-sanitizers-13.0.0-r1/work/compiler-rt/lib/msan/msan_interceptors.cpp:659:50: note: expanded from macro 'SANITIZER_STAT_LINUX'
#define SANITIZER_STAT_LINUX (SANITIZER_LINUX && __GLIBC_PREREQ(2, 33))
                                                 ^
/var/tmp/portage/sys-libs/compiler-rt-sanitizers-13.0.0-r1/work/compiler-rt/lib/msan/msan_interceptors.cpp:699:46: error: function-like macro '__GLIBC_PREREQ' is not defined
#if SANITIZER_FREEBSD || SANITIZER_NETBSD || SANITIZER_STAT_LINUX
                                             ^
/var/tmp/portage/sys-libs/compiler-rt-sanitizers-13.0.0-r1/work/compiler-rt/lib/msan/msan_interceptors.cpp:659:50: note: expanded from macro 'SANITIZER_STAT_LINUX'
#define SANITIZER_STAT_LINUX (SANITIZER_LINUX && __GLIBC_PREREQ(2, 33))
                                                 ^
In file included from /var/tmp/portage/sys-libs/compiler-rt-sanitizers-13.0.0-r1/work/compiler-rt/lib/msan/msan_interceptors.cpp:1362:
In file included from /var/tmp/portage/sys-libs/compiler-rt-sanitizers-13.0.0-r1/work/compiler-rt/lib/msan/../sanitizer_common/sanitizer_platform_interceptors.h:16:
/var/tmp/portage/sys-libs/compiler-rt-sanitizers-13.0.0-r1/work/compiler-rt/lib/msan/../sanitizer_common/sanitizer_glibc_version.h:18:2: warning: "here" [-W#warnings]
#warning "here"
 ^
1 warning and 2 errors generated.

[thesamesam]

Now I understand. I'd missed 5869ea6c6254d848382f22f82b02b698d9c53260 which was added 2 weeks ago!

Thank you both.

compiler-rt/lib/msan/msan_interceptors.cpp

655

Even adding it right below

compiler-rt/lib/msan/msan_interceptors.cpp

 
 INTERCEPTOR(int, putenv, char *string) {
   ENSURE_MSAN_INITED();
   int res = REAL(putenv)(string);
   if (!res) UnpoisonEnviron();
   return res;
 }
 
-#define SANITIZER_STAT_LINUX (SANITIZER_LINUX && __GLIBC_PREREQ(2, 33))
+#define SANITIZER_STAT_LINUX (SANITIZER_LINUX && __GLIBC__ && ((__GLIBC_MAJOR__ == 2 && __GLIBC_MINOR__ >= 33) || __GLIBC_MAJOR__ >= 3))
 #if SANITIZER_FREEBSD || SANITIZER_NETBSD || SANITIZER_STAT_LINUX
 INTERCEPTOR(int, fstat, int fd, void *buf) {
   ENSURE_MSAN_INITED();
   int res = REAL(fstat)(fd, buf);
   if (!res)
     __msan_unpoison(buf, __sanitizer::struct_stat_sz);
   return res;
 }