Paghahanap ng mga bug sa LLVM 8 gamit ang PVS-Studio analyzer

Paghahanap ng mga bug sa LLVM 8 gamit ang PVS-Studio analyzer
Mahigit dalawang taon na ang nakalipas mula noong huling pagsusuri ng code ng proyekto ng LLVM gamit ang aming PVS-Studio analyzer. Siguraduhin natin na ang PVS-Studio analyzer ay isa pa ring nangungunang tool para sa pagtukoy ng mga error at potensyal na kahinaan. Upang gawin ito, susuriin at hahanapin namin ang mga bagong error sa paglabas ng LLVM 8.0.0.

Artikulo na isusulat

Sa totoo lang, hindi ko gustong isulat ang artikulong ito. Hindi kawili-wiling magsulat tungkol sa isang proyekto na ilang beses na naming nasuri (1, 2, 3). Mas mainam na magsulat tungkol sa isang bagong bagay, ngunit wala akong pagpipilian.

Sa bawat oras na ang isang bagong bersyon ng LLVM ay inilabas o ina-update Clang Static Analyzer, nakakatanggap kami ng mga tanong ng sumusunod na uri sa aming mail:

Tingnan, ang bagong bersyon ng Clang Static Analyzer ay natutong maghanap ng mga bagong error! Para sa akin, ang kaugnayan ng paggamit ng PVS-Studio ay bumababa. Nakahanap si Clang ng mas maraming error kaysa dati at naaabot ang mga kakayahan ng PVS-Studio. Ano sa tingin mo tungkol dito?

Dito gusto kong laging sagutin ang isang bagay tulad ng:

Hindi rin kami nakaupong walang ginagawa! Lubos naming napabuti ang mga kakayahan ng PVS-Studio analyzer. Kaya huwag mag-alala, patuloy kaming nangunguna gaya ng dati.

Sa kasamaang palad, ito ay isang masamang sagot. Walang mga patunay dito. At iyon ang dahilan kung bakit isinusulat ko ang artikulong ito ngayon. Kaya, ang proyekto ng LLVM ay muling nasuri at iba't ibang mga error ang natagpuan dito. Ipapakita ko ngayon ang mga tila kawili-wili sa akin. Hindi mahanap ng Clang Static Analyzer ang mga error na ito (o napakahirap gawin ito sa tulong nito). Pero kaya natin. Bukod dito, natagpuan at isinulat ko ang lahat ng mga pagkakamaling ito sa isang gabi.

Ngunit ang pagsulat ng artikulo ay tumagal ng ilang linggo. Hindi ko lang napigilan ang aking sarili na ilagay ang lahat ng ito sa teksto :).

Sa pamamagitan ng paraan, kung interesado ka sa kung anong mga teknolohiya ang ginagamit sa PVS-Studio analyzer upang matukoy ang mga error at potensyal na kahinaan, pagkatapos ay iminumungkahi kong kilalanin ito tandaan.

Bago at lumang mga diagnostic

Tulad ng nabanggit na, mga dalawang taon na ang nakalipas ang proyekto ng LLVM ay muling nasuri, at ang mga error na natagpuan ay naitama. Ngayon ang artikulong ito ay magpapakita ng bagong batch ng mga error. Bakit may nakitang mga bagong bug? Mayroong 3 dahilan para dito:

  1. Ang proyekto ng LLVM ay umuunlad, nagbabago ng lumang code at nagdaragdag ng bagong code. Natural, may mga bagong error sa binago at nakasulat na code. Ito ay malinaw na nagpapakita na ang static na pagsusuri ay dapat gamitin nang regular, at hindi paminsan-minsan. Ang aming mga artikulo ay mahusay na nagpapakita ng mga kakayahan ng PVS-Studio analyzer, ngunit ito ay walang kinalaman sa pagpapabuti ng kalidad ng code at pagbabawas ng gastos sa pag-aayos ng mga error. Gumamit ng static code analyzer nang regular!
  2. Tinatapos at pinapahusay namin ang mga kasalukuyang diagnostic. Samakatuwid, maaaring matukoy ng analyzer ang mga error na hindi nito napansin sa mga nakaraang pag-scan.
  3. Ang mga bagong diagnostic ay lumitaw sa PVS-Studio na hindi umiiral 2 taon na ang nakakaraan. Nagpasya akong i-highlight ang mga ito sa isang hiwalay na seksyon upang malinaw na ipakita ang pagbuo ng PVS-Studio.

Mga depekto na natukoy ng mga diagnostic na umiral 2 taon na ang nakakaraan

Fragment N1: Copy-Paste

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

Babala sa PVS-Studio: V501 [CWE-570] Mayroong magkaparehong mga sub-expression na 'Name.startswith("avx512.mask.permvar.")' sa kaliwa at sa kanan ng '||' operator. AutoUpgrade.cpp 73

Ito ay dobleng sinusuri na ang pangalan ay nagsisimula sa substring na "avx512.mask.permvar.". Sa pangalawang tseke, halatang may gusto silang isulat, ngunit nakalimutan nilang itama ang kinopyang teksto.

Fragment N2: Typo

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

Babala PVS-Studio: V501 Mayroong magkaparehong mga sub-expression na 'CXNameRange_WantQualifier' sa kaliwa at sa kanan ng '|' operator. CIindex.cpp 7245

Dahil sa isang typo, ang parehong pinangalanang constant ay ginagamit nang dalawang beses CXNameRange_WantQualifier.

Fragment N3: Pagkalito sa operator precedence

int PPCTTIImpl::getVectorInstrCost(unsigned Opcode, Type *Val, unsigned Index) {
  ....
  if (ISD == ISD::EXTRACT_VECTOR_ELT && Index == ST->isLittleEndian() ? 1 : 0)
    return 0;
  ....
}

Babala sa PVS-Studio: V502 [CWE-783] Marahil ang operator na '?:' ay gumagana sa ibang paraan kaysa sa inaasahan. Ang operator na '?:' ay may mas mababang priyoridad kaysa sa operator na '=='. PPCTargetTransformInfo.cpp 404

Sa aking palagay, ito ay isang napakagandang pagkakamali. Oo, alam kong may kakaiba akong ideya tungkol sa kagandahan :).

Ngayon, ayon sa mga prayoridad ng operator, ang expression ay sinusuri tulad ng sumusunod:

(ISD == ISD::EXTRACT_VECTOR_ELT && (Index == ST->isLittleEndian())) ? 1 : 0

Mula sa praktikal na pananaw, ang ganitong kondisyon ay hindi makatwiran, dahil maaari itong bawasan sa:

(ISD == ISD::EXTRACT_VECTOR_ELT && Index == ST->isLittleEndian())

Ito ay isang malinaw na pagkakamali. Malamang, gusto nilang ihambing ang 0/1 sa isang variable Index. Upang ayusin ang code kailangan mong magdagdag ng mga panaklong sa paligid ng ternary operator:

if (ISD == ISD::EXTRACT_VECTOR_ELT && Index == (ST->isLittleEndian() ? 1 : 0))

Sa pamamagitan ng paraan, ang ternary operator ay lubhang mapanganib at naghihikayat ng mga lohikal na pagkakamali. Maging maingat dito at huwag maging sakim sa mga panaklong. Tiningnan ko ang paksang ito nang mas detalyado dito, sa kabanata na β€œMag-ingat sa ?: Operator at Ilakip Ito sa Panaklong.”

Fragment N4, N5: Null pointer

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

Babala sa PVS-Studio: V522 [CWE-476] Maaaring maganap ang dereferencing ng null pointer na 'LHS'. TGParser.cpp 2152

Kung ang pointer LHS ay null, dapat magbigay ng babala. Gayunpaman, sa halip, ang parehong null pointer na ito ay aalisin sa sanggunian: LHS->getAsString().

Ito ay isang napaka-karaniwang sitwasyon kapag ang isang error ay nakatago sa isang error handler, dahil walang sinuman ang sumusubok sa kanila. Sinusuri ng mga static analyzer ang lahat ng naaabot na code, gaano man ito kadalas gamitin. Ito ay isang napakagandang halimbawa ng kung paano umaakma ang static na pagsusuri sa iba pang mga diskarte sa pagsubok at proteksyon ng error.

Katulad na error sa paghawak ng pointer RHS pinapayagan sa code sa ibaba lamang: V522 [CWE-476] Maaaring maganap ang dereferencing ng null pointer na 'RHS'. TGParser.cpp 2186

Fragment N6: Gamit ang pointer pagkatapos gumalaw

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

Babala ng PVS-Studio: V522 [CWE-476] Maaaring maganap ang dereferencing ng null pointer na 'ProgClone'. Miscompilation.cpp 601

Sa simula isang matalinong pointer ProgClone huminto sa pagmamay-ari ng bagay:

BD.setNewProgram(std::move(ProgClone));

Sa katunayan, ngayon ProgClone ay isang null pointer. Samakatuwid, ang isang null pointer dereference ay dapat mangyari sa ibaba lamang:

Function *NewF = ProgClone->getFunction(MisCompFunctions[i].first);

Ngunit, sa katotohanan, hindi ito mangyayari! Tandaan na ang loop ay hindi aktwal na naisakatuparan.

Sa simula ng lalagyan Miscompiled Functions na-clear:

MiscompiledFunctions.clear();

Susunod, ang laki ng container na ito ay ginagamit sa kondisyon ng loop:

for (unsigned i = 0, e = MisCompFunctions.size(); i != e; ++i) {

Madaling makita na ang loop ay hindi nagsisimula. Sa tingin ko ito ay isa ring bug at dapat na iba ang pagkakasulat ng code.

Mukhang na-encounter na natin ang sikat na parity of errors na iyon! Ang isang pagkakamali ay nagtatakip ng isa pa :).

Fragment N7: Gamit ang pointer pagkatapos gumalaw

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

Babala ng PVS-Studio: V522 [CWE-476] Maaaring maganap ang dereferencing ng null pointer na 'Test'. Miscompilation.cpp 709

Ang parehong sitwasyon muli. Sa una, ang mga nilalaman ng bagay ay inilipat, at pagkatapos ay ginagamit ito na parang walang nangyari. Nakikita ko ang sitwasyong ito nang higit at mas madalas sa code ng programa pagkatapos lumitaw ang mga semantika ng paggalaw sa C++. Ito ang dahilan kung bakit mahal ko ang C++ na wika! Parami nang parami ang mga bagong paraan upang mabaril ang iyong sariling paa. Ang PVS-Studio analyzer ay palaging may trabaho :).

Fragment N8: Null pointer

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

Babala ng PVS-Studio: V522 [CWE-476] Maaaring maganap ang dereferencing ng null pointer na 'Uri'. PrettyFunctionDumper.cpp 233

Bilang karagdagan sa mga humahawak ng error, kadalasang hindi sinusubok ang mga function ng pag-debug sa pag-print. Mayroon lamang tayong ganitong kaso sa harap natin. Ang pag-andar ay naghihintay para sa gumagamit, na, sa halip na lutasin ang kanyang mga problema, ay mapipilitang ayusin ito.

Tamang:

if (Type)
  Type->dump(*this);
else
  Printer << "<unknown-type>";

Fragment N9: Null pointer

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

Babala ng PVS-Studio: V522 [CWE-476] Maaaring maganap ang dereferencing ng null pointer na 'Ty'. SearchableTableEmitter.cpp 614

Sa tingin ko ang lahat ay malinaw at hindi nangangailangan ng paliwanag.

Fragment N10: Typo

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

Babala sa PVS-Studio: V570 Ang variable na 'Identifier->Uri' ay itinalaga sa sarili nito. FormatTokenLexer.cpp 249

Walang punto sa pagtatalaga ng variable sa sarili nito. Malamang na gusto nilang magsulat:

Identifier->Type = Question->Type;

Fragment N11: Kahina-hinalang break

void SystemZOperand::print(raw_ostream &OS) const {
  switch (Kind) {
    break;
  case KindToken:
    OS << "Token:" << getToken();
    break;
  case KindReg:
    OS << "Reg:" << SystemZInstPrinter::getRegisterName(getReg());
    break;
  ....
}

Babala sa PVS-Studio: V622 [CWE-478] Pag-isipang suriin ang pahayag ng 'switch'. Posibleng nawawala ang unang 'case' operator. SystemZAsmParser.cpp 652

May napakahinalang operator sa simula masira. Nakalimutan mo bang magsulat ng iba dito?

Fragment N12: Pagsusuri ng pointer pagkatapos ng dereferencing

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

Babala sa PVS-Studio: V595 [CWE-476] Ang 'Callee' pointer ay ginamit bago ito na-verify laban sa nullptr. Suriin ang mga linya: 172, 174. AMDGPUInline.cpp 172

Pointer Callee sa simula ay dereference sa oras na ang function ay tinatawag na getTTI.

At pagkatapos ay lumalabas na ang pointer na ito ay dapat suriin para sa pagkakapantay-pantay nullptr:

if (!Callee || Callee->isDeclaration())

Ngunit huli na...

Fragment N13 - N...: Sinusuri ang isang pointer pagkatapos ng dereferencing

Ang sitwasyong tinalakay sa nakaraang fragment ng code ay hindi natatangi. Ito ay lilitaw dito:

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()) {               // <=
  ....
}

Babala ng PVS-Studio: V595 [CWE-476] Ang 'CalleeFn' pointer ay ginamit bago ito na-verify laban sa nullptr. Suriin ang mga linya: 1079, 1081. SimplifyLibCalls.cpp 1079

At dito:

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

Babala ng PVS-Studio: V595 [CWE-476] Ang 'ND' pointer ay ginamit bago ito na-verify laban sa nullptr. Suriin ang mga linya: 532, 534. SemaTemplateInstantiateDecl.cpp 532

At dito:

  • V595 [CWE-476] Ang 'U' pointer ay ginamit bago ito na-verify laban sa nullptr. Suriin ang mga linya: 404, 407. DWARFormValue.cpp 404
  • V595 [CWE-476] Ang 'ND' pointer ay ginamit bago ito na-verify laban sa nullptr. Suriin ang mga linya: 2149, 2151. SemaTemplateInstantiate.cpp 2149

At pagkatapos ay naging hindi ako interesado sa pag-aaral ng mga babala na may numerong V595. Kaya hindi ko alam kung may mga katulad na error bukod sa mga nakalista dito. Malamang meron.

Fragment N17, N18: Kahina-hinalang shift

static inline bool processLogicalImmediate(uint64_t Imm, unsigned RegSize,
                                           uint64_t &Encoding) {
  ....
  unsigned Size = RegSize;
  ....
  uint64_t NImms = ~(Size-1) << 1;
  ....
}

Babala sa PVS-Studio: V629 [CWE-190] Isaalang-alang ang pag-inspeksyon sa '~(Size - 1) << 1' na expression. Bit shifting ng 32-bit na halaga na may kasunod na pagpapalawak sa 64-bit na uri. AArch64AddressingModes.h 260

Maaaring hindi ito isang bug at ang code ay gumagana nang eksakto tulad ng nilalayon. Ngunit ito ay malinaw na isang kahina-hinalang lugar at kailangang suriin.

Sabihin natin ang variable laki ay katumbas ng 16, at pagkatapos ay binalak ng may-akda ng code na makuha ito sa isang variable NImms halaga:

1111111111111111111111111111111111111111111111111111111111100000

Gayunpaman, sa katotohanan ang magiging resulta ay:

0000000000000000000000000000000011111111111111111111111111100000

Ang katotohanan ay ang lahat ng mga kalkulasyon ay nangyayari gamit ang 32-bit unsigned type. At pagkatapos lamang, ang 32-bit na hindi naka-sign na uri na ito ay tuwirang papalawakin sa uint64_t. Sa kasong ito, ang pinakamahalagang bit ay magiging zero.

Maaari mong ayusin ang sitwasyon tulad nito:

uint64_t NImms = ~static_cast<uint64_t>(Size-1) << 1;

Katulad na sitwasyon: V629 [CWE-190] Pag-isipang suriin ang expression na 'Immr << 6'. Bit shifting ng 32-bit na halaga na may kasunod na pagpapalawak sa 64-bit na uri. AArch64AddressingModes.h 269

Fragment N19: Nawawalang keyword iba?

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

Babala sa PVS-Studio: V646 [CWE-670] Isaalang-alang ang pag-inspeksyon sa lohika ng application. Posibleng nawawala ang 'ibang' keyword. AMDGPUAsmParser.cpp 5655

Walang mali dito. Mula noon-block ng una if nagtatapos sa magpatuloy, tapos di bale, may keyword iba o hindi. Sa alinmang paraan gagana ang code nang pareho. Nakakamiss pa rin iba ginagawang mas malabo at mapanganib ang code. Kung sa hinaharap magpatuloy mawala, ang code ay magsisimulang gumana nang ganap na naiiba. Sa aking palagay, mas mainam na magdagdag iba.

Fragment N20: Apat na typo ng parehong uri

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

Mga babala sa PVS-Studio:

  • V655 [CWE-480] Ang mga string ay pinagsama ngunit hindi ginagamit. Pag-isipang suriin ang expression na 'Result + Name.str()'. Simbolo.cpp 32
  • V655 [CWE-480] Ang mga string ay pinagsama ngunit hindi ginagamit. Pag-isipang suriin ang expression na 'Resulta + "(ObjC Class)" + Name.str()'. Simbolo.cpp 35
  • V655 [CWE-480] Ang mga string ay pinagsama ngunit hindi ginagamit. Pag-isipang suriin ang expression na 'Resulta + "(ObjC Class EH) " + Name.str()'. Simbolo.cpp 38
  • V655 [CWE-480] Ang mga string ay pinagsama ngunit hindi ginagamit. Pag-isipang suriin ang 'Resulta + "(ObjC IVar)" + Name.str()' na expression. Simbolo.cpp 41

Sa hindi sinasadya, ang + operator ang ginamit sa halip na ang += operator. Ang resulta ay mga disenyo na walang kahulugan.

Fragment N21: Hindi natukoy na pag-uugali

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

Subukang hanapin ang mapanganib na code sa iyong sarili. At ito ay isang larawan upang makaabala ng atensyon upang hindi agad tumingin sa sagot:

Paghahanap ng mga bug sa LLVM 8 gamit ang PVS-Studio analyzer

Babala sa PVS-Studio: V708 [CWE-758] Mapanganib na konstruksyon ang ginagamit: 'FeaturesMap[Op] = FeaturesMap.size()', kung saan ang 'FeaturesMap' ay nasa 'map' class. Ito ay maaaring humantong sa hindi natukoy na pag-uugali. RISCVCompressInstEmitter.cpp 490

linya ng problema:

FeaturesMap[Op] = FeaturesMap.size();

Kung elemento Op ay hindi natagpuan, pagkatapos ay isang bagong elemento ang nilikha sa mapa at ang bilang ng mga elemento sa mapang ito ay nakasulat doon. Hindi lang alam kung tatawagin ang function laki bago o pagkatapos magdagdag ng bagong elemento.

Fragment N22-N24: Mga paulit-ulit na takdang-aralin

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

Babala sa PVS-Studio: V519 [CWE-563] Ang variable na 'NType' ay itinalaga ang mga halaga nang dalawang beses nang sunud-sunod. Marahil ito ay isang pagkakamali. Suriin ang mga linya: 1663, 1664. MachOObjectFile.cpp 1664

Sa tingin ko ay wala talagang mali dito. Isang hindi kinakailangang paulit-ulit na assignment. Pero blunder pa rin.

Gayundin:

  • V519 [CWE-563] Ang variable na 'B.NDesc' ay itinalaga ang mga halaga nang dalawang beses nang sunud-sunod. Marahil ito ay isang pagkakamali. Suriin ang mga linya: 1488, 1489. llvm-nm.cpp 1489
  • V519 [CWE-563] Ang variable ay itinalaga ang mga halaga nang dalawang beses nang sunud-sunod. Marahil ito ay isang pagkakamali. Suriin ang mga linya: 59, 61. coff2yaml.cpp 61

Fragment N25-N27: Higit pang mga muling pagtatalaga

Ngayon tingnan natin ang isang bahagyang naiibang bersyon ng muling pagtatalaga.

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

Babala ng PVS-Studio: V519 [CWE-563] Ang variable na 'Alignment' ay itinalaga ang mga halaga nang dalawang beses nang sunud-sunod. Marahil ito ay isang pagkakamali. Suriin ang mga linya: 1158, 1160. LoadStoreVectorizer.cpp 1160

Ito ay isang kakaibang code na tila naglalaman ng isang lohikal na error. Sa simula, variable Pagkakahanay ang isang halaga ay itinalaga depende sa kundisyon. At pagkatapos ay naganap muli ang pagtatalaga, ngunit ngayon ay walang anumang tseke.

Ang mga katulad na sitwasyon ay makikita dito:

  • V519 [CWE-563] Ang variable na 'Mga Epekto' ay itinalaga ang mga halaga nang dalawang beses nang sunud-sunod. Marahil ito ay isang pagkakamali. Suriin ang mga linya: 152, 165. WebAssemblyRegStackify.cpp 165
  • V519 [CWE-563] Ang variable na 'ExpectNoDerefChunk' ay itinalaga ang mga halaga nang dalawang beses nang sunud-sunod. Marahil ito ay isang pagkakamali. Suriin ang mga linya: 4970, 4973. SemaType.cpp 4973

Fragment N28: Palaging totoong kondisyon

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

Babala sa PVS-Studio: V547 [CWE-571] Ang ekspresyong 'nextByte != 0x90' ay palaging totoo. X86DisassemblerDecoder.cpp 379

Walang saysay ang pagsuri. Variable susunod naByte palaging hindi katumbas ng halaga 0x90, na sumusunod mula sa nakaraang tseke. Ito ay isang uri ng lohikal na error.

Fragment N29 - N...: Palaging totoo/maling mga kundisyon

Ang analyzer ay nagbibigay ng maraming babala na ang buong kundisyon (V547) o bahagi nito (V560) ay palaging totoo o mali. Kadalasan ang mga ito ay hindi tunay na mga error, ngunit simpleng sloppy code, ang resulta ng macro expansion, at iba pa. Gayunpaman, makatuwirang tingnan ang lahat ng mga babalang ito, dahil ang mga tunay na lohikal na pagkakamali ay nangyayari paminsan-minsan. Halimbawa, ang seksyong ito ng code ay kahina-hinala:

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

Babala sa PVS-Studio: V560 [CWE-570] Ang isang bahagi ng conditional expression ay palaging mali: RegNo == 0xe. ARMDisassembler.cpp 939

Ang pare-parehong 0xE ay ang halagang 14 sa decimal. Pagsusulit RegNo == 0xe walang saysay dahil kung RegNo > 13, pagkatapos ay makukumpleto ng function ang pagpapatupad nito.

Mayroong maraming iba pang mga babala na may mga ID na V547 at V560, ngunit tulad ng sa V595, hindi ako interesado sa pag-aaral ng mga babalang ito. Malinaw na na mayroon akong sapat na materyal upang magsulat ng isang artikulo :). Samakatuwid, hindi alam kung gaano karaming mga error ng ganitong uri ang maaaring makilala sa LLVM gamit ang PVS-Studio.

Bibigyan kita ng halimbawa kung bakit nakakabagot ang pag-aaral ng mga trigger na ito. Ang analyzer ay ganap na tama sa pagbibigay ng babala para sa sumusunod na code. Ngunit hindi ito isang pagkakamali.

bool UnwrappedLineParser::parseBracedList(bool ContinueOnSemicolons,
                                          tok::TokenKind ClosingBraceKind) {
  bool HasError = false;
  ....
  HasError = true;
  if (!ContinueOnSemicolons)
    return !HasError;
  ....
}

Babala ng PVS-Studio: V547 [CWE-570] Ang ekspresyong '!HasError' ay palaging mali. UnwrappedLineParser.cpp 1635

Fragment N30: ​​Kahina-hinalang pagbabalik

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

Babala sa PVS-Studio: V612 [CWE-670] Isang walang kundisyong 'pagbabalik' sa loob ng isang loop. R600OptimizeVectorRegisters.cpp 63

Ito ay alinman sa isang error o isang tiyak na pamamaraan na nilayon upang ipaliwanag ang isang bagay sa mga programmer na nagbabasa ng code. Walang ipinapaliwanag sa akin ang disenyong ito at mukhang napakahinala. Buti na lang hindi ganyan ang pagsusulat :).

Pagod? Pagkatapos ay oras na upang gumawa ng tsaa o kape.

Paghahanap ng mga bug sa LLVM 8 gamit ang PVS-Studio analyzer

Mga depekto na natukoy ng mga bagong diagnostic

Sa tingin ko, sapat na ang 30 pag-activate ng mga lumang diagnostic. Tingnan natin ngayon kung anong mga kawili-wiling bagay ang makikita sa mga bagong diagnostic na lumabas sa analyzer pagkatapos nauna mga tseke. Sa panahong ito, kabuuang 66 na pangkalahatang layunin na diagnostic ang idinagdag sa C++ analyzer.

Fragment N31: Hindi maabot na 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();
}

Babala sa PVS-Studio: V779 [CWE-561] Natukoy ang hindi maabot na code. Posibleng may error. ExecutionUtils.cpp 146

Tulad ng nakikita mo, ang parehong mga sangay ng operator if nagtatapos sa isang tawag sa operator pagbabalik. Alinsunod dito, ang lalagyan CtorDtorsByPriority hindi kailanman malilinawan.

Fragment N32: Hindi maabot na 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;
}

Babala ng PVS-Studio: V779 [CWE-561] Natukoy ang hindi maabot na code. Posibleng may error. LLParser.cpp 835

Kawili-wiling sitwasyon. Tingnan muna natin ang lugar na ito:

return ParseTypeIdEntry(SummaryID);
break;

Sa unang tingin, parang walang mali dito. Parang operator masira may dagdag dito, at maaari mo lang itong tanggalin. Gayunpaman, hindi lahat ay napakasimple.

Ang analyzer ay nagbibigay ng babala sa mga linya:

Lex.setIgnoreColonInIdentifiers(false);
return false;

At sa katunayan, ang code na ito ay hindi maabot. Lahat ng kaso sa lumipat nagtatapos sa isang tawag mula sa operator pagbabalik. At ngayon walang kabuluhan mag-isa masira parang hindi naman harmless! Marahil ang isa sa mga sangay ay dapat magtapos sa masira, hindi sa pagbabalik?

Fragment N33: Random na pag-reset ng matataas na 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);
  ....
}

Babala sa PVS-Studio: V784 Ang laki ng bit mask ay mas mababa sa laki ng unang operand. Ito ay magiging sanhi ng pagkawala ng mas mataas na mga bit. RuntimeDyld.cpp 815

Mangyaring tandaan na ang function getStubAlignment uri ng pagbabalik hindi naka -ignign. Kalkulahin natin ang halaga ng expression, sa pag-aakalang ibabalik ng function ang halaga 8:

~(getStubAlignment() - 1)

~(8u-1)

0xFFFFFFFF8u

Ngayon pansinin na ang variable Laki ng Data ay may 64-bit na unsigned type. Lumalabas na kapag nagsasagawa ng DataSize at 0xFFFFFFF8u na operasyon, ang lahat ng tatlumpu't dalawang high-order bit ay ire-reset sa zero. Malamang, hindi ito ang gusto ng programmer. Pinaghihinalaan ko na gusto niyang kalkulahin: DataSize & 0xFFFFFFFFFFFFFFF8u.

Upang ayusin ang error, dapat mong isulat ito:

DataSize &= ~(static_cast<uint64_t>(getStubAlignment()) - 1);

O kaya:

DataSize &= ~(getStubAlignment() - 1ULL);

Fragment N34: Nabigo ang tahasang uri ng cast

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

Babala sa PVS-Studio: V1028 [CWE-190] Posibleng overflow. Isaalang-alang ang pag-cast ng mga operand ng operator na 'NumElts * Scale' sa uri ng 'size_t', hindi ang resulta. X86ISelLowering.h 1577

Ang tahasang uri ng pag-cast ay ginagamit upang maiwasan ang pag-apaw kapag nagpaparami ng mga variable ng uri int. Gayunpaman, ang tahasang uri ng pag-cast dito ay hindi nagpoprotekta laban sa pag-apaw. Una, ang mga variable ay paramihin, at pagkatapos lamang ang 32-bit na resulta ng pagpaparami ay lalawak sa uri laki_t.

Fragment N35: Nabigong Copy-Paste

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

V778 [CWE-682] Dalawang magkatulad na fragment ng code ang natagpuan. Marahil, ito ay isang typo at 'Op1' na variable ang dapat gamitin sa halip na 'Op0'. InstCombineCompares.cpp 5507

Tinutukoy ng bagong kawili-wiling diagnostic na ito ang mga sitwasyon kung saan nakopya ang isang piraso ng code at nagsimula nang baguhin ang ilang pangalan dito, ngunit sa isang lugar ay hindi nila ito naitama.

Pakitandaan na sa pangalawang bloke ay nagbago sila Op0 sa Op1. Ngunit sa isang lugar ay hindi nila ito naayos. Malamang na ito ay dapat na nakasulat tulad nito:

if (!match(Op1, m_PosZeroFP()) && isKnownNeverNaN(Op1, &TLI)) {
  I.setOperand(1, ConstantFP::getNullValue(Op1->getType()));
  return &I;
}

Fragment N36: Variable Confusion

struct Status {
  unsigned Mask;
  unsigned Mode;

  Status() : Mask(0), Mode(0){};

  Status(unsigned Mask, unsigned Mode) : Mask(Mask), Mode(Mode) {
    Mode &= Mask;
  };
  ....
};

Babala sa PVS-Studio: V1001 [CWE-563] Ang variable na 'Mode' ay itinalaga ngunit hindi ginagamit sa pagtatapos ng function. SIModeRegister.cpp 48

Napakadelikado na bigyan ang mga function argument ng parehong pangalan ng mga miyembro ng klase. Napakadaling malito. Mayroon lamang tayong ganitong kaso sa harap natin. Walang saysay ang ekspresyong ito:

Mode &= Mask;

Nagbabago ang argumento ng function. Iyon lang. Hindi na ginagamit ang argumentong ito. Malamang na dapat ay isinulat mo ito tulad nito:

Status(unsigned Mask, unsigned Mode) : Mask(Mask), Mode(Mode) {
  this->Mode &= Mask;
};

Fragment N37: Variable Confusion

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

Babala PVS-Studio: V1001 [CWE-563] Ang variable na 'Size' ay itinalaga ngunit hindi ginagamit sa pagtatapos ng function. Object.cpp 424

Ang sitwasyon ay katulad ng nauna. Dapat itong isulat:

this->Size += this->EntrySize;

Fragment N38-N47: Nakalimutan nilang suriin ang index

Noong nakaraan, tumingin kami sa mga halimbawa ng diagnostic na pag-trigger V595. Ang kakanyahan nito ay ang pointer ay na-dereference sa simula, at pagkatapos lamang ay nasuri. Mga batang diagnostic V1004 ay ang kabaligtaran sa kahulugan, ngunit nagpapakita rin ng maraming mga pagkakamali. Tinutukoy nito ang mga sitwasyon kung saan ang pointer ay nasuri sa simula at pagkatapos ay nakalimutang gawin ito. Tingnan natin ang mga ganitong kaso na matatagpuan sa loob ng LLVM.

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

Babala ng PVS-Studio: V1004 [CWE-476] Ang 'Ptr' pointer ay ginamit nang hindi ligtas pagkatapos itong ma-verify laban sa nullptr. Suriin ang mga linya: 729, 738. TargetTransformInfoImpl.h 738

Variable Ptr maaaring magkapantay nullptr, gaya ng pinatunayan ng tseke:

if (Ptr != nullptr)

Gayunpaman, sa ibaba ng pointer na ito ay dereference nang walang paunang pagsusuri:

auto PtrSizeBits = DL.getPointerTypeSizeInBits(Ptr->getType());

Isaalang-alang natin ang isa pang katulad na kaso.

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

Babala ng PVS-Studio: V1004 [CWE-476] Ang 'FD' pointer ay ginamit nang hindi ligtas pagkatapos itong ma-verify laban sa nullptr. Suriin ang mga linya: 3228, 3231. CGDebugInfo.cpp 3231

Bigyang-pansin ang tanda FD. Sigurado ako na ang problema ay malinaw na nakikita at walang espesyal na paliwanag ang kinakailangan.

At higit pa:

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

Babala ng PVS-Studio: V1004 [CWE-476] Ang 'PtrTy' pointer ay ginamit nang hindi ligtas pagkatapos itong ma-verify laban sa nullptr. Suriin ang mga linya: 960, 965. InterleavedLoadCombinePass.cpp 965

Paano protektahan ang iyong sarili mula sa gayong mga pagkakamali? Maging mas matulungin sa Code-Review at gamitin ang PVS-Studio static analyzer upang regular na suriin ang iyong code.

Walang kwenta ang pagbanggit ng iba pang mga fragment ng code na may mga ganitong uri ng mga error. Mag-iiwan lang ako ng listahan ng mga babala sa artikulo:

  • V1004 [CWE-476] Ang 'Expr' na pointer ay ginamit nang hindi ligtas pagkatapos itong ma-verify laban sa nullptr. Suriin ang mga linya: 1049, 1078. DebugInfoMetadata.cpp 1078
  • V1004 [CWE-476] Ang 'PI' pointer ay ginamit nang hindi ligtas pagkatapos itong ma-verify laban sa nullptr. Suriin ang mga linya: 733, 753. LegacyPassManager.cpp 753
  • V1004 [CWE-476] Ang 'StatepointCall' na pointer ay ginamit nang hindi ligtas pagkatapos itong ma-verify laban sa nullptr. Suriin ang mga linya: 4371, 4379. Verifier.cpp 4379
  • V1004 [CWE-476] Ang 'RV' pointer ay ginamit nang hindi ligtas pagkatapos itong ma-verify laban sa nullptr. Suriin ang mga linya: 2263, 2268. TGParser.cpp 2268
  • V1004 [CWE-476] Ang 'CalleeFn' pointer ay ginamit nang hindi ligtas pagkatapos itong ma-verify laban sa nullptr. Suriin ang mga linya: 1081, 1096. SimplifyLibCalls.cpp 1096
  • V1004 [CWE-476] Ang 'TC' pointer ay ginamit nang hindi ligtas pagkatapos itong ma-verify laban sa nullptr. Suriin ang mga linya: 1819, 1824. Driver.cpp 1824

Fragment N48-N60: Hindi kritikal, ngunit isang depekto (posibleng tumagas ang memorya)

std::unique_ptr<IRMutator> createISelMutator() {
  ....
  std::vector<std::unique_ptr<IRMutationStrategy>> Strategies;
  Strategies.emplace_back(
      new InjectorIRStrategy(InjectorIRStrategy::getDefaultOps()));
  ....
}

Babala sa PVS-Studio: V1023 [CWE-460] Ang isang pointer na walang may-ari ay idinaragdag sa lalagyan ng 'Mga Diskarte' sa pamamagitan ng pamamaraang 'emplace_back'. Ang isang memory leak ay magaganap sa kaso ng isang pagbubukod. llvm-isel-fuzzer.cpp 58

Upang magdagdag ng isang elemento sa dulo ng isang lalagyan tulad ng std::vector > hindi ka basta basta magsulat xxx.push_back(bagong X), dahil walang implicit na conversion mula sa X* Π² std::unique_ptr.

Ang isang karaniwang solusyon ay ang pagsusulat xxx.emplace_back(bagong X)dahil ito ay nag-compile: pamamaraan emplace_back gumagawa ng isang elemento nang direkta mula sa mga argumento nito at samakatuwid ay maaaring gumamit ng mga tahasang constructor.

Hindi ito ligtas. Kung puno na ang vector, muling ilalaan ang memorya. Maaaring mabigo ang pagpapatakbo ng pagsasauli ng memorya, na magreresulta sa isang pagbubukod na itinapon std::bad_alloc. Sa kasong ito, mawawala ang pointer at hindi matatanggal ang nilikhang bagay.

Ang isang ligtas na solusyon ay ang lumikha unique_ptrna magmamay-ari ng pointer bago subukan ng vector na muling italaga ang memorya:

xxx.push_back(std::unique_ptr<X>(new X))

Dahil C++14, maaari mong gamitin ang 'std::make_unique':

xxx.push_back(std::make_unique<X>())

Ang ganitong uri ng depekto ay hindi kritikal para sa LLVM. Kung hindi mailalaan ang memorya, hihinto lamang ang compiler. Gayunpaman, para sa mga application na may mahabang uptime, na hindi maaaring wakasan kung nabigo ang paglalaan ng memorya, maaari itong maging isang tunay na masamang bug.

Kaya, kahit na ang code na ito ay hindi nagbibigay ng praktikal na banta sa LLVM, nakita kong kapaki-pakinabang na pag-usapan ang pattern ng error na ito at natutunan ng PVS-Studio analyzer na kilalanin ito.

