-
Notifications
You must be signed in to change notification settings - Fork 180
fix: Fix sw64 gcc not support pie by default #3508
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
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
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: 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 |
Reviewer's GuideThis PR enables position-independent code (PIE) across the project to address a sw64 GCC default behavior, by turning on global PIE compilation and adding PIE linker flags to key executables for improved security and compatibility. Flow diagram for CMake build process with PIE flagsflowchart TD
A["Configure CMake"] --> B{"CMAKE_POSITION_INDEPENDENT_CODE"}
B -->|ON in root CMakeLists| C["Compile sources with -fPIE"]
C --> D["Per-target CMakeLists"]
D --> E_dde_file_manager["dde_file_manager adds target_link_options(-pie)"]
D --> E_dde_file_dialog["dde_file_dialog adds target_link_options(-pie)"]
D --> E_dde_file_dialog_x11["dde_file_dialog_x11 adds target_link_options(-pie)"]
D --> E_dde_file_dialog_wayland["dde_file_dialog_wayland adds target_link_options(-pie)"]
D --> E_filepreview["filepreview adds target_link_options(-pie)"]
D --> E_diskencrypt["diskencrypt adds target_link_options(-pie)"]
D --> E_daemon["dde_file_manager_daemon adds target_link_options(-pie)"]
E_dde_file_manager --> F["Link executables with -pie"]
E_dde_file_dialog --> F
E_dde_file_dialog_x11 --> F
E_dde_file_dialog_wayland --> F
E_filepreview --> F
E_diskencrypt --> F
E_daemon --> F
F --> G["PIE-enabled binaries for sw64 gcc"]
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey - I've left some high level feedback:
- Enabling
CMAKE_POSITION_INDEPENDENT_CODEglobally may impact all targets including static libraries; consider restricting this setting to the specific targets or platforms (e.g., only sw64/gcc) that require PIE support. - The
target_link_options(... -pie)additions are duplicated across multiple CMakeLists; consider centralizing this in a common helper function or applying a target property from a shared CMake module to reduce repetition. - It may be safer to gate
-pieusage on compiler and platform checks (e.g.,if (CMAKE_CXX_COMPILER_ID STREQUAL "GNU" AND CMAKE_SYSTEM_PROCESSOR STREQUAL "sw64")) to avoid issues on toolchains that don’t support or need this flag.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- Enabling `CMAKE_POSITION_INDEPENDENT_CODE` globally may impact all targets including static libraries; consider restricting this setting to the specific targets or platforms (e.g., only sw64/gcc) that require PIE support.
- The `target_link_options(... -pie)` additions are duplicated across multiple CMakeLists; consider centralizing this in a common helper function or applying a target property from a shared CMake module to reduce repetition.
- It may be safer to gate `-pie` usage on compiler and platform checks (e.g., `if (CMAKE_CXX_COMPILER_ID STREQUAL "GNU" AND CMAKE_SYSTEM_PROCESSOR STREQUAL "sw64")`) to avoid issues on toolchains that don’t support or need this flag.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这份 1. 修改意图分析开发者通过修改多个
2. 代码审查2.1 逻辑与语法
2.2 代码质量
2.3 性能影响
2.4 安全性
3. 改进意见与建议尽管当前的修改是正确且有益的,但在跨平台兼容性和构建系统健壮性方面还有优化空间: 建议 1:使用 CMake 内置属性替代直接传参(推荐)直接使用 修改建议: target_link_options(${PROJECT_NAME} PRIVATE -pie)改为: set_target_properties(${PROJECT_NAME} PROPERTIES POSITION_INDEPENDENT_CODE TRUE)或者对于可执行文件,确保链接器标志正确,可以使用 include(CheckPIESupported)
check_pie_supported()
# 如果支持,则设置属性
if(PIE_SUPPORTED)
set_target_properties(${PROJECT_NAME} PROPERTIES POSITION_INDEPENDENT_CODE TRUE)
endif()注意:对于可执行文件, 建议 2:增加编译器支持检查为了防止在不支持 PIE 的旧版编译器或特定平台上构建失败,建议添加检查逻辑。 修改示例: # 检查编译器是否支持 PIE
include(CheckCXXCompilerFlag)
check_cxx_compiler_flag("-pie" HAS_PIE_SUPPORT)
if(HAS_PIE_SUPPORT)
target_link_options(${PROJECT_NAME} PRIVATE -pie)
endif()建议 3:统一管理链接选项由于几乎所有的可执行文件都需要添加 修改示例: function(apply_security_settings TARGET)
if(CMAKE_BUILD_TYPE STREQUAL "Release")
# 启用 PIE
if(NOT MSVC) # MSVC 处理方式不同
target_link_options(${TARGET} PRIVATE -pie)
endif()
endif()
endfunction()在子目录中调用: apply_security_settings(${PROJECT_NAME})4. 总结这份代码修改在安全性方面是非常积极且必要的,能够有效提升二进制文件抵抗内存攻击的能力。代码本身没有语法错误。主要的改进方向在于增强跨平台兼容性和减少重复代码,通过使用 CMake 的内置属性或检查宏,可以使构建脚本更加健壮和易于维护。 |
|
/forcemerge |
|
This pr force merged! (status: behind) |
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 executables across key file manager components to address platform-specific PIE defaults and improve security.
Build: