[sanitizer][MIPS] Add --target flag for MIPS32

[slthakur]

Summary

Without the --target flag, clang uses the mips64 triple which selects the n64 abi. We need to add --target=mips-linux-gnu, so that clang can select the correct abi for mips32r2.

[slthakur]

cmake/config-ix.cmake

199–200

I used the tab escape sequence here because while generating asan tests I get the following error :

Scanning dependencies of target Asan-mips64el-with-calls-Noinst-Test
[ 76%] Generating ASAN_NOINST_TEST_OBJECTS.gtest-all.cc.mips64el-with-calls.o
clang-3.7: error: unknown argument: '-mips64r2 -mabi=n64'

[samsonov]

cmake/config-ix.cmake

199–200

Please test if you can provide multiple flags to test_target_arch function:

test_target_arch(mips "" "-mips32r2" "--target=mips-linux-gnu")

If it doesn't work, we should fix it.

[slthakur]

I am able to give multiple flags to test_target_arch function.
But with this the tests under AddressSanitizer-mips64-linux don't get the flags properly.
Since cmake list elements are glued with a semicolon, this semicolon gets inserted in between the two mips flags which divides the compile command into two parts. Therefore I have replaced this semicolon in get_target_flags_for_arch function with a tab.

[samsonov]

cmake/config-ix.cmake

235

You shouldn't modify TARGET_${arch}_CFLAGS here, or do a string replacement. Just glue all flags into a string using foreach() loop, e.g. see set_target_compile_flags in CompilerRTUtils.cmake

[slthakur]

Addressed review comments

[samsonov]

No, I believe you should fix get_target_flags_for_arch function instead, so that it returns the string instead of the list.

[slthakur]

Addressed review comments

[slthakur]

cmake/Modules/CompilerRTCompile.cmake

49 ↗

