Geekly articles weekly

使用PVS-Studio查找LLVM 8中的错误

PVS-Studio和LLVM 8.0.0

自上次通过PVS-Studio检查LLVM项目的代码以来已经有两年了，所以让我们看看PVS-Studio是否仍然是检测错误和安全漏洞的工具中的领导者。 我们将通过扫描LLVM 8.0.0版本中的新错误来实现。

必须写的文章


坦白说，我不想写这篇文章。 谈论我们已经多次检查（ 1，2，3 ）的项目并不有趣。 我宁愿选择新的东西，但别无选择。

每次发行新版本的LLVM或更新Clang Static Analyzer时 ，我们都会收到以下内容的电子邮件：

嘿，新版本的Clang Static Analyzer有了新的诊断！ PVS-Studio似乎变得不那么重要了。 Clang可以检测到比以前更多的bug，现在正赶上PVS-Studio。 你说什么

为此，我很乐意回应：

我们也没有闲逛！ 我们大大提高了PVS-Studio的功能，所以不用担心-我们仍然是最好的。

恐怕这是一个不好的答案。 它没有提供证据，这就是我写这篇文章的原因。 因此，我再次检查了LLVM，发现了大量的各种错误。 我最喜欢的那些将进一步讨论。 Clang Static Analyzer无法检测到这些错误（或使过程非常麻烦），而我们可以。 而且，顺便说一句，我只花了一个晚上就把所有这些错误记录下来了。

不过，这篇文章花了我几个星期才能完成。 我只是无法让自己把收集到的材料写成文字:)。

顺便说一句，如果您想知道PVS-Studio采用了什么技术来检测错误和漏洞，请看一下这篇文章 。

新的和现有的诊断


正如我已经说过的，LLVM的许多检查中的最后一次是在两年前完成的，后来发现的错误已由作者修复。 本文将显示错误的新部分。 怎么会有新的错误？ 有以下三个原因：

  1. LLVM项目正在不断发展； 作者修改现有代码并添加新代码。 修改的零件和新零件自然都具有新的错误。 对于定期而不是时常进行静态分析，这是一个有力的论据。 我们的文章格式非常适合展示PVS-Studio的功能，但与提高代码质量或降低错误修复成本无关。 定期使用静态分析！
  2. 我们修改并改进了现有的诊断程​​序，使分析仪能够检测到以前无法发现的错误。
  3. PVS-Studio已通过两年前不存在的新诊断程序得到了增强。 我将这些警告分为一个单独的部分，以便更清楚地看到PVS-Studio的进度。

现有诊断程序发现的缺陷


片段编号 1：复制粘贴

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诊断消息： V501 [CWE-570]在“ ||”的左侧和右侧有相同的子表达式“ Name.startswith（“ avx512.mask.permvar。”）” 操作员。 自动升级.cpp 73

出现“ avx512.mask.permvar”。 子字符串被检查两次。 第二个条件显然是检查其他内容，但是程序员忘记更改复制的行。

片段编号 2：错别字

 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被使用了两次。

片段编号 3：对运算符优先级的混淆

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

PVS-Studio诊断消息： 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))

三元运算符实际上非常棘手，可能会导致逻辑错误。 小心使用它，不要犹豫在它周围加上其他括号。 在“当心？：运算符并将其括在括号中”部分中将在此详细讨论该主题。

片段编号 4，5：空指针

 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诊断消息： V522 [CWE-476]可能会取消引用空指针“ LHS”。 TGParser.cpp 2152

如果LHS指针恰好为空，则程序应生成警告。 相反，它将取消引用非常空的指针： LHS-> getAsString（） 。

错误处理程序包含错误的情况很常见，因为开发人员无法正确测试它们。 静态分析器会检查所有可访问的代码，无论实际执行的频率是多少。 这是静态分析如何补充其他代码测试和保护手段的一个很好的例子。

还可以找到RHS指针的类似故障处理程序：V522 [CWE-476]可能会取消引用空指针“ RHS”。 TGParser.cpp 2186

片段编号 6：移动后使用指针

 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]可能会取消引用空指针“ ProgClone”。 错误编译.cpp 601

智能指针ProgClone首先释放对象所有权：

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

实际上， ProgClone已成为空指针-因此，从技术上讲，空指针会进一步取消引用：

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

但这不会发生！ 请注意，循环实际上根本不执行。

首先清除了MiscompiledFunctions容器：

 MiscompiledFunctions.clear();

然后在循环条件中使用其大小：

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

显然，循环不会开始。 我认为这也是一个错误，代码的外观也有所不同。

我猜我们在这里看到的是臭名昭著的错误奇偶校验，其中一个错误充当了另一个错误的伪装:)。

片段编号 7：移动后使用指针

 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]可能会取消引用空指针“测试”。 错误编译cpp 709

这与前面的情况相似。 首先移动对象的内容，然后使用它，就好像什么都没发生一样。 在将移动语义添加到C ++之后，此错误变得越来越普遍。 那就是我喜欢这种语言的原因！ 您将获得新的射击方式，这意味着PVS-Studio将始终有工作要做：)。

片段编号 8：空指针

 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]可能会取消引用空指针“类型”。 233.第233章

就像错误处理程序一样，打印调试数据的测试功能通常也无法获得足够的测试覆盖率，这就是一个例子。 该功能不是在帮助用户解决他们的问题，而是在等待他们解决问题。

固定代码：

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

片段编号 9：空指针

 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]可能会取消引用空指针“ Ty”。 SearchableTableEmitter.cpp 614

我认为您不需要对此发表任何评论。

片段编号 10：错别字

 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诊断消息： V570为其分配了“标识符->类型”变量。 格式令牌Lexer.cpp 249

给变量赋值本身是没有意义的操作。 程序员必须具有以下目的：

 Identifier->Type = Question->Type;

片段编号 11：可疑的休息

 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诊断消息： V622 [CWE-478]考虑检查“ switch”语句。 第一个“ case”运算符可能会丢失。 系统ZAsmParser.cpp 652

开头有一个非常可疑的break语句。 这里不应该有别的东西吗？

片段编号 12：解引用后检查指针

 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诊断消息： V595 [CWE-476]在针对nullptr验证了“被调用者”指针之前，已使用该指针。 检查行：172，174。AMDGPUInline.cpp 172

调用getTTI函数时，首先取消引用被调用方指针。

然后发现应该检查指针的nullptr ：

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

太晚了...

片段编号 13-否....：取消引用后检查指针

前面的示例不是唯一的。 在此代码段中发现了相同的问题：

 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]在针对nullptr对其进行验证之前，已使用了'CalleeFn'指针。 检查行：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]在针对nullptr对其进行验证之前，已使用了'ND'指针。 检查行：532、534。SemaTemplateInstantiateDecl.cpp 532

在这里：

  • V595 [CWE-476]在针对nullptr对其进行验证之前，已使用了'U'指针。 检查行：404、407。DWARFFormValue.cpp 404
  • V595 [CWE-476]在针对nullptr对其进行验证之前，已使用了'ND'指针。 检查行：2149，2151。SemaTemplateInstantiate.cpp 2149

然后，我对跟踪V595警告失去了兴趣，因此，除了上面显示的错误之外，我无法告诉您是否还有其他此类错误。 我敢打赌。

片段编号 17、18：可疑的转变

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

PVS-Studio诊断消息： V629 [CWE-190]考虑检查'〜（大小-1）<< 1'表达式。 32位值的位移，随后扩展为64位类型。 260 AArch64AddressingModes.h

这段代码实际上可能是正确的，但是看起来确实很奇怪，需要检查。

假设Size变量的值为16； 那么NImms变量将获得以下值：

11111111111111111111111111111111111111111111111111111111111111100000000

但实际上它将获得价值：

0000000000000000000000000000000000000000001111111111111111111111111100000

发生这种情况是因为所有计算都是在32位无符号类型上完成的，然后才隐式提升为uint64_t ，并将最高有效位清零。

该问题可以解决如下：

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

此类型的另一个错误：V629 [CWE-190]考虑检查'Immr << 6'表达式。 32位值的位移，随后扩展为64位类型。 AArch64AddressingModes.h 269

片段编号 19： 还有其他 关键词 吗 ？

 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诊断消息： V646 [CWE-670]考虑检查应用程序的逻辑。 可能缺少“ else”关键字。 AMDGPUAsmParser.cpp 5655

这不是一个错误。 由于第一个if语句的then块以continue结尾，因此是否具有else关键字无关紧要。 在任何情况下，行为都是相同的。 但是，缺少其他内容会使代码的可读性降低，因此有潜在的危险。 如果有一天继续消失，行为将发生巨大变化。 我强烈建议添加else 。

片段编号 20：四个相同的错别字

 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]字符串已连接，但未使用。 考虑检查“结果+“（ObjC IVar）” + Name.str（）”表达式。 Symbol.cpp 41

程序员不小心使用了+运算符而不是+ =，并最终得到了四个无意义的构造。

片段编号 21：未定义的行为

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

尝试自行发现错误。 我添加了图像，以便您不会立即看到答案：

???


PVS-Studio诊断消息： V708 [CWE-758]使用了危险的构造：'FeaturesMap [Op] = FeatureMap.size （）'，其中'FeaturesMap'是'map'类的。 这可能导致不确定的行为。 RISCVCompressInstEmitter.cpp 490

错误的行是这一行：

 FeaturesMap[Op] = FeaturesMap.size();

如果未找到Op元素，则程序将在映射中创建一个新元素，并为其分配该映射中元素的总数。 您只是不知道在添加新元素之前还是之后将调用size函数。

片段编号 22-没有 24：重复的作业

 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诊断消息： 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

片段编号 25-没有 27：更多重复的作业

这些处理重复分配的版本略有不同。

 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]'Alignment'变量连续两次被赋值。 也许这是一个错误。 检查行：1158、1160。LoadStoreVectorizer.cpp 1160

这是一个非常奇怪的代码段，它可能包含逻辑错误。 首先根据条件为Alignment变量分配值，然后再次为其分配值，但无需任何事先检查。

类似缺陷：

  • V519 [CWE-563]'效果'变量连续两次被赋值。 也许这是一个错误。 检查行：152，165。WebAssemblyRegStackify.cpp 165
  • V519 [CWE-563]'ExpectNoDerefChunk'变量已连续两次分配值。 也许这是一个错误。 检查行：4970，4973。SemaType.cpp 4973

片段编号 28：始终为真状态

 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诊断消息： V547 [CWE-571]表达式'nextByte！= 0x90'始终为true。 X86DisassemblerDecoder.cpp 379

支票没有意义。 nextByte变量从不等于0x90 ：从逻辑上讲，它仅来自上一个检查。 这一定是逻辑错误。

片段编号 29-否....：始终为真/假条件

关于整个条件（ 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-Studio诊断消息： V560 [CWE-570]条件表达式的一部分始终为false：RegNo == 0xe。 ARMDisassembler.cpp 939

0xE常数是十进制数字14。检查RegNo == 0xe没有意义，因为如果RegNo> 13 ，则函数将返回。

我看到了很多其他的V547和V560警告，但是像V595一样 ，我对检查它们并不感到兴奋，因为我已经有足够的材料写一篇文章了：）。 因此，在LLVM中没有此类错误总数。

这是一个示例，说明为什么检查这些警告很无聊。 对以下代码发出警告时，分析仪完全正确。 但这仍然不是错误。

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

PVS-Studio诊断消息：V547 [CWE-570]表达式'！HasError'始终为false。 UnwrappedLineParser.cpp 1635

片段编号 30：可疑的回报

 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诊断消息： V612 [CWE-670]循环内无条件的“返回”。 R600OptimizeVectorRegisters.cpp 63

它可能是错误或特定的编码技术，旨在与其他程序员交流一些想法。 对我来说，它什么也没说，只是那是一段非常可疑的代码。 请不要写这样的代码:)。

感到累了吗？ 好，该喝些茶或咖啡了。

咖啡


新诊断程序发现的缺陷


我认为30个示例足以满足现有的诊断要求。 现在，让我们看看是否可以通过新的诊断找到有趣的东西，这些诊断是在上次检查之后添加的。 在过去的两年中，C ++分析器模块扩展了66种新的诊断程序。

片段编号 31：无法访问的代码

 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诊断消息： V779 [CWE-561]检测到无法访问的代码。 可能存在错误。 ExecutionUtils.cpp 146

如您所见， if语句的两个分支都以return语句结尾，这意味着CtorDtorsByPriority容器将永远不会被清除。

片段编号 32：无法访问的代码

 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;

这段代码似乎没有什么奇怪的。 break语句是不必要的，可以安全地删除。 但这不是那么简单。

该警告由以下几行触发：

 Lex.setIgnoreColonInIdentifiers(false); return false;

确实，此代码不可访问。 switch语句的所有大小写标签都以return结尾，而毫无意义的单独中断看起来不再那么无害了！ 如果其中一个分支的结尾是休息而不是返回，该怎么办？

