ค้นหาข้อบกพร่องใน LLVM 8 โดยใช้ตัววิเคราะห์ PVS-Studio

ค้นหาข้อบกพร่องใน LLVM 8 โดยใช้ตัววิเคราะห์ PVS-Studio
เวลาผ่านไปกว่าสองปีแล้วนับตั้งแต่การตรวจสอบโค้ดล่าสุดของโปรเจ็กต์ LLVM โดยใช้เครื่องวิเคราะห์ PVS-Studio ของเรา มาตรวจสอบให้แน่ใจว่าเครื่องวิเคราะห์ PVS-Studio ยังคงเป็นเครื่องมือชั้นนำในการระบุข้อผิดพลาดและช่องโหว่ที่อาจเกิดขึ้น เมื่อต้องการทำเช่นนี้ เราจะตรวจสอบและค้นหาข้อผิดพลาดใหม่ในรุ่น LLVM 8.0.0

บทความที่จะเขียน

พูดตามตรงฉันไม่อยากเขียนบทความนี้ การเขียนเกี่ยวกับโครงการที่เราตรวจสอบมาแล้วหลายครั้งไม่น่าสนใจ (1, 2, 3). เขียนเกี่ยวกับสิ่งใหม่ๆ ดีกว่า แต่ฉันไม่มีทางเลือก

ทุกครั้งที่มีการเปิดตัวหรืออัปเดต LLVM เวอร์ชันใหม่ เครื่องวิเคราะห์เสียงดังกราวคงที่เราได้รับคำถามประเภทต่อไปนี้ทางไปรษณีย์ของเรา:

ดูสิ Clang Static Analyzer เวอร์ชันใหม่ได้เรียนรู้ที่จะค้นหาข้อผิดพลาดใหม่! สำหรับฉันดูเหมือนว่าความเกี่ยวข้องของการใช้ PVS-Studio กำลังลดลง Clang พบข้อผิดพลาดมากกว่าเดิมและตามทันความสามารถของ PVS-Studio คุณคิดอย่างไรเกี่ยวกับเรื่องนี้?

ฉันมักจะต้องการตอบคำถามเช่น:

เราไม่นั่งเฉยๆเช่นกัน! เราได้ปรับปรุงความสามารถของเครื่องวิเคราะห์ PVS-Studio อย่างมาก ไม่ต้องกังวลเรายังคงเป็นผู้นำเหมือนเดิม

น่าเสียดายที่นี่เป็นคำตอบที่ไม่ดี ไม่มีข้อพิสูจน์ในนั้น และนั่นคือเหตุผลที่ฉันเขียนบทความนี้ตอนนี้ ดังนั้นจึงมีการตรวจสอบโครงการ LLVM อีกครั้งและพบข้อผิดพลาดหลายประการ ตอนนี้ฉันจะสาธิตสิ่งที่น่าสนใจสำหรับฉัน Clang Static Analyzer ไม่พบข้อผิดพลาดเหล่านี้ (หรือไม่สะดวกอย่างยิ่งที่จะทำเช่นนั้น) แต่เราทำได้ ยิ่งไปกว่านั้น ฉันพบและจดข้อผิดพลาดเหล่านี้ไว้ในเย็นวันหนึ่ง

แต่การเขียนบทความใช้เวลาหลายสัปดาห์ ฉันไม่สามารถพาตัวเองใส่ทั้งหมดนี้ลงในข้อความได้ :)

อย่างไรก็ตาม หากคุณสนใจว่าเทคโนโลยีใดที่ใช้ในเครื่องวิเคราะห์ PVS-Studio เพื่อระบุข้อผิดพลาดและช่องโหว่ที่อาจเกิดขึ้น ฉันขอแนะนำให้ทำความคุ้นเคยกับสิ่งนี้ บันทึก.

การวินิจฉัยใหม่และเก่า

ตามที่ระบุไว้แล้ว เมื่อประมาณสองปีที่แล้วโครงการ LLVM ได้รับการตรวจสอบอีกครั้ง และข้อผิดพลาดที่พบได้รับการแก้ไขแล้ว ตอนนี้บทความนี้จะนำเสนอข้อผิดพลาดชุดใหม่ เหตุใดจึงพบข้อบกพร่องใหม่ มี 3 เหตุผลสำหรับสิ่งนี้:

  1. โครงการ LLVM กำลังพัฒนา โดยเปลี่ยนโค้ดเก่าและเพิ่มโค้ดใหม่ โดยปกติแล้วจะมีข้อผิดพลาดใหม่ในโค้ดที่แก้ไขและเขียน สิ่งนี้แสดงให้เห็นอย่างชัดเจนว่าควรใช้การวิเคราะห์แบบคงที่อย่างสม่ำเสมอ ไม่ใช่เป็นครั้งคราว บทความของเราแสดงให้เห็นอย่างดีถึงความสามารถของเครื่องวิเคราะห์ PVS-Studio แต่สิ่งนี้ไม่เกี่ยวข้องกับการปรับปรุงคุณภาพโค้ดและลดต้นทุนในการแก้ไขข้อผิดพลาด ใช้ตัววิเคราะห์โค้ดแบบคงที่เป็นประจำ!
  2. เรากำลังสรุปและปรับปรุงการวินิจฉัยที่มีอยู่ ดังนั้น เครื่องวิเคราะห์จึงสามารถระบุข้อผิดพลาดที่ไม่ได้สังเกตเห็นระหว่างการสแกนครั้งก่อนได้
  3. การวินิจฉัยใหม่ปรากฏใน PVS-Studio ที่ไม่มีเมื่อ 2 ปีที่แล้ว ฉันตัดสินใจเน้นมันในส่วนแยกต่างหากเพื่อแสดงการพัฒนาของ PVS-Studio อย่างชัดเจน

ข้อบกพร่องที่ระบุโดยการวินิจฉัยที่มีอยู่เมื่อ 2 ปีที่แล้ว

แฟรกเมนต์ N1: คัดลอก-วาง

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-สตูดิโอ: V501 [CWE-570] มีนิพจน์ย่อยที่เหมือนกัน 'Name.startswith("avx512.mask.permvar.")' ทางด้านซ้ายและทางขวาของ '||' ตัวดำเนินการ อัปเกรดอัตโนมัติ.cpp 73

มีการตรวจสอบซ้ำอีกครั้งว่าชื่อขึ้นต้นด้วยสตริงย่อย "avx512.mask.permvar" ในการตรวจสอบครั้งที่สอง เห็นได้ชัดว่าพวกเขาต้องการเขียนอย่างอื่น แต่ลืมแก้ไขข้อความที่คัดลอก

แฟรกเมนต์ N2: พิมพ์ผิด

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

