krak/zstd
krak/zstd
1
0
Fork 0
mirror of https://github.com/facebook/zstd.git synced 2025-03-06 16:56:49 +02:00
Code Issues Releases 1 Activity

[cmake] Fix -z noexecstack portability

Browse Source 
Summary:
Issue reported by @ryandesign and @MarcusCalhoun-Lopez.

CMake doesn't support spaces in flags. This caused older versions of gcc to
ignore the unknown flag "-z noexecstack" on MacOS since it was interpreted as a
single flag, not two separate flags. Then, during compilation it was treated as
"-z" "noexecstack", which was correctly forwarded to the linker. But the MacOS
linker does not support `-z noexecstack` so compilation failed.

The fix is to use `-Wl,-z,noexecstack`. This is never misinterpreted by a
compiler. However, not all compilers support this syntax to forward flags to the
linker. To fix this issue, we check if all the relevant `noexecstack` flags have
been successfully set, and if they haven't we disable assembly.

See also PR#4056 and PR#4061. I decided to go a different route because this is
simpler. It might not successfully set these flags on some compilers, but in that
case it also disables assembly, so they aren't required.

Test Plan:
```
mkdir build-cmake
cmake ../build/cmake/CMakeLists.txt
make -j
```

See that the linker flag is successfully detected & that assembly is enabled.

Run the same commands on MacOS which doesn't support `-Wl,-z,noexecstack` and see
that everything compiles and that `LD_FLAG_WL_Z_NOEXECSTACK` and
`ZSTD_HAS_NOEXECSTACK` are both false.
This commit is contained in:
Nick Terrell 2024-12-20 09:40:32 -08:00 committed by Nick Terrell
parent 5a7f5c745c
commit f0937b83d9
2 changed files with 18 additions and 2 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

18
build/cmake/CMakeModules/AddZstdCompilationFlags.cmake
View File

@ -50,6 +50,10 @@ function(EnableCompilerFlag _flag _C _CXX _LD)
endfunction()
macro(ADD_ZSTD_COMPILATION_FLAGS)
# We set ZSTD_HAS_NOEXECSTACK if we are certain we've set all the required
# compiler flags to mark the stack as non-executable.
set(ZSTD_HAS_NOEXECSTACK false)
if (CMAKE_CXX_COMPILER_ID MATCHES "GNU|Clang" OR MINGW) #Not only UNIX but also WIN32 for MinGW
# It's possible to select the exact standard used for compilation.
# It's not necessary, but can be employed for specific purposes.
@ -75,10 +79,22 @@ macro(ADD_ZSTD_COMPILATION_FLAGS)
endif ()
# Add noexecstack flags
# LDFLAGS
EnableCompilerFlag("-z noexecstack" false false true)
EnableCompilerFlag("-Wl,-z,noexecstack" false false true)
# CFLAGS & CXXFLAGS
EnableCompilerFlag("-Qunused-arguments" true true false)
EnableCompilerFlag("-Wa,--noexecstack" true true false)
# NOTE: Using 3 nested ifs because the variables are sometimes
# empty if the condition is false, and sometimes equal to false.
# This implicitly converts them to truthy values. There may be
# a better way to do this, but this reliably works.
if (${LD_FLAG_WL_Z_NOEXECSTACK})
if (${C_FLAG_WA_NOEXECSTACK})
if (${CXX_FLAG_WA_NOEXECSTACK})
# We've succeeded in marking the stack as non-executable
set(ZSTD_HAS_NOEXECSTACK true)
endif()
endif()
endif()
elseif (MSVC) # Add specific compilation flags for Windows Visual
set(ACTIVATE_MULTITHREADED_COMPILATION "ON" CACHE BOOL "activate multi-threaded compilation (/MP flag)")

2
build/cmake/lib/CMakeLists.txt
View File

@ -39,7 +39,7 @@ file(GLOB DecompressSources ${LIBRARY_DIR}/decompress/*.c)
if (MSVC)
add_compile_options(-DZSTD_DISABLE_ASM)
else ()
if(CMAKE_SYSTEM_PROCESSOR MATCHES "amd64.*|AMD64.*|x86_64.*|X86_64.*")
if(CMAKE_SYSTEM_PROCESSOR MATCHES "amd64.*|AMD64.*|x86_64.*|X86_64.*" AND ${ZSTD_HAS_NOEXECSTACK})
set(DecompressSources ${DecompressSources} ${LIBRARY_DIR}/decompress/huf_decompress_amd64.S)
else()
add_compile_options(-DZSTD_DISABLE_ASM)