片段编号 33：意外清除最高有效位

 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诊断消息： V784位掩码的大小小于第一个操作数的大小。 这将导致丢失更高的位。 运行系统Dyld.cpp 815

注意， getStubAlignment函数返回一个无符号值。 让我们看看表达式将如何求值，假设函数将返回值8：

〜（getStubAlignment（）-1）

〜（8u-1）

0xFFFFFFF8u

现在请注意， DataSize变量的类型为64位无符号。 因此，事实证明，执行操作DataSize＆0xFFFFFFF8将导致清除该值的所有32个最高有效位。 我不认为程序员想要那样。 也许他们的意思是DataSize＆0xFFFFFFFFFFFFFFFFFFF8u。

要修复该错误，应按以下方式重写代码：

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

或像这样：

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

片段编号 34：错误的显式类型转换

 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诊断消息： V1028 [CWE-190]可能的溢出。 考虑将'NumElts * Scale'运算符的类型转换为'size_t'类型，而不是结果类型。 1577年

在将类型为int的变量相乘时，使用显式类型转换来避免溢出。 但是，在这种情况下，它不起作用，因为首先会发生乘法，然后才将32位结果提升为size_t类型。

片段编号 35：错误的复制粘贴

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

片段编号 36：变量混合

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

PVS-Studio诊断消息： V1001 [CWE-563]分配了'Mode'变量，但在功能结束时未使用。 SIModeRegister.cpp 48

函数参数的名称与类成员的名称相同是非常危险的，因为您可能会混淆它们。 您在这里看到的就是一个例子。 以下表达式毫无意义：

 Mode &= Mask;

参数已更改，但此后从未使用过。 该代码段可能应如下所示：

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

片段编号 37：变量混合

 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]分配了'Size'变量，但在功能结束时未使用。 Object.cpp 424

这与前面的示例相似。 正确版本：

 this->Size += this->EntrySize;

片段编号 38-没有 47：缺少指针检查

我们稍早看了一些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]在针对nullptr进行验证之后，不安全地使用了'Ptr'指针。 检查行：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]在针对nullptr对其进行验证之后，不安全地使用了“ FD”指针。 检查行：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]在针对nullptr对其进行验证后，不安全地使用了'PtrTy'指针。 检查行：960、965。InterleavedLoadCombinePass.cpp 965

您如何避免这样的错误？ 在检查代码时要非常小心，并定期通过PVS-Studio进行检查。

我认为我们不应该研究这种类型的其他示例，因此这里只是警告列表：
  • V1004 [CWE-476]在针对nullptr进行验证之后，不安全地使用了“ Expr”指针。 检查行：1049、1078。DebugInfoMetadata.cpp 1078
  • V1004 [CWE-476]在针对nullptr对其进行验证之后，不安全地使用了'PI'指针。 检查行：733、753。LegacyPassManager.cpp 753
  • V1004 [CWE-476]在针对nullptr对其进行验证之后，不安全地使用了“ StatepointCall”指针。 检查行：4371、4379。Verifier.cpp 4379
  • V1004 [CWE-476]在针对nullptr对其进行验证之后，不安全地使用了“ RV”指针。 检查行：2263，2268。TGParser.cpp 2268
  • V1004 [CWE-476]在针对nullptr进行验证之后，不安全地使用了'CalleeFn'指针。 检查行：1081、1096。SimplifyLibCalls.cpp 1096
  • V1004 [CWE-476]在针对nullptr进行验证之后，不安全地使用了“ TC”指针。 检查行：1819、1824。Driver.cpp 1824

片段编号 48-没有 60：不重要，但仍然是缺陷（潜在的内存泄漏）

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

PVS-Studio诊断消息： V1023 [CWE-460]通过“ emplace_back”方法将没有所有者的指针添加到“策略”容器中。 发生异常时会发生内存泄漏。 llvm-isel-fuzzer.cpp 58

您不能简单地编写xxx.push_back（新X）以将元素附加到类型std :: vector <std :: unique_ptr <X >>的容器中，因为没有从X *隐式转换为std :: unique_ptr < X> 。

流行的解决方案是编写xxx.emplace_back（新X），因为它是可编译的： emplace_back方法直接从参数构造元素，因此可以使用显式构造函数。

