fix: Fix sw64 gcc not support pie by default#47
fix: Fix sw64 gcc not support pie by default#47deepin-bot[bot] merged 1 commit intolinuxdeepin:masterfrom
Conversation
Add the -fPIE parameter via CMAKE_POSITION_INDEPENDENT_CODE. Add the -pie parameter via target_link_options. Log: Update compiler flags for security enhancements Bug: https://pms.uniontech.com/bug-view-339563.html
Reviewer's guide (collapsed on small PRs)Reviewer's GuideThis PR enables position-independent code and PIE linking for the dde-device-formatter target to address sw64 GCC not enabling PIE by default, by turning on global PIC generation and adding a -pie link option. File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
There was a problem hiding this comment.
Hey - I've left some high level feedback:
- The
CMAKE_POSITION_INDEPENDENT_CODEflag is being set globally, which will affect all current and future targets; consider limiting this to the specific target(s) that require PIE to avoid unintended side effects on libraries or other executables. - The unconditional
target_link_options(${PROJECT_NAME} PRIVATE -pie)may not be supported or appropriate on all toolchains or platforms; consider wrapping this in a check for the sw64 GCC toolchain or CMake'sCMAKE_SYSTEM_PROCESSOR/CMAKE_CXX_COMPILER_IDto avoid build issues elsewhere.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The `CMAKE_POSITION_INDEPENDENT_CODE` flag is being set globally, which will affect all current and future targets; consider limiting this to the specific target(s) that require PIE to avoid unintended side effects on libraries or other executables.
- The unconditional `target_link_options(${PROJECT_NAME} PRIVATE -pie)` may not be supported or appropriate on all toolchains or platforms; consider wrapping this in a check for the sw64 GCC toolchain or CMake's `CMAKE_SYSTEM_PROCESSOR`/`CMAKE_CXX_COMPILER_ID` to avoid build issues elsewhere.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
deepin pr auto review我来对这个 CMakeLists.txt 的修改进行审查:
改进建议:
# 启用位置无关代码,提高安全性(ASLR)
set(CMAKE_POSITION_INDEPENDENT_CODE ON)
...
# 启用位置无关可执行文件,提高安全性
target_link_options(${PROJECT_NAME} PRIVATE -pie)
# 检查平台是否支持 PIE
if(UNIX AND NOT APPLE)
set(CMAKE_POSITION_INDEPENDENT_CODE ON)
target_link_options(${PROJECT_NAME} PRIVATE -pie)
endif()总体来说,这个修改是合理的,主要是为了提高程序的安全性。建议添加适当的注释和平台检查,使代码更加健壮和可维护。 |
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: lzwind, wangrong1069 The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
|
/forcemerge |
|
This pr force merged! (status: unstable) |
Add the -fPIE parameter via CMAKE_POSITION_INDEPENDENT_CODE. Add the -pie parameter via target_link_options.
Log: Update compiler flags for security enhancements
Bug: https://pms.uniontech.com/bug-view-339563.html
Summary by Sourcery
Enable position-independent code and PIE linking for the dde-device-formatter target to ensure compatibility with sw64 GCC defaults.
Enhancements: