Checking Clang 11 con PVS-Studio

(28 ottobre 2020 )

Ogni tanto, dobbiamo scrivere articoli su come abbiamo controllato unaltra nuova versione di qualche compilatore. Non è molto divertente. Tuttavia, come mostra la pratica, se smettiamo di farlo per un po , la gente inizia a dubitare che PVS-Studio valga il suo titolo di buon rilevatore di bug e vulnerabilità. E se il nuovo compilatore potesse fare anche questo? Certo, i compilatori si evolvono, ma anche PVS-Studio si evolve – e dimostra ripetutamente la sua capacità di individuare bug anche in progetti di alta qualità come i compilatori.

È ora di ricontrollare Clang

A dirti la verità, ho scritto questo articolo basato nel post precedente “ Controllo del compilatore GCC 10 con PVS-Studio “. Quindi, se alcuni paragrafi sembrano familiari, è perché li hai già letti prima :).

Non è un segreto che i compilatori impieghino i propri analizzatori di codice statici incorporati e anche quelli si stanno sviluppando. Ecco perché scriviamo articoli ogni tanto per dimostrare che il nostro analizzatore statico, PVS-Studio, può trovare bug anche allinterno dei compilatori e che valiamo il nostro sale :).

In effetti, non puoi confronta i classici analizzatori statici con i compilatori. Gli analizzatori statici non solo rilevano bug nel codice sorgente, ma coinvolgono anche uninfrastruttura altamente sviluppata. Per prima cosa, include lintegrazione con sistemi come SonarQube, PlatformIO, Azure DevOps, Travis CI, CircleCI, GitLab CI / CD, Jenkins e Visual Studio. Include meccanismi per la soppressione di massa degli avvisi, che consente di iniziare a utilizzare PVS-Studio subito anche in un progetto di grandi dimensioni. Include linvio di notifiche tramite e-mail. E così via e così via. Ma la prima domanda che gli sviluppatori chiederanno ancora è: “Il tuo PVS-Studio riesce a trovare qualcosa che i compilatori non possono trovare?” E questo significa che siamo condannati a scrivere articoli su come controlliamo i compilatori stessi più e più volte.

Torniamo a Clang. Non cè bisogno di soffermarsi sullargomento e dirti di cosa tratta il progetto. In realtà, abbiamo controllato non solo il codice di Clang 11 stesso, ma anche il codice della libreria LLVM 11 su cui si basa. Dal punto di vista di questo articolo, non importa se è stato trovato un difetto nel codice del compilatore o della libreria.

Ho trovato il codice di Clang / LLVM molto più chiaro di quello di GCC. Almeno non pullula di tutte quelle orribili macro e utilizza ampiamente le moderne funzionalità di C ++.

Anche così, il progetto è ancora abbastanza grande da rendere noioso lesame del rapporto di analisi senza personalizzazione preventiva. Ciò che più ostacola sono i “semi-falsi” positivi. Per “semi-falsi” positivi intendo i casi in cui lanalizzatore è tecnicamente corretto per segnalare determinati problemi ma tali avvertenze non sono di utilità pratica. Ad esempio, molti di questi avvisi si riferiscono a unit test e codice generato.

Ecco un esempio per unit test:

Spaces.SpacesInParentheses = false; // <=
Spaces.SpacesInCStyleCastParentheses = true; // <=
verifyFormat("Type *A = ( Type * )P;", Spaces);
verifyFormat("Type *A = ( vector )P;", Spaces);
verifyFormat("x = ( int32 )y;", Spaces);
verifyFormat("int a = ( int )(2.0f);", Spaces);
verifyFormat("#define AA(X) sizeof((( X * )NULL)->a)", Spaces);
verifyFormat("my_int a = ( my_int )sizeof(int);", Spaces);
verifyFormat("#define x (( int )-1)", Spaces);// Run the first set of tests again with:
Spaces.SpacesInParentheses = false; // <=
Spaces.SpaceInEmptyParentheses = true;
Spaces.SpacesInCStyleCastParentheses = true; // <=
verifyFormat("call(x, y, z);", Spaces);
verifyFormat("call( );", Spaces);

