在Docker容器中使用PVS-Studio验证LibrePCB项目

PVS-Studio和Docker容器

这是一篇有关我们团队如何使用PVS-Studio静态代码分析器测试开放LibrePCB项目的经典文章。 但是,这篇文章很有趣,因为验证是在Docker容器内进行的。 如果您使用容器,我们希望本文将演示将分析器集成到开发过程中的另一种简单方法。

LibrePCB


LibrePCB是用于设计电子电路和印刷电路板的免费软件。 程序代码是用C ++编写的,并且Qt5用于构建图形界面。 最近,该应用程序首次正式发布,标志着其自身文件格式(* .lp,* .lplib)的稳定。 为Linux,macOS和Windows准备的二进制软件包。

LibrePCB


LibrePCB是一个小型项目,仅包含约30万行用C和C ++编写的非空代码。 同时,25%的非空行是注释。 顺便说一句,这是很大一部分评论。 最有可能的原因是,该项目有许多小文件,其中很大一部分被项目信息和许可证的标题注释所占据。 GitHub站点代码: LibrePCB

该项目对我们来说似乎很有趣,因此我们决定进行检查。 但是测试结果不再那么有趣。 是的,有真正的错误。 但是没有什么特别的,我们当然必须告诉我们文章的读者。 也许我们不会写一篇文章,而只限于将错误发送给项目的开发人员。 但是,有趣的是该项目已在Docker映像中进行了测试,因此我们认为仍然值得写一篇文章。

码头工人


Docker-用于在操作系统级别自动化虚拟化环境中应用程序的部署和管理的软件。 它允许您将应用程序及其所有周围环境和依赖项“打包”到容器中。 尽管该技术已经使用了大约五年,并且许多公司已经在其项目基础架构中长期实施了Docker,但是在开源世界中,直到最近它才引起人们的注意。

我们公司与开源项目紧密合作,使用我们自己的PVS-Studio静态分析器检查源代码。 目前,已经验证了300多个项目 。 此活动中最困难的事情始终是其他人的项目的编译,但是Docker容器的引入极大地简化了此过程。

在Docker中测试开源项目的最初经验是使用Azure Service Fabric 。 在那里,开发人员将源文件目录安装到了容器中,并且分析器集成仅限于编辑在容器中运行的脚本之一:

diff --git a/src/build.sh b/src/build.sh index 290c57d..2a286dc 100755 --- a/src/build.sh +++ b/src/build.sh @@ -193,6 +193,9 @@ BuildDir() cd ${ProjBinRoot}/build.${DirName} + pvs-studio-analyzer analyze --cfg /src/PVS-Studio.cfg \ + -o ./service-fabric-pvs.log -j4 + if [ "false" = ${SkipBuild} ]; then if (( $NumProc <= 0 )); then NumProc=$(($(getconf _NPROCESSORS_ONLN)+0)) 

LibrePCB项目之间的区别在于,他们立即提供了用于构建映像的Dockerfile和其中的项目。 事实证明,这对我们来说更加方便。 这是我们感兴趣的Docker文件的一部分:

 FROM ubuntu:14.04 # install packages RUN DEBIAN_FRONTEND=noninteractive \ apt-get -q update \ && apt-get -qy upgrade \ && apt-get -qy install git g++ qt5-default qttools5-dev-tools qt5-doc \ qtcreator libglu1-mesa-dev dia \ && apt-get clean # checkout librepcb RUN git clone --recursive https://..../LibrePCB.git /opt/LibrePCB \ && cd /opt/LibrePCB .... # build and install librepcb RUN /opt/LibrePCB/dev/docker/make_librepcb.sh .... 

组装映像时,我们不会编译和安装项目。 因此,我们收集了一个图像,其中项目的作者保证了项目的成功组装。

启动容器后,将安装分析器,并执行以下命令来构建和分析项目:

 cd /opt/LibrePCB mkdir build && cd build qmake -r ../librepcb.pro pvs-studio-analyzer trace -- make -j2 pvs-studio-analyzer analyze -l /mnt/Share/PVS-Studio.lic -r /opt/LibrePCB \ -o /opt/LibrePCB/LibrePCB.log -v -j4 cp -R -L -a /opt/LibrePCB /mnt/Share 

顺便说一下,所有操作都是在Windows 10中执行的。非常令人高兴的是,所有流行的操作系统的开发人员也都朝着这个方向发展。 不幸的是,带有Windows的容器不太方便。 特别是由于不可能也容易安装软件。

发现的错误


现在是经典部分,其中包含对我们使用PVS-Studio静态代码分析器发现的错误的描述。 顺便说一句,借此机会,我想提醒您,最近我们正在开发一种分析仪,以支持嵌入式系统的代码分析。 以下是一些我们的读者可能跳过的几篇文章:

  1. PVS-Studio包括对GNU Arm嵌入式工具链的支持
  2. PVS-Studio:支持编码标准MISRA C和MISRA C ++

