O analisador de código está errado, viva o analisador

Foo (std :: move (buffer), line_buffer - buffer.get ());






Combinar várias ações em uma expressão C ++ é ruim, pois esse código é difícil de entender, difícil de manter e também é fácil cometer um erro nele. Por exemplo, crie um bug combinando diferentes ações ao avaliar os argumentos da função. Concordamos com a recomendação clássica de que o código deve ser simples e direto. E agora vamos considerar um caso interessante, quando formalmente o analisador PVS-Studio está errado, mas do ponto de vista prático, o código ainda deve ser alterado.



Ordem de Avaliação de Argumento



O que será contado agora é uma continuação da velha história sobre a ordem de cálculo dos argumentos, sobre a qual escrevemos no artigo " A profundidade de uma toca de coelho ou uma entrevista em C ++ no PVS-Studio ".



A breve essência é a seguinte. A ordem em que os argumentos da função são avaliados é um comportamento não especificado. O padrão não especifica a ordem exata em que os desenvolvedores do compilador devem avaliar os argumentos. Por exemplo, da esquerda para a direita (Clang) ou da direita para a esquerda (GCC, MSVC). Antes do padrão C ++ 17, quando os efeitos colaterais ocorriam ao avaliar os argumentos, isso poderia levar a um comportamento indefinido.



Com o advento do padrão C ++ 17, a situação mudou para melhor: agora o cálculo do argumento e seus efeitos colaterais começarão a ser executados apenas a partir do momento em que todos os cálculos e efeitos colaterais do argumento anterior forem realizados. No entanto, isso não significa que agora não haja espaço para erros.



Considere um programa de teste simples:



#include <cstdio>
int main()
{
  int i = 1;
  printf("%d, %d\n", i, i++);
  return 0;
}
      
      





O que este código imprimirá? A resposta ainda depende do compilador, da versão e do humor. Dependendo do compilador, ambos "1, 1" e "2, 1" podem ser impressos. Na verdade, usando o Compiler Explorer, obtenho os seguintes resultados:



  • um programa compilado com Clang 11.0.0 produz "1, 1".
  • um programa compilado com GCC 10.2 produz "2, 1".


Não existe um comportamento indefinido neste programa, mas existe um comportamento não especificado (ordem de avaliação dos argumentos).



Código do projeto CSV Parser



Vamos voltar ao trecho de código do projeto CSV Parser, que mencionei no artigo " Verificando a coleção de bibliotecas C ++ somente de cabeçalho (awesome-hpp) ".



O analisador e eu sabemos que os argumentos podem ser avaliados em ordem diferente. Portanto, o analisador, e depois ele mesmo, considerou este código errado:



std::unique_ptr<char[]> buffer(new char[BUFFER_UPPER_LIMIT]);
....
this->feed_state->feed_buffer.push_back(
    std::make_pair<>(std::move(buffer), line_buffer - buffer.get()));
      
      





Aviso do PVS-Studio: V769 O ponteiro 'buffer.get ()' na expressão 'line_buffer - buffer.get ()' é igual a nullptr. O valor resultante não faz sentido e não deve ser usado. csv.hpp 4957



Na verdade, ambos estamos errados e não há engano. Falaremos mais sobre as nuances, mas por enquanto vamos começar com uma simples.



Então, vamos ver por que é perigoso escrever um código como este:



Foo(std::move(buffer), line_buffer - buffer.get());
      
      





Eu acho que você pode adivinhar a resposta. O resultado depende da ordem em que os argumentos são avaliados. Considere isso no seguinte código sintético:



#include <iostream>
#include <memory>   

void Print(std::unique_ptr<char[]> p, ptrdiff_t diff)
{
    std::cout << diff << std::endl;
} 

void Print2(ptrdiff_t diff, std::unique_ptr<char[]> p)
{
    std::cout << diff << std::endl;
} 

int main()
{
    {
        std::unique_ptr<char[]> buffer(new char[100]);
        char *ptr = buffer.get() + 22;
        Print(std::move(buffer), ptr - buffer.get());
    }
    {
        std::unique_ptr<char[]> buffer(new char[100]);
        char *ptr = buffer.get() + 22;
        Print2(ptr - buffer.get(), std::move(buffer));
    }
    return 0;
}
      
      





Vamos usar o Compiler Explorer novamente e ver a saída desse programa compilada por diferentes compiladores.



Compilador Clang 11.0.0. Resultado :



23387846
22
      
      





O Compilador GCC 10.2. Resultado :



22
26640070
      
      





Esperamos o resultado e você não pode escrever assim. É exatamente sobre isso que o analisador PVS-Studio avisa.



Eu gostaria de acabar com isso, mas tudo é um pouco mais complicado. O fato é que estamos falando sobre passar argumentos por valor e, ao instanciar o template de função std :: make_pair, tudo será diferente. Vamos continuar a mergulhar nas nuances e descobrir por que o PVS-Studio está errado neste caso.