Il analyzer ci avverte che alle variabili vengono assegnati gli stessi valori che hanno già:

  • V1048 Alla variabile Spaces.SpacesInParentheses è stato assegnato lo stesso valore. FormatTest.cpp 11554
  • V1048 Alla variabile “Spaces.SpacesInCStyleCastParentheses” è stato assegnato lo stesso valore. FormatTest.cpp 11556

Tecnicamente, questo avvertimento è essenziale e lo snippet deve essere semplificato o corretto. Ma è anche chiaro che questo codice va bene così comè e non ha senso aggiustare nulla al suo interno.

Ecco un altro esempio: lanalizzatore genera un sacco di avvisi sul file generato automaticamente Options.inc. Guarda il “muro” del codice che contiene:

Questa massa di codice attiva una marea di avvisi:

  • V501 Sono presenti sottoespressioni identiche a sinistra ea destra delloperatore “==”: nullptr == nullptr Options.inc 26
  • V501 Ci sono sottoespressioni identiche a sinistra ea destra delloperatore ==: nullptr == nullptr Options.inc 27
  • V501 Lì sono sottoespressioni identiche a sinistra ea destra delloperatore “==”: nullptr == nullptr Options.inc 28
  • e così via – un avviso per riga …

Eppure tutto questo non è un grosso problema. può essere risolto escludendo dallanalisi file irrilevanti, contrassegnando determinate macro e funzioni, sopprimendo determinati tipi di diagnostica e così via. Sì, può, ma non è un lavoro molto interessante da fare quando scrivi un articolo. Ecco perché ho fatto la stessa cosa dellarticolo sul controllo del compilatore GCC: ho continuato a leggere il rapporto finché non ho raccolto 11 esempi interessanti da includere nellarticolo. Perché 11? Ho solo pensato che poiché era lundicesima versione di Clang, avevo bisogno di 11 esempi :).

11 snippet di codice sospetti

Snippet 1, operazione modulo su 1

Questo è fantastico! Mi piacciono i bug del genere!

void Act() override {
....
// If the value type is a vector, and we allow vector select, then in 50%
// of the cases generate a vector select.
if (isa(Val0->getType()) && (getRandom() % 1)) {
unsigned NumElem =
cast(Val0->getType())->getNumElements();
CondTy = FixedVectorType::get(CondTy, NumElem);
}
....
}

Messaggio diagnostico di PVS-Studio: V1063 Loperazione modulo per 1 non ha senso. Il risultato sarà sempre zero. llvm-stress.cpp 631

Il programmatore sta usando unoperazione modulo per ottenere un valore casuale di 0 o 1. Ma il valore 1 sembra confondere gli sviluppatori e fai in modo che scrivano il classico anti-pattern in cui loperazione modulo viene eseguita su 1 invece di 2. Loperazione X% 1 è priva di significato in quanto restituisce sempre 0 . Questa è la versione corretta:

if (isa(Val0->getType()) && (getRandom() % 2)) {

La diagnostica V1063 aggiunta di recente è terribilmente semplice, ma, come puoi vedere, funziona perfettamente.

Sappiamo che gli sviluppatori di compilatori tengono docchio il nostro lavoro e prendono in prestito le nostre idee. Va benissimo. È bello sapere che PVS-Studio è la forza trainante del progresso . Vediamo quanto ci vorrà perché una diagnostica simile appaia in Clang e GCC :).

Snippet 2, un errore di battitura in una condizione

