[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 ↗

(On Diff #22870)

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 ↗

(On Diff #22870)

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 ↗

(On Diff #22870)

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 ↗

(On Diff #22870)

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 ↗

(On Diff #22870)

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 ↗

(On Diff #22870)

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

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

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

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

cmake/Modules/CompilerRTUtils.cmake

if(${condition})
     endforeach()
   endif()
 endmacro()
 
 macro(append_no_rtti_flag list)
   append_list_if(COMPILER_RT_HAS_FNO_RTTI_FLAG -fno-rtti ${list})
   append_list_if(COMPILER_RT_HAS_GR_FLAG /GR- ${list})
 endmacro()
+
+macro(append_have_file_definition filename varname list)
+  check_include_file("${filename}" "${varname}")
+  if (NOT "${varname}")
+    set("${varname}" 0)
+  endif()
+  list(APPEND ${list} "${varname}=${${varname}}")
+endmacro()

lib/sanitizer_common/CMakeLists.txt

 if(MSVC)
   list(APPEND SANITIZER_COMMON_DEFINITIONS
     SANITIZER_NEEDS_SEGV=0)
 else()
   list(APPEND SANITIZER_COMMON_DEFINITIONS
     SANITIZER_NEEDS_SEGV=1)
 endif()
 
+include(CheckIncludeFile)
+append_have_file_definition(rpc/xdr.h HAVE_RPC_XDR_H SANITIZER_COMMON_DEFINITIONS)
+append_have_file_definition(tirpc/rpc/xdr.h HAVE_TIRPC_RPC_XDR_H SANITIZER_COMMON_DEFINITIONS)
+
 set(SANITIZER_CFLAGS ${SANITIZER_COMMON_CFLAGS})
 append_no_rtti_flag(SANITIZER_CFLAGS)
 
 append_list_if(SANITIZER_LIMIT_FRAME_SIZE -Wframe-larger-than=512
                SANITIZER_CFLAGS)
 append_list_if(COMPILER_RT_HAS_WGLOBAL_CONSTRUCTORS_FLAG -Wglobal-constructors
                SANITIZER_CFLAGS)
 
 add_custom_target(sanitizer_common)
 set(SANITIZER_RUNTIME_LIBRARIES)
 if(APPLE)

lib/sanitizer_common/sanitizer_platform.h

 #endif
 
 #ifdef __mips__
 # define SANITIZER_POINTER_FORMAT_LENGTH FIRST_32_SECOND_64(8, 10)
 #else
 # define SANITIZER_POINTER_FORMAT_LENGTH FIRST_32_SECOND_64(8, 12)
 #endif
 
+// Assume obsolete RPC headers are available by default
+#if !defined(HAVE_RPC_XDR_H) && !defined(HAVE_TIRPC_RPC_XDR_H)
+# define HAVE_RPC_XDR_H (SANITIZER_LINUX && !SANITIZER_ANDROID)
+# define HAVE_TIRPC_RPC_XDR_H 0
+#endif
+
 #endif // SANITIZER_PLATFORM_H

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_RPC_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 HAVE_RPC_XDR_H || HAVE_TIRPC_RPC_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);