[ASan] Only include rpc headers if they are available.

[ygribov]

Summary

See bug https://gcc.gnu.org/bugzilla/show_bug.cgi?id=64839 for more information.

[kcc]

CMakeLists.txt

183

Can you do it w/o cmake checks?
asan rt is built by many build systems, some of which you don't have access to,
and some of which I don't have access to.
All such checks should be done in the sources code.

[ygribov]

CMakeLists.txt

183

Can you do it w/o cmake checks?

I don't think so, Glibc headers do not have any marker to identify whether xdr.h is available or not.

asan rt is built by many build systems, some of which you don't have access to

I'll do the Autoconf part in GCC myself. As for other build systems I think we can safely assume that checking
for existence of header should be well supported.

[kcc]

CMakeLists.txt

183

Can't you rely on the glibc version?
Anyway, at the very least, the code should build on a modern glibc w/o needing extra compile flags.

I.e. provide default values for these macros in a header file (sanitizer_platform_limits_posix.h?)
Then, If you don't need the cmake part here, just drop it.

[ygribov]

CMakeLists.txt

183

Can't you rely on the glibc version?

No, Glibc is not guaranteed to include xdr.h regardless of version. If boils down to distribution maintainer deciding whether he wants to build his glibc with --enable-obsolete-rpc or not.

Anyway, at the very least, the code should build on a modern glibc w/o needing extra compile flags.
I.e. provide default values for these macros
in a header file (sanitizer_platform_limits_posix.h?)

You mean throw in safe ifndefs? Makes sense. While at it, is the rest of the patch fine?

All such checks should be done in the sources code.

But don't we already have tons of configury done in buildscripts? Like ASAN_LOW_MEMORY=1, etc.

[samsonov]

CMakeLists.txt

183

What Kostya says. I'm OK with optional inclusion of headers, as long as they are #included by default, and we use specific build systems (CMake, autoconf, etc.) to optionally disable them.

184

check_include_file just sets the value of CMake variable HAVE_TIRPC_XDR_H, it will not be turned into compiler definition on its own. You will have to either use generate compiler-rt-config.h (meh), or use this variable to add compiler definitions in CMake (better).

[ygribov]

Updated based on Kostya's suggestions. Also performed more rigorous testing:

• rpc/xdr.h + tirpc/rpc/xdr.h - works
• only rpc/xdr.h - works
• only tirpc/rpc/xdr.h - works
• none - works

[samsonov]

lib/sanitizer_common/CMakeLists.txt

120 ↗

(On Diff #23171)

Does HAVE_TIRPC_XDR_H get the value "1" in this macro? Consider moving this logic (check_include_file + adjusting the variable value) to some common macro.

lib/sanitizer_common/sanitizer_platform.h

134 ↗

(On Diff #23171)

I'd prefer the logic like this:

#if !defined(HAVE_RPC_XDR_H) && !defined(HAVE_TIRPC_XDR_H)
# define HAVE_RPC_XDR_H (SANITIZER_LINUX && !SANITIZER_ANDROID)
# define HAVE_TIRPC_XDR_H 0
#endif

lib/sanitizer_common/sanitizer_platform_limits_posix.cc

1166–1167

Looks like you can use just use

#if (HAVE_RPC_XDR_H || HAVE_TIRPC_XDR_H)

here

[ygribov]

Updated after review. The append_have_file_definition macro is ugly ugly (just as CMake itself with it's crazy language idiosyncrasies is).

[samsonov]

LGTM

CMakeLists.txt

 # COMPILER_RT_DEBUG_PYBOOL is used by lit.common.configured.in.
 pythonize_bool(COMPILER_RT_DEBUG)
 
 #================================
 # Setup Compiler Flags
 #================================
 include(CheckIncludeFile)
 check_include_file(unwind.h HAVE_UNWIND_H)
+check_include_file(rpc/xdr.h HAVE_RPC_XDR_H)
+check_include_file(tirpc/rpc/xdr.h HAVE_TIRPC_XDR_H)
 
 include(config-ix)
 
 if(MSVC)
   append_string_if(COMPILER_RT_HAS_W3_FLAG /W3 CMAKE_C_FLAGS CMAKE_CXX_FLAGS)
 else()
   append_string_if(COMPILER_RT_HAS_WALL_FLAG -Wall CMAKE_C_FLAGS CMAKE_CXX_FLAGS)
 endif()

lib/sanitizer_common/sanitizer_platform_limits_posix.cc

 #if SANITIZER_LINUX && !SANITIZER_ANDROID
 #include <glob.h>
 #include <obstack.h>
 #include <mqueue.h>
 #include <net/if_ppp.h>
 #include <netax25/ax25.h>
 #include <netipx/ipx.h>
 #include <netrom/netrom.h>
-#include <rpc/xdr.h>
+#if HAVE_RPC_XDR_H
+# include <rpc/xdr.h>
+#elif HAVE_TIRPC_XDR_H
+# include <tirpc/rpc/xdr.h>
+#endif
 #include <scsi/scsi.h>
 #include <sys/mtio.h>
 #include <sys/kd.h>
 #include <sys/shm.h>
 #include <sys/statvfs.h>
 #include <sys/timex.h>
 #if defined(__mips64)
 # include <sys/procfs.h>

 
 
 CHECK_TYPE_SIZE(group);
 CHECK_SIZE_AND_OFFSET(group, gr_name);
 CHECK_SIZE_AND_OFFSET(group, gr_passwd);
 CHECK_SIZE_AND_OFFSET(group, gr_gid);
 CHECK_SIZE_AND_OFFSET(group, gr_mem);
 
-#if SANITIZER_LINUX && !SANITIZER_ANDROID
+#if SANITIZER_LINUX && !SANITIZER_ANDROID && \
+    (HAVE_RPC_XDR_H || HAVE_TIRPC_XDR_H)
 CHECK_TYPE_SIZE(XDR);
 CHECK_SIZE_AND_OFFSET(XDR, x_op);
 CHECK_SIZE_AND_OFFSET(XDR, x_ops);
 CHECK_SIZE_AND_OFFSET(XDR, x_public);
 CHECK_SIZE_AND_OFFSET(XDR, x_private);
 CHECK_SIZE_AND_OFFSET(XDR, x_base);
 CHECK_SIZE_AND_OFFSET(XDR, x_handy);
 COMPILER_CHECK(__sanitizer_XDR_ENCODE == XDR_ENCODE);