class ReturnValueSlot {
....
bool isNull() const { return !Addr.isValid(); }
....
};static bool haveSameParameterTypes(ASTContext &Context, const FunctionDecl *F1,
const FunctionDecl *F2, unsigned NumParams) {
....
unsigned I1 = 0, I2 = 0;
for (unsigned I = 0; I != NumParams; ++I) {
QualType T1 = NextParam(F1, I1, I == 0);
QualType T2 = NextParam(F2, I2, I == 0);
if (!T1.isNull() && !T1.isNull() && !Context.hasSameUnqualifiedType(T1, T2))
return false;
}
return true;
}

Messaggio di diagnostica PVS-Studio: V501 Sono presenti sottoespressioni identiche a sinistra ea destra del Operatore “& &”:! T1.isNull () & &! T1.isNull () SemaOverload.cpp 9493

Il controllo ! T1.isNull () viene eseguito due volte. Questo è ovviamente un errore di battitura; la seconda parte della condizione deve controllare la variabile T2 .

Snippet 3, potenziale array-index-out-of- limiti

std::vector DeclsLoaded;SourceLocation ASTReader::getSourceLocationForDeclID(GlobalDeclID ID) {
....
unsigned Index = ID - NUM_PREDEF_DECL_IDS; if (Index > DeclsLoaded.size()) {
Error("declaration ID out-of-range for AST file");
return SourceLocation();
} if (Decl *D = DeclsLoaded[Index])
return D->getLocation();
....
}

Messaggio di diagnostica PVS-Studio: V557 Array overrun è possibile. Lindice “Index” punta oltre il limite dellarray. ASTReader.cpp 7318

Supponi che larray memorizzi un elemento e che anche il valore della variabile Index sia 1. Quindi la condizione (1> 1) è falso e, pertanto, larray verrà indicizzato oltre i suoi limiti. Ecco il controllo corretto:

if (Index >= DeclsLoaded.size()) {

Snippet 4, ordine di valutazione degli argomenti

void IHexELFBuilder::addDataSections() {
....
uint32_t SecNo = 1;
....
Section = &Obj->addSection(
".sec" + std::to_string(SecNo++), RecAddr,
ELF::SHF_ALLOC | ELF::SHF_WRITE, SecNo);
....
}

Messaggio diagnostico di PVS-Studio: V567 Comportamento non specificato. Lordine di valutazione degli argomenti non è definito per la funzione “addSection”. Considera lidea di ispezionare la variabile “SecNo”. Object.cpp 1223

Notare che largomento SecNo viene usato due volte, incrementandosi nel frattempo. Il problema è che non puoi dire in quale ordine esatto verranno valutati gli argomenti. Il risultato, quindi, varierà a seconda della versione del compilatore o dei parametri di compilazione.

Ecco un esempio sintetico per illustrare questo punto:

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

A seconda del compilatore, questo codice può produrre “1, 2” o “2, 1”. Lho eseguito su Compiler Explorer e ho ottenuto i seguenti output:

  • quando compilato con Clang 11.0.0, il programma restituisce 1 , 1.
  • quando compilato con GCC 10.2, il programma output s 2, 1.

È interessante notare che questo semplice caso fa sì che Clang emetta un avviso:

:6:26: warning:
unsequenced modification and access to "i" [-Wunsequenced]
printf("%d, %d\n", i, i++);

Per qualche ragione, però, questo avviso non è stato emesso sul codice reale. O è disabilitato in quanto non molto pratico o quel caso è troppo complicato per il compilatore da affrontare.

Snippet 5, uno strano controllo duplicato

template <class ELFT>
void GNUStyle::printVersionSymbolSection(const ELFFile *Obj,
const Elf_Shdr *Sec) { ....
Expected NameOrErr =
this->dumper()->getSymbolVersionByIndex(Ndx, IsDefault);
if (!NameOrErr) {
if (!NameOrErr) {
unsigned SecNdx = Sec - &cantFail(Obj->sections()).front();
this->reportUniqueWarning(createError(
"unable to get a version for entry " + Twine(I) +
" of SHT_GNU_versym section with index " + Twine(SecNdx) + ": " +
toString(NameOrErr.takeError())));
}
Versions.emplace_back("");
continue;
}
....
}

Messaggio diagnostico PVS-Studio: V571 Controllo ricorrente. La condizione “if (! NameOrErr)” era già verificata nella riga 4666. ELFDumper.cpp 4667

Il secondo controllo è un clone del primo ed è, quindi, ridondante. Forse potrebbe essere rimosso in sicurezza. Ma la cosa più probabile è che contenga un errore di battitura e abbia lo scopo di controllare qualche altra variabile.

Snippet 6, potenziale dereferenziazione del puntatore nullo

void RewriteObjCFragileABI::RewriteObjCClassMetaData(
ObjCImplementationDecl *IDecl, std::string &Result)
{
ObjCInterfaceDecl *CDecl = IDecl->getClassInterface(); if (CDecl->isImplicitInterfaceDecl()) {
RewriteObjCInternalStruct(CDecl, Result);
} unsigned NumIvars = !IDecl->ivar_empty()
? IDecl->ivar_size()
: (CDecl ? CDecl->ivar_size() : 0);
....
}

Messaggio diagnostico di PVS-Studio: V595 Il puntatore” CDecl “è stato utilizzato prima di essere verificato rispetto a nullptr. Linee di controllo: 5275, 5284. RewriteObjC.cpp 5275

Quando esegue il primo controllo, lo sviluppatore non esita mai a dereferenziare il puntatore CDecl :

if (CDecl->isImplicitInterfaceDecl())

Ma se guardi il codice qualche riga più avanti, diventa chiaro che il puntatore può essere nullo:

(CDecl ? CDecl->ivar_size() : 0)

Il primo controllo doveva probabilmente assomigliare a questo:

if (CDecl && CDecl->isImplicitInterfaceDecl())

Snippet 7, potenziale dereferenziazione del puntatore nullo

bool
Sema::InstantiateClass(....)
{
....
NamedDecl *ND = dyn_cast(I->NewDecl);
CXXRecordDecl *ThisContext =
dyn_cast_or_null(ND->getDeclContext());
CXXThisScopeRAII ThisScope(*this, ThisContext, Qualifiers(),
ND && ND->isCXXInstanceMember());
....
}

Messaggio di diagnostica PVS-Studio: V595 Il puntatore “ND” è stato utilizzato prima di essere verificato rispetto nullptr.Controllare le righe: 2803, 2805. SemaTemplateInstantiate.cpp 2803

Questo errore è simile al precedente. È pericoloso dereferenziare un puntatore senza un controllo preventivo quando il suo valore viene acquisito utilizzando un cast di tipo dinamico. Inoltre, il codice successivo conferma che è necessario tale controllo.

Snippet 8, una funzione che continua a funzionare nonostante uno stato di errore

bool VerifyObject(llvm::yaml::Node &N,
std::map Expected) {
....
auto *V = llvm::dyn_cast_or_null(Prop.getValue());
if (!V) {
ADD_FAILURE() << KS << " is not a string";
Match = false;
}
std::string VS = V->getValue(Tmp).str();
....
}

Messaggio diagnostico di PVS-Studio: V1004 Il puntatore” V “è stato utilizzato in modo non sicuro dopo essere stato verificato rispetto a nullptr. Controllare le righe: 61, 65. TraceTests.cpp 65

Il puntatore V può essere un puntatore nullo. Questo è ovviamente uno stato di errore, che viene anche segnalato con un messaggio di errore. Ma la funzione continuerà a funzionare come se non fosse successo nulla e finirà per dereferenziare quel puntatore nullo. Il programmatore probabilmente voleva che la funzione si fermasse a questo punto, nel qual caso dovrebbe essere corretto come segue:

auto *V = llvm::dyn_cast_or_null(Prop.getValue());
if (!V) {
ADD_FAILURE() << KS << " is not a string";
Match = false;
return false;
}
std::string VS = V->getValue(Tmp).str();

Snippet 9, un errore di battitura

const char *tools::SplitDebugName(const ArgList &Args, const InputInfo &Input,
const InputInfo &Output) {
if (Arg *A = Args.getLastArg(options::OPT_gsplit_dwarf_EQ))
if (StringRef(A->getValue()) == "single")
return Args.MakeArgString(Output.getFilename()); Arg *FinalOutput = Args.getLastArg(options::OPT_o);
if (FinalOutput && Args.hasArg(options::OPT_c)) {
SmallString<128> T(FinalOutput->getValue());
llvm::sys::path::replace_extension(T, "dwo");
return Args.MakeArgString(T);
} else {
// Use the compilation dir.
SmallString<128> T(
Args.getLastArgValue(options::OPT_fdebug_compilation_dir));
SmallString<128> F(llvm::sys::path::stem(Input.getBaseInput()));
llvm::sys::path::replace_extension(F, "dwo");
T += F;
return Args.MakeArgString(F); // <=
}
}

Messaggio diagnostico di PVS-Studio: V1001 La “T” variabile è assegnata ma non viene utilizzata entro la fine della funzione. CommonArgs.cpp 873

Guarda le ultime righe della funzione. La variabile locale T cambia ma non viene utilizzata in alcun modo. Deve essere un errore di battitura e la funzione dovrebbe probabilmente terminare come segue:

T += F;
return Args.MakeArgString(T);

Snippet 10, zero come il divisore

typedef int32_t si_int;
typedef uint32_t su_int;typedef union {
du_int all;
struct {
#if _YUGA_LITTLE_ENDIAN
su_int low;
su_int high;
#else
su_int high;
su_int low;
#endif // _YUGA_LITTLE_ENDIAN
} s;
} udwords;COMPILER_RT_ABI du_int __udivmoddi4(du_int a, du_int b, du_int *rem) {
....
if (d.s.low == 0) {
if (d.s.high == 0) {
// K X
// ---
// 0 0
if (rem)
*rem = n.s.high % d.s.low;
return n.s.high / d.s.low;
}
....
}

Messaggi diagnostici di PVS-Studio:

  • V609 Mod di zero. Denominatore “d.s.low” == 0. udivmoddi4.c 61
  • V609 Dividi per zero. Denominatore “d.s.low” == 0. udivmoddi4.c 62

Non so se si tratta di un bug o di un congegno complicato, ma il codice sembra strano. Ha due variabili intere regolari, una delle quali è divisa per laltra. Ma la parte interessante è che loperazione di divisione avviene solo se entrambe le variabili sono zeri. Quale compito dovrebbe svolgere?

Snippet 11, copia-incolla

bool MallocChecker::mayFreeAnyEscapedMemoryOrIsModeledExplicitly(....)
{
....
StringRef FName = II->getName();
....
if (FName == "postEvent" &&
FD->getQualifiedNameAsString() == "QCoreApplication::postEvent") {
return true;
} if (FName == "postEvent" &&
FD->getQualifiedNameAsString() == "QCoreApplication::postEvent") {
return true;
}
....
}

Messaggio diagnostico di PVS-Studio: V581 Le espressioni condizionali delle istruzioni “if” situate una accanto allaltra sono identiche. Controllare le righe: 3108, 3113. MallocChecker.cpp 3113

Un frammento di codice è stato clonato ma mai modificato in seguito. Questo clone dovrebbe essere rimosso o modificato per eseguire qualche utile controllo.

Conclusione

Ricorda che puoi utilizzare questa licenza gratuita opzione per controllare i progetti open source. Forniamo anche altri modi per utilizzare PVS-Studio gratuitamente, alcuni dei quali consentono anche lanalisi di codice proprietario. Consulta lelenco completo delle opzioni qui: “ Modi per ottenere una licenza PVS-Studio gratuita “. Grazie per aver letto!

Lascia un commento

Il tuo indirizzo email non sarà pubblicato. I campi obbligatori sono contrassegnati *