Fix the cross compilation of unit tests. NFC

[sgundapa]

Summary

With COMPILER_RT_INCLUDE_TESTS turned ON and in a cross compiling
environment, the unit tests fail to link. This patch does the following changes

• Rename COMPILER_RT_TEST_CFLAGS to COMPILER_RT_UNITTEST_CFLAGS to reflect the way it's used.
• Add COMPILER_RT_TEST_COMPILER_CFLAGS to COMPILER_RT_UNITTEST_CFLAGS so that cross-compiler would be able to build/compile the unit tests
• Add COMPILER_RT_UNITTEST_LINKFLAGS to COMPILER_RT_UNITTEST_CFLAGS so that cross-compiler would be able to link the unit tests (if needed)

[samsonov]

Out of curiosity, do you need to add ${COMPILER_RT_TEST_COMPILER_CFLAGS} to COMPILER_RT_TEST_CFLAGS as well? Otherwise the name COMPILER_RT_TEST_COMPILER_CFLAGS looks really confusing.

[samsonov]

... or should we at least rename COMPILER_RT_TEST_COMPILER_CFLAGS to COMPILER_RT_TEST_TARGET_CFLAGS ?

[sgundapa]

At least in this scenario, asan_compile uses "CMAKE_C_FLAGS" and that exposed the issue during the linking. Ideally both compilation and linking use either COMPILER_RT_TEST_COMPILER_CFLAGS . I can push a clean up patch to rename COMPILER_RT_TEST_COMPILER_CFLAGS to COMPILER_RT_TEST_TARGET_CFLAGS.

Also, what is the difference between COMPILER_RT_TEST_COMPILER_CFLAGS and COMPILER_RT_TEST_CFLAGS ? There seem to be an awe-full lot of similar flags , I believe

[sgundapa]

Ping, on how to proceed

[samsonov]

Sorry for delay. I agree, this is way too confusing. So, currently:

• COMPILER_RT_TEST_COMPILER_CFLAGS is a user-defined CMake variable and is used to compile/link lit-tests (if we're using a cross-compiler)
• COMPILER_RT_TEST_CFLAGS are used to compile unit tests and is just a variable we construct.

I think the right change would be to:
(a) rename COMPILER_RT_TEST_CFLAGS to COMPILER_RT_UNITTEST_CFLAGS to reflect the way it's used.
(b) add COMPILER_RT_TEST_COMPILER_CFLAGS to COMPILER_RT_UNITTEST_CFLAGS so that cross-compiler would be able to build the tests
(c) (if needed) introduce/do smth. similar to COMPILER_RT_UNITTEST_LINKFLAGS if needed.

[sgundapa]

PING (as I pushed this patch during holiday time and there is a greater chance that this might have been unnoticed)

[samsonov]

Right, sorry for delay caused by holidays. I will take a look later this week.

[samsonov]

cmake/Modules/AddCompilerRT.cmake

174 ↗

(On Diff #43755)

No, please leave COMPILER_RT_GTEST_* variables intact: GTest is the name of unittest framework.

lib/asan/tests/CMakeLists.txt

40

Why do you need quotes here?

[sgundapa]

cmake/Modules/AddCompilerRT.cmake

174 ↗

(On Diff #43755)

Ok. I will push a new patch with out any changes to GTEST_*

lib/asan/tests/CMakeLists.txt

40

I guess, it is generally safe to wrap a variable with quotes which holds a value with spaces/colons. Let me know if you think otherwise.
I am not an expert in CMake

[samsonov]

lib/asan/tests/CMakeLists.txt

40

If COMPILER_RT_UNITTEST_LINKFLAGS is a list (which it's likely is, all other similar varialbes are), then you probably don't want to escape semicolons, which are treated as items delimiters.

[samsonov]

LGTM

[sgundapa]

This patch was reverted as an empty COMPILER_RT_TEST_COMPILER_CFLAGS is causing the cmake configuration to bail out. Pushed a new patch here: http://reviews.llvm.org/D16165

lib/asan/tests/CMakeLists.txt

set(ASAN_UNITTEST_COMMON_CFLAGS
   -I${COMPILER_RT_SOURCE_DIR}/lib/sanitizer_common/tests
   -fno-rtti
   -O2
   -Wno-format
   -Werror=sign-compare
   -Wno-non-virtual-dtor)
 append_list_if(COMPILER_RT_HAS_WVARIADIC_MACROS_FLAG -Wno-variadic-macros ASAN_UNITTEST_COMMON_CFLAGS)
 
+# This will ensure the target linker is used for linking unit tests
+set(ASAN_UNITTEST_COMMON_LINKFLAGS
+  ${COMPILER_RT_TEST_COMPILER_CFLAGS})
+
 # -gline-tables-only must be enough for ASan, so use it if possible.
 if(COMPILER_RT_TEST_COMPILER_ID MATCHES "Clang")
   list(APPEND ASAN_UNITTEST_COMMON_CFLAGS -gline-tables-only)
 else()
   list(APPEND ASAN_UNITTEST_COMMON_CFLAGS -g)
 endif()
 
 # Use -D instead of definitions to please custom compile command.