(On Diff #25141)

Unit tests for ASan, MSan and TSan get their TARGET_CFLAGS from get_target_flags_for_arch () function. And TARGET_CFLAGS need to be in list form, otherwise unit tests fail to compile :

Generating tsan_test.cc.mips64.o
cd /home/kms/sagar/compiler-rt_build/lib/tsan/tests/rtl && /home/kms/sagar/bin/MyLLVM/install/bin/clang -Wall -std=c++11 -Wno-unknown-warning-option -fPIC -fno-builtin -fno-exceptions -fomit-frame-pointer -funwind-tables -fno-stack-protector -fvisibility=hidden -fno-function-sections -fno-lto -O3 -gline-tables-only -Wno-gnu -Wno-variadic-macros -Wno-c99-extensions -Wno-non-virtual-dtor -fPIE -fno-rtti -DGTEST_NO_LLVM_RAW_OSTREAM=1 -DGTEST_HAS_RTTI=0 -I/home/kms/sagar/bin/MyLLVM/llvm/utils/unittest/googletest/include -I/home/kms/sagar/bin/MyLLVM/llvm/utils/unittest/googletest -I/home/kms/sagar/compiler-rt/lib -I/home/kms/sagar/compiler-rt/lib/tsan/rtl -DGTEST_HAS_RTTI=0 \ -mips64r2\ -mabi=n64 -c -o tsan_test.cc.mips64.o /home/kms/sagar/compiler-rt/lib/tsan/tests/rtl/tsan_test.cc
clang-3.7: error: no such file or directory: ' -mips64r2 -mabi=n64'

As get_target_flags_for_arch function now returns a string instead of a list, TARGET_CFLAGS will be passed to clang_compile macro as a string.
Therefore we need to convert these flags back to list here to fix the above shown error.

cmake/config-ix.cmake

199–200

I am able to give multiple flags to test_target_arch funtion.
But with this the tests under AddressSanitizer-mips64-linux don't get the flags properly.
Since cmake list elements are glued with a semicolon, this semicolon gets inserted in between the two mips flags which divides the compile command into two parts. Therefore I have replaced this semicolon in get_target_flags_for_arch with a tab.

[slthakur]

cmake/config-ix.cmake

199–200

Sorry this comment was sent by mistake. It was not getting deleted.

[samsonov]

Alright, so here's the problem: get_target_flags_for_arch is used in two places:

1. Fetching flags eventually passed to clang_compile macro to build unit tests. They *should* be represented as list, as clang_compile description clearly says.
2. Fetching flags passed to clang in lit-tests (through ASAN_TEST_TARGET_CFLAGS CMake variable, which is used while configuring lit.site.cfg.in. They should be represented in a string form (space-separated).

Working with lists is generally more convenient, so let's return to http://reviews.llvm.org/differential/diff/25012/, and correct the usage of get_target_flags_for_arch while generating lit configs. I'm OK with replacing ";" with space. Also note, that you should only do this when you actually call get_target_flags_for_arch, not when you obtain these flags from COMPILER_RT_TEST_COMPILER_CFLAGS variable, which is likely a string. A comment in get_target_flags_for_arch, specifying that it returns a *list* would also be convenient.

[slthakur]

Returned to http://reviews.llvm.org/differential/diff/25012/ and corrected the usage of get_target_flags_for_arch only while generating lit configs.

[samsonov]

LGTM. Thanks for patience.

cmake/config-ix.cmake

else()
   elseif("${LLVM_NATIVE_ARCH}" STREQUAL "Mips")
     # Gcc doesn't accept -m32/-m64 so we do the next best thing and use
     # -mips32r2/-mips64r2. We don't use -mips1/-mips3 because we want to match
     # clang's default CPU's. In the 64-bit case, we must also specify the ABI
     # since the default ABI differs between gcc and clang.
     # FIXME: Ideally, we would build the N32 library too.
     if("${COMPILER_RT_TEST_TARGET_ARCH}" MATCHES "mipsel|mips64el")
       # regex for mipsel, mips64el
-      test_target_arch(mipsel "" "-mips32r2")
-      test_target_arch(mips64el "" "-mips64r2 -mabi=n64")
+      test_target_arch(mipsel "" "-mips32r2" "--target=mipsel-linux-gnu")
+      test_target_arch(mips64el "" "-mips64r2" "-mabi=n64")
     else()
-      test_target_arch(mips "" "-mips32r2")
-      test_target_arch(mips64 "" "-mips64r2 -mabi=n64")
+      test_target_arch(mips "" "-mips32r2" "--target=mips-linux-gnu")
+      test_target_arch(mips64 "" "-mips64r2" "-mabi=n64")
     endif()
   elseif("${COMPILER_RT_TEST_TARGET_ARCH}" MATCHES "arm")
     test_target_arch(arm "" "-march=armv7-a")
   elseif("${COMPILER_RT_TEST_TARGET_ARCH}" MATCHES "aarch32")
     test_target_arch(aarch32 "" "-march=armv8-a")
   elseif("${COMPILER_RT_TEST_TARGET_ARCH}" MATCHES "aarch64")
     test_target_arch(aarch64 "" "-march=armv8-a")
   endif()
foreach(arch ${ARGN})
     list(FIND COMPILER_RT_SUPPORTED_ARCH ${arch} ARCH_INDEX)
     if(NOT (ARCH_INDEX EQUAL -1) AND CAN_TARGET_${arch})
       list(APPEND archs ${arch})
     endif()
   endforeach()
   set(${out_var} ${archs} PARENT_SCOPE)
 endfunction()
 
+# Returns a list of architecture specific target cflags in @out_var list.
 function(get_target_flags_for_arch arch out_var)
   list(FIND COMPILER_RT_SUPPORTED_ARCH ${arch} ARCH_INDEX)
   if(ARCH_INDEX EQUAL -1)
     message(FATAL_ERROR "Unsupported architecture: ${arch}")
   else()
     set(${out_var} ${TARGET_${arch}_CFLAGS} PARENT_SCOPE)
   endif()
 endfunction()
 
 # Architectures supported by compiler-rt libraries.
 filter_available_targets(SANITIZER_COMMON_SUPPORTED_ARCH
   x86_64 i386 i686 powerpc64 powerpc64le arm aarch64 mips mips64 mipsel mips64el)
 # LSan and UBSan common files should be available on all architectures supported
 # by other sanitizers (even if they build into dummy object files).

test/asan/CMakeLists.txt

foreach(arch ${ASAN_SUPPORTED_ARCH})
   get_bits_for_arch(${arch} ASAN_TEST_BITS)
   if(ANDROID OR ${arch} MATCHES "arm|aarch64")
     # This is only true if we are cross-compiling.
     # Build all tests with host compiler and use host tools.
     set(ASAN_TEST_TARGET_CC ${COMPILER_RT_TEST_COMPILER})
     set(ASAN_TEST_TARGET_CFLAGS ${COMPILER_RT_TEST_COMPILER_CFLAGS})
   else()
     get_target_flags_for_arch(${arch} ASAN_TEST_TARGET_CFLAGS)
+    string(REPLACE ";" " " ASAN_TEST_TARGET_CFLAGS "${ASAN_TEST_TARGET_CFLAGS}")
   endif()
   if(ANDROID)
     set(ASAN_TEST_DYNAMIC True)
   else()
     set(ASAN_TEST_DYNAMIC False)
   endif()
   string(TOUPPER ${arch} ARCH_UPPER_CASE)
   set(CONFIG_NAME ${ARCH_UPPER_CASE}${OS_NAME}Config)

test/sanitizer_common/CMakeLists.txt

foreach(arch ${${tool_toupper}_SUPPORTED_ARCH})
     set(SANITIZER_COMMON_LIT_TEST_MODE ${tool})
     set(SANITIZER_COMMON_TEST_TARGET_ARCH ${arch})
     if(${arch} MATCHES "arm|aarch64")
       # This is only true if we're cross-compiling.
       set(SANITIZER_COMMON_TEST_TARGET_CFLAGS
           ${COMPILER_RT_TEST_COMPILER_CFLAGS})
     else()
       get_target_flags_for_arch(${arch} SANITIZER_COMMON_TEST_TARGET_CFLAGS)
+      string(REPLACE ";" " " SANITIZER_COMMON_TEST_TARGET_CFLAGS "${SANITIZER_COMMON_TEST_TARGET_CFLAGS}")
     endif()
     set(CONFIG_NAME ${tool}-${arch}-${OS_NAME})
     configure_lit_site_cfg(
       ${CMAKE_CURRENT_SOURCE_DIR}/lit.site.cfg.in
       ${CMAKE_CURRENT_BINARY_DIR}/${CONFIG_NAME}/lit.site.cfg)
     list(APPEND SANITIZER_COMMON_TESTSUITES
          ${CMAKE_CURRENT_BINARY_DIR}/${CONFIG_NAME})
   endforeach()

test/ubsan/CMakeLists.txt

 
 foreach(arch ${UBSAN_SUPPORTED_ARCH})
   set(UBSAN_TEST_TARGET_ARCH ${arch})
   if(${arch} MATCHES "arm|aarch64")
     # This is only true if we're cross-compiling.
     set(UBSAN_TEST_TARGET_CFLAGS ${COMPILER_RT_TEST_COMPILER_CFLAGS})
   else()
     get_target_flags_for_arch(${arch} UBSAN_TEST_TARGET_CFLAGS)
+    string(REPLACE ";" " " UBSAN_TEST_TARGET_CFLAGS "${UBSAN_TEST_TARGET_CFLAGS}")
   endif()
   add_ubsan_testsuite("Standalone" ubsan ${arch})
 
   if(COMPILER_RT_HAS_ASAN AND ";${ASAN_SUPPORTED_ARCH};" MATCHES ";${arch};")
     add_ubsan_testsuite("AddressSanitizer" asan ${arch})
   endif()
   if(COMPILER_RT_HAS_MSAN AND ";${MSAN_SUPPORTED_ARCH};" MATCHES ";${arch};")
     add_ubsan_testsuite("MemorySanitizer" msan ${arch})