Skip to content

Commit 070ec68

Browse files
callumfarecallumfare
authored
Merge pull request #2222 from RossBrunton/ross/cfi
Enable -flto and -fsanitize=cfi in clang
2 parents 42839a8 + 22b41a9 commit 070ec68

File tree

10 files changed

+71
-20
lines changed

10 files changed

+71
-20
lines changed

‎.github/workflows/build-fuzz-reusable.yml

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,8 @@ jobs:
4747
cmake --build build -j $(nproc)
4848
4949
- name: Configure CMake
50+
# CFI sanitization (or flto?) seems to cause linking to fail
51+
# https://github.com/oneapi-src/unified-runtime/issues/2323
5052
run: >
5153
cmake
5254
-B${{github.workspace}}/build
@@ -58,6 +60,7 @@ jobs:
5860
-DUR_USE_ASAN=ON
5961
-DUR_USE_UBSAN=ON
6062
-DUR_BUILD_ADAPTER_L0=ON
63+
-DUR_USE_CFI=OFF
6164
-DUR_LEVEL_ZERO_LOADER_LIBRARY=${{github.workspace}}/level-zero/build/lib/libze_loader.so
6265
-DUR_LEVEL_ZERO_INCLUDE_DIR=${{github.workspace}}/level-zero/include/
6366
-DUR_DPCXX=${{github.workspace}}/dpcpp_compiler/bin/clang++

‎.github/workflows/build-hw-reusable.yml

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -82,6 +82,8 @@ jobs:
8282
tar -xvf ${{github.workspace}}/dpcpp_compiler.tar.gz -C dpcpp_compiler
8383
8484
- name: Configure CMake
85+
# CFI sanitization seems to fail on our CUDA nodes
86+
# https://github.com/oneapi-src/unified-runtime/issues/2309
8587
run: >
8688
cmake
8789
-B${{github.workspace}}/build
@@ -94,6 +96,7 @@ jobs:
9496
-DUR_BUILD_ADAPTER_${{matrix.adapter.name}}=ON
9597
-DUR_CONFORMANCE_TEST_LOADER=${{ matrix.adapter.other_name != '' && 'ON' || 'OFF' }}
9698
${{ matrix.adapter.other_name != '' && format('-DUR_BUILD_ADAPTER_{0}=ON', matrix.adapter.other_name) || '' }}
99+
-DUR_USE_CFI=${{ matrix.adapter.name == 'CUDA' && 'OFF' || 'ON' }}
97100
-DUR_STATIC_LOADER=${{matrix.adapter.static_Loader}}
98101
-DUR_STATIC_ADAPTER_${{matrix.adapter.name}}=${{matrix.adapter.static_adapter}}
99102
-DUR_DPCXX=${{github.workspace}}/dpcpp_compiler/bin/clang++

‎CMakeLists.txt

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,7 @@ option(UR_USE_ASAN "enable AddressSanitizer" OFF)
4040
option(UR_USE_UBSAN "enable UndefinedBehaviorSanitizer" OFF)
4141
option(UR_USE_MSAN "enable MemorySanitizer" OFF)
4242
option(UR_USE_TSAN "enable ThreadSanitizer" OFF)
43+
option(UR_USE_CFI "enable Control Flow Integrity checks (requires clang and implies -flto)" ON)
4344
option(UR_ENABLE_TRACING "enable api tracing through xpti" OFF)
4445
option(UR_ENABLE_SANITIZER "enable device sanitizer" ON)
4546
option(UR_ENABLE_SYMBOLIZER "enable symoblizer for sanitizer" OFF)

‎README.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -130,6 +130,7 @@ List of options provided by CMake:
130130
| UR_USE_TSAN | Enable ThreadSanitizer | ON/OFF | OFF |
131131
| UR_USE_UBSAN | Enable UndefinedBehavior Sanitizer | ON/OFF | OFF |
132132
| UR_USE_MSAN | Enable MemorySanitizer (clang only) | ON/OFF | OFF |
133+
| UR_USE_CFI | Enable Control Flow Integrity checks (clang only, also enables lto) | ON/OFF | ON |
133134
| UR_ENABLE_TRACING | Enable XPTI-based tracing layer | ON/OFF | OFF |
134135
| UR_ENABLE_SANITIZER | Enable device sanitizer layer | ON/OFF | ON |
135136
| UR_CONFORMANCE_TARGET_TRIPLES | SYCL triples to build CTS device binaries for | Comma-separated list | spir64 |

‎cmake/helpers.cmake

Lines changed: 18 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -63,6 +63,16 @@ if(CMAKE_SYSTEM_NAME STREQUAL Linux)
6363
check_cxx_compiler_flag("-fstack-clash-protection" CXX_HAS_FSTACK_CLASH_PROTECTION)
6464
endif()
6565

