[PATCH] D41159: [asan] LIT: Add lld testing config.

[Vitaly Buka via Phabricator via llvm-commits]

vitalybuka added inline comments.


================
Comment at: test/asan/CMakeLists.txt:124
+
+  if(COMPILER_RT_HAS_LLD AND arch STREQUAL "x86_64" AND NOT (APPLE OR WIN32))
+    add_asan_testsuite(${arch} True False)
----------------
so if complier rt has LLD we are going to run 2x+ as much tests?
cfi has small set of tests, but for entire compiler-rt could be noticeable.
@eugenis, any concerns?


================
Comment at: test/asan/CMakeLists.txt:125
+  if(COMPILER_RT_HAS_LLD AND arch STREQUAL "x86_64" AND NOT (APPLE OR WIN32))
+    add_asan_testsuite(${arch} True False)
+  endif()
----------------
if future you are going to add add_asan_testsuite(${arch} True **True**) call here, right?



================
Comment at: test/asan/lit.site.cfg.in:12
 config.target_arch = "@ASAN_TEST_TARGET_ARCH@"
+config.use_lld = @ASAN_TEST_USE_LLD@
+config.use_thinlto = @ASAN_TEST_USE_THINLTO@
----------------
lebedev.ri wrote:
> vitalybuka wrote:
> > lebedev.ri wrote:
> > > vitalybuka wrote:
> > > > Why just asan? can we go for entire compiler-rt?
> > > I'm not sure what you mean. This is per-sanitizer-config option.
> > > I.e. there is a clang lit config, and a clang lit config with LLD. (and later, with LTO)
> > > I'm not sure how to do that for the entire compiler-rt.
> > >  
> > I think we should put it into compiler-rt/test/lit.common.cfg and try to support the option by all sanitizes
> That is great to know, but can you explain in a few more words, please? :)
> For the reference, i'm modelling this after CFI - it has several lit configs, with lld&thinlto, just lld, just thinlto, etc.
> 
please ignore my comment. I've checked patches from description. msan is already done this way


Repository:
  rCRT Compiler Runtime

https://reviews.llvm.org/D41159