[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