Skip to content

feat: check Qt version w/o actually loading so file#74

Merged
BLumia merged 1 commit intolinuxdeepin:masterfrom
BLumia:no-dlopen
Nov 3, 2025
Merged

feat: check Qt version w/o actually loading so file#74
BLumia merged 1 commit intolinuxdeepin:masterfrom
BLumia:no-dlopen

Conversation

@BLumia
Copy link
Member

@BLumia BLumia commented Oct 31, 2025

历史代码中,会尝试使用 dlopen() 装载 Qt 插件文件,调用 qVersion 获取插件版本信息,并 dlclose() 卸载插件文件. 这在 Qt 6.8.2+ 会导致崩溃在 QCoreApplicationPrivate::sendThroughApplicationEvent, 由 qt/qtbase@2b3e56e 所致, 具体发生在 dlclose() (上一个提交并未添加 QLibrary::unload 所以不会触发崩溃).

此处的目的仅仅是检查插件版本,而 dlopen() 本身即可能会执行一些代码,导致污染当前线程,故转为使用解析 so 文件的 ELF 头的 .qtversion section 来解析 Qt 版本号,并优先使用此版本号进行判断.

此修改还增加了一个命令行选项用来验证 so 文件的 Qt 版本解析,以供临时调试类场景使用.

历史代码中,会尝试使用 dlopen() 装载 Qt 插件文件,调用 qVersion 获取
插件版本信息,并 dlclose() 卸载插件文件. 这在 Qt 6.8.2+ 会导致崩溃
在 QCoreApplicationPrivate::sendThroughApplicationEvent, 由 qtbase
的 2b3e56e2b4198a940bb811964d0342868c391858 所致.

此处的目的仅仅是检查插件版本,而 dlopen() 本身即可能会执行一些代码,
导致污染当前线程,故转为使用解析 so 文件的 ELF 头的 .qtversion section
来解析 Qt 版本号,并优先使用此版本号进行判断.

此修改还增加了一个命令行选项用来验证 so 文件的 Qt 版本解析,以供临时
调试类场景使用.

Log:
@BLumia BLumia requested review from 18202781743 and mhduiy October 31, 2025 08:02
@deepin-ci-robot
Copy link

deepin pr auto review

我来对这个代码变更进行详细审查:

  1. 语法逻辑:
  • 代码语法正确,CMakeLists.txt和C++代码的语法都符合规范
  • ELF头解析部分的逻辑清晰,通过检查ELF魔数、节头表等步骤来读取Qt版本信息
  • 条件编译宏的使用合理,确保了代码的可移植性
  1. 代码质量:
  • 新增了DISABLE_ELF_QT_CHECK选项,提高了灵活性
  • qtVersionFromSo函数有详细的注释,解释了ELF解析过程
  • 代码结构清晰,将ELF解析逻辑封装在单独的函数中
  • 错误处理完善,包含了文件打开失败、读取失败等场景
  1. 代码性能:
  • 通过直接读取ELF头获取Qt版本,避免了加载动态库的开销
  • 使用QFile进行文件操作,效率较高
  • 内存使用合理,使用QVector和QByteArray管理内存
  1. 代码安全:
  • 文件操作前检查文件是否可打开
  • 使用memcpy时确保了缓冲区大小足够
  • ELF头解析时验证魔数,防止解析无效文件
  • 使用条件编译确保在不支持ELF的系统上不会执行相关代码

改进建议:

  1. 错误处理可以更详细:
if (!f.open(QIODevice::ReadOnly)) {
    qWarning() << "Cannot open file:" << path << f.errorString();
    return {};
}
  1. 可以添加对32位ELF的完整支持:
#ifdef ELF_QT_VER_CHECK
    // 添加对32位ELF的支持
    if (is64) {
        // 64位处理
    } else {
        // 32位处理
    }
#endif
  1. 可以添加对文件大小的检查:
if (f.size() < sizeof(Elf64_Ehdr)) {
    qWarning() << "File too small:" << path;
    return {};
}
  1. 可以添加对Qt版本格式的验证:
if (major == 0 || minor > 255 || patch > 255) {
    qWarning() << "Invalid Qt version format:" << major << minor << patch;
    return {};
}
  1. 在utils.h中可以添加函数文档:
/**
 * @brief Parse Qt version from ELF header of a shared library
 * @param path Path to the shared library file
 * @return Qt version number if successful, null version number otherwise
 */
QVersionNumber qtVersionFromSo(const QString &path);

总体来说,这是一个很好的改进,通过直接读取ELF头来获取Qt版本信息,避免了动态库加载的开销,同时提供了良好的可配置性和错误处理。


bool checkLibraryQtVersion(const QString &soPath)
{
QVersionNumber libraryQtVersion = qtVersionFromSo(soPath);
Copy link
Contributor

@18202781743 18202781743 Oct 31, 2025

Choose a reason for hiding this comment

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

这里需要检查qt版本的so,是我们自己插件的库么?如果这样的话,qt本身不需要加载so就能够获取metadata,自定义的metadata有version,

}

QLibrary library(soPath);
if (!library.load()) {
Copy link
Contributor

@18202781743 18202781743 Oct 31, 2025

Choose a reason for hiding this comment

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

这里不加载,应该也能拿到metadata,里面就有version
image

Copy link
Member Author

Choose a reason for hiding this comment

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

不能,这个得走QPluginLoader::metaData,然后我实际尝试的情况是:

Failed to find metadata in lib /usr/lib64/deepin-service-manager/libplugin-dde-appearance.so: '/usr/lib64/deepin-service-manager/libplugin-dde-appearance.so' is not a Qt plugin (metadata not found)

@deepin-ci-robot
Copy link

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: 18202781743, BLumia

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

@BLumia BLumia merged commit 4073e42 into linuxdeepin:master Nov 3, 2025
15 of 17 checks passed
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