Checking Clang 11 with PVS-Studio

(28 października 2020 r. )

Od czasu do czasu musimy pisać artykuły o tym, jak sprawdziliśmy inną świeżą wersję jakiegoś kompilatora. To nie jest zbyt zabawne. Jednak, jak pokazuje praktyka, jeśli przestaniemy to robić na jakiś czas, ludzie zaczną wątpić, czy PVS-Studio jest warte swojego tytułu dobrego łapacza błędów i luk. A co, jeśli nowy kompilator też to potrafi? Jasne, kompilatory ewoluują, ale tak samo dzieje się z PVS-Studio – i wielokrotnie udowadnia swoją zdolność do wychwytywania błędów nawet w projektach wysokiej jakości, takich jak kompilatory.

Czas na ponowne sprawdzenie Clang

Prawdę mówiąc, napisałem ten artykuł na podstawie we wcześniejszym poście „ Sprawdzanie kompilatora GCC 10 z PVS-Studio ”. Więc jeśli niektóre akapity wydają się znajome, to dlatego, że już je przeczytałeś :).

Nie jest tajemnicą, że kompilatory wykorzystują własne wbudowane statyczne analizatory kodu, a one również się rozwijają. Dlatego od czasu do czasu piszemy artykuły, aby pokazać, że nasz statyczny analizator, PVS-Studio, może znaleźć błędy nawet w kompilatorach i że jesteśmy warci naszej soli :).

W rzeczywistości nie możesz porównaj klasyczne analizatory statyczne z kompilatorami. Analizatory statyczne nie tylko wykrywają błędy w kodzie źródłowym, ale również obejmują wysoce rozwiniętą infrastrukturę. Obejmuje integrację z takimi systemami jak SonarQube, PlatformIO, Azure DevOps, Travis CI, CircleCI, GitLab CI / CD, Jenkins czy Visual Studio. Zawiera mechanizmy masowego tłumienia ostrzeżeń, co pozwala na rozpoczęcie korzystania z PVS-Studio od razu nawet w dużym projekcie. Obejmuje wysyłanie powiadomień pocztą elektroniczną. I tak dalej i tak dalej. Ale pierwsze pytanie, które programiści nadal będą zadawać, brzmi: „Czy Twoje PVS-Studio może znaleźć coś, czego kompilatorzy nie mogą?” A to oznacza, że ​​jesteśmy skazani na pisanie artykułów o tym, jak w kółko sprawdzamy same kompilatory.

Wróćmy do Clang. Nie ma potrzeby rozwodzenia się nad tematem i opowiadania, o czym jest projekt. W rzeczywistości sprawdziliśmy nie tylko sam kod Clang 11, ale także kod biblioteki LLVM 11, na której jest oparty. Z punktu widzenia tego artykułu nie ma znaczenia, czy znaleziono defekt w kodzie kompilatora czy biblioteki.

Uważam, że kod Clang / LLVM jest znacznie bardziej przejrzysty niż kod GCC. Przynajmniej nie obfituje w te wszystkie okropne makra i szeroko wykorzystuje nowoczesne funkcje C ++.

Mimo to projekt jest nadal wystarczająco duży, aby badanie raportu z analizy było żmudne bez wcześniejszego dostosowywania. To, co najczęściej przeszkadza, to „częściowo fałszywe” pozytywy. Przez „półfałszywe” pozytywne rozumiem przypadki, w których analizator jest technicznie poprawny, aby wskazać pewne problemy, ale te ostrzeżenia nie mają praktycznego zastosowania. Na przykład wiele takich ostrzeżeń odnosi się do testów jednostkowych i wygenerowanego kodu.

Oto przykład testów jednostkowych:

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);

Analysiser ostrzega nas, że zmiennym przypisano te same wartości, które już mają:

  • V1048 Zmiennej „Spaces.SpacesInParentheses” przypisano tę samą wartość. FormatTest.cpp 11554
  • V1048 Zmiennej „Spaces.SpacesInCStyleCastParentheses” przypisano tę samą wartość. FormatTest.cpp 11556

Z technicznego punktu widzenia to ostrzeżenie jest na temat, a fragment wymaga uproszczenia lub poprawienia. Ale jest też jasne, że ten kod jest w porządku i nie ma sensu niczego w nim naprawiać.

Oto kolejny przykład: analizator wyświetla mnóstwo ostrzeżeń w automatycznie generowanym pliku Options.inc. Spójrz na „ścianę” kodu, który zawiera:

