Skip to content

test(editor): fix heap-buffer-overflow crash in UT_Textedit_MoveText - #518

Merged
lzwind merged 1 commit into
linuxdeepin:masterfrom
pengfeixx:feat/ut-100pct-function-coverage
Aug 12, 2026
Merged

test(editor): fix heap-buffer-overflow crash in UT_Textedit_MoveText#518
lzwind merged 1 commit into
linuxdeepin:masterfrom
pengfeixx:feat/ut-100pct-function-coverage

Conversation

@pengfeixx

@pengfeixx pengfeixx commented Aug 12, 2026

Copy link
Copy Markdown
Contributor

Stub Window::updateModifyStatus in three MoveText tests to avoid invalid downcast in EditWrapper::window() when EditWrapper has no Window parent, which caused heap-buffer-overflow in Tabbar::truePathAt. Also replace QTest::qWait/processEvents with direct timer meta-object invocation in startmanager/window/editorapplication/themepanel tests, add __gcov_dump() in ut_main to ensure coverage data flush, and add new cases for LineBar/WarningNotices.

修复 UT_Textedit_MoveText 三个用例的堆缓冲区溢出崩溃:当 EditWrapper 无 Window 父对象时 EditWrapper::window() 的 static_cast 误转导致越界, 通过 stub Window::updateModifyStatus 规避。同时将多个测试中的
qWait/processEvents 替换为直接通过 meta-object 触发 timer lambda, 在 ut_main 增加 __gcov_dump 确保覆盖率数据落盘,并新增
LineBar/WarningNotices 用例。

Log: 修复单测崩溃并改进测试基础设施
Influence: 仅影响单元测试代码,不改动业务源码;消除 ASan 下的崩溃与 UB。

Summary by Sourcery

Stabilize editor-related unit tests by avoiding unsafe event processing and ensuring coverage data is flushed on test exit.

Bug Fixes:

  • Prevent heap-buffer-overflow and invalid downcast in UT_Textedit_MoveText tests by stubbing Window::updateModifyStatus and avoiding use of a Window parent where none exists.

Enhancements:

  • Refactor multiple tests that rely on QTimer::singleShot lambdas to trigger them via direct meta-object invocation instead of QTest::qWait/processEvents, reducing flakiness and interaction with stale events.
  • Add focused tests for TextEdit highlight/eventFilter lambdas, StartManager timer callbacks, ThemePanel popup animation, and Window find/replace bar focus timers to explicitly exercise previously untested lambda logic.
  • Introduce dedicated tests for LineBar clear-button behavior and WarningNotices save/reload button interactions and signals to improve coverage of UI controls.

Tests:

  • Add new unit tests around TextEdit timer and sort comparator lambdas, StartManager singleShot lambdas, ThemePanel animation callbacks, Window popup bar focus lambdas, LineBar clear button lambda, and WarningNotices button click handlers.
  • Improve test isolation by using event filters and direct signal invocation to avoid processing accumulated DeferredDelete and other stale events between tests.

Chores:

  • Ensure coverage data is flushed at the end of the test harness by calling __gcov_dump and terminating via std::_Exit instead of a normal return from main.

Stub Window::updateModifyStatus in three MoveText tests to avoid
invalid downcast in EditWrapper::window() when EditWrapper has no
Window parent, which caused heap-buffer-overflow in Tabbar::truePathAt.
Also replace QTest::qWait/processEvents with direct timer meta-object
invocation in startmanager/window/editorapplication/themepanel tests,
add __gcov_dump() in ut_main to ensure coverage data flush, and add
new cases for LineBar/WarningNotices.

修复 UT_Textedit_MoveText 三个用例的堆缓冲区溢出崩溃:当 EditWrapper
无 Window 父对象时 EditWrapper::window() 的 static_cast 误转导致越界,
通过 stub Window::updateModifyStatus 规避。同时将多个测试中的
qWait/processEvents 替换为直接通过 meta-object 触发 timer lambda,
在 ut_main 增加 __gcov_dump 确保覆盖率数据落盘,并新增
LineBar/WarningNotices 用例。

Log: 修复单测崩溃并改进测试基础设施
Influence: 仅影响单元测试代码,不改动业务源码;消除 ASan 下的崩溃与 UB。

@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 have reached your weekly rate limit of 500000 diff characters.

Please try again later or upgrade to continue using Sourcery

@sourcery-ai

sourcery-ai Bot commented Aug 12, 2026

Copy link
Copy Markdown

Reviewer's Guide