但是该解决方案并不安全。 如果向量已满，将重新分配内存。 该操作可能失败，并最终引发std :: 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]通过'emplace_back'方法将没有所有者的指针添加到'Passes'容器中。 发生异常时会发生内存泄漏。 PassManager.h 546
  • V1023 [CWE-460] A pointer without owner is added to the 'AAs' container by the 'emplace_back' method. A memory leak will occur in case of an exception. AliasAnalysis.h 324
  • V1023 [CWE-460] A pointer without owner is added to the 'Entries' container by the 'emplace_back' method. A memory leak will occur in case of an exception. DWARFDebugFrame.cpp 519
  • V1023 [CWE-460] A pointer without owner is added to the 'AllEdges' container by the 'emplace_back' method. A memory leak will occur in case of an exception. CFGMST.h 268
  • V1023 [CWE-460] A pointer without owner is added to the 'VMaps' container by the 'emplace_back' method. A memory leak will occur in case of an exception. SimpleLoopUnswitch.cpp 2012
  • V1023 [CWE-460] A pointer without owner is added to the 'Records' container by the 'emplace_back' method. A memory leak will occur in case of an exception. FDRLogBuilder.h 30
  • V1023 [CWE-460] A pointer without owner is added to the 'PendingSubmodules' container by the 'emplace_back' method. A memory leak will occur in case of an exception. ModuleMap.cpp 810
  • V1023 [CWE-460] A pointer without owner is added to the 'Objects' container by the 'emplace_back' method. A memory leak will occur in case of an exception. DebugMap.cpp 88
  • V1023 [CWE-460] A pointer without owner is added to the 'Strategies' container by the 'emplace_back' method. A memory leak will occur in case of an exception. llvm-isel-fuzzer.cpp 60
  • V1023 [CWE-460] A pointer without owner is added to the 'Modifiers' container by the 'emplace_back' method. A memory leak will occur in case of an exception. llvm-stress.cpp 685
  • V1023 [CWE-460] A pointer without owner is added to the 'Modifiers' container by the 'emplace_back' method. A memory leak will occur in case of an exception. llvm-stress.cpp 686
  • V1023 [CWE-460] A pointer without owner is added to the 'Modifiers' container by the 'emplace_back' method. A memory leak will occur in case of an exception. llvm-stress.cpp 688
  • V1023 [CWE-460] A pointer without owner is added to the 'Modifiers' container by the 'emplace_back' method. A memory leak will occur in case of an exception. llvm-stress.cpp 689
  • V1023 [CWE-460] A pointer without owner is added to the 'Modifiers' container by the 'emplace_back' method. A memory leak will occur in case of an exception. llvm-stress.cpp 690
  • V1023 [CWE-460] A pointer without owner is added to the 'Modifiers' container by the 'emplace_back' method. A memory leak will occur in case of an exception. llvm-stress.cpp 691
  • V1023 [CWE-460] A pointer without owner is added to the 'Modifiers' container by the 'emplace_back' method. A memory leak will occur in case of an exception. llvm-stress.cpp 692
  • V1023 [CWE-460] A pointer without owner is added to the 'Modifiers' container by the 'emplace_back' method. A memory leak will occur in case of an exception. llvm-stress.cpp 693
  • V1023 [CWE-460] A pointer without owner is added to the 'Modifiers' container by the 'emplace_back' method. A memory leak will occur in case of an exception. llvm-stress.cpp 694
  • V1023 [CWE-460] A pointer without owner is added to the 'Operands' container by the 'emplace_back' method. A memory leak will occur in case of an exception. GlobalISelEmitter.cpp 1911
  • V1023 [CWE-460] A pointer without owner is added to the 'Stash' container by the 'emplace_back' method. A memory leak will occur in case of an exception. GlobalISelEmitter.cpp 2100
  • V1023 [CWE-460] A pointer without owner is added to the 'Matchers' container by the 'emplace_back' method. A memory leak will occur in case of an exception. GlobalISelEmitter.cpp 2702

结论


我写下了60条警告，然后停止了。PVS-Studio是否在LLVM中发现了其他错误？是的，确实如此。但是当我写下示例时，夜幕降临，所以我决定放弃。

我希望您喜欢阅读本文，并鼓励您自己尝试使用PVS-Studio分析仪。

访问此页面以下载分析仪并获得试用密钥。

最重要的是，定期使用静态分析。一次性检查（如我们为普及静态分析和推广PVS-Studio所做的检查）并非正常情况。

祝您改善代码的质量和可靠性！

Source: https://habr.com/ru/post/zh-CN450002/

More articles:


All Articles