[compiler-rt] [test] Disable sunrpc tests when rpc/xdr.h is missing

[mgorny]

Summary

Disable tests requiring sunrpc when the relevant headers are missing.
In order to accommodate that, move the header check
from sanitizer_common to base-config-ix, and define the check result
as a global variable there. Use it afterwards both for definition
needed by sanitizer_common, and to control 'sunrpc' test feature.

While at it, remove the append_have_file_definition macro that was used
only once, and no longer fits the split check-definition.

[mgorny]

Gentle ping.

[mgorny]

Ping. This is necessary to resolve https://github.com/google/sanitizers/issues/974 and fix tests on systems which don't enable all the backwards compatibility cruft (Arch, Gentoo). See also D47817 as necessary dependency.

[mgorny]

Rebased.

[mgorny]

Updated to use find_package().

[mgorny]

Fix bug in previous patch.

[mgorny]

Need to update this for the new idea of not using libtirpc at all.

[mgorny]

Following the changes to D47817, updated this one to disable relevant tests when sunrpc headers are not available.

[mgorny]

Ping. I'd really like to merge this before the branch.

[Lekensteyn]

The direction of this patch looks reasonable to me. Is it worth mentioning the issue (https://github.com/google/sanitizers/issues/974) in the commit message?

What environment have you tested this in, could you verify that the tests are indeed skipped on a system without these headers, and passed otherwise?

lib/sanitizer_common/CMakeLists.txt

196

The old code defined HAVE_xxx to 0 or 1, but in general wouldn't it be preferable to define it to 1 if available, and not define it at all if unavailable? This would match most other HAVE_xxx checks.

In order to make the lit config work in this case, you have to use something like pythonize_bool(HAVE_RPC_XDR_H).

test/lit.common.configured.in

39

This becomes 0 or 1 instead of True or False, not sure if that will cause issues later.

[mgorny]

In D47819#1356347, @Lekensteyn wrote:

The direction of this patch looks reasonable to me. Is it worth mentioning the issue (https://github.com/google/sanitizers/issues/974) in the commit message?

Yes, makes sense. I'll add it when committing.

What environment have you tested this in, could you verify that the tests are indeed skipped on a system without these headers, and passed otherwise?

I have only tests without those headers. I'll try to rebuild glibc with them enabled if it's still supported but it'd be preferable if somebody else tested it on 'natural' environment.

lib/sanitizer_common/CMakeLists.txt

196

Well, I suppose I could do that but given that the old code did it like this, the 0/1 definition seems to be a common practice in compiler-rt, and there is no apparent benefit from doing it the other way, I don't think it a good thing to do.

test/lit.common.configured.in

39

It shouldn't. 0/1 evaluates the same in boolean context, and other LLVM tests are relying on 0/1 being passed to lit already.

[mgorny]

PASS: MemorySanitizer-X86_64 :: Linux/sunrpc_bytes.cc (2932 of 6195)
PASS: MemorySanitizer-X86_64 :: Linux/sunrpc_string.cc (2935 of 6195)
PASS: MemorySanitizer-X86_64 :: Linux/sunrpc.cc (2974 of 6195)
PASS: ThreadSanitizer-x86_64 :: sunrpc.cc (5110 of 6195)

So yep, it works. Tested both ways now.

[Lekensteyn]

In D47819#1356498, @mgorny wrote:

PASS: MemorySanitizer-X86_64 :: Linux/sunrpc_bytes.cc (2932 of 6195)
PASS: MemorySanitizer-X86_64 :: Linux/sunrpc_string.cc (2935 of 6195)
PASS: MemorySanitizer-X86_64 :: Linux/sunrpc.cc (2974 of 6195)
PASS: ThreadSanitizer-x86_64 :: sunrpc.cc (5110 of 6195)

So yep, it works. Tested both ways now.

Thanks for testing! It looks good to me.

cmake/Modules/CompilerRTUtils.cmake

if(${polarity})
     append_list_if(COMPILER_RT_HAS_FRTTI_FLAG -frtti ${list})
     append_list_if(COMPILER_RT_HAS_GR_FLAG /GR ${list})
   else()
     append_list_if(COMPILER_RT_HAS_FNO_RTTI_FLAG -fno-rtti ${list})
     append_list_if(COMPILER_RT_HAS_GR_FLAG /GR- ${list})
   endif()
 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()
-
 macro(list_intersect output input1 input2)
   set(${output})
   foreach(it ${${input1}})
     list(FIND ${input2} ${it} index)
     if( NOT (index EQUAL -1))
       list(APPEND ${output} ${it})
     endif()
   endforeach()

cmake/base-config-ix.cmake

 # The CompilerRT build system requires CMake version 2.8.8 or higher in order
 # to use its support for building convenience "libraries" as a collection of
 # .o files. This is particularly useful in producing larger, more complex
 # runtime libraries.
 
 include(CheckIncludeFile)
 include(CheckCXXSourceCompiles)
 
 check_include_file(unwind.h HAVE_UNWIND_H)
 
+# Used by sanitizer_common and tests.
+check_include_file(rpc/xdr.h HAVE_RPC_XDR_H)
+if (NOT HAVE_RPC_XDR_H)
+  set(HAVE_RPC_XDR_H 0)
+endif()
+
 # Top level target used to build all compiler-rt libraries.
 add_custom_target(compiler-rt ALL)
 add_custom_target(install-compiler-rt)
 add_custom_target(install-compiler-rt-stripped)
 set_property(
   TARGET
     compiler-rt
     install-compiler-rt

lib/sanitizer_common/CMakeLists.txt

set(SANITIZER_IMPL_HEADERS
   sanitizer_vector.h
   sanitizer_win.h
   sanitizer_win_defs.h
   sanitizer_win_dll_thunk.h
   sanitizer_win_weak_interception.h)
 
 include_directories(..)
 
-set(SANITIZER_COMMON_DEFINITIONS)
-
-include(CheckIncludeFile)
-append_have_file_definition(rpc/xdr.h HAVE_RPC_XDR_H SANITIZER_COMMON_DEFINITIONS)
+set(SANITIZER_COMMON_DEFINITIONS
+  HAVE_RPC_XDR_H=${HAVE_RPC_XDR_H})
 
 set(SANITIZER_CFLAGS ${SANITIZER_COMMON_CFLAGS})
 append_rtti_flag(OFF SANITIZER_CFLAGS)
 
 append_list_if(SANITIZER_LIMIT_FRAME_SIZE -Wframe-larger-than=570
                SANITIZER_CFLAGS)
 append_list_if(COMPILER_RT_HAS_WGLOBAL_CONSTRUCTORS_FLAG -Wglobal-constructors
                SANITIZER_CFLAGS)

test/lit.common.cfg

if config.lto_supported:
   if config.use_thinlto:
     config.available_features.add('thinlto')
     config.lto_flags += ["-flto=thin"]
   else:
     config.lto_flags += ["-flto"]
   if config.use_newpm:
     config.lto_flags += ["-fexperimental-new-pass-manager"]
 
+if config.have_rpc_xdr_h:
+  config.available_features.add('sunrpc')
+
 # Ask llvm-config about assertion mode.
 try:
   llvm_config_cmd = subprocess.Popen(
       [os.path.join(config.llvm_tools_dir, 'llvm-config'), '--assertion-mode'],
       stdout = subprocess.PIPE,
       env=config.environment)
 except OSError:
   print("Could not find llvm-config in " + config.llvm_tools_dir)

test/lit.common.configured.in

 set_default("sanitizer_can_use_cxxabi", @SANITIZER_CAN_USE_CXXABI_PYBOOL@)
 set_default("has_lld", @COMPILER_RT_HAS_LLD_PYBOOL@)
 set_default("can_symbolize", @CAN_SYMBOLIZE@)
 set_default("use_lld", False)
 set_default("use_thinlto", False)
 set_default("use_lto", config.use_thinlto)
 set_default("use_newpm", False)
 set_default("android", @ANDROID_PYBOOL@)
+set_default("have_rpc_xdr_h", @HAVE_RPC_XDR_H@)
 config.available_features.add('target-is-%s' % config.target_arch)
 
 if config.enable_per_target_runtime_dir:
   set_default("target_suffix", "")
 else:
   set_default("target_suffix", "-%s" % config.target_arch)
 
 # LLVM tools dir can be passed in lit parameters, so try to

test/msan/Linux/sunrpc.cc

+// REQUIRES: sunrpc
+
 // RUN: %clangxx_msan -g -O0 -DTYPE=int -DFN=xdr_int %s -o %t && \
 // RUN:     %run %t 2>&1
 // RUN: %clangxx_msan -g -O0 -DTYPE=int -DFN=xdr_int -DUNINIT=1 %s -o %t && \
 // RUN:     not %run %t 2>&1 | FileCheck %s
 // RUN: %clangxx_msan -g -O0 -DTYPE=double -DFN=xdr_double %s -o %t && \
 // RUN:     %run %t 2>&1
 // RUN: %clangxx_msan -g -O0 -DTYPE=double -DFN=xdr_double -DUNINIT=1 %s -o %t && \
 // RUN:     not %run %t 2>&1 | FileCheck %s

test/msan/Linux/sunrpc_bytes.cc

+// REQUIRES: sunrpc
+
 // RUN: %clangxx_msan -g -O0 %s -o %t && \
 // RUN:     %run %t 2>&1
 // RUN: %clangxx_msan -g -O0 -DUNINIT=1 %s -o %t && \
 // RUN:     not %run %t 2>&1 | FileCheck %s
 
 #include <assert.h>
 #include <string.h>
 #include <rpc/xdr.h>

test/msan/Linux/sunrpc_string.cc

+// REQUIRES: sunrpc
+
 // RUN: %clangxx_msan -g -O0 %s -o %t && \
 // RUN:     %run %t 2>&1
 // RUN: %clangxx_msan -g -O0 -DUNINIT=1 %s -o %t && \
 // RUN:     not %run %t 2>&1 | FileCheck %s
 
 #include <assert.h>
 #include <string.h>
 #include <rpc/xdr.h>

test/tsan/sunrpc.cc

+// REQUIRES: sunrpc
+
 // RUN: %clang_tsan -O1 %s -o %t && %run %t 2>&1 | FileCheck %s
 
 #include <pthread.h>
 #include <rpc/types.h>
 #include <rpc/xdr.h>
 #include <stdio.h>
 
 void *thr(void *p) {