This PR fixes a heap-buffer-overflow in UT_Textedit_MoveText by stubbing a problematic Window method, and systematically refactors several unit tests to trigger QTimer/QPropertyAnimation lambdas and other callbacks directly instead of via QTest::qWait/processEvents, adds coverage-flushing in the test main, and introduces new tests for LineBar and WarningNotices behavior.

File-Level Changes

Change Details Files
Prevent heap-buffer-overflow in TextEdit MoveText tests by stubbing Window::updateModifyStatus when EditWrapper has no Window parent.
  • Include QTest header in ut_textedit.cpp for new tests and helpers.
  • Extend MoveText tests to stub Window::updateModifyStatus alongside undo/redo slots to avoid invalid static_cast in EditWrapper::window().
tests/src/editor/ut_textedit.cpp
Refactor editor-related tests to invoke timer and internal lambdas directly instead of relying on processEvents/qWait, avoiding stale timers and UB.
  • Add TE_DeferredDeleteBlocker helper and tests that locate TextEdit-owned QTimers and invoke their timeout slots via QMetaObject::invokeMethod.
  • Add tests for TextEdit::highlight, eventFilter color mark menu Tab QTimer lambda, and calcMarkReplaceList sort comparator, asserting expected outcomes without using qWait/processEvents.
tests/src/editor/ut_textedit.cpp
Simplify EditorApplication extra tests to cover pressSpace’s QTimer lambda via direct timeout invocation and move destructor coverage elsewhere.
  • Replace QTest usage with QTimer/QMetaObject to find singleShot timers created by EditorApplication::pressSpace and invoke their timeout via meta-object.
  • Remove EditorApplication destructor coverage from this file and document that destructor is now tested in zz_ut_exit_destructor.cpp.
tests/src/ut_zz_editorapplication_extra.cpp
Add focused tests for WarningNotices button behavior and signals.
  • Add tests verifying setSaveAsBtn toggles save/reload button visibility.
  • Add tests that call slotreloadBtnClicked and slotsaveAsBtnClicked, connecting to reload/save signals and asserting they fire without fatal errors.
tests/src/controls/ut_warningnotices.cpp
Refactor StartManager lambda tests to avoid qWait-driven event processing and directly trigger singleShot timers.
  • Introduce SM_DeferredDeleteBlocker helper to describe blocking of DeferredDelete events (though primary change is direct timer invocation).
  • Update openFilesInTab and slotCloseWindow tests to iterate StartManager-owned singleShot QTimers and invoke their timeout via QMetaObject::invokeMethod, removing reliance on QTest and adding QThread include as needed.
tests/src/ut_startmanager_lambdas.cpp
Add a targeted LineBar test for the clear-button constructor lambda.
  • Add ClearButtonClickedLambda test that sets text, clicks m_clearButton synchronously, and asserts the line edit text is cleared.
tests/src/controls/ut_linebar.cpp
Update ThemePanel popup test to drive QPropertyAnimation-based lambda directly without processEvents.
  • Include QPropertyAnimation and rework popup test to create/show a ThemePanel, call popup, then find child QPropertyAnimation and emit valueChanged with current geometry to fire adjustScrollbarMargins lambda before deleting panel.
tests/src/thememodule/ut_themepanel.cpp
Update Window find/replace bar popup tests to trigger focus lambdas via direct timer timeout invocation.
  • Include QThread and replace QTest-driven event processing with loops over Window-owned singleShot QTimers, invoking timeout through QMetaObject::invokeMethod in popupFindBar and popupReplaceBar tests.
tests/src/widgets/ut_window.cpp
Ensure test coverage data flush and safer process termination in the unit-test main.
  • Declare extern "C" __gcov_dump(), include cstdlib/cstdio, and call __gcov_dump() after RUN_ALL_TESTS to flush coverage data.
  • Replace normal return with std::_Exit(c) and add fflush(nullptr) to force I/O flush, removing ASan report path handling.
  • Remove conditional __sanitizer_set_report_path block that previously ran after tests.
tests/src/ut_main.cpp

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

★ 总体评分:87分

■ 【总体评价】

代码通过直接触发定时器信号有效解决了单元测试中的事件积压与悬空指针问题,但存在未使用的冗余类定义和重复逻辑
逻辑基本正确但因存在死代码及代码重复扣13分

■ 【详细分析】

  • 1.语法逻辑(基本正确)✓