คำเตือน PVS-Studio: V501 มีนิพจน์ย่อยที่เหมือนกัน 'CXNameRange_WantQualifier' ทางด้านซ้ายและทางด้านขวาของ '|' ตัวดำเนินการ CIndex.cpp 7245

เนื่องจากการพิมพ์ผิด จึงมีการใช้ค่าคงที่ที่มีชื่อเดียวกันสองครั้ง CXNameRange_WantQualifier.

แฟรกเมนต์ N3: ความสับสนกับลำดับความสำคัญของตัวดำเนินการ

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

คำเตือน PVS-สตูดิโอ: V502 [CWE-783] บางทีตัวดำเนินการ '?:' อาจทำงานในลักษณะที่แตกต่างจากที่คาดไว้ ตัวดำเนินการ '?:' มีลำดับความสำคัญต่ำกว่าตัวดำเนินการ '==' PPCTargetTransformInfo.cpp 404

ในความคิดของฉัน นี่เป็นข้อผิดพลาดที่สวยงามมาก ใช่ ฉันรู้ว่าฉันมีไอเดียแปลกๆ เกี่ยวกับความงาม :)

ตอนนี้ตาม ลำดับความสำคัญของผู้ปฏิบัติงานนิพจน์ได้รับการประเมินดังนี้:

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

จากมุมมองเชิงปฏิบัติ เงื่อนไขดังกล่าวไม่สมเหตุสมผล เนื่องจากสามารถลดลงเป็น:

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

นี่เป็นข้อผิดพลาดที่ชัดเจน เป็นไปได้มากว่าพวกเขาต้องการเปรียบเทียบ 0/1 กับตัวแปร ดัชนี. ในการแก้ไขโค้ด คุณต้องเพิ่มวงเล็บล้อมรอบตัวดำเนินการแบบไตรภาค:

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

อย่างไรก็ตามตัวดำเนินการที่ประกอบด้วยสามนั้นอันตรายมากและกระตุ้นให้เกิดข้อผิดพลาดเชิงตรรกะ ระวังให้มากและอย่าโลภกับวงเล็บ ฉันดูหัวข้อนี้โดยละเอียดยิ่งขึ้น ที่นี่ในบท “ระวัง ?: ตัวดำเนินการและใส่ไว้ในวงเล็บ”

แฟรกเมนต์ N4, N5: ตัวชี้ Null

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-สตูดิโอ: V522 [CWE-476] การยกเลิกการอ้างอิงของตัวชี้ null 'LHS' อาจเกิดขึ้น TGParser.cpp 2152

ถ้าใช้พอยน์เตอร์ LHS เป็นโมฆะ ควรออกคำเตือน อย่างไรก็ตาม ตัวชี้ null เดียวกันนี้จะถูกยกเลิกการอ้างอิงแทน: LHS->getAsString().

นี่เป็นสถานการณ์ปกติมากเมื่อมีการซ่อนข้อผิดพลาดในตัวจัดการข้อผิดพลาด เนื่องจากไม่มีใครทดสอบข้อผิดพลาดเหล่านั้น เครื่องวิเคราะห์แบบคงที่จะตรวจสอบโค้ดที่เข้าถึงได้ทั้งหมด ไม่ว่าจะใช้งานบ่อยแค่ไหนก็ตาม นี่เป็นตัวอย่างที่ดีว่าการวิเคราะห์แบบคงที่ช่วยเสริมการทดสอบและการป้องกันข้อผิดพลาดอื่นๆ ได้อย่างไร

ข้อผิดพลาดในการจัดการตัวชี้ที่คล้ายกัน RHS อนุญาตในรหัสด้านล่าง: V522 [CWE-476] การยกเลิกการอ้างอิงของตัวชี้ว่าง 'RHS' อาจเกิดขึ้น TGParser.cpp 2186

แฟรกเมนต์ N6: การใช้ตัวชี้หลังจากเคลื่อนที่

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: V522 [CWE-476] การยกเลิกการอ้างอิงของตัวชี้ null 'ProgClone' อาจเกิดขึ้น คอมไพล์ไม่ถูกต้อง cpp 601

ที่จุดเริ่มต้นตัวชี้อัจฉริยะ ProgClone สิ้นสุดการเป็นเจ้าของวัตถุ:

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

ที่จริงแล้วตอนนี้ ProgClone เป็นตัวชี้ว่าง ดังนั้น การอ้างอิงตัวชี้ null ควรเกิดขึ้นด้านล่าง:

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

แต่ในความเป็นจริง สิ่งนี้จะไม่เกิดขึ้น! โปรดทราบว่าการวนซ้ำไม่ได้ถูกดำเนินการจริง

ที่จุดเริ่มต้นของคอนเทนเนอร์ ฟังก์ชั่นที่คอมไพล์ไม่ถูกต้อง เคลียร์:

MiscompiledFunctions.clear();

ถัดไป ขนาดของคอนเทนเนอร์นี้จะถูกใช้ในเงื่อนไขการวนซ้ำ:

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

สังเกตได้ง่ายว่าการวนซ้ำไม่เริ่มต้น ฉันคิดว่านี่เป็นข้อผิดพลาดด้วย และโค้ดควรเขียนแตกต่างออกไป

ดูเหมือนว่าเราได้พบกับความเท่าเทียมกันของข้อผิดพลาดอันโด่งดัง! ความผิดพลาดอย่างหนึ่งปกปิดอีกอย่าง :)

แฟรกเมนต์ N7: การใช้ตัวชี้หลังจากเคลื่อนที่

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: V522 [CWE-476] การยกเลิกการอ้างอิงของตัวชี้ null 'ทดสอบ' อาจเกิดขึ้น คอมไพล์ไม่ถูกต้อง cpp 709

สถานการณ์เดียวกันอีกครั้ง ในตอนแรก เนื้อหาของวัตถุจะถูกย้าย จากนั้นจึงถูกใช้ราวกับว่าไม่มีอะไรเกิดขึ้น ฉันเห็นสถานการณ์นี้บ่อยขึ้นในโค้ดโปรแกรมหลังจากซีแมนทิกส์การเคลื่อนไหวปรากฏใน C ++ นี่คือเหตุผลที่ฉันชอบภาษา C++! มีวิธีใหม่ๆ ในการยิงขาของคุณเองออกมากขึ้นเรื่อยๆ เครื่องวิเคราะห์ PVS-Studio จะใช้งานได้เสมอ :)

แฟรกเมนต์ N8: ตัวชี้ Null

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: V522 [CWE-476] อาจมีการยกเลิกการอ้างอิงตัวชี้ null 'ประเภท' PrettyFunctionDumper.cpp 233

นอกเหนือจากตัวจัดการข้อผิดพลาดแล้ว โดยปกติแล้วจะไม่มีการทดสอบฟังก์ชันการแก้ไขข้อบกพร่องในการพิมพ์ เรามีกรณีเช่นนี้ต่อหน้าเรา ฟังก์ชั่นกำลังรอผู้ใช้ซึ่งจะถูกบังคับให้แก้ไขปัญหาแทนการแก้ปัญหา

