[ubsan] lit changes for lld testing, future lto testing.

[lebedev.ri]

Summary

As discussed in https://github.com/google/oss-fuzz/issues/933,
it would be really awesome to be able to use ThinLTO for fuzzing.
However, as @kcc has pointed out, it is currently undefined (untested)
whether the sanitizers actually function properly with LLD and/or LTO.

This patch is inspired by the cfi test, which already do test with LTO
(and/or LLD), since LTO is required for CFI to function.

I started with UBSan, because it's cmakelists / lit.* files appeared
to be the cleanest. This patch adds the infrastructure to easily add
LLD and/or LTO sub-variants of the existing lit test configurations.

Also, this patch adds the LLD flavor, that explicitly does use LLD to link.
The check-ubsan does pass on my machine. And to minimize the [initial]
potential buildbot breakage i have put some restrictions on this flavour.

Please review carefully, i have not worked with lit/sanitizer tests before.

[lebedev.ri]

Move config.lto_launch param to before config.compile_wrapper param in the invocation line, same as in cfi/lit.cfg.

[eugenis]

test/ubsan/CMakeLists.txt

35

I'm not sure we want to test all sanitizers with LLD - they don't really interact with the linker, and it doubles testing time. @pcc what do you think?

test/ubsan/lit.common.cfg

68

This code should be shared between cfi/ubsan/asan/msan/sanitizer_common. Please move as much of it as possible one level up.

[lebedev.ri]

test/ubsan/CMakeLists.txt

35

For the context, https://github.com/google/oss-fuzz/issues/295#issuecomment-340847238 did sound like the more testing there is, the better.
While in this case, if needed, it is possible to test only some combinations, i'm not sure (did not look yet) it will be possible for other sanitizers.

TBN the addition of LTO can double the testing time yet again, but i suppose it is possible to always test lld and lto as one configuration.

test/ubsan/lit.common.cfg

68

