[LLVM][compiler-rt] Fix build with LLVM_USE_SANITIZER=Undefined #120006
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
llvmbot
added
cmake
Member
|
@llvm/pr-subscribers-compiler-rt-sanitizer Author: Dmitry Chestnykh (chestnykh) ChangesFix #120003 Full diff: https://github.com/llvm/llvm-project/pull/120006.diff 5 Files Affected:
diff --git a/compiler-rt/cmake/config-ix.cmake b/compiler-rt/cmake/config-ix.cmake
index 6d52eecc9a91fe..25b0f6e2f3e2d5 100644
--- a/compiler-rt/cmake/config-ix.cmake
+++ b/compiler-rt/cmake/config-ix.cmake
@@ -931,7 +931,8 @@ endif()
# calling malloc on first use.
# TODO(hctim): Enable this on Android again. Looks like it's causing a SIGSEGV
# for Scudo and GWP-ASan, further testing needed.
-if (GWP_ASAN_SUPPORTED_ARCH AND
+if (COMPILER_RT_HAS_SANITIZER_COMMON AND
+ GWP_ASAN_SUPPORTED_ARCH AND
COMPILER_RT_BUILD_GWP_ASAN AND
COMPILER_RT_BUILD_SANITIZERS AND
"gwp_asan" IN_LIST COMPILER_RT_SANITIZERS_TO_BUILD AND
diff --git a/compiler-rt/lib/scudo/standalone/tests/CMakeLists.txt b/compiler-rt/lib/scudo/standalone/tests/CMakeLists.txt
index a85eb737dba0a8..d33fb051143757 100644
--- a/compiler-rt/lib/scudo/standalone/tests/CMakeLists.txt
+++ b/compiler-rt/lib/scudo/standalone/tests/CMakeLists.txt
@@ -1,5 +1,7 @@
include_directories(..)
+include(CompilerRTCompile)
+
add_custom_target(ScudoUnitTests)
set_target_properties(ScudoUnitTests PROPERTIES
FOLDER "Compiler-RT Tests")
diff --git a/compiler-rt/test/CMakeLists.txt b/compiler-rt/test/CMakeLists.txt
index f9e23710d3e4f7..3f87e69964f91a 100644
--- a/compiler-rt/test/CMakeLists.txt
+++ b/compiler-rt/test/CMakeLists.txt
@@ -46,75 +46,81 @@ if(NOT ANDROID)
endif()
endif()
-umbrella_lit_testsuite_begin(check-compiler-rt)
-
-function(compiler_rt_test_runtime runtime)
- string(TOUPPER ${runtime} runtime_uppercase)
- if(COMPILER_RT_HAS_${runtime_uppercase} AND COMPILER_RT_INCLUDE_TESTS)
- if (${runtime} STREQUAL cfi AND NOT COMPILER_RT_HAS_UBSAN)
- # CFI tests require diagnostic mode, which is implemented in UBSan.
- elseif (${runtime} STREQUAL scudo_standalone)
- add_subdirectory(scudo/standalone)
- else()
- add_subdirectory(${runtime})
+# XXX: Maybe more precise conditions to enable/disable tests
+# But now the below code is broken at least with LLVM_USE_SANITIZER=Undefined
+if(NOT LLVM_USE_SANITIZER)
+
+ umbrella_lit_testsuite_begin(check-compiler-rt)
+
+ function(compiler_rt_test_runtime runtime)
+ string(TOUPPER ${runtime} runtime_uppercase)
+ if(COMPILER_RT_HAS_${runtime_uppercase} AND COMPILER_RT_INCLUDE_TESTS)
+ if (${runtime} STREQUAL cfi AND NOT COMPILER_RT_HAS_UBSAN)
+ # CFI tests require diagnostic mode, which is implemented in UBSan.
+ elseif (${runtime} STREQUAL scudo_standalone)
+ add_subdirectory(scudo/standalone)
+ else()
+ add_subdirectory(${runtime})
+ endif()
endif()
- endif()
-endfunction()
+ endfunction()
-# Run sanitizer tests only if we're sure that clang would produce
-# working binaries.
-if(COMPILER_RT_CAN_EXECUTE_TESTS)
- if(COMPILER_RT_BUILD_BUILTINS)
- add_subdirectory(builtins)
- endif()
- if(COMPILER_RT_BUILD_SANITIZERS)
- compiler_rt_test_runtime(interception)
+ # Run sanitizer tests only if we're sure that clang would produce
+ # working binaries.
+ if(COMPILER_RT_CAN_EXECUTE_TESTS)
+ if(COMPILER_RT_BUILD_BUILTINS)
+ add_subdirectory(builtins)
+ endif()
+ if(COMPILER_RT_BUILD_SANITIZERS)
+ compiler_rt_test_runtime(interception)
- compiler_rt_test_runtime(lsan)
- compiler_rt_test_runtime(ubsan)
- compiler_rt_test_runtime(sanitizer_common)
+ compiler_rt_test_runtime(lsan)
+ compiler_rt_test_runtime(ubsan)
+ compiler_rt_test_runtime(sanitizer_common)
- # OpenBSD not supporting asan, cannot run the tests
- if(COMPILER_RT_BUILD_LIBFUZZER AND NOT "${CMAKE_SYSTEM_NAME}" STREQUAL "OpenBSD" AND NOT ANDROID)
- compiler_rt_test_runtime(fuzzer)
+ # OpenBSD not supporting asan, cannot run the tests
+ if(COMPILER_RT_BUILD_LIBFUZZER AND NOT "${CMAKE_SYSTEM_NAME}" STREQUAL "OpenBSD" AND NOT ANDROID)
+ compiler_rt_test_runtime(fuzzer)
- # These tests don't need an additional runtime but use asan runtime.
- add_subdirectory(metadata)
- endif()
+ # These tests don't need an additional runtime but use asan runtime.
+ add_subdirectory(metadata)
+ endif()
- foreach(sanitizer ${COMPILER_RT_SANITIZERS_TO_BUILD})
- compiler_rt_test_runtime(${sanitizer})
- endforeach()
- endif()
- if(COMPILER_RT_BUILD_PROFILE AND COMPILER_RT_HAS_PROFILE)
- compiler_rt_test_runtime(profile)
- endif()
- if(COMPILER_RT_BUILD_CTX_PROFILE)
- compiler_rt_test_runtime(ctx_profile)
- endif()
- if(COMPILER_RT_BUILD_MEMPROF)
- compiler_rt_test_runtime(memprof)
- endif()
- if(COMPILER_RT_BUILD_XRAY)
- compiler_rt_test_runtime(xray)
+ foreach(sanitizer ${COMPILER_RT_SANITIZERS_TO_BUILD})
+ compiler_rt_test_runtime(${sanitizer})
+ endforeach()
+ endif()
+ if(COMPILER_RT_BUILD_PROFILE AND COMPILER_RT_HAS_PROFILE)
+ compiler_rt_test_runtime(profile)
+ endif()
+ if(COMPILER_RT_BUILD_CTX_PROFILE)
+ compiler_rt_test_runtime(ctx_profile)
+ endif()
+ if(COMPILER_RT_BUILD_MEMPROF)
+ compiler_rt_test_runtime(memprof)
+ endif()
+ if(COMPILER_RT_BUILD_XRAY)
+ compiler_rt_test_runtime(xray)
+ endif()
+ if(COMPILER_RT_BUILD_ORC)
+ compiler_rt_Test_runtime(orc)
+ endif()
+ # ShadowCallStack does not yet provide a runtime with compiler-rt, the tests
+ # include their own minimal runtime
+ add_subdirectory(shadowcallstack)
endif()
- if(COMPILER_RT_BUILD_ORC)
- compiler_rt_Test_runtime(orc)
+
+ # Now that we've traversed all the directories and know all the lit testsuites,
+ # introduce a rule to run to run all of them.
+ get_property(LLVM_COMPILER_RT_LIT_DEPENDS GLOBAL PROPERTY LLVM_COMPILER_RT_LIT_DEPENDS)
+ add_custom_target(compiler-rt-test-depends)
+ set_target_properties(compiler-rt-test-depends PROPERTIES FOLDER "Compiler-RT/Tests")
+ if(LLVM_COMPILER_RT_LIT_DEPENDS)
+ add_dependencies(compiler-rt-test-depends ${LLVM_COMPILER_RT_LIT_DEPENDS})
endif()
- # ShadowCallStack does not yet provide a runtime with compiler-rt, the tests
- # include their own minimal runtime
- add_subdirectory(shadowcallstack)
-endif()
+ umbrella_lit_testsuite_end(check-compiler-rt)
-# Now that we've traversed all the directories and know all the lit testsuites,
-# introduce a rule to run to run all of them.
-get_property(LLVM_COMPILER_RT_LIT_DEPENDS GLOBAL PROPERTY LLVM_COMPILER_RT_LIT_DEPENDS)
-add_custom_target(compiler-rt-test-depends)
-set_target_properties(compiler-rt-test-depends PROPERTIES FOLDER "Compiler-RT/Tests")
-if(LLVM_COMPILER_RT_LIT_DEPENDS)
- add_dependencies(compiler-rt-test-depends ${LLVM_COMPILER_RT_LIT_DEPENDS})
-endif()
-umbrella_lit_testsuite_end(check-compiler-rt)
+endif(LLVM_USE_SANITIZER)
if(COMPILER_RT_STANDALONE_BUILD)
if(NOT TARGET check-all)
diff --git a/llvm/CMakeLists.txt b/llvm/CMakeLists.txt
index f14065ab037990..f4bd6ceaa681d1 100644
--- a/llvm/CMakeLists.txt
+++ b/llvm/CMakeLists.txt
@@ -707,8 +707,15 @@ endif( LLVM_USE_PERF )
set(LLVM_USE_SANITIZER "" CACHE STRING
"Define the sanitizer used to build binaries and tests.")
option(LLVM_OPTIMIZE_SANITIZED_BUILDS "Pass -O1 on debug sanitizer builds" ON)
+
+if( CMAKE_C_COMPILER_ID MATCHES "Clang" AND CMAKE_CXX_COMPILER_ID MATCHES "Clang" )
+ set(LLVM_UBSAN_FLAGS_CLANG "-fno-sanitize=function")
+else()
+ # gcc doesn't know about -fsanitize=function
+ set(LLVM_UBSAN_FLAGS_CLANG)
+endif()
set(LLVM_UBSAN_FLAGS
- "-fsanitize=undefined -fno-sanitize=vptr,function -fno-sanitize-recover=all"
+ "-fsanitize=undefined -fno-sanitize=vptr ${LLVM_UBSAN_FLAGS_CLANG} -fno-sanitize-recover=all"
CACHE STRING
"Compile flags set to enable UBSan. Only used if LLVM_USE_SANITIZER contains 'Undefined'.")
set(LLVM_LIB_FUZZING_ENGINE "" CACHE PATH
diff --git a/llvm/cmake/modules/HandleLLVMOptions.cmake b/llvm/cmake/modules/HandleLLVMOptions.cmake
index f19125eb6bf273..76ace6a1a951e8 100644
--- a/llvm/cmake/modules/HandleLLVMOptions.cmake
+++ b/llvm/cmake/modules/HandleLLVMOptions.cmake
@@ -1064,7 +1064,8 @@ if(LLVM_USE_SANITIZER)
if (LLVM_USE_SANITIZE_COVERAGE)
append("-fsanitize=fuzzer-no-link" CMAKE_C_FLAGS CMAKE_CXX_FLAGS)
endif()
- if (LLVM_USE_SANITIZER MATCHES ".*Undefined.*")
+ if (LLVM_USE_SANITIZER MATCHES ".*Undefined.*" AND CMAKE_C_COMPILER_ID MATCHES "Clang" AND
+ CMAKE_CXX_COMPILER_ID MATCHES "Clang")
set(IGNORELIST_FILE "${PROJECT_SOURCE_DIR}/utils/sanitizers/ubsan_ignorelist.txt")
if (EXISTS "${IGNORELIST_FILE}")
# Use this option name version since -fsanitize-ignorelist is only
|
Fix llvm#120003 The patch contains fixes to all the errors described in the issue attachement
Contributor
Author
|
But gn asserts that host compiler is clang: So probably this PR is not needed and only in the documentation it is necessary to clearly indicate that the compiler should be clang if LLVM_USE_SANITIZER is not empty |
Contributor
Author
|
UPD: So the patch is needed |
jroelofs
reviewed
Dec 16, 2024
| endif() | ||
| if (LLVM_USE_SANITIZER MATCHES ".*Undefined.*") | ||
| if (LLVM_USE_SANITIZER MATCHES ".*Undefined.*" AND CMAKE_C_COMPILER_ID MATCHES "Clang" AND | ||
| CMAKE_CXX_COMPILER_ID MATCHES "Clang") |
Contributor
There was a problem hiding this comment.
GCC supports -fsanitize=undefined: https://clang.godbolt.org/z/nT3bPW766
Maybe you have an old version that predates that support?
Contributor
Author
There was a problem hiding this comment.
I don't touch -fsanitize=undefined argument, it passes both to clang and gcc. Here i disable -fsanitize-blacklist=... passing to gcc which doesn't supported by it
Contributor
There was a problem hiding this comment.
ohh, I think it would help a lot to put that concept in a variable, e.g:
check_cxx_compiler_flag(-fsanitize-blacklist=${IGNORELIST_FILE} COMPILER_RT_CXX_SUPPORTS_FSANITIZE_BLACKLIST)
instead of tying this to the specific compiler.
Contributor
Author
There was a problem hiding this comment.
Thx, i've done the same for -fno-sanitize=function flag in llvm/CMakeLists.txt
vitalybuka
reviewed
Jan 9, 2025
| # TODO(hctim): Enable this on Android again. Looks like it's causing a SIGSEGV | ||
| # for Scudo and GWP-ASan, further testing needed. | ||
| if (GWP_ASAN_SUPPORTED_ARCH AND | ||
| if (COMPILER_RT_HAS_SANITIZER_COMMON AND |
Collaborator
There was a problem hiding this comment.
Why do we need COMPILER_RT_HAS_SANITIZER_COMMON for GWP_ASAN_SUPPORTED_ARCH?
It should not be a dependency.
vitalybuka
reviewed
Jan 9, 2025
| if (LLVM_USE_SANITIZER MATCHES ".*Undefined.*") | ||
| set(IGNORELIST_FILE "${PROJECT_SOURCE_DIR}/utils/sanitizers/ubsan_ignorelist.txt") | ||
| if (EXISTS "${IGNORELIST_FILE}") | ||
| check_c_compiler_flag(-fsanitize-blacklist=${IGNORELIST_FILE} C_SUPPORTS_FSANITIZE_BLACKLIST_FLAG) |
vitalybuka
reviewed
Jan 9, 2025
vitalybuka
reviewed
Jan 9, 2025
| option(LLVM_OPTIMIZE_SANITIZED_BUILDS "Pass -O1 on debug sanitizer builds" ON) | ||
|
|
||
| check_c_compiler_flag(-fno-sanitize=function C_SUPPORTS_FNO_SANITIZE_FUNCTION_FLAG) | ||
| check_cxx_compiler_flag(-fno-sanitize=function CXX_SUPPORTS_FNO_SANITIZE_FUNCTION_FLAG) |
Collaborator
There was a problem hiding this comment.
Changes in llvm/ caused by unsupported flags, cause is different from compiler-rt changes.
Can you please extract them into a separate PR?
I agree with them and happy to approve.
Collaborator
|
Please try with LLVM_ENABLE_RUNTIMES. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fix #120003
The patch contains fixes to all the errors described in the issue attachement