std :: make_pair



Vamos dar uma olhada no site cppreference e ver como o template de função std :: make_pair mudou .



Até C ++ 11:
template <classe T1, classe T2>

std :: pair <T1, T2> make_pair (T1 t, T2 u);
Desde C ++ 11, até C ++ 14:
template <classe T1, classe T2>

std :: pair <V1, V2> make_pair (T1 && t, T2 && u);
Desde C ++ 14:
template< class T1, class T2 >

constexpr std::pair<V1,V2> make_pair( T1&& t, T2&& u );
Como você pode ver, era uma vez, std :: make_pair assumia argumentos por valor. Se std :: unique_ptr existisse naquela época , então o código discutido acima estaria de fato incorreto. Se esse código funcionou ou não foi uma questão de sorte. Na prática, é claro, essa situação nunca teria surgido, já que std :: unique_ptr apareceu em C ++ 11 como um substituto para std :: auto_ptr .



Voltemos ao nosso tempo. Começando com a versão do padrão C ++ 11, o construtor começou a usar a semântica de movimentação.



Há um ponto sutil aqui em que std :: move não move realmente nada, mas apenas converte o objeto em uma referência rvalue . Isso vai permitirstd :: make_pair passa o ponteiro para o novo std :: unique_ptr , deixando nullptr no ponteiro inteligente original. Mas esta passagem de ponteiro não acontecerá até que entremos em std :: make_pair . Nesse momento, já calculamos line_buffer - buffer.get () e tudo ficará bem. Ou seja, uma chamada para buffer.get () não pode retornar nullptr no momento em que é avaliada, não importa quando isso acontece.



Lamento a descrição complicada. O resultado final é que esse código é perfeitamente válido. E, de fato, o analisador estático PVS-Studio, neste caso, deu um alarme falso. No entanto, nossa equipe não tem certeza de que devemos nos apressar em fazer alterações na lógica do analisador para tais situações.



O rei está morto, vida longa ao rei!



Descobrimos que a operação descrita no artigo era falsa. Obrigado a um de nossos leitores que chamou nossa atenção para a peculiaridade da implementação std :: make_pair .



No entanto, este é o caso quando não temos certeza se vale a pena melhorar o comportamento do analisador. A questão é que esse código é muito complicado. É preciso admitir que o código que analisamos não merece uma investigação tão detalhada, que arrastou um artigo inteiro. Se este código requer tanta atenção, é um código muito ruim.



É oportuno aqui relembrar o artigo "Os falsos positivos são nossos inimigos, mas ainda podem ser seus amigos ". A publicação não é nossa, mas concordamos com ela.



Talvez seja esse o caso. O aviso pode ser falso, mas aponta para um lugar melhor para refatorar. Basta escrever algo assim:



auto delta = line_buffer - buffer.get();
this->feed_state->feed_buffer.push_back(
  std::make_pair(std::move(buffer), delta));
      
      





Ou você pode tornar o código ainda melhor nessa situação usando o método emplace_back :



auto delta = line_buffer - buffer.get();
this->feed_state->feed_buffer.emplace_back(std::move(buffer), delta);
      
      





Este código criará o objeto std :: pair resultante no contêiner "no local", ignorando a criação de um objeto temporário e sua transferência para o contêiner. A propósito, o analisador PVS-Studio sugere fazer essa substituição emitindo um aviso V823 do conjunto de regras para micro-otimizações de código.



O código se tornará inequivocamente mais simples e claro para qualquer leitor e analisador. Não há nenhum mérito em amontoar tantas ações quanto possível em uma linha de código.



Sim, neste caso deu sorte e não há engano. Mas é improvável que o autor, ao escrever este código, tenha guardado tudo o que discutimos em sua cabeça. Provavelmente, foi a sorte que jogou. E em outra ocasião você pode não ter sorte.



Conclusão



Então, descobrimos que não há erro real. O analisador gera um falso positivo. Talvez removamos o aviso apenas para esses casos, ou talvez não. Vamos pensar sobre isso mais tarde. Afinal, esse é um caso bastante raro, e o código em que os argumentos são avaliados com efeitos colaterais é perigoso em geral, e é melhor evitá-lo. Vale a pena refatorar pelo menos para fins preventivos.



Ver código:



Foo(std::move(buffer), line_buffer - buffer.get());
      
      





fácil de quebrar alterando algo em outra parte do programa. Um código como esse é difícil de manter. E também é desagradável porque pode dar a falsa impressão de que tudo está funcionando corretamente. Na verdade, isso é apenas uma coincidência e tudo pode falhar se você alterar as configurações do compilador ou de otimização.



Torne seu código mais fácil!









Se você deseja compartilhar este artigo com um público que fala inglês, use o link de tradução: Andrey Karpov. O Code Analyzer está errado. Viva o analisador! ...



All Articles