Skip to content

Commit e5a2c3d

Browse files
jgmelberclaude
andcommitted
Address Copilot review comments
- Guard the new -fPIC/-fPIE blocks with NOT MSVC too: clang-cl reports CMAKE_CXX_COMPILER_ID "Clang" but uses the MSVC-style driver, which doesn't accept these GNU-style flags. - Drop -fPIE from target_link_options(bootgen ...); it's a compile-only flag, and passing it at link time can break toolchains where linking doesn't go through the compiler driver. Only -pie belongs there. Re-verified locally under the same forced -fno-pie -fno-PIC -no-pie global configuration as before: bootgen still links as a true PIE (ET_DYN) binary and runs correctly. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
1 parent 04274fb commit e5a2c3d

1 file changed

Lines changed: 10 additions & 17 deletions

File tree

tools/bootgen/CMakeLists.txt

Lines changed: 10 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -144,16 +144,12 @@ if (CMAKE_CXX_COMPILER_ID MATCHES "MSVC")
144144
target_compile_options(bootgen-lib PRIVATE /wd4996 /wd4840 /wd4005 /wd5033)
145145
endif()
146146
target_compile_options(bootgen-lib PRIVATE ${bootgen_warning_ignores})
147-
if(CMAKE_CXX_COMPILER_ID MATCHES "Clang|GNU")
148-
# Explicit -fPIC, not just the POSITION_INDEPENDENT_CODE property above:
149-
# that property relies on CMake's CMP0083 + the CheckPIESupported module
150-
# to inject the right compiler/linker flags, and this project never calls
151-
# check_pie_supported(), so the property silently does nothing on some
152-
# toolchains (observed on the manylinux_2_28 gcc-toolset used to build
153-
# release wheels, which -- unlike a distro's system gcc -- does not
154-
# default to PIE/PIC either). Passed last so it wins (compilers apply
155-
# "last flag wins" for -fPIC/-fno-PIC) over anything inherited from the
156-
# linked MLIR/LLVM package's own build configuration.
147+
if(CMAKE_CXX_COMPILER_ID MATCHES "Clang|GNU" AND NOT MSVC)
148+
# The POSITION_INDEPENDENT_CODE property above doesn't reliably force PIC
149+
# here (this project never calls check_pie_supported(), which CMP0083
150+
# needs), so pass -fPIC explicitly, last, so it wins over any inherited
151+
# -fno-PIC. NOT MSVC excludes clang-cl, which also matches "Clang" but
152+
# can't parse this flag.
157153
target_compile_options(bootgen-lib PRIVATE -fPIC)
158154
endif()
159155
target_include_directories(bootgen-lib PRIVATE ${BOOTGEN_SOURCE} ${OPENSSL_INCLUDE_DIR})
@@ -178,14 +174,11 @@ if (CMAKE_CXX_COMPILER_ID MATCHES "MSVC")
178174
target_compile_options(bootgen PRIVATE /EHsc)
179175
target_compile_options(bootgen PRIVATE /wd4996 /wd4840 /wd4005 /wd5033)
180176
endif()
181-
if(CMAKE_CXX_COMPILER_ID MATCHES "Clang|GNU")
182-
# Explicit -fPIE/-pie (see bootgen-lib above for why the
183-
# POSITION_INDEPENDENT_CODE property alone can't be trusted). Passed last
184-
# so it wins -- compilers apply "last flag wins" for -fPIE/-fno-PIE and
185-
# -pie/-no-pie -- over anything inherited from the linked MLIR/LLVM
186-
# package's own build configuration.
177+
if(CMAKE_CXX_COMPILER_ID MATCHES "Clang|GNU" AND NOT MSVC)
178+
# See bootgen-lib above. -fPIE is compile-only; -pie is the link-mode
179+
# switch (kept separate since some toolchains don't accept -fPIE at link).
187180
target_compile_options(bootgen PRIVATE -fPIE)
188-
target_link_options(bootgen PRIVATE -fPIE -pie)
181+
target_link_options(bootgen PRIVATE -pie)
189182
endif()
190183
target_compile_definitions(bootgen PRIVATE OPENSSL_USE_APPLINK)
191184
target_link_libraries(bootgen PRIVATE bootgen-lib OpenSSL::SSL OpenSSL::applink)

0 commit comments

Comments
 (0)