错别字


 SymbolPreviewGraphicsItem::SymbolPreviewGraphicsItem( const IF_GraphicsLayerProvider& layerProvider, const QStringList& localeOrder, const Symbol& symbol, const Component* cmp, const tl::optional<Uuid>& symbVarUuid, const tl::optional<Uuid>& symbVarItemUuid) noexcept { if (mComponent && symbVarUuid && symbVarItemUuid) .... if (mComponent && symbVarItemUuid && symbVarItemUuid) // <= .... } 

PVS-Studio警告: V501 CWE-571“ &&”运算符的左侧和右侧都有相同的子表达式“ symbVarItemUuid”。 symbolpreviewgraphicsitem.cpp 74

经典错字:连续两次检查变量symbVarItemUuid 。 上面有类似的检查,并且看一下,很显然,要检查的第二个变量应该是symbVarUuid

以下错字代码段:

 void Clipper::DoMaxima(TEdge *e) { .... if (e->OutIdx >= 0) { AddOutPt(e, e->Top); e->OutIdx = Unassigned; } DeleteFromAEL(e); if (eMaxPair->OutIdx >= 0) { AddOutPt(eMaxPair, e->Top); // <= eMaxPair->OutIdx = Unassigned; } DeleteFromAEL(eMaxPair); .... } 

PVS-Studio警告: V778 CWE-682发现了两个相似的代码片段。 也许这是一个错字,应该使用“ eMaxPair”变量而不是“ e”。 clipper.cpp 2999

该代码很可能是使用复制粘贴编写的。 由于第二段文本的疏忽,他们忘记了用eMaxPair-> Top替换e- > Top

额外检查


 static int rndr_emphasis(hoedown_buffer *ob, const hoedown_buffer *content, const hoedown_renderer_data *data) { if (!content || !content->size) return 0; HOEDOWN_BUFPUTSL(ob, "<em>"); if (content) hoedown_buffer_put(ob, content->data, content->size); HOEDOWN_BUFPUTSL(ob, "</em>"); return 1; } 

PVS-Studio警告: V547 CWE-571表达式“内容”始终为真。 html.c 162

这可能仍然不是错误,而只是冗余代码。 无需重新检查内容指针。 如果为零,则该函数立即完成其工作。

类似的情况:

 void Clipper::DoMaxima(TEdge *e) { .... else if( e->OutIdx >= 0 && eMaxPair->OutIdx >= 0 ) { if (e->OutIdx >= 0) AddLocalMaxPoly(e, eMaxPair, e->Top); DeleteFromAEL(e); DeleteFromAEL(eMaxPair); } .... } 

PVS-Studio 警告V547 CWE-571表达式'e-> OutIdx> = 0'始终为true。 第2983章

重新检查(e-> OutIdx> = 0)没有意义。 但是,也许这是一个错误。 例如,我们假设应该检查变量e-> Top 。 但是,这只是预感。 我们不熟悉项目代码,因此无法区分错误和冗余代码:)。

还有另一种情况:

 QString SExpression::toString(int indent) const { .... if (child.isLineBreak() && nextChildIsLineBreak) { if (child.isLineBreak() && (i > 0) && mChildren.at(i - 1).isLineBreak()) { // too many line breaks ;) } else { str += '\n'; } } .... } 

PVS-Studio警告: V571 CWE-571定期检查。 “ child.isLineBreak()”条件已在第208行中进行了验证。sexpression.cpp 209

逻辑错误


 void FootprintPreviewGraphicsItem::paint(....) noexcept { .... for (const Circle& circle : mFootprint.getCircles()) { layer = mLayerProvider.getLayer(*circle.getLayerName()); if (!layer) continue; // <= if (layer) { // <= pen = QPen(....); painter->setPen(pen); } else painter->setPen(Qt::NoPen); .... } .... } 

PVS-Studio警告: V547 CWE-571表达式“图层”始终为真。 足迹previewgraphicsitem.cpp 177

由于第二个if语句中的条件始终为true,因此永远不会满足else分支。

忘记了指针检查


 extern int ZEXPORT unzGetGlobalComment ( unzFile file, char * szComment, uLong uSizeBuf) { .... if (uReadThis>0) { *szComment='\0'; if (ZREAD64(s->z_filefunc,s->filestream,szComment,uReadThis)!=uReadThis) return UNZ_ERRNO; } if ((szComment != NULL) && (uSizeBuf > s->gi.size_comment)) *(szComment+s->gi.size_comment)='\0'; .... } 

PVS-Studio警告: V595 CWE-476在针对nullptr进行验证之前,已使用了'szComment'指针。 检查行:2068、2073。unzip.c 2068

如果uReadThis> 0 ,则取消引用szComment指针。 这很危险,因为此指针可能为空。 分析器基于进一步检查此指针是否等于NULL的事实得出这样的结论。

未初始化的班级成员


 template <class T> class Edge { public: using VertexType = Vector2<T>; Edge(const VertexType &p1, const VertexType &p2, T w=-1) : p1(p1), p2(p2), weight(w) {}; // <= Edge(const Edge &e) : p1(e.p1), p2(e.p2), weight(e.weight), isBad(false) {}; Edge() : p1(0,0), p2(0,0), weight(0), isBad(false) {} VertexType p1; VertexType p2; T weight=0; bool isBad; }; 

PVS-Studio警告: V730 CWE-457并非类的所有成员都在构造函数中初始化。 考虑检查:isBad。 edge.h 14

除第一个构造函数外,所有构造函数均初始化isBad类的字段。 最有可能的是,第一个构造函数只是偶然地忘记了执行此初始化。 结果,第一个构造函数创建了一个未完全初始化的对象,这可能导致未定义的程序行为。

还有11个V730诊断触发器 。 但是,由于我们对代码不熟悉,因此很难说这些警告是否表示实际问题。 我认为这些警告最好由该项目的作者进行研究。

内存泄漏


 template <typename ElementType> void ProjectLibrary::loadElements(....) { .... ElementType* element = new ElementType(elementDir, false); // can throw if (elementList.contains(element->getUuid())) { throw RuntimeError( __FILE__, __LINE__, QString(tr("There are multiple library elements with the same " "UUID in the directory \"%1\"")) .arg(subdirPath.toNative())); } .... } 

PVS-Studio警告: V773 CWE-401在未释放“元素”指针的情况下引发了异常。 可能发生内存泄漏。 projectlibrary.cpp 245

如果元素已经在列表中,则将引发异常。 但这不会破坏先前创建的对象,该对象的指针存储在element变量中。

无效的异常类型


 bool CmdRemoveSelectedSchematicItems::performExecute() { .... throw new LogicError(__FILE__, __LINE__); .... } 

PVS-Studio警告: V1022 CWE-755指针引发异常。 考虑改为按值扔掉它。 cmdremoveselectedschematicitems.cpp 143

分析器检测到指针引发的异常。 通常,通常按值抛出异常并按引用捕获。 抛出指针可能导致不捕获异常,因为该异常将通过引用捕获。 同样,使用指针会强制拦截器调用delete运算符以破坏创建的对象,从而不会发生内存泄漏。

通常, 运算符是偶然写在这里的,应该删除。 在所有其他地方都说:

 throw LogicError(__FILE__, __LINE__); 

危险使用dynamic_cast


 void GraphicsView::handleMouseWheelEvent( QGraphicsSceneWheelEvent* event) noexcept { if (event->modifiers().testFlag(Qt::ShiftModifier)) .... } bool GraphicsView::eventFilter(QObject* obj, QEvent* event) { .... handleMouseWheelEvent(dynamic_cast<QGraphicsSceneWheelEvent*>(event)); .... } 

PVS-Studio警告: V522 CWE-628可能会取消引用空指针“事件”。 潜在的空指针将传递给“ handleMouseWheelEvent”函数。 检查第一个参数。 检查行:143,252。graphicsview.cpp 143

dynamic_cast运算符产生的指针将传递到handleMouseWheelEvent函数。 在其中,无需事先验证就可以取消引用该指针。

这是危险的,因为dynamic_cast运算符可能返回nullptr 。 事实证明,此代码并不比仅使用更快的static_cast好

使用前,应在此代码中添加显式指针检查。

同样,这种代码很常见:

 bool GraphicsView::eventFilter(QObject* obj, QEvent* event) { .... QGraphicsSceneMouseEvent* e = dynamic_cast<QGraphicsSceneMouseEvent*>(event); Q_ASSERT(e); if (e->button() == Qt::MiddleButton) .... } 

PVS-Studio警告: V522 CWE-690可能会取消引用潜在的空指针'e'。 graphicsview.cpp 206

使用Q_ASSERT宏检查指针。 这是他的描述:
如果test为false,则输出包含源代码文件名和行号的警告消息。

Q_ASSERT()对于在开发过程中测试前置条件和后置条件非常有用。 如果在编译过程中定义了QT_NO_DEBUG,则不执行任何操作。

Q_ASSERT是在使用之前检查指针不好方法。 通常,在发行版本中未定义QT_NO_DEBUG 。 我不知道LibrePCB项目的情况如何。 但是,如果在Release中定义了QT_NO_DEBUG ,则这是一个奇怪且非标准的解决方案。

如果宏扩展为空白,则表明没有验证。 然后还不清楚为什么完全使用dynamic_cast 。 为什么不使用static_cast呢?

通常,此代码是无味的,值得对所有类似的代码片段进行回顾。 顺便说一下,其中有很多。 我算出82个类似的案例!

结论


总的来说,LibrePCB项目对我们来说似乎是高质量的。 尽管如此,我们还是建议项目的作者自己使用PVS-Studio工具,并对分析仪指示的代码部分进行独立的代码审查。 我们准备为他们提供一个月的免费许可证,以对项目进行全面分析。 此外,由于项目代码已公开并发布在GitHub网站上,因此他们可以使用免费的分析仪许可选项。 我们将尽快撰写有关此许可选项的文章。



如果您想与讲英语的读者分享这篇文章,请使用以下链接:Andrey Karpov,Svyatoslav Razmyslov。 在Docker容器中使用PVS-Studio检查LibrePCB

Source: https://habr.com/ru/post/zh-CN433562/


All Articles