Could you please clarify, do you mean to move build_invocation() into test/lit.common.cfg, or the variables?
Moving the function seemed cleaner, but if i do that, i get NameError: name 'build_invocation' is not defined, so i'm most likely missing something...
`

[vitalybuka]

test/ubsan/lit.common.cfg

68

I think it's about moving entire use lld and lto stuff from ubsan to parent dir so all sanitizers can use it

[eugenis]

test/ubsan/CMakeLists.txt

35

lld and lto as one configuration sounds good.

test/ubsan/lit.common.cfg

68

Yes. Maybe just the variables. Basically, adding "use_lld" to the config should switch any sanitizer's tests to lld, no reason for this to be specific to ubsan.

70

"run_wrapper" is a misnomer, it is actually a compiler wrapper, as I understand. I'm not sure why it is not part of config.compile_wrapper.

[lebedev.ri]

Aha, i see.
How about this approach?

There is something weird going on with config.cxx_mode_flags in cfi/lit.cfg.
If i *don't* append it at the end (but use config.clangxx, which already contains it), then test/cfi/overwrite.cpp breaks, maybe others.

[eugenis]

test/lit.common.cfg

341 ↗

(On Diff #122267)

Now asan will add target_cflags twice: first here, then in asan/lig.cfg build_invocation.

Perhaps its better to keep compiler and flags in separate config variables, and let individual test projects join them. How about adding run_wrapper and compile_wrapper to config.clang, and -fuse-ld and lto_flags to config.target_cflags?

[eugenis]

About cxx_mode_flags - I'm not sure what's wrong. You'd need to compare the generated test command lines.

[lebedev.ri]

test/lit.common.cfg

341 ↗

(On Diff #122267)

Now asan will add target_cflags twice: first here, then in asan/lig.cfg build_invocation.

Yes, because i have only touched cfi and ubsan.
If these changes make sense, i would look into asan/msan next.

How about adding run_wrapper and compile_wrapper to config.clang

https://reviews.llvm.org/D39508#inline-345840 - i.e. how do i do that?

[lebedev.ri]

Rebased.
Changed the code to simply update the config.clang and config.target_cflags, and projects will have to join them by themselves.
Does this look better?

[eugenis]

Looks fine.
You need to update other sanitizers like asan to not add compile_wrapper the second time.

test/lit.common.cfg

345 ↗

(On Diff #124613)

why not keep target_cflags as a list of strings? Looks like all users are joining them anyway.

[lebedev.ri]

In D39508#938255, @eugenis wrote:

Looks fine.

Great, finally onto something here :)

You need to update other sanitizers like asan to not add compile_wrapper the second time.

Yep, done.

For UBSan, the LLD testing is added, for others - nothing yet.
Should i do that in this patch, or split the non-cleanup stuff into the next patch?

[lebedev.ri]

test/lit.common.cfg

345 ↗

(On Diff #124613)

Not *all* of them.
There are some places that treat it as a string (via re.search e.g.)
So i *think* it is best to keep it as string, not list of strings.

[eugenis]

re.search('mthumb', config.target_cflags)

can be rewritten as

'-mthumb' in config.target_cflags

if target_cflags is a list. It's IMO both easier to read and less likely to match something unexpected, say part of a --sysroot path.

It's fine to add ubsan+lld testing in the same patch.

[lebedev.ri]

Changed target_cflags to be a list.
The differential are starting to become a little bit messy :)

[eugenis]

test/asan/lit.cfg

172 ↗

(On Diff #124891)

I don't think "is not None" part is necessary.

test/lit.common.cfg

330 ↗

(On Diff #124891)

Ah, I did not realize that cmake passes multiple flags as a string. Then mthumb in target_cflags check would not work.

Also, if there are no flags, this would create a 1-element [None] list, which would fail to stringify later.

Sorry for the extra work. Let's revert to target_cflags being a string.

[lebedev.ri]

And back to target_cflags being a string.

[lebedev.ri]

Re-uploaded previous diff (124711).

[eugenis]

Looks good!

[lebedev.ri]

ninja check-sanitizer check-asan check-asan-dynamic check-cfi check-cfi-and-supported check-scudo check-tsan check-ubsan check-ubsan-minimal check-msan passes, not sure what else to check, i'm gonna land this and revert if any bot is not happy.

[lebedev.ri]

Reverted due to some *strange* buildbot failure:

This change has introduced a problem with the Lit tests build for compiler-rt using Gold: http://lab.llvm.org:8011/builders/sanitizer-x86_64-linux/builds/6047/steps/test%20standalone%20compiler-rt/logs/stdio

llvm-lit: /b/sanitizer-x86_64-linux/build/llvm/utils/lit/lit/TestingConfig.py:101: fatal: unable to parse config file '/b/sanitizer-x86_64-linux/build/llvm/projects/compiler-rt/test/profile/Linux/lit.local.cfg', traceback: Traceback (most recent call last):
  File "/b/sanitizer-x86_64-linux/build/llvm/utils/lit/lit/TestingConfig.py", line 88, in load_from_path
    exec(compile(data, path, 'exec'), cfg_globals, None)
  File "/b/sanitizer-x86_64-linux/build/llvm/projects/compiler-rt/test/profile/Linux/lit.local.cfg", line 37, in <module>
    if root.host_os not in ['Linux'] or not is_gold_linker_available():
  File "/b/sanitizer-x86_64-linux/build/llvm/projects/compiler-rt/test/profile/Linux/lit.local.cfg", line 27, in is_gold_linker_available
    stderr = subprocess.PIPE)
  File "/usr/lib/python2.7/subprocess.py", line 390, in __init__
    errread, errwrite)
  File "/usr/lib/python2.7/subprocess.py", line 1024, in _execute_child
    raise child_exception
OSError: [Errno 2] No such file or directory

[lebedev.ri]

Ok, reproduced with (ld is gold)

$ CC=clang CXX=clang++ cmake ../compiler-rt -DLLVM_CONFIG_PATH=/usr/bin/llvm-config-6.0 -DCOMPILER_RT_INCLUDE_TESTS=ON -DLLVM_MAIN_SRC_DIR=/build/llvm
$ make -j9
$ make check-all -j9
...
[100%] Running all regression tests
llvm-lit: /build/llvm/utils/lit/lit/TestingConfig.py:101: fatal: unable to parse config file '/build/compiler-rt/test/profile/Linux/lit.local.cfg', traceback: Traceback (most recent call last):
  File "/build/llvm/utils/lit/lit/TestingConfig.py", line 88, in load_from_path
    exec(compile(data, path, 'exec'), cfg_globals, None)
  File "/build/compiler-rt/test/profile/Linux/lit.local.cfg", line 37, in <module>
    if root.host_os not in ['Linux'] or not is_gold_linker_available():
  File "/build/compiler-rt/test/profile/Linux/lit.local.cfg", line 27, in is_gold_linker_available
    stderr = subprocess.PIPE)
  File "/usr/lib/python2.7/subprocess.py", line 390, in __init__
    errread, errwrite)
  File "/usr/lib/python2.7/subprocess.py", line 1025, in _execute_child
    raise child_exception
OSError: [Errno 2] No such file or directory

[lebedev.ri]

And the reason is very simple, config.clang is /usr/local/bin/clang (yes, with spaces) there, so naturally such path/binary/executable does not exist.
I guess we now know why`config.compile_wrapper` and run_wrapper were separate before.
Let's see if python has some less safe subprocess.Popen() variant that would be ok with it.

[lebedev.ri]

Ok, so this does fix the standalone build, but it is extra-crappy because of shell=True.
Thoughs, ideas?

[eugenis]

I'm OK with shell=True.
Another option to make config.clang a list of strings, like we tried with target_cflags.

[lebedev.ri]

In D39508#942501, @eugenis wrote:

I'm OK with shell=True.

Ok, will re-commit, let's see if there are other failure modes i did not find yet :/

Another option to make config.clang a list of strings, like we tried with target_cflags.

Hmm, could work actually, but let's not cram everything into one differential :)

[lebedev.ri]

Alright, no new failures thus far :)

I will finish the simple stuff of modifying the rest of the tests to set the config.use_lld/config.use_lto
(and enable the lld testing at the same time) in the next differential and leave the fun lto stuff until after that.

Roman.

test/ubsan/CMakeLists.txt

 set(UBSAN_LIT_TESTS_DIR ${CMAKE_CURRENT_SOURCE_DIR})
 
 set(UBSAN_TESTSUITES)
 set(UBSAN_TEST_DEPS ${SANITIZER_COMMON_LIT_TEST_DEPS})
 
-macro(add_ubsan_testsuite test_mode sanitizer arch)
+macro(add_ubsan_testsuite test_mode sanitizer arch lld thinlto)
   set(UBSAN_LIT_TEST_MODE "${test_mode}")
-  set(CONFIG_NAME ${UBSAN_LIT_TEST_MODE}-${arch})
+  set(CONFIG_NAME ${UBSAN_LIT_TEST_MODE})
+  if (${lld})
+    set(CONFIG_NAME ${CONFIG_NAME}-lld)
+    list(APPEND UBSAN_TEST_DEPS lld)
+  endif()
+  if (${thinlto})
+    set(CONFIG_NAME ${CONFIG_NAME}-thinlto)
+    list(APPEND UBSAN_TEST_DEPS LTO)
+  endif()
+  set(UBSAN_TEST_USE_LLD ${lld})
+  set(UBSAN_TEST_USE_THINLTO ${thinlto})
+  set(CONFIG_NAME ${CONFIG_NAME}-${arch})
   configure_lit_site_cfg(
     ${CMAKE_CURRENT_SOURCE_DIR}/lit.site.cfg.in
     ${CMAKE_CURRENT_BINARY_DIR}/${CONFIG_NAME}/lit.site.cfg)
   list(APPEND UBSAN_TESTSUITES ${CMAKE_CURRENT_BINARY_DIR}/${CONFIG_NAME})
   if(NOT COMPILER_RT_STANDALONE_BUILD)
     list(APPEND UBSAN_TEST_DEPS ${sanitizer})
   endif()
 endmacro()
 
+macro(add_ubsan_testsuites test_mode sanitizer arch)
+  add_ubsan_testsuite(${test_mode} ${sanitizer} ${arch} False False)
+
+  if(COMPILER_RT_HAS_LLD AND arch STREQUAL "x86_64" AND NOT (APPLE OR WIN32))
+    add_ubsan_testsuite(${test_mode} ${sanitizer} ${arch} True False)
+  endif()
+endmacro()
+
 set(UBSAN_TEST_ARCH ${UBSAN_SUPPORTED_ARCH})
 if(APPLE)
   darwin_filter_host_archs(UBSAN_SUPPORTED_ARCH UBSAN_TEST_ARCH)
 endif()
 
 foreach(arch ${UBSAN_TEST_ARCH})
   set(UBSAN_TEST_TARGET_ARCH ${arch})
   get_test_cc_for_arch(${arch} UBSAN_TEST_TARGET_CC UBSAN_TEST_TARGET_CFLAGS)
-  add_ubsan_testsuite("Standalone" ubsan ${arch})
+  add_ubsan_testsuites("Standalone" ubsan ${arch})
 
   if(COMPILER_RT_HAS_ASAN AND ";${ASAN_SUPPORTED_ARCH};" MATCHES ";${arch};")
     # TODO(wwchrome): Re-enable ubsan for asan win 64-bit when ready.
     # Disable ubsan with AddressSanitizer tests for Windows 64-bit.
     if(NOT OS_NAME MATCHES "Windows" OR CMAKE_SIZEOF_VOID_P EQUAL 4)
-      add_ubsan_testsuite("AddressSanitizer" asan ${arch})
+      add_ubsan_testsuites("AddressSanitizer" asan ${arch})
     endif()
   endif()
   if(COMPILER_RT_HAS_MSAN AND ";${MSAN_SUPPORTED_ARCH};" MATCHES ";${arch};")
-    add_ubsan_testsuite("MemorySanitizer" msan ${arch})
+    add_ubsan_testsuites("MemorySanitizer" msan ${arch})
   endif()
   if(COMPILER_RT_HAS_TSAN AND ";${TSAN_SUPPORTED_ARCH};" MATCHES ";${arch};" AND NOT ANDROID)
-    add_ubsan_testsuite("ThreadSanitizer" tsan ${arch})
+    add_ubsan_testsuites("ThreadSanitizer" tsan ${arch})
   endif()
 endforeach()
 
 if(APPLE)
   foreach(arch ${UBSAN_TEST_ARCH})
     set(UBSAN_TEST_TARGET_ARCH ${arch})
     get_test_cc_for_arch(${arch} UBSAN_TEST_TARGET_CC UBSAN_TEST_TARGET_CFLAGS)
     set(UBSAN_TEST_TARGET_CFLAGS "${UBSAN_TEST_TARGET_CFLAGS} -lc++abi")
-    add_ubsan_testsuite("StandaloneStatic" ubsan ${arch})
+    add_ubsan_testsuites("StandaloneStatic" ubsan ${arch})
   endforeach()
 endif()
 
 add_lit_testsuite(check-ubsan "Running UndefinedBehaviorSanitizer tests"
   ${UBSAN_TESTSUITES}
   DEPENDS ${UBSAN_TEST_DEPS})
 set_target_properties(check-ubsan PROPERTIES FOLDER "Compiler-RT Misc")

test/ubsan/lit.common.cfg

 # -*- Python -*-
 
 import os
 
 def get_required_attr(config, attr_name):
   attr_value = getattr(config, attr_name, None)
   if attr_value == None:
     lit_config.fatal(
       "No attribute %r in test configuration! You may need to run "
       "tests from your build directory or add this attribute "
       "to lit.site.cfg " % attr_name)
   return attr_value
 
+# Setup config name.
+config.name = 'UBSan-' + config.name_suffix
+
 # Setup source root.
 config.test_source_root = os.path.dirname(__file__)
 
 default_ubsan_opts = list(config.default_sanitizer_opts)
 # Choose between standalone and UBSan+ASan modes.
 ubsan_lit_test_mode = get_required_attr(config, 'ubsan_lit_test_mode')
 if ubsan_lit_test_mode == "Standalone":
-  config.name = 'UBSan-Standalone-' + config.target_arch
   config.available_features.add("ubsan-standalone")
   clang_ubsan_cflags = []
 elif ubsan_lit_test_mode == "StandaloneStatic":
-  config.name = 'UBSan-StandaloneStatic-' + config.target_arch
   config.available_features.add("ubsan-standalone-static")
   clang_ubsan_cflags = ['-static-libsan']
 elif ubsan_lit_test_mode == "AddressSanitizer":
-  config.name = 'UBSan-ASan-' + config.target_arch
   config.available_features.add("ubsan-asan")
   clang_ubsan_cflags = ["-fsanitize=address"]
   default_ubsan_opts += ['detect_leaks=0']
 elif ubsan_lit_test_mode == "MemorySanitizer":
-  config.name = 'UBSan-MSan-' + config.target_arch
   config.available_features.add("ubsan-msan")
   clang_ubsan_cflags = ["-fsanitize=memory"]
 elif ubsan_lit_test_mode == "ThreadSanitizer":
-  config.name = 'UBSan-TSan-' + config.target_arch
   config.available_features.add("ubsan-tsan")
   clang_ubsan_cflags = ["-fsanitize=thread"]
 else:
   lit_config.fatal("Unknown UBSan test mode: %r" % ubsan_lit_test_mode)
 
 # Platform-specific default for lit tests.
 if config.target_arch == 's390x':
   # On SystemZ we need -mbackchain to make the fast unwinder work.
   clang_ubsan_cflags.append("-mbackchain")
 
 default_ubsan_opts_str = ':'.join(default_ubsan_opts)
 if default_ubsan_opts_str:
   config.environment['UBSAN_OPTIONS'] = default_ubsan_opts_str
   default_ubsan_opts_str += ':'
 # Substitution to setup UBSAN_OPTIONS in portable way.
 config.substitutions.append(('%env_ubsan_opts=',
                              'env UBSAN_OPTIONS=' + default_ubsan_opts_str))
 
+run_wrapper = []
+target_cflags = [get_required_attr(config, "target_cflags")]
+
+if config.use_lld and config.has_lld:
+  target_cflags += ["-fuse-ld=lld"]
+elif config.use_lld and (not config.has_lld):
+  config.unsupported = True
+
+if config.use_thinlto and config.lto_supported:
+  run_wrapper += config.lto_launch
+  target_cflags += config.lto_flags
+elif config.use_thinlto and (not config.lto_supported):
+  config.unsupported = True
+
 def build_invocation(compile_flags):
-  return " " + " ".join([config.compile_wrapper, config.clang] + compile_flags) + " "
+  return " " + " ".join(run_wrapper + [config.compile_wrapper, config.clang] + compile_flags) + " "
 
-target_cflags = [get_required_attr(config, "target_cflags")]
 clang_ubsan_cflags += target_cflags
 clang_ubsan_cxxflags = config.cxx_mode_flags + clang_ubsan_cflags
 
 # Define %clang and %clangxx substitutions to use in test RUN lines.
 config.substitutions.append( ("%clang ", build_invocation(clang_ubsan_cflags)) )
 config.substitutions.append( ("%clangxx ", build_invocation(clang_ubsan_cxxflags)) )
 config.substitutions.append( ("%gmlt ", " ".join(config.debug_info_flags) + " ") )
 
 # Default test suffixes.
 config.suffixes = ['.c', '.cc', '.cpp']
 
 # Check that the host supports UndefinedBehaviorSanitizer tests
 if config.host_os not in ['Linux', 'Darwin', 'FreeBSD', 'Windows', 'NetBSD']:
   config.unsupported = True
 
 config.available_features.add('arch=' + config.target_arch)

test/ubsan/lit.site.cfg.in

 @LIT_SITE_CFG_IN_HEADER@
 
 # Tool-specific config options.
+config.name_suffix = "@CONFIG_NAME@"
 config.ubsan_lit_test_mode = "@UBSAN_LIT_TEST_MODE@"
 config.target_cflags = "@UBSAN_TEST_TARGET_CFLAGS@"
 config.target_arch = "@UBSAN_TEST_TARGET_ARCH@"
+config.use_lld = @UBSAN_TEST_USE_LLD@
+config.use_thinlto = @UBSAN_TEST_USE_THINLTO@
 
 # Load common config for all compiler-rt lit tests.
 lit_config.load_config(config, "@COMPILER_RT_BINARY_DIR@/test/lit.common.configured")
 
 # Load tool-specific config that would do the real work.
 lit_config.load_config(config, "@UBSAN_LIT_TESTS_DIR@/lit.common.cfg")