Ta większość kodu wywołuje zalew ostrzeżeń:

  • V501 Istnieją identyczne wyrażenia podrzędne po lewej i po prawej stronie operatora „==”: nullptr == nullptr Options.inc 26
  • V501 Istnieją identyczne wyrażenia podrzędne po lewej i prawej stronie operatora ==: nullptr == nullptr Options.inc 27
  • V501 Tam są identycznymi wyrażeniami podrzędnymi po lewej i po prawej stronie operatora „==”: nullptr == nullptr Options.inc 28
  • i tak dalej – jedno ostrzeżenie w każdym wierszu…

Ale to nie jest wielka sprawa. można rozwiązać, wykluczając z analizy nieistotne pliki, zaznaczając określone makra i funkcje, wyłączając określone typy diagnostyczne i tak dalej. Tak, może, ale nie jest to zbyt interesująca praca podczas pisania artykułu. Dlatego zrobiłem to samo, co w artykule o sprawdzaniu kompilatora GCC: czytałem raport, aż zebrałem 11 interesujących przykładów do umieszczenia w artykule. Dlaczego 11? Pomyślałem, że skoro to 11-ta wersja Clanga, potrzebowałem 11 przykładów :).

11 podejrzanych fragmentów kodu

Fragment 1, operacja modulo na 1

To jest fajne! Lubię takie błędy!

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);
}
....
}

Komunikat diagnostyczny PVS-Studio: V1063 Operacja modulo by 1 jest bez znaczenia. Wynik zawsze będzie wynosił zero. llvm-stress.cpp 631

Programista używa operacji modulo, aby uzyskać losową wartość 0 lub 1. Jednak wartość 1 wydaje się mylić programistów i zmusić ich do napisania klasycznego wzorca anty-wzorca, w którym operacja modulo jest wykonywana na 1 zamiast na 2. Operacja X% 1 jest bez znaczenia, ponieważ zawsze daje 0 . Oto poprawiona wersja:

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

Niedawno dodana diagnostyka V1063 jest okropnie prosta, ale jak widać, działa doskonale.

Wiemy, że programiści kompilatorów pilnują naszej pracy i zapożyczają nasze pomysły. To jest w porządku. Miło wiedzieć, że PVS-Studio jest siłą napędową postępu . Zobaczmy, ile zajmie pojawienie się podobnej diagnostyki w Clang i GCC :).

Snippet 2, literówka w warunku

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;
}

Komunikat diagnostyczny PVS-Studio: V501 Istnieją identyczne wyrażenia podrzędne po lewej i po prawej stronie Operator & & operator:! T1.isNull () & &! T1.isNull () SemaOverload.cpp 9493

Sprawdzenie ! T1.isNull () jest wykonywane dwukrotnie. To oczywiście literówka; druga część warunku musi sprawdzać zmienną T2 .

Fragment 3, potencjalny indeks tablicy poza bounds

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();
....
}

Komunikat diagnostyczny PVS-Studio: V557 Przepełnienie tablicy jest możliwe. Indeks „Indeks” wskazuje poza granicę tablicy. ASTReader.cpp 7318

Załóżmy, że tablica przechowuje jeden element i wartość zmiennej Index również wynosi 1. Następnie warunek (1> 1) jest fałszem i dlatego tablica będzie indeksowana poza jej granicami. Oto poprawne sprawdzenie:

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

Snippet 4, kolejność oceny argumentów

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

Komunikat diagnostyczny PVS-Studio: V567 Nieokreślone zachowanie. Kolejność oceny argumentów nie jest zdefiniowana dla funkcji „addSection”. Rozważ sprawdzenie zmiennej „SecNo”. Object.cpp 1223

Zauważ, że argument SecNo jest używany dwukrotnie, aw międzyczasie jest zwiększany. Problem polega na tym, że nie można powiedzieć, w jakiej dokładnej kolejności argumenty będą oceniane. Wynik będzie zatem różny w zależności od wersji kompilatora lub parametrów kompilacji.

Oto syntetyczny przykład ilustrujący ten punkt:

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

W zależności od kompilatora, ten kod może wyświetlać „1, 2” lub „2, 1”. Uruchomiłem go w Compiler Explorer i otrzymałem następujące dane wyjściowe:

  • po skompilowaniu z Clang 11.0.0 program wyświetla 1 , 1.
  • po kompilacji z GCC 10.2, program wyjście s 2, 1.

Co ciekawe, ten prosty przypadek powoduje, że Clang generuje ostrzeżenie:

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

Z jakiegoś powodu to ostrzeżenie nie zostało jednak wydane na rzeczywistym kodzie. Albo jest wyłączony jako niezbyt praktyczny, albo ten przypadek jest zbyt skomplikowany, aby kompilator sobie z nim poradził.

Fragment 5, dziwny duplikat sprawdzenia

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;
}
....
}

Komunikat diagnostyczny PVS-Studio: V571 Sprawdzanie cykliczne. Warunek „if (! NameOrErr)” został już zweryfikowany w wierszu 4666. ELFDumper.cpp 4667

