Skip to content

chore: Update CMake configuration for improved security and threading…#146

Merged
lzwind merged 1 commit into
linuxdeepin:masterfrom
dengzhongyuan365-dev:master
Jun 21, 2025
Merged

chore: Update CMake configuration for improved security and threading…#146
lzwind merged 1 commit into
linuxdeepin:masterfrom
dengzhongyuan365-dev:master

Conversation

@dengzhongyuan365-dev

@dengzhongyuan365-dev dengzhongyuan365-dev commented Jun 21, 2025

Copy link
Copy Markdown
Contributor

… support

  • Added required Threads package to CMakeLists.txt
  • Enhanced compiler flags for position-independent code and security features
  • Updated build dependencies in debian/control to include libc6-dev

Summary by Sourcery

Update the CMake configuration to enable threading support, strengthen security compiler/linker flags, and refresh Debian build dependencies.

Enhancements:

  • Require and link the Threads package in the CMake build
  • Harden binaries with position-independent code, stack protector, and RELRO/now/noexecstack/PIE flags
  • Refine flag usage by moving gc-sections to linker settings

Build:

  • Add libc6-dev to Debian build dependencies

… support

- Added required Threads package to CMakeLists.txt
- Enhanced compiler flags for position-independent code and security features
- Updated build dependencies in debian/control to include libc6-dev
@sourcery-ai

sourcery-ai Bot commented Jun 21, 2025

Copy link
Copy Markdown

Reviewer's Guide

This PR refines the CMake build by enabling threading support, augmenting compiler and linker flags for position-independent code and security hardening, and updates the Debian packaging dependencies accordingly.

File-Level Changes

Change Details Files
Integrate threading support
  • Added Threads package requirement in top-level CMakeLists
  • Linked Threads::Threads to the basestruct target
CMakeLists.txt
basestruct/CMakeLists.txt
Harden build with PIC and security flags
  • Moved gc-sections linker flags out of compiler flags
  • Added position-independent code and stack protector compile flags
  • Extended linker flags with relro, now, noexecstack, and pie
CMakeLists.txt
Update packaging dependencies
  • Included libc6-dev in Debian build-depends
debian/control

Tips and commands

Interacting with Sourcery

  • Trigger a new review: Comment @sourcery-ai review on the pull request.
  • Continue discussions: Reply directly to Sourcery's review comments.
  • Generate a GitHub issue from a review comment: Ask Sourcery to create an
    issue from a review comment by replying to it. You can also reply to a
    review comment with @sourcery-ai issue to create an issue from it.
  • Generate a pull request title: Write @sourcery-ai anywhere in the pull
    request title to generate a title at any time. You can also comment
    @sourcery-ai title on the pull request to (re-)generate the title at any time.
  • Generate a pull request summary: Write @sourcery-ai summary anywhere in
    the pull request body to generate a PR summary at any time exactly where you
    want it. You can also comment @sourcery-ai summary on the pull request to
    (re-)generate the summary at any time.
  • Generate reviewer's guide: Comment @sourcery-ai guide on the pull
    request to (re-)generate the reviewer's guide at any time.
  • Resolve all Sourcery comments: Comment @sourcery-ai resolve on the
    pull request to resolve all Sourcery comments. Useful if you've already
    addressed all the comments and don't want to see them anymore.
  • Dismiss all Sourcery reviews: Comment @sourcery-ai dismiss on the pull
    request to dismiss all existing Sourcery reviews. Especially useful if you
    want to start fresh with a new review - don't forget to comment
    @sourcery-ai review to trigger a new review!

Customizing Your Experience

Access your dashboard to:

  • Enable or disable review features such as the Sourcery-generated pull request
    summary, the reviewer's guide, and others.
  • Change the review language.
  • Add, remove or edit custom review instructions.
  • Adjust other review settings.

Getting Help

@sourcery-ai sourcery-ai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey @dengzhongyuan365-dev - I've reviewed your changes - here's some feedback:

  • Wrap Linux-specific linker flags (-z relro, ‑z now, ‑z noexecstack) in an if(UNIX AND CMAKE_SYSTEM_NAME STREQUAL "Linux") block to prevent portability issues on other platforms.
  • Unify PIC/PIE usage by applying ‑fPIC only to shared libraries and ‑fPIE to executables rather than globally setting ‑fPIC in CMAKE_C_FLAGS to avoid unnecessary binary overhead.
  • Instead of mutating CMAKE_EXE_LINKER_FLAGS globally, consider using target_link_options or target_compile_options for each target to scope the security and optimization flags more precisely.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- Wrap Linux-specific linker flags (-z relro, ‑z now, ‑z noexecstack) in an if(UNIX AND CMAKE_SYSTEM_NAME STREQUAL "Linux") block to prevent portability issues on other platforms.
- Unify PIC/PIE usage by applying ‑fPIC only to shared libraries and ‑fPIE to executables rather than globally setting ‑fPIC in CMAKE_C_FLAGS to avoid unnecessary binary overhead.
- Instead of mutating CMAKE_EXE_LINKER_FLAGS globally, consider using target_link_options or target_compile_options for each target to scope the security and optimization flags more precisely.

Sourcery is free for open source - if you like our reviews please consider sharing them ✨
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.

@deepin-ci-robot

Copy link
Copy Markdown

deepin pr auto review

代码审查意见:

  1. CMakeLists.txt文件中,添加了find_package(Threads REQUIRED),这是一个好的做法,因为它确保了在编译时能够找到线程库。但是,如果项目没有使用多线程功能,添加这个包可能会增加不必要的依赖。

  2. CMakeLists.txt文件中,移除了-Wl,--gc-sections链接器选项,这可能会影响代码的优化程度。如果移除这个选项后,代码大小和性能有所下降,应该重新评估这一改动。

  3. CMakeLists.txt文件中,添加了位置无关代码和安全编译标志,这是一个好的安全实践。但是,如果项目已经使用了这些标志,那么重复添加可能会引起问题。

  4. CMakeLists.txt文件中,移除了-fstack-protector-all -z relro -z now -z noexecstack -pie编译和链接标志,这可能会降低程序的安全性。如果移除这些标志后,程序的安全性有所下降,应该重新评估这一改动。

  5. basestruct/CMakeLists.txt文件中,添加了Threads::Threads库的链接,这是一个好的做法,因为它确保了在编译时能够找到线程库。

  6. debian/control文件中,添加了libc6-dev作为构建依赖,这是一个好的做法,因为它确保了在构建过程中能够找到C库的头文件。

总体来说,这些改动提高了代码的安全性,但是需要确保这些改动不会对项目的构建和运行产生负面影响。如果可能,应该在测试环境中验证这些改动的影响。

@lzwind lzwind merged commit 93bb914 into linuxdeepin:master Jun 21, 2025
15 of 17 checks passed
@deepin-ci-robot

Copy link
Copy Markdown

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: dengzhongyuan365-dev, lzwind

The full list of commands accepted by this bot can be found here.

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants