我们的团队会不时地重新检查我们已经写过的项目。 另一个经过重新检查的项目是Qt。 我们上一次在2014年用PVS-Studio测试它。 自2014年以来,在Coverity的帮助下开始定期检查该项目。 这很有趣。 让我们看看是否现在可以使用PVS-Studio查找任何有趣的错误。
t
以前的文章:
这次测试了
Qt Base (Core,Gui,Widgets,Network等)和
Qt5超级模块 。 关于Qt Creator,我们计划稍后再写一篇单独的文章。 为了进行验证,我们使用了PVS-Studio静态分析仪,您可以从网站上
下载其试用版。
在我看来,Qt代码已经变得更好。 自上次测试以来,多年来,PVS-Studio分析仪中出现了许多新的诊断方法。 尽管如此,在对警告的回顾研究中,我发现这个规模的项目没有太多错误。 我再说一遍,这是我个人的印象。 我那时或现在都没有对错误的密度做任何特殊的研究。
最有可能的是,使用Coverity静态分析器进行的定期检查很可能会影响代码的质量。 2014年,在Coverity的帮助下,开始检查Qt项目(
qt-project ),并在2016年检查了Qt Creator(
qt-creator )。 我的看法:如果您正在开发一个开源项目,那么
Coverity Scan可以是一个很好的免费解决方案,它将显着提高项目的质量和可靠性。
但是,正如读者可以猜测的那样,如果我没有在PVS-Studio报告中注意到任何有趣的事情,那么就不会有文章了:)。 既然有一篇文章,那就是缺陷。 让我们看看它们。 总共我写出96个错误。
复制粘贴和错字不成功
让我们从流派的经典开始,当错误的原因是注意力不集中时。 这些错误被程序员低估了。 对于那些尚未阅读的人,建议您阅读以下两篇文章:
这些错误是中间语言。 例如,第二篇文章提供了许多用C,C ++和C#编写的比较函数中的错误示例。 现在,在PVS-Studio中实现Java语言支持时,我们会遇到相同的错误模式。 例如,这是我们最近在
Hibernate库中发现的错误:
public boolean equals(Object other) { if (other instanceof Id) { Id that = (Id) other; return purchaseSequence.equals(this.purchaseSequence) && that.purchaseNumber == this.purchaseNumber; } else { return false; } }
如果仔细观察,结果是将
purchaseSequence字段与其自身进行了比较。 正确的选项:
return that.purchaseSequence.equals(this.purchaseSequence) && that.purchaseNumber == this.purchaseNumber;
总的来说,一切都一如既往,PVS-Studio分析仪将不得不在Java项目中“夺取Augean马“”。 顺便说一下,我们邀请所有人参加测试PVS-Studio for Java的Beta版,该版本将在不久的将来出现。 为此,请
写信给我们 (选择“我想要Java的分析器”)。
现在回到Qt项目中的错误。
缺陷N1 static inline int windowDpiAwareness(HWND hwnd) { return QWindowsContext::user32dll.getWindowDpiAwarenessContext && QWindowsContext::user32dll.getWindowDpiAwarenessContext ? QWindowsContext::user32dll.getAwarenessFromDpiAwarenessContext( QWindowsContext::user32dll.getWindowDpiAwarenessContext(hwnd)) : -1; }
PVS-Studio警告:V501 CWE-571在'&&'运算符的左侧和右侧有相同的子表达式'QWindowsContext :: user32dll.getWindowDpiAwarenessContext'。 qwindowscontext.cpp 150
除了分析仪消息外,此处不需要任何特殊说明。 在我看来,该表达式应该像这样:
return QWindowsContext::user32dll.getAwarenessFromDpiAwarenessContext && QWindowsContext::user32dll.getWindowDpiAwarenessContext ? QWindowsContext::user32dll.getAwarenessFromDpiAwarenessContext( QWindowsContext::user32dll.getWindowDpiAwarenessContext(hwnd)) : -1;
缺陷N2,N3 void QReadWriteLockPrivate::release() { Q_ASSERT(!recursive); Q_ASSERT(!waitingReaders && !waitingReaders && !readerCount && !writerCount); freelist->release(id); }
PVS-Studio警告:V501 CWE-571'&&'运算符的左侧和右侧有相同的子表达式:!WaitingReaders && !! WaitingReaders qreadwritelock.cpp 632
该错误在
Q_ASSERT宏
条件内部,因此不重要。 但这仍然是一个错误。
仔细检查了
waitingReaders变量。 显然他们忘记了检查其他变量。
在qreadwritelock.cpp文件的625行中发现了相同的错误。 复制粘贴万岁! :)
缺陷N4 QString QGraphicsSceneBspTree::debug(int index) const { .... if (node->type == Node::Horizontal) { tmp += debug(firstChildIndex(index)); tmp += debug(firstChildIndex(index) + 1); } else { tmp += debug(firstChildIndex(index)); tmp += debug(firstChildIndex(index) + 1); } .... }
PVS-Studio警告:V523 CWE-691'then'语句等效于'else'语句。 qgraphicsscene_bsp.cpp 179
最有可能复制了该文本块,但他们忘记了对其进行更正。
缺陷N5 enum FillRule { OddEvenFill, WindingFill }; QDataStream &operator>>(QDataStream &s, QPainterPath &p) { .... int fillRule; s >> fillRule; Q_ASSERT(fillRule == Qt::OddEvenFill || Qt::WindingFill); .... }
PVS-Studio警告:V768 CWE-571枚举常量'WindingFill'用作布尔型变量。 qpainterpath.cpp 2479
同意,这是一个美丽的家伙!
Q_ASSERT不检查任何内容,因为条件始终为true。 条件成立,因为命名常量
Qt :: WindingFill为1。
缺陷N6 bool QVariant::canConvert(int targetTypeId) const { .... if (currentType == QMetaType::SChar || currentType == QMetaType::Char) currentType = QMetaType::UInt; if (targetTypeId == QMetaType::SChar || currentType == QMetaType::Char) targetTypeId = QMetaType::UInt; .... }
阅读警告之前,请尝试自己找出一个错字。 通过添加图片,我将帮助您不要立即阅读分析器消息:)。
PVS-Studio警告:V560 CWE-570条件表达式的一部分始终为false:currentType == QMetaType :: Char。 qvariant.cpp 3529
在条件
if中首先检查条件“ currentType == QMetaType :: Char”。 如果满足条件,则为变量
currentType分配值
QMetaType :: UInt 。 因此,变量
currentType不再可以等于
QMetaType :: Char 。 因此,分析器报告第二个子表达式“ currentType == QMetaType :: Char”始终为假。
实际上,第二个
if应该是这样的:
if (targetTypeId == QMetaType::SChar || targetTypeId == QMetaType::Char) targetTypeId = QMetaType::UInt;
V560诊断说明该报告发现了许多V560警告。 但是,一旦发现这篇文章的有趣案例,我就不再查看它们了,该案例在上面被认为是缺陷N6。
绝大多数消息V560不能被称为假,但是它们没有用。 换句话说,在文章中描述它们并不有趣。 为了弄清楚我到底是什么意思,请考虑一种这样的情况。
QString QTextHtmlExporter::findUrlForImage(const QTextDocument *doc, ....) { QString url; if (!doc) return url; if (QTextDocument *parent = qobject_cast<QTextDocument *>(doc->parent())) return findUrlForImage(parent, cacheKey, isPixmap); if (doc && doc->docHandle()) {
警告PVS-Stuidio:V560 CWE-571条件表达式的一部分始终为true:doc。 qtextdocument.cpp 2992
分析器是绝对正确的,它在重新检查时
doc指针并不总是为
nullptr 。 但这不是一个错误,只是程序员是安全的。 您可以通过编写以下代码来简化代码:
if (doc->docHandle()) {
缺陷N7最后一种情况,可以归类为错别字。 出现错误是由于常量名称混乱,仅在首字母不同的情况下。
class QWindowsCursor : public QPlatformCursor { public: enum CursorState { CursorShowing, CursorHidden, CursorSuppressed }; .... } QWindowsCursor::CursorState QWindowsCursor::cursorState() { enum { cursorShowing = 0x1, cursorSuppressed = 0x2 }; CURSORINFO cursorInfo; cursorInfo.cbSize = sizeof(CURSORINFO); if (GetCursorInfo(&cursorInfo)) { if (cursorInfo.flags & CursorShowing) .... }
PVS-Studio警告:V616 CWE-480在按位操作中使用名为“ CursorShowing”的常量,值为0。 qwindowscursor.cpp 669
更详细地讲,我已经在一个单独的小笔记中分析了此错误:“
再次证明,PVS-Studio分析仪比
人更专心 。”
安全漏洞
实际上,本文中讨论的所有错误都可以称为安全缺陷。 它们均根据
常见弱点枚举进行分类(请参阅分析器消息中的CWE ID)。 如果将错误分类为CWE,则可能存在安全风险。 在
PVS-Studio SAST页上对此进行了详细说明。
但是,我想将一些错误归为一个单独的小组。 让我们看看它们。
缺陷N8,N9 bool QLocalServerPrivate::addListener() { .... SetSecurityDescriptorOwner(pSD.data(), pTokenUser->User.Sid, FALSE); SetSecurityDescriptorGroup(pSD.data(), pTokenGroup->PrimaryGroup, FALSE); .... }
PVS-Studio警告:
- V530 CWE-252需要使用函数'SetSecurityDescriptorOwner'的返回值。 qlocalserver_win.cpp 167
- V530 CWE-252需要使用功能'SetSecurityDescriptorGroup'的返回值。 qlocalserver_win.cpp 168
有与访问控制有关的各种功能。 函数
SetSecurityDescriptorOwner和
SetSecurityDescriptorGroup就在其中。
使用此类功能,您需要非常小心地工作。 例如,您必须检查它们返回的状态。 如果对这些函数的调用失败,会发生什么? 猜测不是必需的,有必要编写代码来处理这种情况。
不必利用缺乏验证的优势并将此类错误转化为漏洞。 但是,在任何情况下都不存在风险,您需要编写更安全的代码。
缺陷N10 bool QLocalServerPrivate::addListener() { .... InitializeAcl(acl, aclSize, ACL_REVISION_DS); .... }
PVS-Studio警告:V530 CWE-252需要使用函数“ InitializeAcl”的返回值。 qlocalserver_win.cpp 144
这种情况类似于上面讨论的情况。
缺陷N11,N12 static inline void sha1ProcessChunk(....) { .... quint8 chunkBuffer[64]; .... #ifdef SHA1_WIPE_VARIABLES .... memset(chunkBuffer, 0, 64); #endif }
PVS-Studio警告:V597 CWE-14编译器可能会删除“ memset”函数调用,该函数调用用于刷新“ chunkBuffer”缓冲区。 RtlSecureZeroMemory()函数应用于擦除私有数据。 第189章
编译器将删除
memset函数调用。 我已经在文章中多次分析了这种情况。 我不想重复自己。 我指的是“
安全清除私有数据 ”一文。
另一个错误是在同一个sha1.cpp文件中,在第247行。
空指针
现在该谈论指针了。 在这个话题上有很多错误。
缺陷N13 QByteArray &QByteArray::append(const char *str, int len) { if (len < 0) len = qstrlen(str); if (str && len) { .... }
PVS-Studio警告:V595 CWE-476在针对nullptr进行验证之前,已使用了'str'指针。 检查行:2118,2119。qbytearray.cpp 2118
典型的情况是,在开始时使用指针,然后检查
nullptr是否相等。 这是一个非常常见的错误模式,我们几乎在所有项目中都经常
看到它。
缺陷N14,N15 static inline const QMetaObjectPrivate *priv(const uint* data) { return reinterpret_cast<const QMetaObjectPrivate*>(data); } bool QMetaEnum::isFlag() const { const int offset = priv(mobj->d.data)->revision >= 8 ? 2 : 1; return mobj && mobj->d.data[handle + offset] & EnumIsFlag; }
PVS-Studio警告:V595 CWE-476在针对nullptr进行验证之前,已使用了'mobj'指针。 检查行:2671,2672。qmetaobject.cpp 2671
以防万一,我带来了
priv函数的主体。 由于某些原因,有时读者会想出代码可以工作的情况。 我不知道这种不信任来自何处,也不希望看到错误的棘手功能:)。 例如,某人可能在评论中建议
priv是以下形式的宏:
#define priv(A) foo(sizeof(A))
然后一切都会正常。
为了避免此类讨论,我尝试引用代码片段,其中提供了所有确认错误存在的信息。
因此,
modj指针
被取消引用,然后检查。
现场还出现了“强大而可怕的”复制粘贴。 由于在
isScoped函数中检测到完全相同的错误,因此:
bool QMetaEnum::isScoped() const { const int offset = priv(mobj->d.data)->revision >= 8 ? 2 : 1; return mobj && mobj->d.data[handle + offset] & EnumIsScoped; }
PVS-Studio警告:V595 CWE-476在针对nullptr进行验证之前,已使用了'mobj'指针。 检查行:2683、2684。qmetaobject.cpp 2683
缺陷N16-N21考虑另一个例子,我认为足够了。
void QTextCursor::insertFragment(const QTextDocumentFragment &fragment) { if (!d || !d->priv || fragment.isEmpty()) return; d->priv->beginEditBlock(); d->remove(); fragment.d->insert(*this); d->priv->endEditBlock(); if (fragment.d && fragment.d->doc) d->priv->mergeCachedResources(fragment.d->doc->docHandle()); }
PVS-Studio警告:V595 CWE-476在对nullptr进行验证之前,已使用了'fragment.d'指针。 检查行:2238、2241。qtextcursor.cpp 2238
都一样 注意存储在变量
fragment.d中的指针的工作顺序。
此类型的其他错误:
- V595 CWE-476在针对nullptr验证之前使用了“窗口”指针。 检查行:1846、1848。qapplication.cpp 1846
- V595 CWE-476在针对nullptr验证之前使用了“窗口”指针。 检查行:1858、1860。qapplication.cpp 1858
- V595 CWE-476在针对nullptr进行验证之前,已使用了“答复”指针。 检查行:492、502。qhttpnetworkconnectionchannel.cpp 492
- V595 CWE-476在针对nullptr对其进行验证之前,已使用了'newHandle'指针。 检查行:877,883。qsplitter.cpp 877
- V595 CWE-476在针对nullptr对其进行验证之前,已使用了“小部件”指针。 检查行:2320、2322。qwindowsvistastyle.cpp 2320
- 实际上,还有更多错误。 我很快就厌倦了学习V595警告,对于这篇文章,我已经写出了足够的代码片段。
缺陷N22-N33在代码中检查了
新操作符返回的指针。 在很多地方不检查
malloc函数结果的情况下,这尤其有趣(请参阅以下错误组)。
bool QTranslatorPrivate::do_load(const QString &realname, const QString &directory) { .... d->unmapPointer = new char[d->unmapLength]; if (d->unmapPointer) { file.seek(0); qint64 readResult = file.read(d->unmapPointer, d->unmapLength); if (readResult == qint64(unmapLength)) ok = true; } .... }
PVS-Studio警告:V668 CWE-571对'd-> unmap Pointer'指针针对null进行测试没有任何意义,因为使用'new'运算符分配了内存。 如果内存分配错误,将生成异常。 qtranslator.cpp 596
检查指针没有意义,因为在发生内存分配错误的情况下,将
引发异常
std :: bad_alloc 。 如果要让
新运算符在没有足够内存的情况下返回
nullptr ,则应编写:
d->unmapPointer = new (std::nothrow) char[d->unmapLength];
分析器知道使用
新运算符的情况,在这种情况下不会发出警告。
其他错误:我将给他们
qt-V668.txt文件 。
缺陷N34-N70如所承诺的,现在轮到它们不检查调用
malloc ,
calloc ,
strdup等函数的结果时了。 这些错误比乍看之下要严重得多。 更多详细信息:“
检查malloc函数返回什么为什么很重要 。”
SourceFiles::SourceFiles() { nodes = (SourceFileNode**)malloc(sizeof(SourceFileNode*)*(num_nodes=3037)); for(int n = 0; n < num_nodes; n++) nodes[n] = nullptr; }
PVS-Studio警告:V522 CWE-690可能会取消引用潜在的空指针“节点”。 检查行:138,136。makefiledeps.cpp 138
该指针无需事先验证即可使用。
所有这些错误属于同一类型,因此我将不对其进行详细介绍。 我将给出其他警告列表:
qt-V522-V575.txt 。
条件中的逻辑错误
缺陷N71 QString QEdidParser::parseEdidString(const quint8 *data) { QByteArray buffer(reinterpret_cast<const char *>(data), 13);
PVS-Studio警告:V547 CWE-570表达式'buffer [i] <'\ 040'&& buffer [i]>'\ 176'始终为假。 qedidparser.cpp 169
该功能必须执行以下操作“用破折号替换不可打印的字符”。 但是,她没有。 让我们仔细看看这种情况:
if (buffer[i] < '\040' && buffer[i] > '\176')
这没有道理。 字符不能同时小于“ \ 040”并大于“ \ 176”。 在这种情况下,必须使用运算符“ ||”。 正确的代码是:
if (buffer[i] < '\040' || buffer[i] > '\176')
缺陷N72由于Windows用户不走运,出现了类似的错误。
#if defined(Q_OS_WIN) static QString driveSpec(const QString &path) { if (path.size() < 2) return QString(); char c = path.at(0).toLatin1(); if (c < 'a' && c > 'z' && c < 'A' && c > 'Z') return QString(); if (path.at(1).toLatin1() != ':') return QString(); return path.mid(0, 2); } #endif
分析仪立即生成两个警告:
- V590 CWE-571考虑检查'c <'a'&& c>'z'&& c <'A'&& c>'Z''表达式。 表达式过多或打印错误。 qdir.cpp 77
- V560 CWE-570条件表达式的一部分始终为false:c>'z'。 qdir.cpp 77
逻辑错误处于以下情况:
if (c < 'a' && c > 'z' && c < 'A' && c > 'Z')
据我了解,程序员希望找到一个不是拉丁字母的字符。 在这种情况下,条件应如下所示:
if ((c < 'a' || c > 'z') && (c < 'A' || c > 'Z'))
缺陷N73 enum SelectionMode { NoSelection, SingleSelection, MultiSelection, ExtendedSelection, ContiguousSelection }; void QAccessibleTableCell::unselectCell() { QAbstractItemView::SelectionMode selectionMode = view->selectionMode(); if (!m_index.isValid() || (selectionMode & QAbstractItemView::NoSelection)) return; .... }
PVS-Studio警告:V616 CWE-480在按位运算中使用名为“ QAbstractItemView :: NoSelection”的常量(值为0)。 itemviews.cpp 976
命名常量
QAbstractItemView :: NoSelection为零。 因此,子表达式
(selectionMode和QAbstractItemView :: NoSelection)没有意义。 永远为0。
我认为应该在这里写:
if (!m_index.isValid() || (selectionMode == QAbstractItemView::NoSelection))
缺陷N74以下代码使我难以理解。 他错了,但我不知道他应该是什么。 对函数进行注释也无济于事。
PVS-Studio警告:V547 CWE-570表达式“消息”始终为假。 qwindowscontext.cpp 802
程序员可能假设
FormatMessage函数将更改
消息指针的值。 但是事实并非如此。
FormatMessage函数不能更改指针的值,因为它是按值传递给函数的。 这是此函数的原型:
DWORD __stdcall FormatMessageW( DWORD dwFlags, LPCVOID lpSource, DWORD dwMessageId, DWORD dwLanguageId, LPWSTR lpBuffer, DWORD nSize, va_list *Arguments );
潜在的内存泄漏
缺陷N75-N92 struct SourceDependChildren { SourceFile **children; int num_nodes, used_nodes; SourceDependChildren() : children(nullptr), num_nodes(0), used_nodes(0) { } ~SourceDependChildren() { if (children) free(children); children = nullptr; } void addChild(SourceFile *s) { if(num_nodes <= used_nodes) { num_nodes += 200; children = (SourceFile**)realloc(children, sizeof(SourceFile*)*(num_nodes)); } children[used_nodes++] = s; } };
PVS-Studio警告:V701 CWE-401 realloc()可能泄漏:当realloc()分配内存失败时,原始指针'children'丢失。 考虑将realloc()分配给一个临时指针。 makefiledeps.cpp 103
缓冲区扩展以危险的方式实现。 如果
realloc函数无法分配内存,它将返回
NULL 。 此
NULL将立即放置在
children变量中,并且不可能以某种方式释放较早分配的缓冲区。 将会发生内存泄漏。
类似错误:
qt-701.txt 。
杂项
缺陷N93 template<class GradientBase, typename BlendType> static inline const BlendType * QT_FASTCALL qt_fetch_linear_gradient_template(....) { .... if (t+inc*length < qreal(INT_MAX >> (FIXPT_BITS + 1)) && t+inc*length > qreal(INT_MIN >> (FIXPT_BITS + 1))) { .... }
PVS-Studio警告:V610 CWE-758未指定的行为。 检查移位运算符“ >>”。 左操作数'(-2147483647-1)'为负。 qdrawhelper.cpp 4015
INT_MIN的负值不能移位。 这是未指定的行为,因此您不能依赖此操作的结果。 最高有效位可以等于0或1。
缺陷N94 void QObjectPrivate::addConnection(int signal, Connection *c) { .... if (signal >= connectionLists->count()) connectionLists->resize(signal + 1); ConnectionList &connectionList = (*connectionLists)[signal]; .... if (signal < 0) { .... }
PVS-Studio警告:V781 CWE-129使用后,将检查“信号”变量的值。 程序逻辑中可能有一个错误。 检查行:397,413。qobject.cpp 397
校验
(信号<0)表示
信号自变量的值可能为负。 但是,此参数以前曾用于索引数组。 事实证明,检查执行得太迟了。 该程序将被破坏。
缺陷N95 bool QXmlStreamWriterPrivate::finishStartElement(bool contents) { .... if (inEmptyElement) { write("/>"); QXmlStreamWriterPrivate::Tag &tag = tagStack_pop(); lastNamespaceDeclaration = tag.namespaceDeclarationsSize; lastWasStartElement = false; } else { write(">"); } inStartElement = inEmptyElement = false; lastNamespaceDeclaration = namespaceDeclarations.size(); return hadSomethingWritten; }
PVS-Studio警告:V519 CWE-563'lastNamespaceDeclaration'变量已连续两次分配值。 也许这是一个错误。 检查行:3188、3194。qxmlstream.cpp 3194
我将强调错误的实质:
if (inEmptyElement) { lastNamespaceDeclaration = tag.namespaceDeclarationsSize; } lastNamespaceDeclaration = namespaceDeclarations.size();
缺陷N96 void QRollEffect::scroll() { .... if (currentHeight != totalHeight) { currentHeight = totalHeight * (elapsed/duration) + (2 * totalHeight * (elapsed%duration) + duration) / (2 * duration);
V519 CWE-563“完成”变量连续两次分配了值。 也许这是一个错误。 检查行:509、511。qeffects.cpp 511
一切与前面的情况相同。 注意
完成变量。
结论
即使从表面上浏览报告,我也写出了近100个错误。 我对PVS-Studio的结果感到满意。
当然,这种罕见的代码检查与提高代码的质量和可靠性无关。 它们仅演示代码分析器的功能。 静态分析工具应定期使用。 在这种情况下,它们减少了修复错误的成本,并保护了应用程序免受许多潜在漏洞的侵害。
谢谢您的关注。 为了与我们的新出版物保持同步,我邀请您订阅我们的一种渠道:
- VK.com:pvsstudio_rus
- “老派” RSS: viva64-blog-ru
- 推特: @pvsstudio_rus
- Instagram的: @pvsstudio_rus
- 电报: @pvsstudio_rus

如果您想与说英语的读者分享这篇文章,请使用以下链接:Andrey Karpov。
使用PVS-Studio对Qt 5进行第三次检查