แก้ไข:

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

แฟรกเมนต์ N9: ตัวชี้ Null

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: V522 [CWE-476] การยกเลิกการอ้างอิงตัวชี้ null 'Ty' อาจเกิดขึ้น SearchableTableEmitter.cpp 614

ฉันคิดว่าทุกอย่างชัดเจนและไม่จำเป็นต้องมีคำอธิบาย

แฟรกเมนต์ N10: พิมพ์ผิด

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-สตูดิโอ: V570 ตัวแปร 'Identifier->Type' ถูกกำหนดให้กับตัวมันเอง FormatTokenLexer.cpp 249

ไม่มีประโยชน์ที่จะกำหนดตัวแปรให้กับตัวมันเอง เป็นไปได้มากว่าพวกเขาต้องการเขียน:

Identifier->Type = Question->Type;

ชิ้นส่วน N11: การแตกหักที่น่าสงสัย

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-สตูดิโอ: V622 [CWE-478] พิจารณาตรวจสอบคำสั่ง 'สวิตช์' อาจเป็นไปได้ว่าตัวดำเนินการ 'กรณี' แรกหายไป SystemZAsmParser.cpp 652

มีผู้ดำเนินการที่น่าสงสัยมากในตอนเริ่มต้น ทำลาย. คุณลืมเขียนอย่างอื่นที่นี่หรือเปล่า?

แฟรกเมนต์ N12: การตรวจสอบตัวชี้หลังจากการยกเลิกการอ้างอิง

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-สตูดิโอ: V595 [CWE-476] ตัวชี้ 'Callee' ถูกใช้ก่อนที่จะได้รับการตรวจสอบกับ nullptr ตรวจสอบบรรทัด: 172, 174. AMDGPUInline.cpp 172

ตัวชี้ แคลลี่ ที่จุดเริ่มต้นจะถูกยกเลิกการอ้างอิงในเวลาที่เรียกใช้ฟังก์ชัน getTTI.

แล้วปรากฎว่าควรตรวจสอบความเท่าเทียมกันของตัวชี้นี้ nullptr:

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

แต่มันสายเกินไปแล้ว…

แฟรกเมนต์ N13 - N...: การตรวจสอบตัวชี้หลังการยกเลิกการอ้างอิง

สถานการณ์ที่กล่าวถึงในส่วนของโค้ดก่อนหน้านี้ไม่ซ้ำกัน มันปรากฏที่นี่:

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: V595 [CWE-476] ตัวชี้ 'CalleeFn' ถูกใช้ก่อนที่จะได้รับการยืนยันกับ nullptr ตรวจสอบบรรทัด: 1079, 1081. SimplifyLibCalls.cpp 1079

และที่นี่:

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: V595 [CWE-476] ตัวชี้ 'ND' ถูกใช้ก่อนที่จะได้รับการตรวจสอบกับ nullptr ตรวจสอบบรรทัด: 532, 534 SemaTemplateInstantiateDecl.cpp 532

และที่นี่:

  • V595 [CWE-476] ตัวชี้ 'U' ถูกใช้ก่อนที่จะได้รับการตรวจสอบกับ nullptr ตรวจสอบบรรทัด: 404, 407 DWARFormValue.cpp 404
  • V595 [CWE-476] ตัวชี้ 'ND' ถูกใช้ก่อนที่จะได้รับการตรวจสอบกับ nullptr ตรวจสอบบรรทัด: 2149, 2151 SemaTemplateInstantiate.cpp 2149

แล้วฉันก็ไม่สนใจที่จะศึกษาคำเตือนด้วยหมายเลข V595 ฉันไม่รู้ว่ามีข้อผิดพลาดที่คล้ายกันมากกว่านี้นอกเหนือจากที่ระบุไว้ที่นี่หรือไม่ เป็นไปได้มากว่าจะมี

ส่วน N17, N18: การเปลี่ยนแปลงที่น่าสงสัย

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

คำเตือน PVS-สตูดิโอ: V629 [CWE-190] พิจารณาตรวจสอบนิพจน์ '~(Size - 1) << 1' การเปลี่ยนบิตของค่า 32 บิตพร้อมกับการขยายเป็นประเภท 64 บิตในภายหลัง AArch64AddressingModes.h 260

อาจไม่ใช่จุดบกพร่องและโค้ดทำงานได้ตรงตามที่ต้องการ แต่เห็นได้ชัดว่าเป็นสถานที่ที่น่าสงสัยมากและจำเป็นต้องได้รับการตรวจสอบ

สมมุติว่าตัวแปร ขนาด มีค่าเท่ากับ 16 จากนั้นผู้เขียนโค้ดวางแผนที่จะรับมันไว้ในตัวแปร นิมม์ มูลค่า:

1111111111111111111111111111111111111111111111111111111111100000

อย่างไรก็ตาม ในความเป็นจริงผลลัพธ์จะเป็น:

0000000000000000000000000000000011111111111111111111111111100000

ความจริงก็คือการคำนวณทั้งหมดเกิดขึ้นโดยใช้ประเภท 32 บิตที่ไม่ได้ลงนาม และเมื่อถึงตอนนั้น ประเภท 32 บิตที่ไม่ได้ลงชื่อนี้จะถูกขยายโดยปริยาย uint64_t. ในกรณีนี้ บิตที่สำคัญที่สุดจะเป็นศูนย์

คุณสามารถแก้ไขสถานการณ์เช่นนี้:

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

สถานการณ์ที่คล้ายกัน: V629 [CWE-190] พิจารณาตรวจสอบนิพจน์ 'Immr << 6' การเปลี่ยนบิตของค่า 32 บิตพร้อมกับการขยายเป็นประเภท 64 บิตในภายหลัง AArch64AddressingModes.h 269

ส่วน N19: ไม่มีคำหลัก อื่น?

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-สตูดิโอ: V646 [CWE-670] พิจารณาตรวจสอบตรรกะของแอปพลิเคชัน อาจเป็นไปได้ว่าคำหลัก 'else' หายไป AMDGPUAsmParser.cpp 5655

ไม่มีข้อผิดพลาดที่นี่ ตั้งแต่นั้นมาก็บล็อกแรก if ลงท้ายด้วย ต่อแล้วมันไม่สำคัญว่ามีคีย์เวิร์ดอยู่ อื่น หรือไม่. ทั้งสองวิธีรหัสจะทำงานเหมือนกัน ยังคิดถึงอยู่ อื่น ทำให้โค้ดไม่ชัดเจนและอันตรายมากขึ้น หากในอนาคต. ต่อ หายไปโค้ดจะเริ่มทำงานแตกต่างไปจากเดิมอย่างสิ้นเชิง ในความคิดของฉันมันจะดีกว่าที่จะเพิ่ม อื่น.

ส่วน N20: พิมพ์ผิดประเภทเดียวกันสี่ครั้ง

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:

  • V655 [CWE-480] สตริงถูกต่อเข้าด้วยกันแต่ไม่ได้ใช้ พิจารณาตรวจสอบนิพจน์ 'Result + Name.str()' Symbol.cpp 32
  • V655 [CWE-480] สตริงถูกต่อเข้าด้วยกันแต่ไม่ได้ใช้ พิจารณาตรวจสอบนิพจน์ 'Result + "(ObjC Class)" + Name.str()' Symbol.cpp 35
  • V655 [CWE-480] สตริงถูกต่อเข้าด้วยกันแต่ไม่ได้ใช้ พิจารณาตรวจสอบนิพจน์ 'Result + "(ObjC Class EH) " + Name.str()' Symbol.cpp 38
  • V655 [CWE-480] สตริงถูกต่อเข้าด้วยกันแต่ไม่ได้ใช้ พิจารณาตรวจสอบนิพจน์ 'Result + "(ObjC IVar)" + Name.str()' Symbol.cpp 41

โดยบังเอิญ ตัวดำเนินการ + จะถูกใช้แทนตัวดำเนินการ += ผลลัพธ์ที่ได้คือการออกแบบที่ไม่มีความหมาย

แฟรกเมนต์ N21: พฤติกรรมที่ไม่ได้กำหนด

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

พยายามค้นหารหัสอันตรายด้วยตัวเอง และนี่คือภาพเพื่อเบี่ยงเบนความสนใจเพื่อไม่ให้ดูคำตอบในทันที:

ค้นหาข้อบกพร่องใน LLVM 8 โดยใช้ตัววิเคราะห์ PVS-Studio

คำเตือน PVS-สตูดิโอ: V708 [CWE-758] มีการใช้โครงสร้างที่เป็นอันตราย: 'FeaturesMap[Op] = FeaturesMap.size()' โดยที่ 'FeaturesMap' อยู่ในคลาส 'map' สิ่งนี้อาจนำไปสู่พฤติกรรมที่ไม่ได้กำหนดไว้ RISCVCompressInstEmitter.cpp 490

บรรทัดปัญหา:

FeaturesMap[Op] = FeaturesMap.size();

ถ้าธาตุ Op ไม่พบองค์ประกอบใหม่จะถูกสร้างขึ้นในแผนที่และจำนวนองค์ประกอบในแผนที่นี้จะถูกเขียนไว้ที่นั่น ไม่ทราบว่าจะมีการเรียกใช้ฟังก์ชันนี้หรือไม่ ขนาด ก่อนหรือหลังการเพิ่มองค์ประกอบใหม่

แฟรกเมนต์ N22-N24: การมอบหมายซ้ำ

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-สตูดิโอ: V519 [CWE-563] ตัวแปร 'NType' ได้รับการกำหนดค่าสองครั้งติดต่อกัน บางทีนี่อาจเป็นความผิดพลาด ตรวจสอบบรรทัด: 1663, 1664 MachOObjectFile.cpp 1664

ฉันไม่คิดว่าจะมีข้อผิดพลาดจริงที่นี่ แค่มอบหมายงานซ้ำๆ โดยไม่จำเป็น แต่ก็ยังมีข้อผิดพลาดอยู่

เช่นเดียวกัน:

  • V519 [CWE-563] ตัวแปร 'B.NDesc' ได้รับการกำหนดค่าสองครั้งติดต่อกัน บางทีนี่อาจเป็นความผิดพลาด ตรวจสอบบรรทัด: 1488, 1489. llvm-nm.cpp 1489
  • V519 [CWE-563] กำหนดค่าตัวแปรสองครั้งติดต่อกัน บางทีนี่อาจเป็นความผิดพลาด ตรวจสอบบรรทัด: 59, 61. coff2yaml.cpp 61

แฟรกเมนต์ N25-N27: การมอบหมายใหม่เพิ่มเติม

ตอนนี้เรามาดูเวอร์ชันที่แตกต่างกันเล็กน้อยของการกำหนดใหม่

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: V519 [CWE-563] ตัวแปร 'การจัดตำแหน่ง' ได้รับการกำหนดค่าสองครั้งติดต่อกัน บางทีนี่อาจเป็นความผิดพลาด ตรวจสอบบรรทัด: 1158, 1160 LoadStoreVectorizer.cpp 1160

นี่เป็นโค้ดที่แปลกมากซึ่งเห็นได้ชัดว่ามีข้อผิดพลาดเชิงตรรกะ ที่จุดเริ่มต้นแปรผัน การวางแนว ค่าจะถูกกำหนดค่าตามเงื่อนไข แล้วการมอบหมายก็เกิดขึ้นอีกครั้ง แต่ตอนนี้ไม่มีการตรวจสอบใดๆ

สามารถดูสถานการณ์ที่คล้ายกันได้ที่นี่:

  • V519 [CWE-563] ตัวแปร 'Effects' ได้รับการกำหนดค่าสองครั้งติดต่อกัน บางทีนี่อาจเป็นความผิดพลาด ตรวจสอบบรรทัด: 152, 165 WebAssemblyRegStackify.cpp 165
  • V519 [CWE-563] ตัวแปร 'ExpectNoDerefChunk' ได้รับการกำหนดค่าสองครั้งติดต่อกัน บางทีนี่อาจเป็นความผิดพลาด ตรวจสอบบรรทัด: 4970, 4973. SemaType.cpp 4973

แฟรกเมนต์ N28: สภาพจริงเสมอ

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-สตูดิโอ: V547 [CWE-571] นิพจน์ 'nextByte != 0x90' เป็นจริงเสมอ X86DisassemblerDecoder.cpp 379

การตรวจสอบไม่สมเหตุสมผล ตัวแปร ไบต์ถัดไป ไม่เท่ากับมูลค่าเสมอไป 0x90ซึ่งตามมาจากการตรวจสอบครั้งก่อน นี่เป็นข้อผิดพลาดเชิงตรรกะบางประเภท

แฟรกเมนต์ N29 - N...: เงื่อนไขจริง/เท็จเสมอ

เครื่องวิเคราะห์ออกคำเตือนมากมายว่าสภาวะทั้งหมด (V547) หรือบางส่วน (V560) เป็นจริงหรือเท็จเสมอ บ่อยครั้งสิ่งเหล่านี้ไม่ใช่ข้อผิดพลาดจริง แต่เป็นเพียงโค้ดที่เลอะเทอะ ซึ่งเป็นผลมาจากการขยายมาโคร และอื่นๆ ที่คล้ายกัน อย่างไรก็ตาม การดูคำเตือนเหล่านี้ทั้งหมดก็สมเหตุสมผล เนื่องจากข้อผิดพลาดเชิงตรรกะที่แท้จริงเกิดขึ้นเป็นครั้งคราว ตัวอย่างเช่น โค้ดส่วนนี้น่าสงสัย:

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-สตูดิโอ: V560 [CWE-570] ส่วนหนึ่งของนิพจน์เงื่อนไขจะเป็นเท็จเสมอ: RegNo == 0xe ARMDisassembler.cpp 939

ค่าคงที่ 0xE คือค่า 14 ในรูปทศนิยม การตรวจสอบ RegNo == 0xe ไม่สมเหตุสมผลเพราะถ้า เลขทะเบียน > 13จากนั้นฟังก์ชันจะดำเนินการให้เสร็จสิ้น

มีคำเตือนอื่นๆ อีกมากมายเกี่ยวกับ ID V547 และ V560 แต่ก็เช่นเดียวกัน V595ฉันไม่สนใจที่จะศึกษาคำเตือนเหล่านี้ ชัดเจนแล้วว่าฉันมีเนื้อหาเพียงพอที่จะเขียนบทความ :) ดังนั้นจึงไม่ทราบว่าสามารถระบุข้อผิดพลาดประเภทนี้ใน LLVM ได้จำนวนเท่าใดโดยใช้ PVS-Studio

ฉันจะยกตัวอย่างว่าทำไมการศึกษาสิ่งกระตุ้นเหล่านี้จึงน่าเบื่อ เครื่องวิเคราะห์มีสิทธิ์อย่างยิ่งในการออกคำเตือนสำหรับรหัสต่อไปนี้ แต่นี่ก็ไม่ใช่ข้อผิดพลาดเช่นกัน

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

คำเตือน PVS-Studio: V547 [CWE-570] นิพจน์ '!HasError' จะเป็นเท็จเสมอ แกะ LineParser.cpp 1635 ออก

ชิ้นส่วน N30: ​​การกลับมาที่น่าสงสัย

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-สตูดิโอ: V612 [CWE-670] 'การส่งคืน' โดยไม่มีเงื่อนไขภายในลูป R600OptimizeVectorRegisters.cpp 63

นี่เป็นข้อผิดพลาดหรือเทคนิคเฉพาะที่มีจุดประสงค์เพื่ออธิบายบางสิ่งให้โปรแกรมเมอร์อ่านโค้ด การออกแบบนี้ไม่ได้อธิบายอะไรให้ฉันฟังและดูน่าสงสัยมาก อย่าเขียนแบบนั้นดีกว่า :)

เหนื่อย? จากนั้นก็ถึงเวลาชงชาหรือกาแฟ

ค้นหาข้อบกพร่องใน LLVM 8 โดยใช้ตัววิเคราะห์ PVS-Studio

ข้อบกพร่องที่ระบุโดยการวินิจฉัยใหม่

ฉันคิดว่าการเปิดใช้งานการวินิจฉัยแบบเก่า 30 ครั้งก็เพียงพอแล้ว ตอนนี้เรามาดูกันว่าสิ่งที่น่าสนใจสามารถพบได้ด้วยการวินิจฉัยใหม่ที่ปรากฏในเครื่องวิเคราะห์หลังจากนั้น ก่อน เช็ค ในช่วงเวลานี้ มีการเพิ่มการวินิจฉัยวัตถุประสงค์ทั่วไปทั้งหมด 66 รายการลงในเครื่องวิเคราะห์ C++

ส่วน N31: รหัสที่ไม่สามารถเข้าถึงได้

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-สตูดิโอ: V779 [CWE-561] ตรวจพบรหัสที่ไม่สามารถเข้าถึงได้ เป็นไปได้ว่ามีข้อผิดพลาดเกิดขึ้น ExecutionUtils.cpp 146

อย่างที่คุณเห็นทั้งสองสาขาของโอเปอเรเตอร์ if ลงท้ายด้วยการโทรไปยังโอเปอเรเตอร์ กลับ. ตามภาชนะนั้น CtorDtorsByPriority จะไม่มีวันถูกล้าง

ส่วน N32: รหัสที่ไม่สามารถเข้าถึงได้

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: V779 [CWE-561] ตรวจพบรหัสที่ไม่สามารถเข้าถึงได้ เป็นไปได้ว่ามีข้อผิดพลาดเกิดขึ้น LLParser.cpp 835

สถานการณ์ที่น่าสนใจ มาดูสถานที่นี้กันก่อน:

return ParseTypeIdEntry(SummaryID);
break;

เมื่อมองแวบแรกดูเหมือนว่าไม่มีข้อผิดพลาดที่นี่ ดูเหมือนผู้ดำเนินการ ทำลาย มีอันพิเศษอยู่ที่นี่ และคุณสามารถลบมันได้ อย่างไรก็ตามไม่ใช่เรื่องง่ายทั้งหมด

เครื่องวิเคราะห์จะออกคำเตือนในบรรทัด:

Lex.setIgnoreColonInIdentifiers(false);
return false;

และแน่นอนว่ารหัสนี้ไม่สามารถเข้าถึงได้ ทุกกรณีใน สลับ ปิดท้ายด้วยการโทรจากโอเปอเรเตอร์ กลับ. และตอนนี้ก็ไร้สติเพียงลำพัง ทำลาย ดูไม่เป็นอันตรายเลย! บางทีกิ่งหนึ่งควรจะลงท้ายด้วย ทำลาย, ไม่บน กลับ?

แฟรกเมนต์ N33: การรีเซ็ตบิตสูงแบบสุ่ม

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-สตูดิโอ: V784 ขนาดของบิตมาสก์จะน้อยกว่าขนาดของตัวถูกดำเนินการตัวแรก ซึ่งจะทำให้สูญเสียบิตที่สูงกว่า RuntimeDyld.cpp 815

โปรดทราบว่าฟังก์ชั่น getStubAlignment ประเภทการส่งคืน ไม่ได้ลงนาม. ลองคำนวณค่าของนิพจน์ โดยสมมติว่าฟังก์ชันส่งคืนค่า 8:

~(getStubAlignment() - 1)

~(8u-1)

0xFFFFFFFF8u

ตอนนี้สังเกตว่าตัวแปร ขนาดข้อมูล มีประเภทที่ไม่ได้ลงนาม 64 บิต ปรากฎว่าเมื่อดำเนินการ DataSize & 0xFFFFFF8u บิตที่มีลำดับสูงทั้งหมดสามสิบสองบิตจะถูกรีเซ็ตเป็นศูนย์ เป็นไปได้มากว่านี่ไม่ใช่สิ่งที่โปรแกรมเมอร์ต้องการ ฉันสงสัยว่าเขาต้องการคำนวณ: DataSize & 0xFFFFFFFFFFFFFF8u

เพื่อแก้ไขข้อผิดพลาด คุณควรเขียนสิ่งนี้:

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

หรือดังนั้น:

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

แฟรกเมนต์ N34: การส่งประเภทที่ชัดเจนล้มเหลว

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-สตูดิโอ: V1028 [CWE-190] อาจมีน้ำล้น พิจารณาแปลงตัวถูกดำเนินการของตัวดำเนินการ 'NumElts * Scale' เป็นประเภท 'size_t' ไม่ใช่ผลลัพธ์ X86ISelLowering.h 1577

การหล่อประเภทที่ชัดเจนใช้เพื่อหลีกเลี่ยงการล้นเมื่อคูณตัวแปรประเภท int. อย่างไรก็ตาม การคัดเลือกประเภทที่ชัดเจนที่นี่ไม่ได้ป้องกันการล้น ขั้นแรก ตัวแปรจะถูกคูณ จากนั้นผลลัพธ์ของการคูณแบบ 32 บิตเท่านั้นที่จะขยายเป็นประเภท ขนาด_t.

แฟรกเมนต์ N35: การคัดลอก-วางล้มเหลว

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] พบโค้ดสองส่วนที่คล้ายกัน บางทีนี่อาจเป็นการพิมพ์ผิดและควรใช้ตัวแปร 'Op1' แทน 'Op0' InstCombineCompares.cpp 5507

การวินิจฉัยใหม่ที่น่าสนใจนี้ระบุสถานการณ์ที่มีการคัดลอกโค้ดบางส่วนและชื่อบางส่วนในนั้นเริ่มมีการเปลี่ยนแปลง แต่ในที่เดียวพวกเขาไม่ได้แก้ไข

โปรดทราบว่าในบล็อกที่สองมีการเปลี่ยนแปลง Op0 บน Op1. แต่ที่แห่งหนึ่งพวกเขาไม่ได้ซ่อมมัน เป็นไปได้มากว่าควรจะเขียนดังนี้:

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

แฟรกเมนต์ N36: ความสับสนที่แปรผัน

struct Status {
  unsigned Mask;
  unsigned Mode;

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

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

คำเตือน PVS-สตูดิโอ: V1001 [CWE-563] ตัวแปร 'Mode' ถูกกำหนดไว้แต่ไม่ได้ถูกใช้ในตอนท้ายของฟังก์ชัน SIModeRegister.cpp 48

การกำหนดอาร์กิวเมนต์ของฟังก์ชันด้วยชื่อเดียวกันกับสมาชิกชั้นเรียนเป็นสิ่งที่อันตรายมาก มันง่ายมากที่จะสับสน เรามีกรณีเช่นนี้ต่อหน้าเรา สำนวนนี้ไม่สมเหตุสมผล:

Mode &= Mask;

อาร์กิวเมนต์ของฟังก์ชันเปลี่ยนไป นั่นคือทั้งหมดที่ อาร์กิวเมนต์นี้ไม่ได้ใช้อีกต่อไป เป็นไปได้มากว่าคุณควรเขียนแบบนี้:

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

แฟรกเมนต์ N37: ความสับสนที่แปรผัน

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

คำเตือน PVS-Studio: V1001 [CWE-563] ตัวแปร 'ขนาด' ถูกกำหนดไว้แล้ว แต่จะไม่ได้ใช้ในตอนท้ายของฟังก์ชัน อ็อบเจ็กต์.cpp 424

สถานการณ์คล้ายกับสถานการณ์ก่อนหน้า ควรเขียนว่า:

this->Size += this->EntrySize;

ชิ้นส่วน N38-N47: พวกเขาลืมตรวจสอบดัชนี

ก่อนหน้านี้ เราได้ดูตัวอย่างการกระตุ้นการวินิจฉัย V595. สาระสำคัญของมันคือตัวชี้ถูกยกเลิกการอ้างอิงตั้งแต่เริ่มต้น และจากนั้นจึงตรวจสอบเท่านั้น การวินิจฉัยรุ่นเยาว์ V1004 เป็นความหมายที่ตรงกันข้ามแต่ยังเผยให้เห็นข้อผิดพลาดมากมาย โดยจะระบุสถานการณ์ที่มีการตรวจสอบตัวชี้ตั้งแต่เริ่มต้นแล้วลืมทำ ลองดูกรณีดังกล่าวที่พบใน 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());  // <=
  ....
}

คำเตือน PVS-Studio: V1004 [CWE-476] ตัวชี้ 'Ptr' ถูกใช้อย่างไม่ปลอดภัยหลังจากได้รับการยืนยันกับ nullptr ตรวจสอบบรรทัด: 729, 738 TargetTransformInfoImpl.h 738

ตัวแปร ptr อาจจะเท่ากัน nullptrดังที่เห็นได้จากเช็ค:

if (Ptr != nullptr)

อย่างไรก็ตาม ด้านล่างตัวชี้นี้ถูกยกเลิกการอ้างอิงโดยไม่มีการตรวจสอบเบื้องต้น:

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

ลองพิจารณาอีกกรณีที่คล้ายกัน

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: V1004 [CWE-476] ตัวชี้ 'FD' ถูกใช้อย่างไม่ปลอดภัยหลังจากได้รับการยืนยันกับ nullptr ตรวจสอบบรรทัด: 3228, 3231 CGDebugInfo.cpp 3231

ให้ความสนใจกับป้าย FD. ฉันแน่ใจว่าปัญหานั้นมองเห็นได้ชัดเจนและไม่ต้องการคำอธิบายพิเศษ

และต่อไป:

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: V1004 [CWE-476] ตัวชี้ 'PtrTy' ถูกใช้อย่างไม่ปลอดภัยหลังจากได้รับการยืนยันกับ nullptr ตรวจสอบบรรทัด: 960, 965. InterleavedLoadCombinePass.cpp 965

จะป้องกันตนเองจากข้อผิดพลาดดังกล่าวได้อย่างไร? ให้ความสำคัญกับการตรวจสอบโค้ดมากขึ้น และใช้เครื่องวิเคราะห์แบบคงที่ PVS-Studio เพื่อตรวจสอบโค้ดของคุณเป็นประจำ

ไม่มีประโยชน์ที่จะอ้างถึงส่วนย่อยของโค้ดอื่นที่มีข้อผิดพลาดประเภทนี้ ฉันจะทิ้งเฉพาะรายการคำเตือนไว้ในบทความ:

  • V1004 [CWE-476] ตัวชี้ 'Expr' ถูกใช้อย่างไม่ปลอดภัยหลังจากที่ได้รับการตรวจสอบกับ nullptr ตรวจสอบบรรทัด: 1049, 1078 DebugInfoMetadata.cpp 1078
  • V1004 [CWE-476] ตัวชี้ 'PI' ถูกใช้อย่างไม่ปลอดภัยหลังจากที่ได้รับการตรวจสอบกับ nullptr ตรวจสอบบรรทัด: 733, 753. LegacyPassManager.cpp 753
  • V1004 [CWE-476] ตัวชี้ 'StatepointCall' ถูกใช้อย่างไม่ปลอดภัยหลังจากที่ได้รับการตรวจสอบกับ nullptr ตรวจสอบบรรทัด: 4371, 4379. Verifier.cpp 4379
  • V1004 [CWE-476] ตัวชี้ 'RV' ถูกใช้อย่างไม่ปลอดภัยหลังจากที่ได้รับการตรวจสอบกับ nullptr ตรวจสอบบรรทัด: 2263, 2268 TGParser.cpp 2268
  • V1004 [CWE-476] ตัวชี้ 'CalleeFn' ถูกใช้อย่างไม่ปลอดภัยหลังจากที่ได้รับการตรวจสอบกับ nullptr ตรวจสอบบรรทัด: 1081, 1096 SimplifyLibCalls.cpp 1096
  • V1004 [CWE-476] ตัวชี้ 'TC' ถูกใช้อย่างไม่ปลอดภัยหลังจากได้รับการยืนยันกับ nullptr ตรวจสอบบรรทัด: 1819, 1824. Driver.cpp 1824

แฟรกเมนต์ N48-N60: ไม่สำคัญ แต่เป็นข้อบกพร่อง (หน่วยความจำรั่วได้)

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

คำเตือน PVS-สตูดิโอ: V1023 [CWE-460] ตัวชี้ที่ไม่มีเจ้าของถูกเพิ่มลงในคอนเทนเนอร์ 'กลยุทธ์' โดยวิธี 'emplace_back' หน่วยความจำรั่วจะเกิดขึ้นในกรณีที่มีข้อยกเว้น llvm-isel-fuzzer.cpp 58

หากต้องการเพิ่มองค์ประกอบที่ส่วนท้ายของคอนเทนเนอร์เช่น มาตรฐาน::เวกเตอร์ > คุณไม่สามารถเขียนได้ xxx.push_back(X ใหม่)เนื่องจากไม่มีการแปลงโดยนัยจาก X* в มาตรฐาน::unique_ptr.

วิธีแก้ไขทั่วไปคือการเขียน xxx.emplace_back(X ใหม่)เนื่องจากมันคอมไพล์: method emplace_back สร้างองค์ประกอบโดยตรงจากอาร์กิวเมนต์ของมัน และดังนั้นจึงสามารถใช้ตัวสร้างที่ชัดเจนได้

มันไม่ปลอดภัย หากเวกเตอร์เต็ม หน่วยความจำจะถูกจัดสรรใหม่ การดำเนินการจัดสรรหน่วยความจำอาจล้มเหลว ส่งผลให้มีข้อยกเว้นเกิดขึ้น มาตรฐาน::bad_alloc. ในกรณีนี้ ตัวชี้จะหายไปและวัตถุที่สร้างขึ้นจะไม่ถูกลบ

ทางออกที่ปลอดภัยคือการสร้าง Unique_ptrซึ่งจะเป็นเจ้าของตัวชี้ก่อนที่เวกเตอร์จะพยายามจัดสรรหน่วยความจำใหม่:

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

ตั้งแต่ C ++ 14 คุณสามารถใช้ 'std::make_unique':

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

ข้อบกพร่องประเภทนี้ไม่สำคัญสำหรับ LLVM หากไม่สามารถจัดสรรหน่วยความจำได้ คอมไพเลอร์จะหยุดทำงานทันที อย่างไรก็ตามสำหรับการใช้งานที่มีระยะเวลานาน เวลาทำงานซึ่งไม่สามารถยุติได้หากการจัดสรรหน่วยความจำล้มเหลว นี่อาจเป็นข้อผิดพลาดที่น่ารังเกียจจริงๆ

ดังนั้น แม้ว่าโค้ดนี้จะไม่ก่อให้เกิดภัยคุกคามในทางปฏิบัติต่อ LLVM แต่ฉันพบว่าการพูดถึงรูปแบบข้อผิดพลาดนี้มีประโยชน์ และผู้วิเคราะห์ PVS-Studio ได้เรียนรู้ที่จะระบุมันแล้ว

คำเตือนอื่นๆ ประเภทนี้:

  • V1023 [CWE-460] ตัวชี้ที่ไม่มีเจ้าของถูกเพิ่มลงในคอนเทนเนอร์ 'Passes' โดยวิธี 'emplace_back' หน่วยความจำรั่วจะเกิดขึ้นในกรณีที่มีข้อยกเว้น PassManager.h 546
  • V1023 [CWE-460] ตัวชี้ที่ไม่มีเจ้าของจะถูกเพิ่มลงในคอนเทนเนอร์ 'AAs' โดยวิธี 'emplace_back' หน่วยความจำรั่วจะเกิดขึ้นในกรณีที่มีข้อยกเว้น AliasAnalysis.h 324
  • V1023 [CWE-460] ตัวชี้ที่ไม่มีเจ้าของถูกเพิ่มลงในคอนเทนเนอร์ 'รายการ' โดยวิธี 'emplace_back' หน่วยความจำรั่วจะเกิดขึ้นในกรณีที่มีข้อยกเว้น DWARFDebugFrame.cpp 519
  • V1023 [CWE-460] ตัวชี้ที่ไม่มีเจ้าของถูกเพิ่มลงในคอนเทนเนอร์ 'AllEdges' โดยวิธี 'emplace_back' หน่วยความจำรั่วจะเกิดขึ้นในกรณีที่มีข้อยกเว้น CFGMST.h 268
  • V1023 [CWE-460] ตัวชี้ที่ไม่มีเจ้าของถูกเพิ่มลงในคอนเทนเนอร์ 'VMaps' โดยวิธี 'emplace_back' หน่วยความจำรั่วจะเกิดขึ้นในกรณีที่มีข้อยกเว้น SimpleLoopUnswitch.cpp 2012
  • V1023 [CWE-460] ตัวชี้ที่ไม่มีเจ้าของถูกเพิ่มลงในคอนเทนเนอร์ 'บันทึก' โดยวิธี 'emplace_back' หน่วยความจำรั่วจะเกิดขึ้นในกรณีที่มีข้อยกเว้น FDRLogBuilder.h 30
  • V1023 [CWE-460] ตัวชี้ที่ไม่มีเจ้าของถูกเพิ่มลงในคอนเทนเนอร์ 'PendingSubmodules' โดยวิธี 'emplace_back' หน่วยความจำรั่วจะเกิดขึ้นในกรณีที่มีข้อยกเว้น ModuleMap.cpp 810
  • V1023 [CWE-460] ตัวชี้ที่ไม่มีเจ้าของถูกเพิ่มลงในคอนเทนเนอร์ 'Objects' โดยวิธี 'emplace_back' หน่วยความจำรั่วจะเกิดขึ้นในกรณีที่มีข้อยกเว้น DebugMap.cpp 88
  • V1023 [CWE-460] ตัวชี้ที่ไม่มีเจ้าของถูกเพิ่มลงในคอนเทนเนอร์ 'กลยุทธ์' โดยวิธี 'emplace_back' หน่วยความจำรั่วจะเกิดขึ้นในกรณีที่มีข้อยกเว้น llvm-isel-fuzzer.cpp 60
  • V1023 [CWE-460] ตัวชี้ที่ไม่มีเจ้าของถูกเพิ่มลงในคอนเทนเนอร์ 'ตัวแก้ไข' โดยวิธี 'emplace_back' หน่วยความจำรั่วจะเกิดขึ้นในกรณีที่มีข้อยกเว้น llvm-stress.cpp 685
  • V1023 [CWE-460] ตัวชี้ที่ไม่มีเจ้าของถูกเพิ่มลงในคอนเทนเนอร์ 'ตัวแก้ไข' โดยวิธี 'emplace_back' หน่วยความจำรั่วจะเกิดขึ้นในกรณีที่มีข้อยกเว้น llvm-stress.cpp 686
  • V1023 [CWE-460] ตัวชี้ที่ไม่มีเจ้าของถูกเพิ่มลงในคอนเทนเนอร์ 'ตัวแก้ไข' โดยวิธี 'emplace_back' หน่วยความจำรั่วจะเกิดขึ้นในกรณีที่มีข้อยกเว้น llvm-stress.cpp 688
  • V1023 [CWE-460] ตัวชี้ที่ไม่มีเจ้าของถูกเพิ่มลงในคอนเทนเนอร์ 'ตัวแก้ไข' โดยวิธี 'emplace_back' หน่วยความจำรั่วจะเกิดขึ้นในกรณีที่มีข้อยกเว้น llvm-stress.cpp 689
  • V1023 [CWE-460] ตัวชี้ที่ไม่มีเจ้าของถูกเพิ่มลงในคอนเทนเนอร์ 'ตัวแก้ไข' โดยวิธี 'emplace_back' หน่วยความจำรั่วจะเกิดขึ้นในกรณีที่มีข้อยกเว้น llvm-stress.cpp 690
  • V1023 [CWE-460] ตัวชี้ที่ไม่มีเจ้าของถูกเพิ่มลงในคอนเทนเนอร์ 'ตัวแก้ไข' โดยวิธี 'emplace_back' หน่วยความจำรั่วจะเกิดขึ้นในกรณีที่มีข้อยกเว้น llvm-stress.cpp 691
  • V1023 [CWE-460] ตัวชี้ที่ไม่มีเจ้าของถูกเพิ่มลงในคอนเทนเนอร์ 'ตัวแก้ไข' โดยวิธี 'emplace_back' หน่วยความจำรั่วจะเกิดขึ้นในกรณีที่มีข้อยกเว้น llvm-stress.cpp 692
  • V1023 [CWE-460] ตัวชี้ที่ไม่มีเจ้าของถูกเพิ่มลงในคอนเทนเนอร์ 'ตัวแก้ไข' โดยวิธี 'emplace_back' หน่วยความจำรั่วจะเกิดขึ้นในกรณีที่มีข้อยกเว้น llvm-stress.cpp 693
  • V1023 [CWE-460] ตัวชี้ที่ไม่มีเจ้าของถูกเพิ่มลงในคอนเทนเนอร์ 'ตัวแก้ไข' โดยวิธี 'emplace_back' หน่วยความจำรั่วจะเกิดขึ้นในกรณีที่มีข้อยกเว้น llvm-stress.cpp 694
  • V1023 [CWE-460] ตัวชี้ที่ไม่มีเจ้าของถูกเพิ่มลงในคอนเทนเนอร์ 'ตัวถูกดำเนินการ' โดยวิธี 'emplace_back' หน่วยความจำรั่วจะเกิดขึ้นในกรณีที่มีข้อยกเว้น GlobalISelEmitter.cpp 1911
  • V1023 [CWE-460] ตัวชี้ที่ไม่มีเจ้าของถูกเพิ่มลงในคอนเทนเนอร์ 'Stash' โดยวิธี 'emplace_back' หน่วยความจำรั่วจะเกิดขึ้นในกรณีที่มีข้อยกเว้น GlobalISelEmitter.cpp 2100
  • V1023 [CWE-460] ตัวชี้ที่ไม่มีเจ้าของถูกเพิ่มลงในคอนเทนเนอร์ 'Matchers' โดยวิธี 'emplace_back' หน่วยความจำรั่วจะเกิดขึ้นในกรณีที่มีข้อยกเว้น GlobalISelEmitter.cpp 2702

ข้อสรุป

ฉันออกคำเตือนทั้งหมด 60 ครั้งแล้วหยุดลง มีข้อบกพร่องอื่น ๆ ที่เครื่องวิเคราะห์ PVS-Studio ตรวจพบใน LLVM หรือไม่ ใช่ฉันมี. อย่างไรก็ตาม ตอนที่ฉันเขียนโค้ดบางส่วนสำหรับบทความ มันเป็นช่วงเย็นหรือค่อนข้างจะกลางคืนด้วยซ้ำ และฉันก็ตัดสินใจว่าถึงเวลาที่ต้องเรียกวันนี้แล้ว

ฉันหวังว่าคุณจะพบว่ามันน่าสนใจและอยากลองใช้เครื่องวิเคราะห์ PVS-Studio

คุณสามารถดาวน์โหลดตัววิเคราะห์และรับรหัสเรือกวาดทุ่นระเบิดได้ที่ หน้านี้.

สิ่งสำคัญที่สุดคือใช้การวิเคราะห์แบบคงที่อย่างสม่ำเสมอ การตรวจสอบเพียงครั้งเดียวดำเนินการโดยเราเพื่อทำให้วิธีการวิเคราะห์แบบคงที่เป็นที่นิยมและ PVS-Studio ไม่ใช่สถานการณ์ปกติ

ขอให้โชคดีในการปรับปรุงคุณภาพและความน่าเชื่อถือของโค้ดของคุณ!

ค้นหาข้อบกพร่องใน LLVM 8 โดยใช้ตัววิเคราะห์ PVS-Studio

หากคุณต้องการแบ่งปันบทความนี้กับผู้ชมที่พูดภาษาอังกฤษ โปรดใช้ลิงก์การแปล: Andrey Karpov ค้นหาข้อบกพร่องใน LLVM 8 ด้วย PVS-Studio.

ที่มา: will.com

เพิ่มความคิดเห็น