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

Alexey Samsonov vonosmas at gmail.com
Tue Mar 31 14:01:46 PDT 2015


REPOSITORY
  rL LLVM

================
Comment at: CMakeLists.txt:183
@@ -182,1 +182,3 @@
 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)
----------------
ygribov wrote:
> kcc wrote:
> > ygribov wrote:
> > > kcc wrote:
> > > > 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. 
> > > > 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.
> > 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. 
> > 
> > 
> > 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.
> 
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.

================
Comment at: CMakeLists.txt:184
@@ -183,1 +183,3 @@
+check_include_file(rpc/xdr.h HAVE_RPC_XDR_H)
+check_include_file(tirpc/rpc/xdr.h HAVE_TIRPC_XDR_H)
 
----------------
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).

http://reviews.llvm.org/D8698

EMAIL PREFERENCES
  http://reviews.llvm.org/settings/panel/emailpreferences/






More information about the llvm-commits mailing list