66+
if (UR_USE_CFI)
67+
set(SAVED_CMAKE_REQUIRED_FLAGS ${CMAKE_REQUIRED_FLAGS})
68+
set(CMAKE_REQUIRED_FLAGS "-flto -fvisibility=hidden")
69+
check_cxx_compiler_flag("-fsanitize=cfi" CXX_HAS_CFI_SANITIZE)
70+
set(CMAKE_REQUIRED_FLAGS ${SAVED_CMAKE_REQUIRED_FLAGS})
71+
else()
72+
# If CFI checking is disabled, pretend we don't support it
73+
set(CXX_HAS_CFI_SANITIZE OFF)
74+
endif()
75+
6676
function(add_ur_target_compile_options name)
6777
if(NOT MSVC)
6878
target_compile_definitions(${name} PRIVATE -D_FORTIFY_SOURCE=2)
@@ -78,11 +88,10 @@ function(add_ur_target_compile_options name)
7888
# Hardening options
7989
-fPIC
8090
-fstack-protector-strong
81-
-fvisibility=hidden # Required for -fsanitize=cfi
82-
# -fsanitize=cfi requires -flto, which breaks a lot of things
83-
# See: https://github.com/oneapi-src/unified-runtime/issues/2120
84-
# -flto
85-
# $<$<CXX_COMPILER_ID:Clang,AppleClang>:-fsanitize=cfi>
91+
-fvisibility=hidden
92+
# cfi-icall requires called functions in shared libraries to also be built with cfi-icall, which we can't
93+
# guarantee. -fsanitize=cfi depends on -flto
94+
$<$<BOOL:${CXX_HAS_CFI_SANITIZE}>:-flto -fsanitize=cfi -fno-sanitize=cfi-icall>
8695
$<$<BOOL:${CXX_HAS_FCF_PROTECTION_FULL}>:-fcf-protection=full>
8796
$<$<BOOL:${CXX_HAS_FSTACK_CLASH_PROTECTION}>:-fstack-clash-protection>
8897

@@ -119,7 +128,10 @@ endfunction()
119128
function(add_ur_target_link_options name)
120129
if(NOT MSVC)
121130
if (NOT APPLE)
122-
target_link_options(${name} PRIVATE "LINKER:-z,relro,-z,now,-z,noexecstack")
131+
target_link_options(${name} PRIVATE
132+
$<$<BOOL:${CXX_HAS_CFI_SANITIZE}>:-flto -fsanitize=cfi -fno-sanitize=cfi-icall>
133+
"LINKER:-z,relro,-z,now,-z,noexecstack"
134+
)
123135
if (UR_DEVELOPER_MODE)
124136
target_link_options(${name} PRIVATE -Werror -Wextra)
125137
endif()

‎test/adapters/level_zero/v2/CMakeLists.txt

Lines changed: 18 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -33,19 +33,24 @@ add_unittest(level_zero_command_list_cache
3333
${PROJECT_SOURCE_DIR}/source/adapters/level_zero/v2/command_list_cache.cpp
3434
)
3535

36-
add_unittest(level_zero_event_pool
37-
event_pool_test.cpp
38-
${PROJECT_SOURCE_DIR}/source/ur/ur.cpp
39-
${PROJECT_SOURCE_DIR}/source/adapters/level_zero/adapter.cpp
40-
${PROJECT_SOURCE_DIR}/source/adapters/level_zero/device.cpp
41-
${PROJECT_SOURCE_DIR}/source/adapters/level_zero/platform.cpp
42-
${PROJECT_SOURCE_DIR}/source/adapters/level_zero/v2/event_pool.cpp
43-
${PROJECT_SOURCE_DIR}/source/adapters/level_zero/v2/event_pool_cache.cpp
44-
${PROJECT_SOURCE_DIR}/source/adapters/level_zero/v2/event_provider_normal.cpp
45-
${PROJECT_SOURCE_DIR}/source/adapters/level_zero/v2/event_provider_counter.cpp
46-
${PROJECT_SOURCE_DIR}/source/adapters/level_zero/v2/event.cpp
47-
${PROJECT_SOURCE_DIR}/source/adapters/level_zero/v2/queue_api.cpp
48-
)
36+
if(CXX_HAS_CFI_SANITIZE)
37+
message(WARNING "Level Zero V2 Event Pool tests are disabled when using CFI sanitizer")
38+
message(NOTE "See https://github.com/oneapi-src/unified-runtime/issues/2324")
39+
else()
40+
add_unittest(level_zero_event_pool
41+
event_pool_test.cpp
42+
${PROJECT_SOURCE_DIR}/source/ur/ur.cpp
43+
${PROJECT_SOURCE_DIR}/source/adapters/level_zero/adapter.cpp
44+
${PROJECT_SOURCE_DIR}/source/adapters/level_zero/device.cpp
45+
${PROJECT_SOURCE_DIR}/source/adapters/level_zero/platform.cpp
46+
${PROJECT_SOURCE_DIR}/source/adapters/level_zero/v2/event_pool.cpp
47+
${PROJECT_SOURCE_DIR}/source/adapters/level_zero/v2/event_pool_cache.cpp
48+
${PROJECT_SOURCE_DIR}/source/adapters/level_zero/v2/event_provider_normal.cpp
49+
${PROJECT_SOURCE_DIR}/source/adapters/level_zero/v2/event_provider_counter.cpp
50+
${PROJECT_SOURCE_DIR}/source/adapters/level_zero/v2/event.cpp
51+
${PROJECT_SOURCE_DIR}/source/adapters/level_zero/v2/queue_api.cpp
52+
)
53+
endif()
4954

