chore: Update compiler flags for security enhancements#99
Merged
deepin-bot[bot] merged 1 commit intolinuxdeepin:masterfrom Nov 24, 2025
Merged
chore: Update compiler flags for security enhancements#99deepin-bot[bot] merged 1 commit intolinuxdeepin:masterfrom
deepin-bot[bot] merged 1 commit intolinuxdeepin:masterfrom
Conversation
Reviewer's guide (collapsed on small PRs)Reviewer's GuideThis PR tightens compiler security settings across the app, service, and test targets by adding the -D_FORTIFY_SOURCE=2 macro definition to both C and C++ compilation flags in their respective qmake project files. File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey there - I've reviewed your changes - here's some feedback:
- Consider centralizing the common security compile flags in a shared .pri file so you don't have to repeat the same QMAKE_*FLAGS in app.pro, service.pro, and tests.pro.
- Since _FORTIFY_SOURCE requires optimization to be effective, you may want to ensure these flags are only applied (or are adjusted) for builds compiled with at least -O1/-O2 to avoid unexpected behavior in non-optimized builds.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- Consider centralizing the common security compile flags in a shared .pri file so you don't have to repeat the same QMAKE_*FLAGS in app.pro, service.pro, and tests.pro.
- Since _FORTIFY_SOURCE requires optimization to be effective, you may want to ensure these flags are only applied (or are adjusted) for builds compiled with at least -O1/-O2 to avoid unexpected behavior in non-optimized builds.
## Individual Comments
### Comment 1
<location> `src/app/app.pro:8` </location>
<code_context>
QMAKE_LFLAGS += -z noexecstack -pie -z relro -z now
-QMAKE_CFLAGS += -fstack-protector-all -fPIE
-QMAKE_CXXFLAGS += -fstack-protector-all -fPIE
+QMAKE_CFLAGS += -fstack-protector-all -fPIE -D_FORTIFY_SOURCE=2
+QMAKE_CXXFLAGS += -fstack-protector-all -fPIE -D_FORTIFY_SOURCE=2
</code_context>
<issue_to_address>
**🚨 suggestion (security):** Consider ensuring optimization is enabled so _FORTIFY_SOURCE=2 actually has effect.
_FORTIFY_SOURCE only works with optimization (e.g., -O1 or higher). If your default CFLAGS don’t guarantee that, consider only defining it in optimized/release builds or ensure an optimization level like -O2 is always used, so the protections actually take effect.
Suggested implementation:
```
#添加安全编译参数
QMAKE_LFLAGS += -z noexecstack -pie -z relro -z now
QMAKE_CFLAGS += -fstack-protector-all -fPIE
QMAKE_CXXFLAGS += -fstack-protector-all -fPIE
# 仅在启用优化的 release 构建中启用 _FORTIFY_SOURCE，以确保其实际生效
CONFIG(release, debug|release) {
QMAKE_CFLAGS_RELEASE += -O2 -D_FORTIFY_SOURCE=2
QMAKE_CXXFLAGS_RELEASE += -O2 -D_FORTIFY_SOURCE=2
}
## 添加内存泄露检测
#CONFIG(debug, debug|release) {
```
- 请确认项目的其他位置（例如全局 qmake 配置、上层 .pro 或 .pri 文件）没有为 release 构建强制设置其他优化等级（如 -O0 或 -Os），以避免与这里的 -O2 冲突。
- 如果全局已经统一设置了优化等级（例如所有 release 都是 -O2），可以将上面 block 中的 -O2 去掉，只保留 -D_FORTIFY_SOURCE=2，以避免重复指定优化等级。
- 若你希望在 debug 模式下也启用部分优化和 FORTIFY（不常见，但有时有需求），可以类似地为 QMAKE_CFLAGS_DEBUG / QMAKE_CXXFLAGS_DEBUG 添加合适的 -O 级别和 -D_FORTIFY_SOURCE=2。
</issue_to_address>
### Comment 2
<location> `src/service/service.pro:13-14` </location>
<code_context>
QMAKE_LFLAGS += -z noexecstack -pie -z relro -z now
-QMAKE_CFLAGS += -fstack-protector-all -fPIE
-QMAKE_CXXFLAGS += -fstack-protector-all -fPIE
+QMAKE_CFLAGS += -fstack-protector-all -fPIE -D_FORTIFY_SOURCE=2
+QMAKE_CXXFLAGS += -fstack-protector-all -fPIE -D_FORTIFY_SOURCE=2
## 添加内存泄露检测
</code_context>
<issue_to_address>
**suggestion:** These duplicated security flags across .pro files could be centralized for consistency and easier maintenance.
Since both projects now define identical security flags, consider moving them into a shared .pri or common config file that each .pro includes. This will reduce duplication and help prevent configuration drift when updating flags like _FORTIFY_SOURCE or PIE settings in the future.
Suggested implementation:
```
# 添加公共安全编译参数（集中在共享配置文件中，避免各 .pro 重复配置）
include(../common/security_flags.pri)
TEMPLATE = app
```
To fully implement the suggestion and centralize the security flags, you should also:
1. Create a new file (if it doesn't exist yet), for example at `src/common/security_flags.pri` (or adjust the include path above to match your project layout), with the shared flags:
- QMAKE_LFLAGS += -z noexecstack -pie -z relro -z now
- QMAKE_CFLAGS += -fstack-protector-all -fPIE -D_FORTIFY_SOURCE=2
- QMAKE_CXXFLAGS += -fstack-protector-all -fPIE -D_FORTIFY_SOURCE=2
2. In the other .pro files that currently duplicate these flags, remove their individual QMAKE_LFLAGS/QMAKE_CFLAGS/QMAKE_CXXFLAGS security flag lines and add the same `include(../common/security_flags.pri)` (with the correct relative path from each .pro).
3. If different targets need slight variations of the flags in the future, keep the common baseline in `security_flags.pri` and override/append only the differences locally in specific .pro files.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Add "-D_FORTIFY_SOURCE=2" Log: Update compiler flags for security enhancements Bug: https://pms.uniontech.com/bug-view-337059.html
deepin pr auto review我来对这个git diff进行详细审查：
在app.pro、service.pro和tests.pro三个文件中都添加了
在.reuse/dep5文件中添加了
a) 关于_FORTIFY_SOURCE：
b) 关于安全编译选项：
c) 关于tests.pro：
|
lzwind
approved these changes
Nov 24, 2025
|
[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 |
Contributor
Author
|
/merge |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Add "-D_FORTIFY_SOURCE=2"
Log: Update compiler flags for security enhancements
Bug: https://pms.uniontech.com/bug-view-337059.html
Summary by Sourcery
Build: