Er zijn meer dan twee jaar verstreken sinds de laatste codecontrole van het LLVM-project met behulp van onze PVS-Studio-analysator. Laten we ervoor zorgen dat de PVS-Studio-analysator nog steeds een toonaangevend hulpmiddel is voor het identificeren van fouten en potentiële kwetsbaarheden. Om dit te doen, zullen we nieuwe fouten in de LLVM 8.0.0-release controleren en vinden.
Artikel dat geschreven moet worden
Eerlijk gezegd had ik dit artikel niet willen schrijven. Het is niet interessant om te schrijven over een project dat we al verschillende keren hebben gecontroleerd (
Elke keer dat er een nieuwe versie van LLVM wordt uitgebracht of bijgewerkt
Kijk, de nieuwe versie van Clang Static Analyzer heeft geleerd nieuwe fouten te vinden! Het lijkt mij dat de relevantie van het gebruik van PVS-Studio afneemt. Clang vindt meer fouten dan voorheen en maakt kennis met de mogelijkheden van PVS-Studio. Wat vind je hiervan?
Hierop wil ik altijd iets antwoorden als:
Ook wij zitten niet stil! We hebben de mogelijkheden van de PVS-Studio-analysator aanzienlijk verbeterd. Maak je dus geen zorgen, we blijven leidinggeven zoals voorheen.
Helaas is dit een slecht antwoord. Er zitten geen bewijzen in. En daarom schrijf ik dit artikel nu. Het LLVM-project is dus opnieuw gecontroleerd en er zijn verschillende fouten in aangetroffen. Ik zal nu degenen demonstreren die mij interessant leken. Clang Static Analyzer kan deze fouten niet vinden (of het is uiterst lastig om dit met zijn hulp te doen). Maar we kunnen. Bovendien heb ik al deze fouten op één avond gevonden en opgeschreven.
Maar het schrijven van het artikel duurde enkele weken. Ik kon mezelf er gewoon niet toe brengen dit allemaal in tekst te verwerken :).
Trouwens, als je geïnteresseerd bent in welke technologieën in de PVS-Studio-analysator worden gebruikt om fouten en potentiële kwetsbaarheden te identificeren, dan raad ik aan om hiermee kennis te maken
Nieuwe en oude diagnostiek
Zoals reeds opgemerkt is ongeveer twee jaar geleden het LLVM-project opnieuw gecontroleerd en zijn de gevonden fouten gecorrigeerd. Nu zal dit artikel een nieuwe reeks fouten presenteren. Waarom zijn er nieuwe bugs gevonden? Hiervoor zijn 3 redenen:
- Het LLVM-project evolueert, waarbij oude code wordt gewijzigd en nieuwe code wordt toegevoegd. Uiteraard zitten er nieuwe fouten in de gewijzigde en geschreven code. Dit toont duidelijk aan dat statische analyse regelmatig moet worden gebruikt, en niet af en toe. Onze artikelen laten de mogelijkheden van de PVS-Studio-analysator goed zien, maar dit heeft niets te maken met het verbeteren van de codekwaliteit en het verlagen van de kosten voor het oplossen van fouten. Gebruik regelmatig een statische code-analysator!
- We finaliseren en verbeteren de bestaande diagnostiek. Daarom kan de analysator fouten identificeren die hij tijdens eerdere scans niet heeft opgemerkt.
- Er zijn nieuwe diagnostieken verschenen in PVS-Studio die 2 jaar geleden nog niet bestonden. Ik besloot ze in een aparte sectie uit te lichten om de ontwikkeling van PVS-Studio duidelijk te laten zien.
Defecten geïdentificeerd door diagnostiek die 2 jaar geleden bestond
Fragment N1: kopiëren en plakken
static bool ShouldUpgradeX86Intrinsic(Function *F, StringRef Name) {
if (Name == "addcarryx.u32" || // Added in 8.0
....
Name == "avx512.mask.cvtps2pd.128" || // Added in 7.0
Name == "avx512.mask.cvtps2pd.256" || // Added in 7.0
Name == "avx512.cvtusi2sd" || // Added in 7.0
Name.startswith("avx512.mask.permvar.") || // Added in 7.0 // <=
Name.startswith("avx512.mask.permvar.") || // Added in 7.0 // <=
Name == "sse2.pmulu.dq" || // Added in 7.0
Name == "sse41.pmuldq" || // Added in 7.0
Name == "avx2.pmulu.dq" || // Added in 7.0
....
}
PVS-Studio waarschuwing:
Er wordt dubbel gecontroleerd of de naam begint met de substring "avx512.mask.permvar.". Bij de tweede controle wilden ze uiteraard iets anders schrijven, maar vergaten ze de gekopieerde tekst te corrigeren.
Fragment N2: Typfout
enum CXNameRefFlags {
CXNameRange_WantQualifier = 0x1,
CXNameRange_WantTemplateArgs = 0x2,
CXNameRange_WantSinglePiece = 0x4
};
void AnnotateTokensWorker::HandlePostPonedChildCursor(
CXCursor Cursor, unsigned StartTokenIndex) {
const auto flags = CXNameRange_WantQualifier | CXNameRange_WantQualifier;
....
}
Waarschuwing PVS-Studio: V501 Er zijn identieke subexpressies 'CXNameRange_WantQualifier' links en rechts van de '|' exploitant. CIndex.cpp 7245
Door een typefout wordt dezelfde constante twee keer gebruikt CXNameRange_WantQualifier.
Fragment N3: Verwarring met voorrang van operator
int PPCTTIImpl::getVectorInstrCost(unsigned Opcode, Type *Val, unsigned Index) {
....
if (ISD == ISD::EXTRACT_VECTOR_ELT && Index == ST->isLittleEndian() ? 1 : 0)
return 0;
....
}
PVS-Studio waarschuwing:
Naar mijn mening is dit een heel mooie fout. Ja, ik weet dat ik vreemde ideeën heb over schoonheid :).
Nu, volgens
(ISD == ISD::EXTRACT_VECTOR_ELT && (Index == ST->isLittleEndian())) ? 1 : 0
Vanuit praktisch oogpunt heeft een dergelijke voorwaarde geen zin, omdat deze kan worden herleid tot:
(ISD == ISD::EXTRACT_VECTOR_ELT && Index == ST->isLittleEndian())
Dit is een duidelijke fout. Hoogstwaarschijnlijk wilden ze 0/1 vergelijken met een variabele Index. Om de code te corrigeren, moet je haakjes rond de ternaire operator toevoegen:
if (ISD == ISD::EXTRACT_VECTOR_ELT && Index == (ST->isLittleEndian() ? 1 : 0))
Trouwens, de ternaire operator is erg gevaarlijk en veroorzaakt logische fouten. Wees er heel voorzichtig mee en wees niet hebzuchtig met haakjes. Ik heb dit onderwerp nader bekeken
Fragment N4, N5: nulaanwijzer
Init *TGParser::ParseValue(Record *CurRec, RecTy *ItemType, IDParseMode Mode) {
....
TypedInit *LHS = dyn_cast<TypedInit>(Result);
....
LHS = dyn_cast<TypedInit>(
UnOpInit::get(UnOpInit::CAST, LHS, StringRecTy::get())
->Fold(CurRec));
if (!LHS) {
Error(PasteLoc, Twine("can't cast '") + LHS->getAsString() +
"' to string");
return nullptr;
}
....
}
PVS-Studio waarschuwing:
Als de wijzer LHS nul is, moet een waarschuwing worden gegeven. In plaats daarvan wordt echter van dezelfde null-pointer geen verwijzing meer gemaakt: LHS->getAsString().
Dit is een zeer typische situatie waarin een fout verborgen is in een foutafhandelaar, aangezien niemand deze test. Statische analysers controleren alle bereikbare code, ongeacht hoe vaak deze wordt gebruikt. Dit is een zeer goed voorbeeld van hoe statische analyse andere test- en foutbeschermingstechnieken aanvult.
Soortgelijke fout bij het afhandelen van de aanwijzer RHS toegestaan in de onderstaande code: V522 [CWE-476] Er kan een dereferentie van de null-pointer 'RHS' plaatsvinden. TGParser.cpp 2186
Fragment N6: Gebruik van de aanwijzer na verplaatsing
static Expected<bool>
ExtractBlocks(....)
{
....
std::unique_ptr<Module> ProgClone = CloneModule(BD.getProgram(), VMap);
....
BD.setNewProgram(std::move(ProgClone)); // <=
MiscompiledFunctions.clear();
for (unsigned i = 0, e = MisCompFunctions.size(); i != e; ++i) {
Function *NewF = ProgClone->getFunction(MisCompFunctions[i].first); // <=
assert(NewF && "Function not found??");
MiscompiledFunctions.push_back(NewF);
}
....
}
PVS-Studio-waarschuwing: V522 [CWE-476] Er kan een dereferentie van de null-pointer 'ProgClone' plaatsvinden. Miscompilatie.cpp 601
In het begin een slimme aanwijzer ProgKloon houdt op eigenaar te zijn van het object:
BD.setNewProgram(std::move(ProgClone));
Sterker nog, nu ProgKloon is een nulwijzer. Daarom zou een null pointer-dereferentie net onder moeten plaatsvinden:
Function *NewF = ProgClone->getFunction(MisCompFunctions[i].first);
Maar in werkelijkheid zal dit niet gebeuren! Merk op dat de lus niet daadwerkelijk wordt uitgevoerd.
Aan het begin van de container Verkeerd gecompileerde functies gewist:
MiscompiledFunctions.clear();
Vervolgens wordt de grootte van deze container gebruikt in de lusvoorwaarde:
for (unsigned i = 0, e = MisCompFunctions.size(); i != e; ++i) {
Het is gemakkelijk te zien dat de lus niet begint. Ik denk dat dit ook een bug is en dat de code anders moet worden geschreven.
Het lijkt erop dat we die beroemde pariteit van fouten zijn tegengekomen! De ene fout maskeert de andere :).
Fragment N7: Gebruik van de aanwijzer na verplaatsing
static Expected<bool> TestOptimizer(BugDriver &BD, std::unique_ptr<Module> Test,
std::unique_ptr<Module> Safe) {
outs() << " Optimizing functions being tested: ";
std::unique_ptr<Module> Optimized =
BD.runPassesOn(Test.get(), BD.getPassesToRun());
if (!Optimized) {
errs() << " Error running this sequence of passes"
<< " on the input program!n";
BD.setNewProgram(std::move(Test)); // <=
BD.EmitProgressBitcode(*Test, "pass-error", false); // <=
if (Error E = BD.debugOptimizerCrash())
return std::move(E);
return false;
}
....
}
PVS-Studio waarschuwing: V522 [CWE-476] Er kan een dereferentie van de nulpointer 'Test' plaatsvinden. Miscompilatie.cpp 709
Opnieuw dezelfde situatie. In eerste instantie wordt de inhoud van het object verplaatst en vervolgens gebruikt alsof er niets is gebeurd. Ik zie deze situatie steeds vaker in programmacode nadat de bewegingssemantiek in C++ verscheen. Dit is waarom ik van de C++-taal houd! Er zijn steeds meer nieuwe manieren om je eigen been eraf te schieten. De PVS-Studio-analysator heeft altijd werk :).
Fragment N8: nulwijzer
void FunctionDumper::dump(const PDBSymbolTypeFunctionArg &Symbol) {
uint32_t TypeId = Symbol.getTypeId();
auto Type = Symbol.getSession().getSymbolById(TypeId);
if (Type)
Printer << "<unknown-type>";
else
Type->dump(*this);
}
PVS-Studio waarschuwing: V522 [CWE-476] Er kan een dereferentie van de null pointer 'Type' plaatsvinden. PrettyFunctionDumper.cpp 233
Naast foutafhandelaars worden de debug-afdrukfuncties meestal niet getest. Wij hebben precies zo'n geval voor ons. De functie wacht op de gebruiker, die, in plaats van zijn problemen op te lossen, gedwongen zal worden deze op te lossen.
corrigeren:
if (Type)
Type->dump(*this);
else
Printer << "<unknown-type>";
Fragment N9: nulwijzer
void SearchableTableEmitter::collectTableEntries(
GenericTable &Table, const std::vector<Record *> &Items) {
....
RecTy *Ty = resolveTypes(Field.RecType, TI->getType());
if (!Ty) // <=
PrintFatalError(Twine("Field '") + Field.Name + "' of table '" +
Table.Name + "' has incompatible type: " +
Ty->getAsString() + " vs. " + // <=
TI->getType()->getAsString());
....
}
PVS-Studio waarschuwing: V522 [CWE-476] Er kan een dereferentie van de nulpointer 'Ty' plaatsvinden. SearchableTableEmitter.cpp 614
Ik denk dat alles duidelijk is en geen uitleg behoeft.
Fragment N10: Typfout
bool FormatTokenLexer::tryMergeCSharpNullConditionals() {
....
auto &Identifier = *(Tokens.end() - 2);
auto &Question = *(Tokens.end() - 1);
....
Identifier->ColumnWidth += Question->ColumnWidth;
Identifier->Type = Identifier->Type; // <=
Tokens.erase(Tokens.end() - 1);
return true;
}
PVS-Studio waarschuwing:
Het heeft geen zin om een variabele aan zichzelf toe te wijzen. Hoogstwaarschijnlijk wilden ze schrijven:
Identifier->Type = Question->Type;
Fragment N11: Verdachte breuk
void SystemZOperand::print(raw_ostream &OS) const {
switch (Kind) {
break;
case KindToken:
OS << "Token:" << getToken();
break;
case KindReg:
OS << "Reg:" << SystemZInstPrinter::getRegisterName(getReg());
break;
....
}
PVS-Studio waarschuwing:
Er is in het begin een zeer verdachte operator breken. Ben je vergeten hier nog iets te schrijven?
Fragment N12: Controle van een aanwijzer na dereferentie
InlineCost AMDGPUInliner::getInlineCost(CallSite CS) {
Function *Callee = CS.getCalledFunction();
Function *Caller = CS.getCaller();
TargetTransformInfo &TTI = TTIWP->getTTI(*Callee);
if (!Callee || Callee->isDeclaration())
return llvm::InlineCost::getNever("undefined callee");
....
}
PVS-Studio waarschuwing:
wijzer Calle aan het begin wordt de referentie verwijderd op het moment dat de functie wordt aangeroepen krijgTTI.
En dan blijkt dat deze aanwijzer op gelijkheid moet worden gecontroleerd nulptr:
if (!Callee || Callee->isDeclaration())
Maar het is te laat…
Fragment N13 - N...: Een aanwijzer controleren na dereferentie
De in het vorige codefragment besproken situatie is niet uniek. Het verschijnt hier:
static Value *optimizeDoubleFP(CallInst *CI, IRBuilder<> &B,
bool isBinary, bool isPrecise = false) {
....
Function *CalleeFn = CI->getCalledFunction();
StringRef CalleeNm = CalleeFn->getName(); // <=
AttributeList CalleeAt = CalleeFn->getAttributes();
if (CalleeFn && !CalleeFn->isIntrinsic()) { // <=
....
}
PVS-Studio waarschuwing: V595 [CWE-476] De 'CalleeFn'-aanwijzer werd gebruikt voordat deze werd geverifieerd tegen nullptr. Controleer lijnen: 1079, 1081. SimplifyLibCalls.cpp 1079
En hier:
void Sema::InstantiateAttrs(const MultiLevelTemplateArgumentList &TemplateArgs,
const Decl *Tmpl, Decl *New,
LateInstantiatedAttrVec *LateAttrs,
LocalInstantiationScope *OuterMostScope) {
....
NamedDecl *ND = dyn_cast<NamedDecl>(New);
CXXRecordDecl *ThisContext =
dyn_cast_or_null<CXXRecordDecl>(ND->getDeclContext()); // <=
CXXThisScopeRAII ThisScope(*this, ThisContext, Qualifiers(),
ND && ND->isCXXInstanceMember()); // <=
....
}
PVS-Studio waarschuwing: V595 [CWE-476] De 'ND'-aanwijzer werd gebruikt voordat deze werd geverifieerd tegen nullptr. Controleer regels: 532, 534. SemaTemplateInstantiateDecl.cpp 532
En hier:
- V595 [CWE-476] De 'U'-aanwijzer werd gebruikt voordat deze werd geverifieerd tegen nullptr. Controleer regels: 404, 407. DWARFormValue.cpp 404
- V595 [CWE-476] De 'ND'-aanwijzer werd gebruikt voordat deze werd geverifieerd tegen nullptr. Controleer regels: 2149, 2151. SemaTemplateInstantiate.cpp 2149
En toen raakte ik ongeïnteresseerd in het bestuderen van de waarschuwingen met nummer V595. Ik weet dus niet of er meer soortgelijke fouten zijn dan de fouten die hier worden vermeld. Hoogstwaarschijnlijk is dat zo.
Fragment N17, N18: Verdachte verschuiving
static inline bool processLogicalImmediate(uint64_t Imm, unsigned RegSize,
uint64_t &Encoding) {
....
unsigned Size = RegSize;
....
uint64_t NImms = ~(Size-1) << 1;
....
}
PVS-Studio waarschuwing:
Het is mogelijk geen bug en de code werkt precies zoals bedoeld. Maar dit is duidelijk een zeer verdachte plaats en moet worden gecontroleerd.
Laten we zeggen de variabele Maat is gelijk aan 16, en toen was de auteur van de code van plan om het in een variabele te krijgen Nimms betekenis:
1111111111111111111111111111111111111111111111111111111111100000
In werkelijkheid zal het resultaat echter zijn:
0000000000000000000000000000000011111111111111111111111111100000
Feit is dat alle berekeningen plaatsvinden met behulp van het 32-bits type zonder teken. En alleen dan zal dit 32-bits niet-ondertekende type impliciet worden uitgebreid naar uint64_t. In dit geval zullen de meest significante bits nul zijn.
U kunt de situatie als volgt oplossen:
uint64_t NImms = ~static_cast<uint64_t>(Size-1) << 1;
Soortgelijke situatie: V629 [CWE-190] Overweeg de expressie 'Immr << 6' te inspecteren. Bitverschuiving van de 32-bits waarde met een daaropvolgende uitbreiding naar het 64-bits type. AArch64AddressingModes.h 269
Fragment N19: Ontbrekend trefwoord anders?
void AMDGPUAsmParser::cvtDPP(MCInst &Inst, const OperandVector &Operands) {
....
if (Op.isReg() && Op.Reg.RegNo == AMDGPU::VCC) {
// VOP2b (v_add_u32, v_sub_u32 ...) dpp use "vcc" token.
// Skip it.
continue;
} if (isRegOrImmWithInputMods(Desc, Inst.getNumOperands())) { // <=
Op.addRegWithFPInputModsOperands(Inst, 2);
} else if (Op.isDPPCtrl()) {
Op.addImmOperands(Inst, 1);
} else if (Op.isImm()) {
// Handle optional arguments
OptionalIdx[Op.getImmTy()] = I;
} else {
llvm_unreachable("Invalid operand type");
}
....
}
PVS-Studio waarschuwing:
Er is hier geen sprake van een fout. Sinds het toenmalige blok van de eerste if eindigt met voortzetten, dan maakt het niet uit, er is een trefwoord anders of niet. Hoe dan ook, de code zal hetzelfde werken. Nog steeds gemist anders maakt de code onduidelijker en gevaarlijker. Als in de toekomst voortzetten verdwijnt, zal de code compleet anders gaan werken. Volgens mij is het beter om toe te voegen anders.
Fragment N20: Vier typefouten van hetzelfde type
LLVM_DUMP_METHOD void Symbol::dump(raw_ostream &OS) const {
std::string Result;
if (isUndefined())
Result += "(undef) ";
if (isWeakDefined())
Result += "(weak-def) ";
if (isWeakReferenced())
Result += "(weak-ref) ";
if (isThreadLocalValue())
Result += "(tlv) ";
switch (Kind) {
case SymbolKind::GlobalSymbol:
Result + Name.str(); // <=
break;
case SymbolKind::ObjectiveCClass:
Result + "(ObjC Class) " + Name.str(); // <=
break;
case SymbolKind::ObjectiveCClassEHType:
Result + "(ObjC Class EH) " + Name.str(); // <=
break;
case SymbolKind::ObjectiveCInstanceVariable:
Result + "(ObjC IVar) " + Name.str(); // <=
break;
}
OS << Result;
}
PVS-Studio-waarschuwingen:
- V655 [CWE-480] De strings zijn aaneengeschakeld, maar worden niet gebruikt. Overweeg de expressie 'Result + Name.str()' te inspecteren. Symbool.cpp 32
- V655 [CWE-480] De strings zijn aaneengeschakeld, maar worden niet gebruikt. Overweeg de expressie 'Result + "(ObjC Class)" + Name.str()' te inspecteren. Symbool.cpp 35
- V655 [CWE-480] De strings zijn aaneengeschakeld, maar worden niet gebruikt. Overweeg de expressie 'Result + "(ObjC Class EH) " + Name.str()' te inspecteren. Symbool.cpp 38
- V655 [CWE-480] De strings zijn aaneengeschakeld, maar worden niet gebruikt. Overweeg de expressie 'Result + "(ObjC IVar)" + Name.str()' te inspecteren. Symbool.cpp 41
Per ongeluk wordt de + operator gebruikt in plaats van de += operator. Het resultaat zijn ontwerpen zonder betekenis.
Fragment N21: Ongedefinieerd gedrag
static void getReqFeatures(std::map<StringRef, int> &FeaturesMap,
const std::vector<Record *> &ReqFeatures) {
for (auto &R : ReqFeatures) {
StringRef AsmCondString = R->getValueAsString("AssemblerCondString");
SmallVector<StringRef, 4> Ops;
SplitString(AsmCondString, Ops, ",");
assert(!Ops.empty() && "AssemblerCondString cannot be empty");
for (auto &Op : Ops) {
assert(!Op.empty() && "Empty operator");
if (FeaturesMap.find(Op) == FeaturesMap.end())
FeaturesMap[Op] = FeaturesMap.size();
}
}
}
Probeer zelf de gevaarlijke code te vinden. En dit is een foto om de aandacht af te leiden, zodat je niet meteen naar het antwoord kijkt:
PVS-Studio waarschuwing:
Probleemlijn:
FeaturesMap[Op] = FeaturesMap.size();
Als onderdeel Op wordt niet gevonden, dan wordt er een nieuw element op de kaart gemaakt en wordt het aantal elementen op deze kaart daar geschreven. Het is alleen onbekend of de functie zal worden aangeroepen grootte voor of na het toevoegen van een nieuw element.
Fragment N22-N24: Herhaalde opdrachten
Error MachOObjectFile::checkSymbolTable() const {
....
} else {
MachO::nlist STE = getSymbolTableEntry(SymDRI);
NType = STE.n_type; // <=
NType = STE.n_type; // <=
NSect = STE.n_sect;
NDesc = STE.n_desc;
NStrx = STE.n_strx;
NValue = STE.n_value;
}
....
}
PVS-Studio waarschuwing:
Ik denk niet dat er hier sprake is van een echte fout. Gewoon een onnodige herhaalde opdracht. Maar toch een blunder.
Evenzo:
- V519 [CWE-563] Aan de variabele 'B.NDesc' worden twee keer achter elkaar waarden toegewezen. Misschien is dit een vergissing. Controleer lijnen: 1488, 1489. llvm-nm.cpp 1489
- V519 [CWE-563] Aan de variabele worden twee keer achter elkaar waarden toegewezen. Misschien is dit een vergissing. Controleer regels: 59, 61. coff2yaml.cpp 61
Fragment N25-N27: Meer hertoewijzingen
Laten we nu eens kijken naar een iets andere versie van hertoewijzing.
bool Vectorizer::vectorizeLoadChain(
ArrayRef<Instruction *> Chain,
SmallPtrSet<Instruction *, 16> *InstructionsProcessed) {
....
unsigned Alignment = getAlignment(L0);
....
unsigned NewAlign = getOrEnforceKnownAlignment(L0->getPointerOperand(),
StackAdjustedAlignment,
DL, L0, nullptr, &DT);
if (NewAlign != 0)
Alignment = NewAlign;
Alignment = NewAlign;
....
}
PVS-Studio waarschuwing: V519 [CWE-563] Aan de variabele 'Alignment' worden twee keer achter elkaar waarden toegewezen. Misschien is dit een vergissing. Controleer regels: 1158, 1160. LoadStoreVectorizer.cpp 1160
Dit is een zeer vreemde code die blijkbaar een logische fout bevat. In het begin variabel Uitlijning afhankelijk van de voorwaarde wordt een waarde toegekend. En dan vindt de toewijzing opnieuw plaats, maar nu zonder enige controle.
Soortgelijke situaties zijn hier te zien:
- V519 [CWE-563] Aan de variabele 'Effecten' worden twee keer achter elkaar waarden toegewezen. Misschien is dit een vergissing. Controleer regels: 152, 165. WebAssemblyRegStackify.cpp 165
- V519 [CWE-563] Aan de variabele 'ExpectNoDerefChunk' worden tweemaal achtereen waarden toegewezen. Misschien is dit een vergissing. Controleer regels: 4970, 4973. SemaType.cpp 4973
Fragment N28: Altijd ware toestand
static int readPrefixes(struct InternalInstruction* insn) {
....
uint8_t byte = 0;
uint8_t nextByte;
....
if (byte == 0xf3 && (nextByte == 0x88 || nextByte == 0x89 ||
nextByte == 0xc6 || nextByte == 0xc7)) {
insn->xAcquireRelease = true;
if (nextByte != 0x90) // PAUSE instruction support // <=
break;
}
....
}
PVS-Studio waarschuwing:
Controleren heeft geen zin. Variabel volgendeByte altijd niet gelijk aan de waarde 0x90, die volgt uit de vorige controle. Dit is een soort logische fout.
Fragment N29 - N...: Altijd waar/onwaar-voorwaarden
De analysator geeft veel waarschuwingen dat de hele toestand (
static DecodeStatus DecodeGPRPairRegisterClass(MCInst &Inst, unsigned RegNo,
uint64_t Address, const void *Decoder) {
DecodeStatus S = MCDisassembler::Success;
if (RegNo > 13)
return MCDisassembler::Fail;
if ((RegNo & 1) || RegNo == 0xe)
S = MCDisassembler::SoftFail;
....
}
PVS-Studio waarschuwing:
De constante 0xE is de waarde 14 in decimalen. Inspectie Regnr == 0xe heeft geen zin, want als Regnr > 13, waarna de functie de uitvoering zal voltooien.
Er waren nog veel meer waarschuwingen met ID's V547 en V560, maar net als bij
Ik zal je een voorbeeld geven van waarom het bestuderen van deze triggers saai is. De analysator heeft volkomen gelijk als hij een waarschuwing geeft voor de volgende code. Maar dit is geen vergissing.
bool UnwrappedLineParser::parseBracedList(bool ContinueOnSemicolons,
tok::TokenKind ClosingBraceKind) {
bool HasError = false;
....
HasError = true;
if (!ContinueOnSemicolons)
return !HasError;
....
}
PVS-Studio-waarschuwing: V547 [CWE-570] Expressie '!HasError' is altijd onwaar. UnwrappedLineParser.cpp 1635
Fragment N30: Verdachte terugkeer
static bool
isImplicitlyDef(MachineRegisterInfo &MRI, unsigned Reg) {
for (MachineRegisterInfo::def_instr_iterator It = MRI.def_instr_begin(Reg),
E = MRI.def_instr_end(); It != E; ++It) {
return (*It).isImplicitDef();
}
....
}
PVS-Studio waarschuwing:
Dit is een fout of een specifieke techniek die bedoeld is om iets uit te leggen aan programmeurs die de code lezen. Dit ontwerp verklaart mij niets en ziet er erg verdacht uit. Het is beter om niet zo te schrijven :).
Moe? Dan is het tijd om thee of koffie te zetten.
Defecten geïdentificeerd door nieuwe diagnostiek
Ik denk dat 30 activeringen van oude diagnostiek voldoende zijn. Laten we nu eens kijken welke interessante dingen er kunnen worden gevonden met de nieuwe diagnostiek die daarna in de analysator verscheen
Fragment N31: onbereikbare code
Error CtorDtorRunner::run() {
....
if (auto CtorDtorMap =
ES.lookup(JITDylibSearchList({{&JD, true}}), std::move(Names),
NoDependenciesToRegister, true))
{
....
return Error::success();
} else
return CtorDtorMap.takeError();
CtorDtorsByPriority.clear();
return Error::success();
}
PVS-Studio waarschuwing:
Zoals u kunt zien, beide takken van de operator if eindigt met een oproep aan de telefoniste terugkeer. Dienovereenkomstig, de container CtorDtorsByPriority zal nooit worden opgeruimd.
Fragment N32: onbereikbare code
bool LLParser::ParseSummaryEntry() {
....
switch (Lex.getKind()) {
case lltok::kw_gv:
return ParseGVEntry(SummaryID);
case lltok::kw_module:
return ParseModuleEntry(SummaryID);
case lltok::kw_typeid:
return ParseTypeIdEntry(SummaryID); // <=
break; // <=
default:
return Error(Lex.getLoc(), "unexpected summary kind");
}
Lex.setIgnoreColonInIdentifiers(false); // <=
return false;
}
PVS-Studio waarschuwing: V779 [CWE-561] Onbereikbare code gedetecteerd. Het is mogelijk dat er een fout aanwezig is. LLPaser.cpp 835
Interessante situatie. Laten we eerst naar deze plek kijken:
return ParseTypeIdEntry(SummaryID);
break;
Op het eerste gezicht lijkt het erop dat hier geen sprake is van een fout. Het lijkt op de operator breken er is hier nog een extra, en je kunt deze eenvoudig verwijderen. Echter, niet allemaal zo eenvoudig.
De analysator geeft een waarschuwing op de regels:
Lex.setIgnoreColonInIdentifiers(false);
return false;
En inderdaad, deze code is onbereikbaar. Alle gevallen binnen schakelaar eindigt met een oproep van de telefoniste terugkeer. En nu zinloos alleen breken ziet er niet zo onschuldig uit! Misschien moet een van de takken eindigen op brekenmaar niet aan terugkeer?
Fragment N33: Willekeurige reset van hoge bits
unsigned getStubAlignment() override {
if (Arch == Triple::systemz)
return 8;
else
return 1;
}
Expected<unsigned>
RuntimeDyldImpl::emitSection(const ObjectFile &Obj,
const SectionRef &Section,
bool IsCode) {
....
uint64_t DataSize = Section.getSize();
....
if (StubBufSize > 0)
DataSize &= ~(getStubAlignment() - 1);
....
}
PVS-Studio waarschuwing:
Houd er rekening mee dat de functie getStubAlignment retourneert soort Ongesigneerd. Laten we de waarde van de uitdrukking berekenen, ervan uitgaande dat de functie de waarde 8 retourneert:
~(getStubAlignment() - 1)
~(8u-1)
0xFFFFFFFF8u
Merk nu op dat de variabele Gegevensgrootte heeft een 64-bits niet-ondertekend type. Het blijkt dat bij het uitvoeren van de DataSize & 0xFFFFFFF8u-bewerking alle tweeëndertig bits van hoge orde op nul worden teruggezet. Hoogstwaarschijnlijk is dit niet wat de programmeur wilde. Ik vermoed dat hij wilde berekenen: DataSize & 0xFFFFFFFFFFFFFFF8u.
Om de fout op te lossen, moet u dit schrijven:
DataSize &= ~(static_cast<uint64_t>(getStubAlignment()) - 1);
Of zo:
DataSize &= ~(getStubAlignment() - 1ULL);
Fragment N34: Expliciete typecast mislukt
template <typename T>
void scaleShuffleMask(int Scale, ArrayRef<T> Mask,
SmallVectorImpl<T> &ScaledMask) {
assert(0 < Scale && "Unexpected scaling factor");
int NumElts = Mask.size();
ScaledMask.assign(static_cast<size_t>(NumElts * Scale), -1);
....
}
PVS-Studio waarschuwing:
Expliciete typecasting wordt gebruikt om overflow te voorkomen bij het vermenigvuldigen van typevariabelen int. Expliciete typecasting biedt hier echter geen bescherming tegen overflow. Eerst worden de variabelen vermenigvuldigd en pas daarna wordt het 32-bits resultaat van de vermenigvuldiging uitgebreid naar het type
Fragment N35: Kopiëren en plakken mislukt
Instruction *InstCombiner::visitFCmpInst(FCmpInst &I) {
....
if (!match(Op0, m_PosZeroFP()) && isKnownNeverNaN(Op0, &TLI)) {
I.setOperand(0, ConstantFP::getNullValue(Op0->getType()));
return &I;
}
if (!match(Op1, m_PosZeroFP()) && isKnownNeverNaN(Op1, &TLI)) {
I.setOperand(1, ConstantFP::getNullValue(Op0->getType())); // <=
return &I;
}
....
}
Deze nieuwe interessante diagnose identificeert situaties waarin een stukje code is gekopieerd en sommige namen erin zijn veranderd, maar op één plek hebben ze het niet gecorrigeerd.
Houd er rekening mee dat ze in het tweede blok zijn gewijzigd Op0 op Op1. Maar op één plek hebben ze het niet opgelost. Waarschijnlijk had het zo geschreven moeten worden:
if (!match(Op1, m_PosZeroFP()) && isKnownNeverNaN(Op1, &TLI)) {
I.setOperand(1, ConstantFP::getNullValue(Op1->getType()));
return &I;
}
Fragment N36: Variabele verwarring
struct Status {
unsigned Mask;
unsigned Mode;
Status() : Mask(0), Mode(0){};
Status(unsigned Mask, unsigned Mode) : Mask(Mask), Mode(Mode) {
Mode &= Mask;
};
....
};
PVS-Studio waarschuwing:
Het is erg gevaarlijk om functieargumenten dezelfde naam te geven als klasseleden. Het is heel gemakkelijk om in de war te raken. Wij hebben precies zo'n geval voor ons. Deze uitdrukking slaat nergens op:
Mode &= Mask;
Het functieargument verandert. Dat is alles. Dit argument wordt niet meer gebruikt. Waarschijnlijk had je het zo moeten schrijven:
Status(unsigned Mask, unsigned Mode) : Mask(Mask), Mode(Mode) {
this->Mode &= Mask;
};
Fragment N37: Variabele verwarring
class SectionBase {
....
uint64_t Size = 0;
....
};
class SymbolTableSection : public SectionBase {
....
};
void SymbolTableSection::addSymbol(Twine Name, uint8_t Bind, uint8_t Type,
SectionBase *DefinedIn, uint64_t Value,
uint8_t Visibility, uint16_t Shndx,
uint64_t Size) {
....
Sym.Value = Value;
Sym.Visibility = Visibility;
Sym.Size = Size;
Sym.Index = Symbols.size();
Symbols.emplace_back(llvm::make_unique<Symbol>(Sym));
Size += this->EntrySize;
}
Waarschuwing PVS-Studio: V1001 [CWE-563] De variabele 'Grootte' is toegewezen, maar wordt aan het einde van de functie niet gebruikt. Object.cpp 424
De situatie is vergelijkbaar met de vorige. Er moet geschreven worden:
this->Size += this->EntrySize;
Fragment N38-N47: Ze vergaten de index te controleren
Eerder hebben we gekeken naar voorbeelden van diagnostische triggering
int getGEPCost(Type *PointeeType, const Value *Ptr,
ArrayRef<const Value *> Operands) {
....
if (Ptr != nullptr) { // <=
assert(....);
BaseGV = dyn_cast<GlobalValue>(Ptr->stripPointerCasts());
}
bool HasBaseReg = (BaseGV == nullptr);
auto PtrSizeBits = DL.getPointerTypeSizeInBits(Ptr->getType()); // <=
....
}
PVS-Studio waarschuwing: V1004 [CWE-476] De 'Ptr'-aanwijzer is op onveilige wijze gebruikt nadat deze was geverifieerd tegen nullptr. Controleer regels: 729, 738. TargetTransformInfoImpl.h 738
Variabel Ptr kan gelijk zijn nulptr, zoals blijkt uit de cheque:
if (Ptr != nullptr)
Onder deze aanwijzer wordt echter de referentie verwijderd zonder voorafgaande controle:
auto PtrSizeBits = DL.getPointerTypeSizeInBits(Ptr->getType());
Laten we een ander soortgelijk geval bekijken.
llvm::DISubprogram *CGDebugInfo::getFunctionFwdDeclOrStub(GlobalDecl GD,
bool Stub) {
....
auto *FD = dyn_cast<FunctionDecl>(GD.getDecl());
SmallVector<QualType, 16> ArgTypes;
if (FD) // <=
for (const ParmVarDecl *Parm : FD->parameters())
ArgTypes.push_back(Parm->getType());
CallingConv CC = FD->getType()->castAs<FunctionType>()->getCallConv(); // <=
....
}
PVS-Studio-waarschuwing: V1004 [CWE-476] De 'FD'-pointer is op onveilige wijze gebruikt nadat deze was geverifieerd tegen nullptr. Controleer regels: 3228, 3231. CGDebugInfo.cpp 3231
Let op het bord FD. Ik weet zeker dat het probleem duidelijk zichtbaar is en dat er geen speciale uitleg nodig is.
En verder:
static void computePolynomialFromPointer(Value &Ptr, Polynomial &Result,
Value *&BasePtr,
const DataLayout &DL) {
PointerType *PtrTy = dyn_cast<PointerType>(Ptr.getType());
if (!PtrTy) { // <=
Result = Polynomial();
BasePtr = nullptr;
}
unsigned PointerBits =
DL.getIndexSizeInBits(PtrTy->getPointerAddressSpace()); // <=
....
}
PVS-Studio waarschuwing: V1004 [CWE-476] De 'PtrTy'-aanwijzer werd op onveilige wijze gebruikt nadat deze was geverifieerd tegen nullptr. Controleer lijnen: 960, 965. InterleavedLoadCombinePass.cpp 965
Hoe kunt u uzelf tegen dergelijke fouten beschermen? Let beter op Code-Review en gebruik de statische analyser van PVS-Studio om uw code regelmatig te controleren.
Het heeft geen zin andere codefragmenten met dit soort fouten te citeren. Ik zal alleen een lijst met waarschuwingen in het artikel achterlaten:
- V1004 [CWE-476] De 'Expr'-aanwijzer werd op onveilige wijze gebruikt nadat deze was geverifieerd tegen nullptr. Controleer regels: 1049, 1078. DebugInfoMetadata.cpp 1078
- V1004 [CWE-476] De 'PI'-pointer werd op onveilige wijze gebruikt nadat deze was geverifieerd tegen nullptr. Controleer lijnen: 733, 753. LegacyPassManager.cpp 753
- V1004 [CWE-476] De 'StatepointCall'-pointer is op onveilige wijze gebruikt nadat deze was geverifieerd tegen nullptr. Controleer regels: 4371, 4379. Verifier.cpp 4379
- V1004 [CWE-476] De 'RV'-aanwijzer werd op onveilige wijze gebruikt nadat deze was geverifieerd tegen nullptr. Controleer lijnen: 2263, 2268. TGParser.cpp 2268
- V1004 [CWE-476] De 'CalleeFn'-aanwijzer werd op onveilige wijze gebruikt nadat deze was geverifieerd tegen nullptr. Controleer lijnen: 1081, 1096. SimplifyLibCalls.cpp 1096
- V1004 [CWE-476] De 'TC'-pointer werd op onveilige wijze gebruikt nadat deze was geverifieerd tegen nullptr. Controleer lijnen: 1819, 1824. Driver.cpp 1824
Fragment N48-N60: Niet kritisch, maar een defect (mogelijk geheugenlek)
std::unique_ptr<IRMutator> createISelMutator() {
....
std::vector<std::unique_ptr<IRMutationStrategy>> Strategies;
Strategies.emplace_back(
new InjectorIRStrategy(InjectorIRStrategy::getDefaultOps()));
....
}
PVS-Studio waarschuwing:
Om een element aan het einde van een container toe te voegen, zoals std::vector > je kunt niet zomaar schrijven xxx.push_back(nieuwe X), aangezien er geen impliciete conversie is van X* в std::unieke_ptr.
Een veel voorkomende oplossing is schrijven xxx.emplace_back(nieuwe X)sinds het compileert: method emplace_terug construeert een element rechtstreeks uit zijn argumenten en kan daarom expliciete constructors gebruiken.
Het is niet veilig. Als de vector vol is, wordt het geheugen opnieuw toegewezen. De bewerking voor het opnieuw toewijzen van geheugen mislukt mogelijk, waardoor er een uitzondering wordt gegenereerd std::bad_alloc. In dit geval gaat de aanwijzer verloren en wordt het gemaakte object nooit verwijderd.
Een veilige oplossing is creëren uniek_ptrdie de aanwijzer zal bezitten voordat de vector het geheugen opnieuw probeert toe te wijzen:
xxx.push_back(std::unique_ptr<X>(new X))
Sinds C++14 kun je 'std::make_unique' gebruiken:
xxx.push_back(std::make_unique<X>())
Dit type defect is niet kritisch voor LLVM. Als er geen geheugen kan worden toegewezen, stopt de compiler gewoon. Echter, voor toepassingen met lange
Dus hoewel deze code geen praktische bedreiging vormt voor LLVM, vond ik het nuttig om over dit foutpatroon te praten en dat de PVS-Studio-analysator heeft geleerd dit te identificeren.
Andere waarschuwingen van dit type:
- V1023 [CWE-460] Een pointer zonder eigenaar wordt toegevoegd aan de 'Passes'-container door de 'emplace_back'-methode. In het geval van een uitzondering zal er een geheugenlek optreden. PassManager.h 546
- V1023 [CWE-460] Een pointer zonder eigenaar wordt toegevoegd aan de 'AAs'-container door de 'emplace_back'-methode. In het geval van een uitzondering zal er een geheugenlek optreden. AliasAnalysis.h 324
- V1023 [CWE-460] Er wordt een pointer zonder eigenaar toegevoegd aan de 'Entries'-container door de 'emplace_back'-methode. In het geval van een uitzondering zal er een geheugenlek optreden. DWARFDebugFrame.cpp 519
- V1023 [CWE-460] Een pointer zonder eigenaar wordt toegevoegd aan de 'AllEdges'-container door de 'emplace_back'-methode. In het geval van een uitzondering zal er een geheugenlek optreden. CFGMST.h 268
- V1023 [CWE-460] Een pointer zonder eigenaar wordt toegevoegd aan de 'VMaps'-container door de 'emplace_back'-methode. In het geval van een uitzondering zal er een geheugenlek optreden. SimpleLoopUnswitch.cpp 2012
- V1023 [CWE-460] Een pointer zonder eigenaar wordt toegevoegd aan de 'Records'-container door de 'emplace_back'-methode. In het geval van een uitzondering zal er een geheugenlek optreden. FDRLogBuilder.h 30
- V1023 [CWE-460] Een pointer zonder eigenaar wordt toegevoegd aan de 'PendingSubmodules' container door de 'emplace_back' methode. In het geval van een uitzondering zal er een geheugenlek optreden. ModuleMap.cpp 810
- V1023 [CWE-460] Een pointer zonder eigenaar wordt toegevoegd aan de 'Objects'-container door de 'emplace_back'-methode. In het geval van een uitzondering zal er een geheugenlek optreden. DebugMap.cpp 88
- V1023 [CWE-460] Een pointer zonder eigenaar wordt toegevoegd aan de 'Strategies'-container door de 'emplace_back'-methode. In het geval van een uitzondering zal er een geheugenlek optreden. llvm-isel-fuzzer.cpp 60
- V1023 [CWE-460] Een pointer zonder eigenaar wordt toegevoegd aan de 'Modifiers'-container door de 'emplace_back'-methode. In het geval van een uitzondering zal er een geheugenlek optreden. llvm-stress.cpp 685
- V1023 [CWE-460] Een pointer zonder eigenaar wordt toegevoegd aan de 'Modifiers'-container door de 'emplace_back'-methode. In het geval van een uitzondering zal er een geheugenlek optreden. llvm-stress.cpp 686
- V1023 [CWE-460] Een pointer zonder eigenaar wordt toegevoegd aan de 'Modifiers'-container door de 'emplace_back'-methode. In het geval van een uitzondering zal er een geheugenlek optreden. llvm-stress.cpp 688
- V1023 [CWE-460] Een pointer zonder eigenaar wordt toegevoegd aan de 'Modifiers'-container door de 'emplace_back'-methode. In het geval van een uitzondering zal er een geheugenlek optreden. llvm-stress.cpp 689
- V1023 [CWE-460] Een pointer zonder eigenaar wordt toegevoegd aan de 'Modifiers'-container door de 'emplace_back'-methode. In het geval van een uitzondering zal er een geheugenlek optreden. llvm-stress.cpp 690
- V1023 [CWE-460] Een pointer zonder eigenaar wordt toegevoegd aan de 'Modifiers'-container door de 'emplace_back'-methode. In het geval van een uitzondering zal er een geheugenlek optreden. llvm-stress.cpp 691
- V1023 [CWE-460] Een pointer zonder eigenaar wordt toegevoegd aan de 'Modifiers'-container door de 'emplace_back'-methode. In het geval van een uitzondering zal er een geheugenlek optreden. llvm-stress.cpp 692
- V1023 [CWE-460] Een pointer zonder eigenaar wordt toegevoegd aan de 'Modifiers'-container door de 'emplace_back'-methode. In het geval van een uitzondering zal er een geheugenlek optreden. llvm-stress.cpp 693
- V1023 [CWE-460] Een pointer zonder eigenaar wordt toegevoegd aan de 'Modifiers'-container door de 'emplace_back'-methode. In het geval van een uitzondering zal er een geheugenlek optreden. llvm-stress.cpp 694
- V1023 [CWE-460] Een pointer zonder eigenaar wordt toegevoegd aan de 'Operands'-container door de 'emplace_back'-methode. In het geval van een uitzondering zal er een geheugenlek optreden. GlobalISelEmitter.cpp 1911
- V1023 [CWE-460] Een pointer zonder eigenaar wordt toegevoegd aan de 'Stash'-container door de 'emplace_back'-methode. In het geval van een uitzondering zal er een geheugenlek optreden. GlobalISelEmitter.cpp 2100
- V1023 [CWE-460] Een pointer zonder eigenaar wordt toegevoegd aan de 'Matchers'-container door de 'emplace_back'-methode. In het geval van een uitzondering zal er een geheugenlek optreden. GlobalISelEmitter.cpp 2702
Conclusie
Ik heb in totaal 60 waarschuwingen gegeven en ben toen gestopt. Zijn er andere defecten die de PVS-Studio-analysator in LLVM detecteert? Ja ik heb. Toen ik echter codefragmenten voor het artikel aan het schrijven was, was het laat in de avond, of beter gezegd zelfs nacht, en besloot ik dat het tijd was om er een einde aan te maken.
Ik hoop dat je het interessant vond en dat je de PVS-Studio-analysator wilt proberen.
Je kunt de analysator downloaden en de mijnenvegersleutel verkrijgen op
Het belangrijkste is dat u regelmatig statische analyses gebruikt. Eenmalige controles, door ons uitgevoerd om de methodologie van statische analyse en PVS-Studio populair te maken, zijn geen normaal scenario.
Veel succes met het verbeteren van de kwaliteit en betrouwbaarheid van uw code!
Als u dit artikel met een Engelssprekend publiek wilt delen, gebruik dan de vertaallink: Andrey Karpov.
Bron: www.habr.com