Este é um artigo clássico sobre como nossa equipe testou um projeto aberto do LibrePCB usando o analisador de código estático PVS-Studio. No entanto, o artigo é interessante, pois a verificação foi realizada dentro do contêiner do Docker. Se você usar contêineres, esperamos que o artigo demonstre outra maneira simples de integrar o analisador ao processo de desenvolvimento.
LibrePCB
O LibrePCB é um software gratuito para projetar circuitos eletrônicos e placas de circuito impresso. O código do programa é escrito em C ++ e o Qt5 é usado para criar a interface gráfica. Recentemente, ocorreu o primeiro lançamento oficial desse aplicativo, que marcou a estabilização de seu próprio formato de arquivo (* .lp, * .lplib). Pacotes binários preparados para Linux, macOS e Windows.
O LibrePCB é um pequeno projeto que contém apenas cerca de 300.000 linhas de código não vazias em C e C ++. Ao mesmo tempo, 25% das linhas não vazias são comentários. A propósito, essa é uma grande porcentagem de comentários. Provavelmente, isso se deve ao fato de o projeto ter muitos arquivos pequenos, uma parte significativa da qual é ocupada por comentários de cabeçalho das informações do projeto e da licença. Código do site GitHub:
LibrePCB .
O projeto nos pareceu interessante e decidimos dar uma olhada. Mas os resultados dos testes não eram mais tão interessantes. Sim, houve erros reais. Mas não havia nada de especial sobre o qual certamente devemos contar aos leitores de nossos artigos. Talvez não escrevamos um artigo e nos limitemos a enviar os erros aos desenvolvedores do projeto. No entanto, o ponto interessante foi que o projeto foi testado na imagem do Docker e, portanto, decidimos que ainda valia a pena escrever um artigo.
Docker
Docker - software para automatizar a implantação e o gerenciamento de aplicativos em um ambiente de virtualização no nível do sistema operacional. Ele permite que você "empacote" o aplicativo com todos os seus arredores e dependências em um contêiner. Embora essa tecnologia tenha cerca de cinco anos e muitas empresas implementem o Docker em sua infraestrutura de projeto, no mundo do código aberto isso não era muito perceptível até recentemente.
Nossa empresa trabalha em estreita colaboração com projetos de código aberto, verificando o código-fonte usando nosso próprio analisador estático PVS-Studio. Atualmente,
mais de 300 projetos foram verificados. A coisa mais difícil nessa atividade sempre foi a compilação de projetos de outras pessoas, mas a introdução de contêineres do Docker simplificou bastante esse processo.
A primeira experiência de teste de um projeto de código aberto no Docker foi com o
Azure Service Fabric . Lá, os desenvolvedores fizeram a instalação do diretório do arquivo de origem no contêiner e a integração do analisador foi limitada à edição de um dos scripts em execução no contêiner:
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))
A diferença entre o projeto LibrePCB é que eles imediatamente forneceram o
Dockerfile para criar a imagem e o projeto nela. Acabou sendo ainda mais conveniente para nós. Aqui está a parte do arquivo Docker em que estamos interessados:
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:
Não faremos a compilação e instalação do projeto durante a montagem da imagem. Assim, coletamos uma imagem na qual o autor do projeto garante a montagem bem-sucedida do projeto.
Após o início do contêiner, o analisador foi instalado e os seguintes comandos foram executados para criar e analisar o projeto:
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
A propósito, todas as ações foram executadas no Windows 10. É muito agradável que os desenvolvedores de todos os sistemas operacionais populares também estejam desenvolvendo nessa direção. Infelizmente, os contêineres com Windows não são tão convenientes. Especialmente devido à impossibilidade de também instalar software facilmente.
Erros encontrados
Agora, uma seção clássica contendo uma descrição dos erros que encontramos usando o analisador de código estático do PVS-Studio. A propósito, aproveitando esta oportunidade, quero lembrá-lo de que recentemente desenvolvemos um analisador no sentido de oferecer suporte à análise de código para sistemas embarcados. Aqui estão alguns artigos que alguns de nossos leitores podem ter pulado:
- O PVS-Studio inclui suporte para o GNU Arm Embedded Toolchain ;
- PVS-Studio: suporte para os padrões de codificação MISRA C e MISRA C ++ .
Erros de digitação
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 Warning:
V501 CWE-571 Existem sub-expressões idênticas 'symbVarItemUuid' à esquerda e à direita do operador '&&'. symbolpreviewgraphicsitem.cpp 74
Erro de digitação
clássico : a variável
symbVarItemUuid é verificada duas vezes seguidas. Há uma verificação semelhante acima e, olhando para ela, fica claro que a segunda variável a ser verificada deve ser
symbVarUuid .
O seguinte trecho de código de erro de digitação:
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);
Aviso do PVS-Studio:
V778 CWE-682 Dois fragmentos de código semelhantes foram encontrados. Talvez seja um erro de digitação e a variável 'eMaxPair' deva ser usada em vez de 'e'. clipper.cpp 2999
Provavelmente, o código foi escrito usando Copiar e Colar. Devido a uma supervisão no segundo bloco de texto, eles esqueceram de substituir
e-> Top por
eMaxPair-> Top .
Cheques extras
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 Warning:
V547 CWE-571 A expressão 'content' é sempre verdadeira. html.c 162
Provavelmente ainda não é um erro, mas simplesmente um código redundante. Não há necessidade de verificar novamente o ponteiro de
conteúdo . Se for zero, a função termina imediatamente seu trabalho.
Uma situação semelhante:
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); } .... }
Aviso do PVS-Studio:
V547 CWE-571 A expressão 'e-> OutIdx> = 0' é sempre verdadeira. clipper.cpp 2983
Verificar novamente (e-> OutIdx> = 0) não faz sentido. No entanto, talvez isso seja um erro. Por exemplo, podemos assumir que a variável
e-> Top deve ser verificada. No entanto, isso é apenas um palpite. Não estamos familiarizados com o código do projeto e não podemos distinguir erros do código redundante :).
E outro caso:
QString SExpression::toString(int indent) const { .... if (child.isLineBreak() && nextChildIsLineBreak) { if (child.isLineBreak() && (i > 0) && mChildren.at(i - 1).isLineBreak()) {
PVS-Studio Warning:
V571 CWE-571 Verificação recorrente. A condição 'child.isLineBreak ()' já foi verificada na linha 208. sexpression.cpp 209
Erro na lógica
void FootprintPreviewGraphicsItem::paint(....) noexcept { .... for (const Circle& circle : mFootprint.getCircles()) { layer = mLayerProvider.getLayer(*circle.getLayerName()); if (!layer) continue;
PVS-Studio Warning:
V547 CWE-571 A expressão 'layer' é sempre verdadeira. footprintpreviewgraphicsitem.cpp 177
Como a condição na segunda
se a declaração é sempre verdadeira, o ramo
else nunca é satisfeito.
Verificação esquecida do ponteiro
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'; .... }
Aviso do PVS-Studio:
V595 CWE-476 O ponteiro 'szComment' foi utilizado antes de ser verificado no nullptr. Verifique as linhas: 2068, 2073. unzip.c 2068
Se
uReadThis> 0 , o ponteiro
szComment é
desreferenciado . Isso é perigoso, pois esse ponteiro pode ser nulo. O analisador conclui com base no fato de que ainda mais esse ponteiro é verificado quanto à igualdade
NULL .
Membro da classe não inicializado
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) {};
Aviso do PVS-Studio:
V730 CWE-457 Nem todos os membros de uma classe são inicializados dentro do construtor. Considere inspecionar: isBad. edge.h 14
Todos os construtores, exceto o primeiro, inicializam o campo da classe
isBad . Provavelmente, o primeiro construtor simplesmente esqueceu acidentalmente de fazer essa inicialização. Como resultado, o primeiro construtor cria um objeto incompletamente inicializado, o que pode levar ao comportamento indefinido do programa.
Existem mais 11
gatilhos de diagnóstico
V730 . No entanto, como não estamos familiarizados com o código, é difícil dizer se esses avisos indicam problemas reais. Eu acho que esses avisos são melhor estudados pelos autores do projeto.
Vazamento de memória
template <typename ElementType> void ProjectLibrary::loadElements(....) { .... ElementType* element = new ElementType(elementDir, false);
Aviso do PVS-Studio:
V773 CWE-401 A exceção foi lançada sem liberar o ponteiro 'elemento'. É possível um vazamento de memória. projectlibrary.cpp 245
Se um elemento já estiver na lista, uma exceção será lançada. Mas isso não destrói o objeto criado anteriormente, cujo ponteiro é armazenado na variável do
elemento .
Tipo de exceção inválido
bool CmdRemoveSelectedSchematicItems::performExecute() { .... throw new LogicError(__FILE__, __LINE__); .... }
PVS-Studio Warning:
V1022 CWE-755 Uma exceção foi lançada pelo ponteiro. Considere jogá-lo por valor. cmdremoveselectedschematicitems.cpp 143
O analisador detectou uma exceção lançada pelo ponteiro. Na maioria das vezes, é costume lançar exceções por valor e capturar por referência. Arremessar um ponteiro pode fazer com que a exceção não seja capturada, pois será capturada por referência. Além disso, o uso de um ponteiro força o interceptador a chamar o operador de
exclusão para destruir o objeto criado, para que não ocorram vazamentos de memória.
Em geral, o
novo operador é escrito aqui por acidente e deve ser excluído. Que isso é um erro é confirmado pelo fato de que em todos os outros lugares diz:
throw LogicError(__FILE__, __LINE__);
Uso perigoso de 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 Warning:
V522 CWE-628 A desreferenciação do ponteiro nulo 'evento' pode ocorrer. O potencial ponteiro nulo é passado para a função 'handleMouseWheelEvent'. Inspecione o primeiro argumento. Verifique as linhas: 143, 252. graphicsview.cpp 143
O ponteiro resultante do operador
dynamic_cast é passado para a função
handleMouseWheelEvent . Nele, esse ponteiro é desreferenciado sem verificação prévia.
Isso é perigoso, pois o operador
dynamic_cast pode retornar
nullptr . Acontece que esse código não é melhor do que apenas usar o
static_cast mais rápido.
Uma verificação explícita do ponteiro deve ser adicionada a esse código antes do uso.
Além disso, código desse tipo é muito comum:
bool GraphicsView::eventFilter(QObject* obj, QEvent* event) { .... QGraphicsSceneMouseEvent* e = dynamic_cast<QGraphicsSceneMouseEvent*>(event); Q_ASSERT(e); if (e->button() == Qt::MiddleButton) .... }
Aviso do PVS-Studio:
V522 CWE-690 Pode haver desreferenciação de um ponteiro nulo potencial 'e'. graphicsview.cpp 206
O ponteiro é verificado usando a macro
Q_ASSERT . Aqui está a descrição dele:
Imprime uma mensagem de aviso contendo o nome do arquivo de código-fonte e o número da linha, se o teste for falso.
Q_ASSERT () é útil para testar pré e pós-condições durante o desenvolvimento. Não faz nada se QT_NO_DEBUG foi definido durante a compilação.
|
Q_ASSERT é uma maneira ruim de verificar os ponteiros antes do uso. Como regra, na versão Release
QT_NO_DEBUG não
está definida. Não sei como estão as coisas no projeto LibrePCB. No entanto, se
QT_NO_DEBUG estiver definido no Release, essa é uma solução estranha e não padrão.
Se a macro se expandir para o vazio, acontece que não há verificação. E então não está claro por que usar
dynamic_cast . Por que não usar
static_cast então ?
Em geral, esse código é inodoro e merece uma revisão de todos os fragmentos de código semelhantes. E há, a propósito, muitos deles.
Contei 82 casos semelhantes!Conclusão
Em geral, o projeto LibrePCB nos parecia de alta qualidade. No entanto, recomendamos que os autores do projeto se armam com a ferramenta PVS-Studio e conduzam independentemente uma Revisão de Código das seções de código indicadas pelo analisador. Estamos prontos para fornecer a eles uma licença gratuita por um mês para uma análise completa do projeto. Além disso, eles podem usar a opção de licenciamento do analisador gratuito, pois o código do projeto é aberto e publicado no site do GitHub. Escreveremos sobre esta opção de licenciamento em breve.
Se você deseja compartilhar este artigo com um público que fala inglês, use o link para a tradução: Andrey Karpov, Svyatoslav Razmyslov.
Verificando o LibrePCB com o PVS-Studio dentro de um contêiner Docker .