Drugie sprawdzenie jest klonem pierwszego i dlatego jest zbędne. Może da się go bezpiecznie usunąć. Ale bardziej prawdopodobne jest to, że zawiera literówkę i ma na celu sprawdzenie innej zmiennej.

Snippet 6, potencjalna dereferencja wskaźnika zerowego

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);
....
}

Komunikat diagnostyczny PVS-Studio: V595 Wskaźnik„ CDecl ”został użyty przed zweryfikowaniem go z wartością nullptr. Linie kontrolne: 5275, 5284. RewriteObjC.cpp 5275

Podczas wykonywania pierwszego sprawdzenia programista nigdy nie waha się wyłuskać wskaźnika CDecl :

if (CDecl->isImplicitInterfaceDecl())

Ale jeśli spojrzysz na kod kilka wierszy dalej, stanie się jasne, że wskaźnik może być pusty:

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

Pierwsze sprawdzenie prawdopodobnie miało wyglądać następująco:

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

Snippet 7, potencjalna dereferencja wskaźnika zerowego

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());
....
}

Komunikat diagnostyczny PVS-Studio: V595 Wskaźnik „ND” został użyty przed weryfikacją nullptr.Linie kontrolne: 2803, 2805. SemaTemplateInstantiate.cpp 2803

Ten błąd jest podobny do poprzedniego. Wyłuskiwanie wskaźnika bez uprzedniego sprawdzenia, gdy jego wartość jest uzyskiwana za pomocą rzutowania typu dynamicznego, jest niebezpieczne. Co więcej, kolejny kod potwierdza, że ​​takie sprawdzenie jest potrzebne.

Fragment 8, funkcja działająca pomimo stanu błędu

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();
....
}

Komunikat diagnostyczny PVS-Studio: V1004 Wskaźnik„ V ”został użyty w niebezpieczny sposób po zweryfikowaniu go pod kątem wartości nullptr. Linie kontrolne: 61, 65. TraceTests.cpp 65

Wskaźnik V może być wskaźnikiem zerowym. Jest to oczywiście stan błędu, który jest nawet zgłaszany wraz z komunikatem o błędzie. Ale funkcja będzie działać tak, jakby nic się nie stało i zakończy dereferencję tego bardzo zerowego wskaźnika. Programista prawdopodobnie chciał, aby funkcja zatrzymała się w tym momencie, w takim przypadku należy to naprawić w następujący sposób:

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, literówka

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); // <=
}
}

Komunikat diagnostyczny PVS-Studio: V1001 „T” zmienna jest przypisana, ale nie jest używana na końcu funkcji. CommonArgs.cpp 873

Spójrz na ostatnie wiersze funkcji. Zmienna lokalna T zmienia się, ale nie jest w żaden sposób używana. To musi być literówka, a funkcja powinna prawdopodobnie zakończyć się następująco:

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

Snippet 10, zero as dzielnik

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;
}
....
}

Komunikaty diagnostyczne PVS-Studio:

  • V609 Mod przez zero. Mianownik „d.s.low” == 0. udivmoddi4.c 61
  • V609 Podziel przez zero. Mianownik „d.s.low” == 0. udivmoddi4.c 62

Nie wiem, czy to błąd, czy jakieś podstępne urządzenie, ale kod wygląda dziwnie. Ma dwie zwykłe zmienne całkowite, z których jedna jest podzielona przez drugą. Ale interesujące jest to, że operacja dzielenia ma miejsce tylko wtedy, gdy obie zmienne są zerami. Jakie zadanie ma on wykonać?

Urywek 11, kopiuj-wklej

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;
}
....
}

Komunikat diagnostyczny PVS-Studio: V581 Wyrażenia warunkowe instrukcji „if” umieszczonych obok siebie są identyczne. Linie kontrolne: 3108, 3113. MallocChecker.cpp 3113

Fragment kodu został sklonowany, ale nigdy później nie został zmodyfikowany. Ten klon powinien zostać usunięty lub zmodyfikowany, aby przeprowadzić przydatne sprawdzenie.

Wniosek

Pamiętaj, że możesz użyć tej bezpłatnej licencji opcja , aby sprawdzić projekty open source. Udostępniamy również inne sposoby korzystania z PVS-Studio za darmo, niektóre z nich pozwalają nawet na analizę zastrzeżonego kodu. Zobacz pełną listę opcji tutaj: „ Sposoby uzyskania bezpłatnej licencji PVS-Studio ”. Dziękuję za przeczytanie!

Dodaj komentarz

Twój adres email nie zostanie opublikowany. Pola, których wypełnienie jest wymagane, są oznaczone symbolem *