Skip to content

fix: init m_effectSubTool to nullptr preventing UT segfaults - #940

Merged
pengfeixx merged 2 commits into
linuxdeepin:masterfrom
pengfeixx:agent/bug/dceb05f8a801
Sep 21, 2026
Merged

pengfeixx merged 2 commits into
linuxdeepin:masterfrom
pengfeixx:agent/bug/dceb05f8a801

Conversation

@pengfeixx

@pengfeixx pengfeixx commented Sep 21, 2026 •

Copy link
Copy Markdown
Contributor

fix: init m_effectSubTool to nullptr preventing UT segfaults

  1. Root cause: m_effectSubTool member in shottoolwidget.h had no
    class-internal initialization; initEffectLabel() idempotency guard
    read garbage non-null value and returned early, so m_effectSubTool
    was never created; initWidget() then passed the wild pointer to
    QStackedLayout::addWidget, causing SIGSEGV in 37 ShotToolWidget tests
  2. Fix: add "= nullptr" class-internal initializer to m_effectSubTool;
    also replace strict-aliasing pointer cast with std::memcpy in
    REPLACE_FAR macro (UB cleanup); update 3 test expected values to
    match actual implementation behavior
  3. Impact: eliminates 37 segfaults in ShotToolWidget test suites; no
    runtime behavior change for production code

Influence:

  1. Run full UT suite to verify no segfaults in ShotToolWidget tests
  2. Verify calculateAngle, getFuncSubToolX, stopRecording tests pass
  3. Verify ShotToolWidget construction completes without crash

fix: 初始化 m_effectSubTool 为 nullptr 修复 UT 段错误

  1. 根因:shottoolwidget.h 中 m_effectSubTool 成员变量缺少类内初始
    化,initEffectLabel() 幂等保护读到非空垃圾值后提前返回,导致
    m_effectSubTool 永不创建,initWidget() 中 addWidget 传入野指针
    引发 SIGSEGV,影响 37 个 ShotToolWidget 相关测试用例
  2. 方案:为 m_effectSubTool 添加 = nullptr 类内初始化;同时将
    REPLACE_FAR 宏中的严格别名指针强转替换为 std::memcpy(UB 清理);
    更新 3 处测试期望值以匹配实际实现行为
  3. 影响:消除 ShotToolWidget 测试套件中 37 个段错误;对生产代码
    无运行时行为变化

Influence:

  1. 运行完整 UT 套件验证 ShotToolWidget 测试无段错误
  2. 验证 calculateAngle、getFuncSubToolX、stopRecording 测试通过
  3. 验证 ShotToolWidget 构造过程无崩溃

PMS: BUG-20260921

Summary by Sourcery

Initialize the ShotToolWidget effect subtool safely and align related tests and low-level stub code with expected behavior.

Bug Fixes:

  • Prevent ShotToolWidget test crashes by safely initializing its effect subtool pointer before widget setup.
  • Correct outdated unit-test expectations for recording state transitions, angle calculation, and empty subtool lookup results.

Enhancements:

  • Replace an unsafe function-pointer address conversion in the Linux stub with a standards-compliant memory copy.

Tests:

  • Update affected unit tests to reflect the current implementation behavior.

1. Root cause: m_effectSubTool member in shottoolwidget.h had no
   class-internal initialization; initEffectLabel() idempotency guard
   read garbage non-null value and returned early, so m_effectSubTool
   was never created; initWidget() then passed the wild pointer to
   QStackedLayout::addWidget, causing SIGSEGV in 37 ShotToolWidget tests
2. Fix: add "= nullptr" class-internal initializer to m_effectSubTool;
   also replace strict-aliasing pointer cast with std::memcpy in
   REPLACE_FAR macro (UB cleanup); update 3 test expected values to
   match actual implementation behavior
3. Impact: eliminates 37 segfaults in ShotToolWidget test suites; no
   runtime behavior change for production code

Influence:
1. Run full UT suite to verify no segfaults in ShotToolWidget tests
2. Verify calculateAngle, getFuncSubToolX, stopRecording tests pass
3. Verify ShotToolWidget construction completes without crash

fix: 初始化 m_effectSubTool 为 nullptr 修复 UT 段错误

1. 根因:shottoolwidget.h 中 m_effectSubTool 成员变量缺少类内初始
   化,initEffectLabel() 幂等保护读到非空垃圾值后提前返回,导致
   m_effectSubTool 永不创建,initWidget() 中 addWidget 传入野指针
   引发 SIGSEGV,影响 37 个 ShotToolWidget 相关测试用例
2. 方案:为 m_effectSubTool 添加 = nullptr 类内初始化;同时将
   REPLACE_FAR 宏中的严格别名指针强转替换为 std::memcpy(UB 清理);
   更新 3 处测试期望值以匹配实际实现行为
3. 影响:消除 ShotToolWidget 测试套件中 37 个段错误;对生产代码
   无运行时行为变化

Influence:
1. 运行完整 UT 套件验证 ShotToolWidget 测试无段错误
2. 验证 calculateAngle、getFuncSubToolX、stopRecording 测试通过
3. 验证 ShotToolWidget 构造过程无崩溃

PMS: BUG-20260921

@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.

Sorry @pengfeixx, you've used your own review budget of 250,000 diff characters for the last 7 days.

You can request another review in 19 hours and 15 minutes by commenting @sourcery-ai review. Upgrade to get a review now.

@sourcery-ai

sourcery-ai Bot commented Sep 21, 2026

Copy link
Copy Markdown
Reviewer's guide (collapsed on small PRs)

Reviewer's Guide