Iba pang mga babala ng ganitong uri:

  • V1023 [CWE-460] Ang isang pointer na walang may-ari ay idinaragdag sa lalagyan ng 'Passes' sa pamamagitan ng 'emplace_back' na paraan. Ang isang memory leak ay magaganap sa kaso ng isang pagbubukod. PassManager.h 546
  • V1023 [CWE-460] Ang isang pointer na walang may-ari ay idinaragdag sa 'AAs' container sa pamamagitan ng 'emplace_back' na paraan. Ang isang memory leak ay magaganap sa kaso ng isang pagbubukod. AlyasAnalysis.h 324
  • V1023 [CWE-460] Ang isang pointer na walang may-ari ay idinaragdag sa lalagyan ng 'Mga Entry' sa pamamagitan ng 'emplace_back' na paraan. Ang isang memory leak ay magaganap sa kaso ng isang pagbubukod. DWARFDebugFrame.cpp 519
  • V1023 [CWE-460] Ang isang pointer na walang may-ari ay idinaragdag sa 'AllEdges' na lalagyan sa pamamagitan ng 'emplace_back' na paraan. Ang isang memory leak ay magaganap sa kaso ng isang pagbubukod. CFGMST.h 268
  • V1023 [CWE-460] Ang isang pointer na walang may-ari ay idinaragdag sa 'VMaps' container sa pamamagitan ng 'emplace_back' na paraan. Ang isang memory leak ay magaganap sa kaso ng isang pagbubukod. SimpleLoopUnswitch.cpp 2012
  • V1023 [CWE-460] Ang isang pointer na walang may-ari ay idinaragdag sa lalagyan ng 'Mga Tala' sa pamamagitan ng pamamaraang 'emplace_back'. Ang isang memory leak ay magaganap sa kaso ng isang pagbubukod. FDRLogBuilder.h 30
  • V1023 [CWE-460] Ang isang pointer na walang may-ari ay idinaragdag sa 'PendingSubmodules' na lalagyan sa pamamagitan ng 'emplace_back' na paraan. Ang isang memory leak ay magaganap sa kaso ng isang pagbubukod. ModuleMap.cpp 810
  • V1023 [CWE-460] Ang isang pointer na walang may-ari ay idinaragdag sa lalagyan ng 'Mga Bagay' sa pamamagitan ng pamamaraang 'emplace_back'. Ang isang memory leak ay magaganap sa kaso ng isang pagbubukod. DebugMap.cpp 88
  • V1023 [CWE-460] Ang isang pointer na walang may-ari ay idinaragdag sa lalagyan ng 'Mga Diskarte' sa pamamagitan ng pamamaraang 'emplace_back'. Ang isang memory leak ay magaganap sa kaso ng isang pagbubukod. llvm-isel-fuzzer.cpp 60
  • V1023 [CWE-460] Ang isang pointer na walang may-ari ay idinaragdag sa lalagyan ng 'Mga Modifier' sa pamamagitan ng pamamaraang 'emplace_back'. Ang isang memory leak ay magaganap sa kaso ng isang pagbubukod. llvm-stress.cpp 685
  • V1023 [CWE-460] Ang isang pointer na walang may-ari ay idinaragdag sa lalagyan ng 'Mga Modifier' sa pamamagitan ng pamamaraang 'emplace_back'. Ang isang memory leak ay magaganap sa kaso ng isang pagbubukod. llvm-stress.cpp 686
  • V1023 [CWE-460] Ang isang pointer na walang may-ari ay idinaragdag sa lalagyan ng 'Mga Modifier' sa pamamagitan ng pamamaraang 'emplace_back'. Ang isang memory leak ay magaganap sa kaso ng isang pagbubukod. llvm-stress.cpp 688
  • V1023 [CWE-460] Ang isang pointer na walang may-ari ay idinaragdag sa lalagyan ng 'Mga Modifier' sa pamamagitan ng pamamaraang 'emplace_back'. Ang isang memory leak ay magaganap sa kaso ng isang pagbubukod. llvm-stress.cpp 689
  • V1023 [CWE-460] Ang isang pointer na walang may-ari ay idinaragdag sa lalagyan ng 'Mga Modifier' sa pamamagitan ng pamamaraang 'emplace_back'. Ang isang memory leak ay magaganap sa kaso ng isang pagbubukod. llvm-stress.cpp 690
  • V1023 [CWE-460] Ang isang pointer na walang may-ari ay idinaragdag sa lalagyan ng 'Mga Modifier' sa pamamagitan ng pamamaraang 'emplace_back'. Ang isang memory leak ay magaganap sa kaso ng isang pagbubukod. llvm-stress.cpp 691
  • V1023 [CWE-460] Ang isang pointer na walang may-ari ay idinaragdag sa lalagyan ng 'Mga Modifier' sa pamamagitan ng pamamaraang 'emplace_back'. Ang isang memory leak ay magaganap sa kaso ng isang pagbubukod. llvm-stress.cpp 692
  • V1023 [CWE-460] Ang isang pointer na walang may-ari ay idinaragdag sa lalagyan ng 'Mga Modifier' sa pamamagitan ng pamamaraang 'emplace_back'. Ang isang memory leak ay magaganap sa kaso ng isang pagbubukod. llvm-stress.cpp 693
  • V1023 [CWE-460] Ang isang pointer na walang may-ari ay idinaragdag sa lalagyan ng 'Mga Modifier' sa pamamagitan ng pamamaraang 'emplace_back'. Ang isang memory leak ay magaganap sa kaso ng isang pagbubukod. llvm-stress.cpp 694
  • V1023 [CWE-460] Ang isang pointer na walang may-ari ay idinaragdag sa lalagyan ng 'Operands' sa pamamagitan ng 'emplace_back' na paraan. Ang isang memory leak ay magaganap sa kaso ng isang pagbubukod. GlobalISelEmitter.cpp 1911
  • V1023 [CWE-460] Ang isang pointer na walang may-ari ay idinaragdag sa 'Stash' na lalagyan sa pamamagitan ng 'emplace_back' na paraan. Ang isang memory leak ay magaganap sa kaso ng isang pagbubukod. GlobalISelEmitter.cpp 2100
  • V1023 [CWE-460] Ang isang pointer na walang may-ari ay idinaragdag sa lalagyan ng 'Matchers' sa pamamagitan ng 'emplace_back' na paraan. Ang isang memory leak ay magaganap sa kaso ng isang pagbubukod. GlobalISelEmitter.cpp 2702

Konklusyon

Naglabas ako ng 60 na babala sa kabuuan at pagkatapos ay huminto. Mayroon bang iba pang mga depekto na nakita ng PVS-Studio analyzer sa LLVM? Oo meron ako. Gayunpaman, noong nagsusulat ako ng mga fragment ng code para sa artikulo, gabi na, o sa halip ay gabi, at napagpasyahan kong oras na para tawagan ito ng isang araw.

Umaasa ako na nakita mo itong kawili-wili at nais mong subukan ang PVS-Studio analyzer.

Maaari mong i-download ang analyzer at makuha ang minesweeper key sa ang pahinang ito.

Pinakamahalaga, regular na gumamit ng static na pagsusuri. Isang beses na mga tseke, na isinagawa sa amin upang gawing popular ang pamamaraan ng static na pagsusuri at ang PVS-Studio ay hindi isang normal na senaryo.

Good luck sa pagpapabuti ng kalidad at pagiging maaasahan ng iyong code!

Paghahanap ng mga bug sa LLVM 8 gamit ang PVS-Studio analyzer

Kung nais mong ibahagi ang artikulong ito sa isang madla na nagsasalita ng Ingles, mangyaring gamitin ang link ng pagsasalin: Andrey Karpov. Paghahanap ng Mga Bug sa LLVM 8 gamit ang PVS-Studio.

Pinagmulan: www.habr.com

Magdagdag ng komento