ut_textedit.cpp中定义了TE_DeferredDeleteBlocker类,ut_startmanager_lambdas.cpp中定义了SM_DeferredDeleteBlocker类,但在其所在的文件及后续测试用例中均未被实例化或安装为事件过滤器,属于无效的死代码。ut_textedit.cppeventFilter_ColorMarkMenuTabLambda测试中使用new QKeyEvent分配内存并在使用后手动delete,虽然eventFilter是同步调用不会导致double-free,但不符合Qt事件对象的常规栈分配惯例,增加了维护风险。
潜在问题:死代码会误导后续维护者认为事件过滤器已生效;手动管理QKeyEvent生命周期存在隐患。
建议:删除未使用的TE_DeferredDeleteBlockerSM_DeferredDeleteBlocker类定义;将QKeyEvent改为栈上分配QKeyEvent e(QEvent::KeyRelease, Qt::Key_Tab, Qt::NoModifier);

  • 2.代码质量(一般)✕

多个测试文件(ut_textedit.cpput_startmanager_lambdas.cpput_zz_editorapplication_extra.cpput_window.cpp)中高度重复使用相同的“遍历对象树寻找单次定时器并触发”的逻辑模式,违反了DRY(Don't Repeat Yourself)原则。一旦未来QTimer的触发机制或查找策略需要调整,需要修改多处分散的代码。
潜在问题:重复代码块降低测试代码的可维护性;未使用的类定义降低代码整洁度。
建议:提取一个公共的静态辅助函数,例如void triggerFirstSingleShotTimer(QObject* parent),放置在公共测试头文件中,供所有测试用例复用。

  • 3.代码性能(高效)✓

使用findChildren遍历对象树的时间复杂度为O(N),但在单元测试场景下对象树规模极小,性能开销可忽略不计。通过QMetaObject::invokeMethodQt::DirectConnection方式直接执行目标lambda,完全绕过了事件循环的排队与分发开销,相比于原先使用QTest::qWaitprocessEvents阻塞等待的方式,执行效率有显著提升。
建议:保持当前实现方式。

  • 4.代码安全(存在0个安全漏洞)✓

漏洞对比统计:新增漏洞 0 个,减少漏洞 0 个,持平 0 个
本次修改仅涉及单元测试代码的内部逻辑重构,不包含任何网络通信、文件系统操作、命令执行或动态内存越界访问等高风险操作。ut_main.cpp中引入的extern "C" void __gcov_dump(void);是GCC标准内建函数声明,用于安全地落盘覆盖率数据,配合std::_Exit(c)强制跳过全局析构,属于标准的测试生命周期管理手段,不存在安全风险。
建议:保持当前安全状态。

■ 【改进建议代码示例】

// 建议在公共测试头文件(如 test_utils.h)中提取重复的定时器触发逻辑
#ifndef TEST_UTILS_H
#define TEST_UTILS_H

#include <QObject>
#include <QTimer>
#include <QMetaObject>

namespace TestHelper {
// 提取公共方法:查找并触发对象树中的第一个单次定时器
inline void triggerFirstSingleShotTimer(QObject *parent) {
    for (QTimer *t : parent->findChildren<QTimer *>()) {
        if (t->isSingleShot()) {
            QMetaObject::invokeMethod(t, "timeout", Qt::DirectConnection);
            break;
        }
    }
}
} // namespace TestHelper

#endif // TEST_UTILS_H
// ut_textedit.cpp 中的修改示例
#include "test_utils.h"

// 删除未使用的 TE_DeferredDeleteBlocker 类

// highlight() internal QTimer::singleShot(0,...) lambda
TEST(UT_test_textedit_highlight, highlight_TriggersLambda)
{
    TextEdit *edit = new TextEdit;
    EditWrapper *wra = new EditWrapper;
    edit->m_wrapper = wra;

    edit->highlight();
    // 使用提取的公共方法替代重复代码
    TestHelper::triggerFirstSingleShotTimer(edit);

    edit->deleteLater();
    wra->deleteLater();
}

// eventFilter color mark menu Tab key: QTimer::singleShot(0,...) lambda
TEST(UT_test_textedit_eventFilter, eventFilter_ColorMarkMenuTabLambda)
{
    TextEdit *edit = new TextEdit;
    EditWrapper *wra = new EditWrapper;
    edit->m_wrapper = wra;

    edit->m_colorMarkMenu = new QMenu;
    // 修复:改为栈上分配,避免手动管理内存
    QKeyEvent e(QEvent::KeyRelease, Qt::Key_Tab, Qt::NoModifier);

    edit->eventFilter(edit->m_colorMarkMenu, &e);

    // 使用提取的公共方法替代重复代码
    TestHelper::triggerFirstSingleShotTimer(edit);

    delete edit->m_colorMarkMenu;
    edit->deleteLater();
    wra->deleteLater();
}

@lzwind
lzwind merged commit 34266ff into linuxdeepin:master Aug 12, 2026
20 checks passed
@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

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