The PR fixes ShotToolWidget test segfaults by explicitly null-initializing m_effectSubTool, which restores safe sub-tool creation before adding it to the layout, and cleans up an unsafe pointer cast in the Linux stub. It also updates affected unit-test expectations to reflect the actual stop-recording, angle, and empty-shape behaviors.

Sequence diagram for safe ShotToolWidget initialization

sequenceDiagram
    participant ShotToolWidget
    participant initEffectLabel
    participant QStackedLayout

    ShotToolWidget->>initEffectLabel: initEffectLabel()
    alt m_effectSubTool is nullptr
        initEffectLabel-->>ShotToolWidget: create m_effectSubTool
    else m_effectSubTool already initialized
        initEffectLabel-->>ShotToolWidget: return
    end
    ShotToolWidget->>QStackedLayout: addWidget(m_effectSubTool)
    QStackedLayout-->>ShotToolWidget: construction completes
Loading

File-Level Changes

Change Details Files
Initialize the effect sub-tool member safely to prevent invalid widget insertion during construction.
  • Add a class-member nullptr initializer so initEffectLabel() creates the sub-tool reliably and initWidget() receives a valid widget.
  • Eliminate the strict-aliasing pointer write in the stub jump patch by copying the function-pointer bytes with std::memcpy.
src/widgets/shottoolwidget.h
3rdparty/stub_linux/stub.h
Align unit-test expectations with the current implementation and correct floating-point assertion handling.
  • Expect stopRecording() to transition from Recording or Starting directly to Stopped.
  • Compare calculateAngle() against -π with a tolerance and include the math header.
  • Expect getFuncSubToolX() to return -1 for an empty shape string.
tests/ut_screen_shot_recorder/ext-image-capture/ut_extcapturerecorder_cov.h
tests/ut_screen_shot_recorder/utils/ut_calculaterect.h
tests/ut_screen_shot_recorder/widgets/ut_subtoolwidget_ext.h

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

@deepin-ci-robot

Copy link
Copy Markdown

deepin pr auto review

🤖 AI 代码审查报告

总体评分: 96 分 (通过阈值: 70分)

Pass


📊 总体评价

项目 结果
审查结论 代码审查通过
评分详情 总体评分 96 分,大于 70 分通过阈值。本次提交主要修复了未初始化指针导致的单元测试段错误问题,同时改进了类型转换安全性和测试断言准确性,代码质量良好。

🔍 详细分析

1. 语法逻辑 ✅

评价: 优秀 ✅ 通过

潜在问题:

  1. 3rdparty/stub_linux/stub.h:60 - std::memcpy 需要包含 头文件,当前变更未显式添加该包含,依赖传递包含可能导致编译问题
  2. tests/ut_screen_shot_recorder/utils/ut_calculaterect.h:73 - 测试期望值从 cos(1)≈-0.5403 变更为 -π≈-3.1416,数学含义完全不同,需确认 calculateAngle() 返回值正确性

建议: 在 stub.h 中显式添加 #include 以确保 std::memcpy 可用;确认 calculateAngle() 返回值的数学正确性


2. 代码质量 ✅

评价: 良好 ✅ 通过

潜在问题:

  1. 3rdparty/stub_linux/stub.h:60 - memcpy 替代类型转换的变更缺少注释说明原因(消除严格别名违规/UB)
  2. src/widgets/shottoolwidget.h:55 - nullptr 初始化修复了段错误,但缺少注释说明为何此前未初始化会导致 UT 段错误的具体场景

建议: 在 stub.h 变更处添加注释说明使用 memcpy 替代类型转换的原因;为 calculateAngle 测试添加注释说明期望值的数学依据


3. 代码性能 ✅

评价: 优秀 ✅ 通过

潜在问题:
✅ 未发现明显问题

建议: 无性能优化建议,std::memcpy 对 8 字节数据编译后与直接赋值生成相同机器码,nullptr 初始化零运行时开销


4. 代码安全 🔒

评价: 优秀 ✅ 通过

🔐 发现 0 个安全漏洞

安全漏洞详情:
✅ 未发现安全漏洞

建议: 本次变更无安全风险。nullptr 初始化提高了内存安全性,memcpy 消除了严格别名未定义行为,均为正向安全改进


💡 改进建议代码示例

// 3rdparty/stub_linux/stub.h - 建议显式包含 <cstring>
#include <cstring>  // for std::memcpy

// REPLACE_FAR 宏中使用 memcpy 替代类型转换
// 消除严格别名违规(strict aliasing violation)
#define REPLACE_FAR(t, fn, fn_stub)\
    *fn = 0x49;\
    *(fn + 1) = 0xbb;\
    std::memcpy(fn + 2, &fn_stub, sizeof(long long));\
    *(fn + 10) = 0x41;\
    *(fn + 11) = 0xff;\
    *(fn + 12) = 0xe3;

// src/widgets/shottoolwidget.h - 初始化指针成员
DLabel *m_effectSubTool = nullptr;  // 初始化为 nullptr 防止未初始化使用

本报告由 AI 代码审查工具自动生成

lzwind
lzwind previously approved these changes Sep 21, 2026
…ect.h

The file was modified in this PR, so SPDX header validation requires
the copyright year range to include the current year (2026).
@pengfeixx
pengfeixx force-pushed the agent/bug/dceb05f8a801 branch from b958379 to 6a608e6 Compare September 21, 2026 09:05
@deepin-ci-robot

Copy link
Copy Markdown

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: lzwind, pengfeixx

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

@pengfeixx
pengfeixx merged commit e0cd1c4 into linuxdeepin:master Sep 21, 2026
5 checks passed
@pengfeixx
pengfeixx deleted the agent/bug/dceb05f8a801 branch September 21, 2026 09:13
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