5055
add_adapter_test(level_zero_memory_residency
5156
FIXTURE DEVICES

‎test/conformance/enqueue/enqueue_adapter_opencl.match

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
# Note: This file is only for use with cts_exe.py
2+
# Fails when -fsanitize=cfi
3+
{{OPT}}urEnqueueEventsWaitMultiDeviceMTTest.EnqueueWaitOnAllQueues/MultiThread

‎test/conformance/exp_command_buffer/exp_command_buffer_adapter_cuda.match

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
# Note: This file is only for use with cts_exe.py
2+
# These cause SIGILL when built with -fsanitize=cfi on Nvidia
3+
{{OPT}}urCommandBufferKernelHandleUpdateTest.Success/*
4+
{{OPT}}urCommandBufferKernelHandleUpdateTest.UpdateAgain/*
5+
{{OPT}}urCommandBufferKernelHandleUpdateTest.RestoreOriginalKernel/*
6+
{{OPT}}urCommandBufferKernelHandleUpdateTest.KernelAlternativeNotRegistered/*
7+
{{OPT}}urCommandBufferKernelHandleUpdateTest.RegisterInvalidKernelAlternative/*
8+
{{OPT}}urCommandBufferValidUpdateParametersTest.UpdateDimensionsWithoutUpdatingKernel/*
9+
{{OPT}}urCommandBufferValidUpdateParametersTest.UpdateOnlyLocalWorkSize/*
10+
{{OPT}}urCommandBufferValidUpdateParametersTest.SuccessNullptrHandle/*
11+
{{OPT}}KernelCommandEventSyncUpdateTest.TwoWaitEvents/*

‎test/conformance/exp_command_buffer/exp_command_buffer_adapter_hip.match

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
# Note: This file is only for use with cts_exe.py
2+
# These cause SIGILL when built with -fsanitize=cfi on AMD
3+
{{OPT}}urCommandBufferKernelHandleUpdateTest.Success/*
4+
{{OPT}}urCommandBufferKernelHandleUpdateTest.UpdateAgain/*
5+
{{OPT}}urCommandBufferKernelHandleUpdateTest.RestoreOriginalKernel/*
6+
{{OPT}}urCommandBufferKernelHandleUpdateTest.KernelAlternativeNotRegistered/*
7+
{{OPT}}urCommandBufferKernelHandleUpdateTest.RegisterInvalidKernelAlternative/*
8+
{{OPT}}urCommandBufferValidUpdateParametersTest.UpdateDimensionsWithoutUpdatingKernel/*
9+
{{OPT}}urCommandBufferValidUpdateParametersTest.UpdateOnlyLocalWorkSize/*
10+
{{OPT}}urCommandBufferValidUpdateParametersTest.SuccessNullptrHandle/*

‎test/fuzz/CMakeLists.txt

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -51,7 +51,9 @@ target_link_libraries(fuzztest-base
5151
${PROJECT_NAME}::headers
5252
${PROJECT_NAME}::common
5353
-fsanitize=fuzzer)
54-
target_compile_options(fuzztest-base PRIVATE -g -fsanitize=fuzzer)
54+
# When built with -g and -flto (which is required by some hardening flags), this causes a segfault in (upstream)
55+
# LLVM 14-15 while linking when CMAKE_BUILD_TYPE is Release
56+
target_compile_options(fuzztest-base PRIVATE -fsanitize=fuzzer)
5557
target_compile_definitions(fuzztest-base PRIVATE -DKERNEL_IL_PATH="${UR_CONFORMANCE_DEVICE_BINARIES_DIR}/fill/spir64.bin.0")
5658
target_include_directories(fuzztest-base PRIVATE ${UR_CONFORMANCE_DEVICE_BINARIES_DIR})
5759
add_dependencies(fuzztest-base generate_device_binaries)

0 commit comments

Comments
 (0)