fix(cmake): add -z nostart-stop-gc for LLVM/Clang toolchains#5
Conversation
When using LLVM-based toolchains (e.g. ST STM32CubeCLT arm-clang), ld.lld discards __start_/__stop_ encapsulation symbols under --gc-sections, causing undefined symbol errors for ctshell_cmd_section. Export CTSHELL_LINK_OPTIONS with -z nostart-stop-gc automatically when LLVM linker is detected. Also update porting docs (en/zh) to clarify GCC vs LLVM vs MDK-ARM linker script requirements.
7479749 to
722fd6e
Compare
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@CMakeLists.txt`:
- Around line 121-122: The current conditional uses compiler IDs and causes the
ELF-only flag "-z nostart-stop-gc" to be appended whenever Clang is used; change
the check to detect the actual linker binary (CMAKE_LINKER) and only append to
CTSHELL_LINK_OPTIONS when CMAKE_LINKER resolves to the LLVM lld binary (e.g.,
matches "ld.lld" or ends with "ld.lld"); update the if(...) that references
CMAKE_LINKER so it no longer tests CMAKE_C_COMPILER_ID or CMAKE_CXX_COMPILER_ID
but instead tests CMAKE_LINKER MATCHES ".*ld.lld$" (or equivalent exact-path
check) before appending "-z" "nostart-stop-gc" to CTSHELL_LINK_OPTIONS.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 6cffc1b6-bb5c-414c-a455-3429a397c774
📒 Files selected for processing (3)
CMakeLists.txtdocs/en/quickstart/porting.rstdocs/zh/quickstart/porting.rst
| if(CMAKE_LINKER MATCHES "lld" OR CMAKE_C_COMPILER_ID STREQUAL "Clang" OR CMAKE_CXX_COMPILER_ID STREQUAL "Clang") | ||
| list(APPEND CTSHELL_LINK_OPTIONS "-z" "nostart-stop-gc") |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
cache_file="$(fd -HI '^CMakeCache.txt$' | head -n1 || true)"
if [[ -z "${cache_file}" ]]; then
echo "No CMakeCache.txt found. Run CMake configure first."
exit 1
fi
linker="$(rg -n '^CMAKE_LINKER:FILEPATH=' "$cache_file" | sed 's/^[^=]*=//' | head -n1)"
echo "Resolved linker: $linker"
if [[ ! -x "$linker" ]]; then
echo "Linker binary is not executable: $linker"
exit 1
fi
"$linker" --help 2>&1 | rg -n 'nostart-stop-gc|start-stop-gc'Repository: MDLZCOOL/ctshell
Length of output: 111
🏁 Script executed:
cat -n CMakeLists.txt | sed -n '110,135p'Repository: MDLZCOOL/ctshell
Length of output: 857
🏁 Script executed:
rg -n "CMAKE_LINKER|CMAKE_C_COMPILER_ID|CMAKE_CXX_COMPILER_ID|lld|nostart-stop-gc" CMakeLists.txtRepository: MDLZCOOL/ctshell
Length of output: 371
Narrow linker detection to prevent passing -z nostart-stop-gc to unsupported linkers.
The condition on line 121 uses OR with Clang compiler checks, which means the flag is appended whenever Clang is used as the compiler, regardless of the actual linker. This causes -z nostart-stop-gc (an ELF-specific flag for LLVM lld) to be passed to incompatible linkers like Apple's ld64 or GNU ld, resulting in link failures.
The fix should check only if CMAKE_LINKER actually resolves to the ld.lld binary:
🔧 Proposed fix
set(CTSHELL_LINK_OPTIONS "")
-if(CMAKE_LINKER MATCHES "lld" OR CMAKE_C_COMPILER_ID STREQUAL "Clang" OR CMAKE_CXX_COMPILER_ID STREQUAL "Clang")
+if(CMAKE_LINKER MATCHES [[(^|.*/)ld\.lld(\.exe)?$]])
list(APPEND CTSHELL_LINK_OPTIONS "-z" "nostart-stop-gc")
endif()📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if(CMAKE_LINKER MATCHES "lld" OR CMAKE_C_COMPILER_ID STREQUAL "Clang" OR CMAKE_CXX_COMPILER_ID STREQUAL "Clang") | |
| list(APPEND CTSHELL_LINK_OPTIONS "-z" "nostart-stop-gc") | |
| if(CMAKE_LINKER MATCHES [[(^|.*/)ld\.lld(\.exe)?$]]) | |
| list(APPEND CTSHELL_LINK_OPTIONS "-z" "nostart-stop-gc") |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@CMakeLists.txt` around lines 121 - 122, The current conditional uses compiler
IDs and causes the ELF-only flag "-z nostart-stop-gc" to be appended whenever
Clang is used; change the check to detect the actual linker binary
(CMAKE_LINKER) and only append to CTSHELL_LINK_OPTIONS when CMAKE_LINKER
resolves to the LLVM lld binary (e.g., matches "ld.lld" or ends with "ld.lld");
update the if(...) that references CMAKE_LINKER so it no longer tests
CMAKE_C_COMPILER_ID or CMAKE_CXX_COMPILER_ID but instead tests CMAKE_LINKER
MATCHES ".*ld.lld$" (or equivalent exact-path check) before appending "-z"
"nostart-stop-gc" to CTSHELL_LINK_OPTIONS.
|
LGTM, Thanks! |
When using LLVM-based toolchains (e.g. ST STM32CubeCLT arm-clang), ld.lld discards _start/_stop encapsulation symbols under --gc-sections, causing undefined symbol errors for ctshell_cmd_section. Export CTSHELL_LINK_OPTIONS with -z nostart-stop-gc automatically when LLVM linker is detected.
Also update porting docs (en/zh) to clarify GCC vs LLVM vs MDK-ARM linker script requirements.
Summary by CodeRabbit
New Features
Documentation