]> git.ipfire.org Git - thirdparty/zstd.git/commitdiff
[cmake] Fix -z noexecstack portability
authorNick Terrell <terrelln@fb.com>
Fri, 20 Dec 2024 17:40:32 +0000 (09:40 -0800)
committerNick Terrell <nickrterrell@gmail.com>
Fri, 20 Dec 2024 23:06:23 +0000 (15:06 -0800)
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.

build/cmake/CMakeModules/AddZstdCompilationFlags.cmake
build/cmake/lib/CMakeLists.txt

index 153c51bae7174fb7bd1f512c4780ec80190142a9..5f381c656cfa0d81e6af86927c476ef5245efebe 100644 (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("-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)")
index 43b14d1753b5dc325c085d85ef5ece9f8f3dd29c..d07b1f5ff7ae4383b0fbbb3223cf9eb649a586cc 100644 (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)