[clang] [clang][NFC] Prefer usings over typedefs (PR #82920)
https://github.com/steakhal created https://github.com/llvm/llvm-project/pull/82920 None >From fd85686af33570f0baf8ba60a0b0b8113e999c4a Mon Sep 17 00:00:00 2001 From: Balazs Benics Date: Sun, 25 Feb 2024 12:37:49 +0100 Subject: [PATCH] [clang][NFC] Prefer usings over typedefs --- .../clang/Analysis/FlowSensitive/DataflowValues.h| 12 ++-- 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/clang/include/clang/Analysis/FlowSensitive/DataflowValues.h b/clang/include/clang/Analysis/FlowSensitive/DataflowValues.h index 2248bcdf3a512f..8e88f9c4d9c2df 100644 --- a/clang/include/clang/Analysis/FlowSensitive/DataflowValues.h +++ b/clang/include/clang/Analysis/FlowSensitive/DataflowValues.h @@ -45,12 +45,12 @@ class DataflowValues { //======// public: - typedef typename ValueTypes::ValTy ValTy; - typedef typename ValueTypes::AnalysisDataTy AnalysisDataTy; - typedef _AnalysisDirTag AnalysisDirTag; - typedef llvm::DenseMap EdgeDataMapTy; - typedef llvm::DenseMap BlockDataMapTy; - typedef llvm::DenseMap StmtDataMapTy; + using ValTy = typename ValueTypes::ValTy; + using AnalysisDataTy = typename ValueTypes::AnalysisDataTy; + using AnalysisDirTag = _AnalysisDirTag; + using EdgeDataMapTy = llvm::DenseMap; + using BlockDataMapTy = llvm::DenseMap; + using StmtDataMapTy = llvm::DenseMap; //======// // Predicates. ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][NFC] Trim license header comments to 81 characters (PR #82919)
https://github.com/steakhal created https://github.com/llvm/llvm-project/pull/82919 clang-format would format these headers poorly by splitting it into multiple lines. ```regex //=.{78,} ``` >From 319329630e0d2a86b22dd435985026ea236f8e56 Mon Sep 17 00:00:00 2001 From: Balazs Benics Date: Sun, 25 Feb 2024 12:48:06 +0100 Subject: [PATCH] [clang][NFC] Trim license header comments to 81 characters clang-format would format these headers poorly by splitting it into multiple lines. ```regex //=.{78,} ``` --- clang/include/clang/AST/DeclOpenMP.h | 2 +- clang/include/clang/AST/ParentMapContext.h| 2 +- clang/include/clang/Basic/OpenCLExtensionTypes.def| 2 +- clang/include/clang/Basic/RISCVVTypes.def | 8 clang/include/clang/Basic/arm_neon_incl.td| 2 +- .../include/clang/Serialization/PCHContainerOperations.h | 2 +- .../clang/StaticAnalyzer/Core/PathDiagnosticConsumers.h | 2 +- clang/lib/ARCMigrate/TransGCAttrs.cpp | 2 +- clang/lib/AST/Interp/ByteCodeEmitter.h| 2 +- clang/lib/AST/Interp/FunctionPointer.h| 2 +- clang/lib/AST/Interp/PrimType.h | 2 +- clang/lib/Driver/ToolChains/Arch/Mips.h | 2 +- clang/lib/Driver/ToolChains/Arch/Sparc.h | 2 +- clang/lib/Headers/llvm_libc_wrappers/assert.h | 2 +- clang/lib/Sema/AnalysisBasedWarnings.cpp | 2 +- clang/lib/Sema/SemaChecking.cpp | 2 +- .../lib/StaticAnalyzer/Checkers/DirectIvarAssignment.cpp | 2 +- .../Checkers/ObjCAutoreleaseWriteChecker.cpp | 2 +- .../lib/StaticAnalyzer/Checkers/STLAlgorithmModeling.cpp | 2 +- clang/lib/StaticAnalyzer/Checkers/SimpleStreamChecker.cpp | 2 +- clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp | 2 +- clang/lib/Tooling/Refactoring/AtomicChange.cpp| 2 +- clang/test/Analysis/misc-ps-region-store.mm | 4 ++-- clang/test/Rewriter/rewrite-modern-class.mm | 2 +- .../tools/clang-fuzzer/proto-to-llvm/loop_proto_to_llvm.h | 2 +- clang/tools/diagtool/DiagTool.cpp | 2 +- clang/unittests/AST/ASTImporterODRStrategiesTest.cpp | 2 +- clang/unittests/ASTMatchers/Dynamic/RegistryTest.cpp | 4 ++-- clang/unittests/ASTMatchers/Dynamic/VariantValueTest.cpp | 4 ++-- .../CXXOperatorCallExprTraverser.cpp | 2 +- .../RecursiveASTVisitorTests/CallbacksBinaryOperator.cpp | 4 ++-- .../CallbacksCompoundAssignOperator.cpp | 2 +- .../RecursiveASTVisitorTests/CallbacksUnaryOperator.cpp | 4 ++-- .../InitListExprPostOrderNoQueue.cpp | 2 +- .../InitListExprPreOrderNoQueue.cpp | 2 +- .../TemplateArgumentLocTraverser.cpp | 2 +- 36 files changed, 44 insertions(+), 44 deletions(-) diff --git a/clang/include/clang/AST/DeclOpenMP.h b/clang/include/clang/AST/DeclOpenMP.h index 73725e6e85666a..8fdfddb6c1fd74 100644 --- a/clang/include/clang/AST/DeclOpenMP.h +++ b/clang/include/clang/AST/DeclOpenMP.h @@ -1,4 +1,4 @@ -//===- DeclOpenMP.h - Classes for representing OpenMP directives -*- C++ -*-===// +//===- DeclOpenMP.h - Classes for representing OpenMP directives -*- C++ -*-==// // // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. // See https://llvm.org/LICENSE.txt for license information. diff --git a/clang/include/clang/AST/ParentMapContext.h b/clang/include/clang/AST/ParentMapContext.h index d3b2e3986a9935..6f79038627d9e1 100644 --- a/clang/include/clang/AST/ParentMapContext.h +++ b/clang/include/clang/AST/ParentMapContext.h @@ -1,4 +1,4 @@ -//===- ParentMapContext.h - Map of parents using DynTypedNode ---*- C++ -*-===// +//===- ParentMapContext.h - Map of parents using DynTypedNode ---*- C++ -*-===// // // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. // See https://llvm.org/LICENSE.txt for license information. diff --git a/clang/include/clang/Basic/OpenCLExtensionTypes.def b/clang/include/clang/Basic/OpenCLExtensionTypes.def index 17c72d69a02065..50ea826c18a77c 100644 --- a/clang/include/clang/Basic/OpenCLExtensionTypes.def +++ b/clang/include/clang/Basic/OpenCLExtensionTypes.def @@ -1,4 +1,4 @@ -//===-- OpenCLExtensionTypes.def - Metadata about BuiltinTypes --*- C++ -*-===// +//===-- OpenCLExtensionTypes.def - Metadata about BuiltinTypes --*- C++ -*-===// // // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. // See https://llvm.org/LICENSE.txt for license information. diff --git a/clang/include/clang/Basic/RISCVVTypes.def b/clang/include/clang/Basic/RISCVVTypes.def index 6620de8ad50e01..ccb8cb39068e2b 100644 --- a/clang/include/clang/Basic/RISCVVTypes.def +++ b/clang/include/clang/Basic/RISCVVTypes.def @@ -383,7 +383,7 @@
[clang] [DRAFT][analyzer][NFC] clang-format our folders (PR #82599)
@@ -33,30 +33,17 @@ namespace ento { /// checking. /// /// \sa CheckerContext -class CheckerDocumentation : public Checker< check::PreStmt, - check::PostStmt, - check::PreObjCMessage, - check::PostObjCMessage, - check::ObjCMessageNil, - check::PreCall, - check::PostCall, - check::BranchCondition, - check::NewAllocator, - check::Location, - check::Bind, - check::DeadSymbols, - check::BeginFunction, - check::EndFunction, - check::EndAnalysis, - check::EndOfTranslationUnit, - eval::Call, - eval::Assume, - check::LiveSymbols, - check::RegionChanges, - check::PointerEscape, - check::ConstPointerEscape, - check::Event, - check::ASTDecl > { +class CheckerDocumentation +: public Checker< + check::PreStmt, check::PostStmt, + check::PreObjCMessage, check::PostObjCMessage, check::ObjCMessageNil, + check::PreCall, check::PostCall, check::BranchCondition, + check::NewAllocator, check::Location, check::Bind, check::DeadSymbols, + check::BeginFunction, check::EndFunction, check::EndAnalysis, + check::EndOfTranslationUnit, eval::Call, eval::Assume, + check::LiveSymbols, check::RegionChanges, check::PointerEscape, + check::ConstPointerEscape, check::Event, + check::ASTDecl> { steakhal wrote: I was thinking of something like this: ```c++ class CheckerDocumentation : public Checker< // check::PreStmt, // check::PostStmt,// check::PreObjCMessage,// check::PostObjCMessage, // check::ObjCMessageNil,// check::PreCall, // check::PostCall, // check::BranchCondition, // check::NewAllocator, // check::Location, // check::Bind, // check::DeadSymbols, // check::BeginFunction, // check::EndFunction, // check::EndAnalysis, // check::EndOfTranslationUnit, // eval::Call, // eval::Assume, // check::LiveSymbols, // check::RegionChanges, // check::PointerEscape, // check::ConstPointerEscape,// check::Event, // check::ASTDecl // > { ``` https://github.com/llvm/llvm-project/pull/82599 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [DRAFT][analyzer][NFC] clang-format our folders (PR #82599)
@@ -1,4 +1,5 @@ -//===-- STLAlgorithmModeling.cpp ---*- C++ -*--// +//===-- STLAlgorithmModeling.cpp ---*- C++ +//-*--// steakhal wrote: ```suggestion //===-- STLAlgorithmModeling.cpp --*- C++ -*--// ``` https://github.com/llvm/llvm-project/pull/82599 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [DRAFT][analyzer][NFC] clang-format our folders (PR #82599)
@@ -1,4 +1,5 @@ -//=- DirectIvarAssignment.cpp - Check rules on ObjC properties -*- C++ *-==// +//=- DirectIvarAssignment.cpp - Check rules on ObjC properties -*- C++ +//*-==// steakhal wrote: Bad formatting. ```suggestion //=- DirectIvarAssignment.cpp - Check rules on ObjC properties -*- C++ ---*-==// ``` https://github.com/llvm/llvm-project/pull/82599 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [DRAFT][analyzer][NFC] clang-format our folders (PR #82599)
@@ -219,17 +219,19 @@ void WalkAST::VisitCallExpr(CallExpr *CE) { if (containsBadStrncatPattern(CE)) { const Expr *DstArg = CE->getArg(0); const Expr *LenArg = CE->getArg(2); - PathDiagnosticLocation Loc = -PathDiagnosticLocation::createBegin(LenArg, BR.getSourceManager(), AC); + PathDiagnosticLocation Loc = PathDiagnosticLocation::createBegin( + LenArg, BR.getSourceManager(), AC); StringRef DstName = getPrintableName(DstArg); SmallString<256> S; llvm::raw_svector_ostream os(S); os << "Potential buffer overflow. "; if (!DstName.empty()) { -os << "Replace with 'sizeof(" << DstName << ") " - "- strlen(" << DstName <<") - 1'"; +os << "Replace with 'sizeof(" << DstName + << ") " + "- strlen(" + << DstName << ") - 1'"; steakhal wrote: Bad formatting. https://github.com/llvm/llvm-project/pull/82599 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [DRAFT][analyzer][NFC] clang-format our folders (PR #82599)
@@ -110,21 +111,41 @@ void ExprEngine::VisitBinaryOperator(const BinaryOperator* B, continue; } -assert (B->isCompoundAssignmentOp()); +assert(B->isCompoundAssignmentOp()); switch (Op) { - default: -llvm_unreachable("Invalid opcode for compound assignment."); - case BO_MulAssign: Op = BO_Mul; break; - case BO_DivAssign: Op = BO_Div; break; - case BO_RemAssign: Op = BO_Rem; break; - case BO_AddAssign: Op = BO_Add; break; - case BO_SubAssign: Op = BO_Sub; break; - case BO_ShlAssign: Op = BO_Shl; break; - case BO_ShrAssign: Op = BO_Shr; break; - case BO_AndAssign: Op = BO_And; break; - case BO_XorAssign: Op = BO_Xor; break; - case BO_OrAssign: Op = BO_Or; break; +default: + llvm_unreachable("Invalid opcode for compound assignment."); +case BO_MulAssign: + Op = BO_Mul; + break; +case BO_DivAssign: + Op = BO_Div; + break; +case BO_RemAssign: + Op = BO_Rem; + break; +case BO_AddAssign: + Op = BO_Add; + break; +case BO_SubAssign: + Op = BO_Sub; + break; +case BO_ShlAssign: + Op = BO_Shl; + break; +case BO_ShrAssign: + Op = BO_Shr; + break; +case BO_AndAssign: + Op = BO_And; + break; +case BO_XorAssign: + Op = BO_Xor; + break; +case BO_OrAssign: + Op = BO_Or; + break; steakhal wrote: ```suggestion // clang-format off default: llvm_unreachable("Invalid opcode for compound assignment."); case BO_MulAssign: Op = BO_Mul; break; case BO_DivAssign: Op = BO_Div; break; case BO_RemAssign: Op = BO_Rem; break; case BO_AddAssign: Op = BO_Add; break; case BO_SubAssign: Op = BO_Sub; break; case BO_ShlAssign: Op = BO_Shl; break; case BO_ShrAssign: Op = BO_Shr; break; case BO_AndAssign: Op = BO_And; break; case BO_XorAssign: Op = BO_Xor; break; case BO_OrAssign: Op = BO_Or; break; // clang-format on ``` https://github.com/llvm/llvm-project/pull/82599 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [DRAFT][analyzer][NFC] clang-format our folders (PR #82599)
@@ -49,54 +50,44 @@ const char *IsARPBind = "isautoreleasepoolbind"; class ObjCAutoreleaseWriteChecker : public Checker { public: - void checkASTCodeBody(const Decl *D, -AnalysisManager , + void checkASTCodeBody(const Decl *D, AnalysisManager , BugReporter ) const; + private: std::vector SelectorsWithAutoreleasingPool = { // Common to NSArray, NSSet, NSOrderedSet - "enumerateObjectsUsingBlock:", - "enumerateObjectsWithOptions:usingBlock:", + "enumerateObjectsUsingBlock:", "enumerateObjectsWithOptions:usingBlock:", // Common to NSArray and NSOrderedSet "enumerateObjectsAtIndexes:options:usingBlock:", "indexOfObjectAtIndexes:options:passingTest:", "indexesOfObjectsAtIndexes:options:passingTest:", - "indexOfObjectPassingTest:", - "indexOfObjectWithOptions:passingTest:", + "indexOfObjectPassingTest:", "indexOfObjectWithOptions:passingTest:", "indexesOfObjectsPassingTest:", "indexesOfObjectsWithOptions:passingTest:", // NSDictionary "enumerateKeysAndObjectsUsingBlock:", "enumerateKeysAndObjectsWithOptions:usingBlock:", - "keysOfEntriesPassingTest:", - "keysOfEntriesWithOptions:passingTest:", + "keysOfEntriesPassingTest:", "keysOfEntriesWithOptions:passingTest:", // NSSet - "objectsPassingTest:", - "objectsWithOptions:passingTest:", + "objectsPassingTest:", "objectsWithOptions:passingTest:", "enumerateIndexPathsWithOptions:usingBlock:", // NSIndexSet - "enumerateIndexesWithOptions:usingBlock:", - "enumerateIndexesUsingBlock:", + "enumerateIndexesWithOptions:usingBlock:", "enumerateIndexesUsingBlock:", "enumerateIndexesInRange:options:usingBlock:", - "enumerateRangesUsingBlock:", - "enumerateRangesWithOptions:usingBlock:", - "enumerateRangesInRange:options:usingBlock:", - "indexPassingTest:", - "indexesPassingTest:", - "indexWithOptions:passingTest:", - "indexesWithOptions:passingTest:", - "indexInRange:options:passingTest:", - "indexesInRange:options:passingTest:" - }; + "enumerateRangesUsingBlock:", "enumerateRangesWithOptions:usingBlock:", + "enumerateRangesInRange:options:usingBlock:", "indexPassingTest:", + "indexesPassingTest:", "indexWithOptions:passingTest:", + "indexesWithOptions:passingTest:", "indexInRange:options:passingTest:", + "indexesInRange:options:passingTest:"}; steakhal wrote: Bad formatting for the `SelectorsWithAutoreleasingPool` member. https://github.com/llvm/llvm-project/pull/82599 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [DRAFT][analyzer][NFC] clang-format our folders (PR #82599)
@@ -1,4 +1,5 @@ -//===-- SimpleStreamChecker.cpp -*- C++ -*--// +//===-- SimpleStreamChecker.cpp -*- C++ +//-*--// steakhal wrote: ```suggestion //===-- SimpleStreamChecker.cpp ---*- C++ -*--// ``` https://github.com/llvm/llvm-project/pull/82599 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [DRAFT][analyzer][NFC] clang-format our folders (PR #82599)
https://github.com/steakhal commented: Yeey, my review is done. We only have a handful (<50) debatable formatting problems. I'll try to fix them in an other PR. https://github.com/llvm/llvm-project/pull/82599 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [DRAFT][analyzer][NFC] clang-format our folders (PR #82599)
@@ -244,24 +246,25 @@ void WalkAST::VisitCallExpr(CallExpr *CE) { if (containsBadStrlcpyStrlcatPattern(CE)) { const Expr *DstArg = CE->getArg(0); const Expr *LenArg = CE->getArg(2); - PathDiagnosticLocation Loc = -PathDiagnosticLocation::createBegin(LenArg, BR.getSourceManager(), AC); + PathDiagnosticLocation Loc = PathDiagnosticLocation::createBegin( + LenArg, BR.getSourceManager(), AC); StringRef DstName = getPrintableName(DstArg); SmallString<256> S; llvm::raw_svector_ostream os(S); - os << "The third argument allows to potentially copy more bytes than it should. "; + os << "The third argument allows to potentially copy more bytes than it " +"should. "; os << "Replace with the value "; steakhal wrote: Bad formatting. https://github.com/llvm/llvm-project/pull/82599 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [DRAFT][analyzer][NFC] clang-format our folders (PR #82599)
@@ -33,30 +33,17 @@ namespace ento { /// checking. /// /// \sa CheckerContext -class CheckerDocumentation : public Checker< check::PreStmt, - check::PostStmt, - check::PreObjCMessage, - check::PostObjCMessage, - check::ObjCMessageNil, - check::PreCall, - check::PostCall, - check::BranchCondition, - check::NewAllocator, - check::Location, - check::Bind, - check::DeadSymbols, - check::BeginFunction, - check::EndFunction, - check::EndAnalysis, - check::EndOfTranslationUnit, - eval::Call, - eval::Assume, - check::LiveSymbols, - check::RegionChanges, - check::PointerEscape, - check::ConstPointerEscape, - check::Event, - check::ASTDecl > { +class CheckerDocumentation +: public Checker< + check::PreStmt, check::PostStmt, + check::PreObjCMessage, check::PostObjCMessage, check::ObjCMessageNil, + check::PreCall, check::PostCall, check::BranchCondition, + check::NewAllocator, check::Location, check::Bind, check::DeadSymbols, + check::BeginFunction, check::EndFunction, check::EndAnalysis, + check::EndOfTranslationUnit, eval::Call, eval::Assume, + check::LiveSymbols, check::RegionChanges, check::PointerEscape, + check::ConstPointerEscape, check::Event, + check::ASTDecl> { steakhal wrote: Bad formatting. https://github.com/llvm/llvm-project/pull/82599 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [DRAFT][analyzer][NFC] clang-format our folders (PR #82599)
@@ -380,27 +376,41 @@ enum CFNumberType { }; static std::optional GetCFNumberSize(ASTContext , uint64_t i) { - static const unsigned char FixedSize[] = { 8, 16, 32, 64, 32, 64 }; + static const unsigned char FixedSize[] = {8, 16, 32, 64, 32, 64}; if (i < kCFNumberCharType) -return FixedSize[i-1]; +return FixedSize[i - 1]; QualType T; switch (i) { -case kCFNumberCharType: T = Ctx.CharTy; break; -case kCFNumberShortType:T = Ctx.ShortTy;break; -case kCFNumberIntType: T = Ctx.IntTy; break; -case kCFNumberLongType: T = Ctx.LongTy; break; -case kCFNumberLongLongType: T = Ctx.LongLongTy; break; -case kCFNumberFloatType:T = Ctx.FloatTy;break; -case kCFNumberDoubleType: T = Ctx.DoubleTy; break; -case kCFNumberCFIndexType: -case kCFNumberNSIntegerType: -case kCFNumberCGFloatType: - // FIXME: We need a way to map from names to Type*. -default: - return std::nullopt; + case kCFNumberCharType: +T = Ctx.CharTy; +break; + case kCFNumberShortType: +T = Ctx.ShortTy; +break; + case kCFNumberIntType: +T = Ctx.IntTy; +break; + case kCFNumberLongType: +T = Ctx.LongTy; +break; + case kCFNumberLongLongType: +T = Ctx.LongLongTy; +break; + case kCFNumberFloatType: +T = Ctx.FloatTy; +break; + case kCFNumberDoubleType: +T = Ctx.DoubleTy; +break; + case kCFNumberCFIndexType: + case kCFNumberNSIntegerType: + case kCFNumberCGFloatType: +// FIXME: We need a way to map from names to Type*. + default: +return std::nullopt; steakhal wrote: ```suggestion // clang-format off case kCFNumberCharType: T = Ctx.CharTy; break; case kCFNumberShortType:T = Ctx.ShortTy;break; case kCFNumberIntType: T = Ctx.IntTy; break; case kCFNumberLongType: T = Ctx.LongTy; break; case kCFNumberLongLongType: T = Ctx.LongLongTy; break; case kCFNumberFloatType:T = Ctx.FloatTy;break; case kCFNumberDoubleType: T = Ctx.DoubleTy; break; case kCFNumberCFIndexType: case kCFNumberNSIntegerType: case kCFNumberCGFloatType: // FIXME: We need a way to map from names to Type*. default: return std::nullopt; // clang-format on ``` https://github.com/llvm/llvm-project/pull/82599 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [DRAFT][analyzer][NFC] clang-format our folders (PR #82599)
@@ -18,37 +18,57 @@ using namespace til; StringRef til::getUnaryOpcodeString(TIL_UnaryOpcode Op) { switch (Op) { -case UOP_Minus:return "-"; -case UOP_BitNot: return "~"; -case UOP_LogicNot: return "!"; + case UOP_Minus: +return "-"; + case UOP_BitNot: +return "~"; + case UOP_LogicNot: +return "!"; } return {}; } StringRef til::getBinaryOpcodeString(TIL_BinaryOpcode Op) { switch (Op) { -case BOP_Mul: return "*"; -case BOP_Div: return "/"; -case BOP_Rem: return "%"; -case BOP_Add: return "+"; -case BOP_Sub: return "-"; -case BOP_Shl: return "<<"; -case BOP_Shr: return ">>"; -case BOP_BitAnd: return "&"; -case BOP_BitXor: return "^"; -case BOP_BitOr:return "|"; -case BOP_Eq: return "=="; -case BOP_Neq: return "!="; -case BOP_Lt: return "<"; -case BOP_Leq: return "<="; -case BOP_Cmp: return "<=>"; -case BOP_LogicAnd: return "&&"; -case BOP_LogicOr: return "||"; + case BOP_Mul: +return "*"; + case BOP_Div: +return "/"; + case BOP_Rem: +return "%"; + case BOP_Add: +return "+"; + case BOP_Sub: +return "-"; + case BOP_Shl: +return "<<"; + case BOP_Shr: +return ">>"; + case BOP_BitAnd: +return "&"; + case BOP_BitXor: +return "^"; + case BOP_BitOr: +return "|"; + case BOP_Eq: +return "=="; + case BOP_Neq: +return "!="; + case BOP_Lt: +return "<"; + case BOP_Leq: +return "<="; + case BOP_Cmp: +return "<=>"; + case BOP_LogicAnd: +return "&&"; + case BOP_LogicOr: +return "||"; steakhal wrote: ```suggestion // clang-format off case BOP_Mul: return "*"; case BOP_Div: return "/"; case BOP_Rem: return "%"; case BOP_Add: return "+"; case BOP_Sub: return "-"; case BOP_Shl: return "<<"; case BOP_Shr: return ">>"; case BOP_BitAnd: return "&"; case BOP_BitXor: return "^"; case BOP_BitOr:return "|"; case BOP_Eq: return "=="; case BOP_Neq: return "!="; case BOP_Lt: return "<"; case BOP_Leq: return "<="; case BOP_Cmp: return "<=>"; case BOP_LogicAnd: return "&&"; case BOP_LogicOr: return "||"; // clang-format on ``` https://github.com/llvm/llvm-project/pull/82599 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [DRAFT][analyzer][NFC] clang-format our folders (PR #82599)
@@ -535,7 +545,8 @@ void CFNumberChecker::checkPreStmt(const CallExpr *CE, } //===--===// -// CFRetain/CFRelease/CFMakeCollectable/CFAutorelease checking for null arguments. +// CFRetain/CFRelease/CFMakeCollectable/CFAutorelease checking for null +// arguments. steakhal wrote: This is debatable formatting. https://github.com/llvm/llvm-project/pull/82599 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [DRAFT][analyzer][NFC] clang-format our folders (PR #82599)
https://github.com/steakhal edited https://github.com/llvm/llvm-project/pull/82599 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [DRAFT][analyzer][NFC] clang-format our folders (PR #82599)
@@ -565,37 +565,67 @@ til::SExpr *SExprBuilder::translateBinaryOperator(const BinaryOperator *BO, case BO_PtrMemI: return new (Arena) til::Undefined(BO); - case BO_Mul: return translateBinOp(til::BOP_Mul, BO, Ctx); - case BO_Div: return translateBinOp(til::BOP_Div, BO, Ctx); - case BO_Rem: return translateBinOp(til::BOP_Rem, BO, Ctx); - case BO_Add: return translateBinOp(til::BOP_Add, BO, Ctx); - case BO_Sub: return translateBinOp(til::BOP_Sub, BO, Ctx); - case BO_Shl: return translateBinOp(til::BOP_Shl, BO, Ctx); - case BO_Shr: return translateBinOp(til::BOP_Shr, BO, Ctx); - case BO_LT: return translateBinOp(til::BOP_Lt, BO, Ctx); - case BO_GT: return translateBinOp(til::BOP_Lt, BO, Ctx, true); - case BO_LE: return translateBinOp(til::BOP_Leq, BO, Ctx); - case BO_GE: return translateBinOp(til::BOP_Leq, BO, Ctx, true); - case BO_EQ: return translateBinOp(til::BOP_Eq, BO, Ctx); - case BO_NE: return translateBinOp(til::BOP_Neq, BO, Ctx); - case BO_Cmp: return translateBinOp(til::BOP_Cmp, BO, Ctx); - case BO_And: return translateBinOp(til::BOP_BitAnd, BO, Ctx); - case BO_Xor: return translateBinOp(til::BOP_BitXor, BO, Ctx); - case BO_Or: return translateBinOp(til::BOP_BitOr,BO, Ctx); - case BO_LAnd: return translateBinOp(til::BOP_LogicAnd, BO, Ctx); - case BO_LOr: return translateBinOp(til::BOP_LogicOr, BO, Ctx); - - case BO_Assign:return translateBinAssign(til::BOP_Eq, BO, Ctx, true); - case BO_MulAssign: return translateBinAssign(til::BOP_Mul, BO, Ctx); - case BO_DivAssign: return translateBinAssign(til::BOP_Div, BO, Ctx); - case BO_RemAssign: return translateBinAssign(til::BOP_Rem, BO, Ctx); - case BO_AddAssign: return translateBinAssign(til::BOP_Add, BO, Ctx); - case BO_SubAssign: return translateBinAssign(til::BOP_Sub, BO, Ctx); - case BO_ShlAssign: return translateBinAssign(til::BOP_Shl, BO, Ctx); - case BO_ShrAssign: return translateBinAssign(til::BOP_Shr, BO, Ctx); - case BO_AndAssign: return translateBinAssign(til::BOP_BitAnd, BO, Ctx); - case BO_XorAssign: return translateBinAssign(til::BOP_BitXor, BO, Ctx); - case BO_OrAssign: return translateBinAssign(til::BOP_BitOr, BO, Ctx); + case BO_Mul: +return translateBinOp(til::BOP_Mul, BO, Ctx); + case BO_Div: +return translateBinOp(til::BOP_Div, BO, Ctx); + case BO_Rem: +return translateBinOp(til::BOP_Rem, BO, Ctx); + case BO_Add: +return translateBinOp(til::BOP_Add, BO, Ctx); + case BO_Sub: +return translateBinOp(til::BOP_Sub, BO, Ctx); + case BO_Shl: +return translateBinOp(til::BOP_Shl, BO, Ctx); + case BO_Shr: +return translateBinOp(til::BOP_Shr, BO, Ctx); + case BO_LT: +return translateBinOp(til::BOP_Lt, BO, Ctx); + case BO_GT: +return translateBinOp(til::BOP_Lt, BO, Ctx, true); + case BO_LE: +return translateBinOp(til::BOP_Leq, BO, Ctx); + case BO_GE: +return translateBinOp(til::BOP_Leq, BO, Ctx, true); + case BO_EQ: +return translateBinOp(til::BOP_Eq, BO, Ctx); + case BO_NE: +return translateBinOp(til::BOP_Neq, BO, Ctx); + case BO_Cmp: +return translateBinOp(til::BOP_Cmp, BO, Ctx); + case BO_And: +return translateBinOp(til::BOP_BitAnd, BO, Ctx); + case BO_Xor: +return translateBinOp(til::BOP_BitXor, BO, Ctx); + case BO_Or: +return translateBinOp(til::BOP_BitOr, BO, Ctx); + case BO_LAnd: +return translateBinOp(til::BOP_LogicAnd, BO, Ctx); + case BO_LOr: +return translateBinOp(til::BOP_LogicOr, BO, Ctx); + + case BO_Assign: +return translateBinAssign(til::BOP_Eq, BO, Ctx, true); + case BO_MulAssign: +return translateBinAssign(til::BOP_Mul, BO, Ctx); + case BO_DivAssign: +return translateBinAssign(til::BOP_Div, BO, Ctx); + case BO_RemAssign: +return translateBinAssign(til::BOP_Rem, BO, Ctx); + case BO_AddAssign: +return translateBinAssign(til::BOP_Add, BO, Ctx); + case BO_SubAssign: +return translateBinAssign(til::BOP_Sub, BO, Ctx); + case BO_ShlAssign: +return translateBinAssign(til::BOP_Shl, BO, Ctx); + case BO_ShrAssign: +return translateBinAssign(til::BOP_Shr, BO, Ctx); + case BO_AndAssign: +return translateBinAssign(til::BOP_BitAnd, BO, Ctx); + case BO_XorAssign: +return translateBinAssign(til::BOP_BitXor, BO, Ctx); + case BO_OrAssign: +return translateBinAssign(til::BOP_BitOr, BO, Ctx); steakhal wrote: ```suggestion // clang-format off case BO_Mul: return translateBinOp(til::BOP_Mul, BO, Ctx); case BO_Div: return translateBinOp(til::BOP_Div, BO, Ctx); case BO_Rem: return translateBinOp(til::BOP_Rem, BO, Ctx); case BO_Add: return translateBinOp(til::BOP_Add, BO, Ctx); case BO_Sub: return translateBinOp(til::BOP_Sub, BO, Ctx); case BO_Shl: return translateBinOp(til::BOP_Shl, BO, Ctx); case BO_Shr: return translateBinOp(til::BOP_Shr, BO, Ctx); case BO_LT: return translateBinOp(til::BOP_Lt, BO, Ctx); case BO_GT: return translateBinOp(til::BOP_Lt, BO,
[clang] [Clang] Fixes to immediate-escalating functions (PR #82281)
steakhal wrote: I've submitted a PR #82609 to backport this commit to release/18.x as it fixes a crash and would be also important for us downstream. https://github.com/llvm/llvm-project/pull/82281 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [DRAFT][analyzer][NFC] clang-format our folders (PR #82599)
https://github.com/steakhal edited https://github.com/llvm/llvm-project/pull/82599 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [DRAFT][analyzer][NFC] clang-format our folders (PR #82599)
@@ -520,18 +518,19 @@ class TypedRegion : public SubRegion { bool isBoundable() const override { return true; } - static bool classof(const MemRegion* R) { + static bool classof(const MemRegion *R) { unsigned k = R->getKind(); return k >= BEGIN_TYPED_REGIONS && k <= END_TYPED_REGIONS; } }; -/// TypedValueRegion - An abstract class representing regions having a typed value. +/// TypedValueRegion - An abstract class representing regions having a typed +/// value. steakhal wrote: ```suggestion /// An abstract class representing regions having a typed value. ``` https://github.com/llvm/llvm-project/pull/82599 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [DRAFT][analyzer][NFC] clang-format our folders (PR #82599)
@@ -128,14 +128,14 @@ enum TIL_CastOpcode : unsigned char { CAST_objToPtr }; -const TIL_Opcode COP_Min = COP_Future; -const TIL_Opcode COP_Max = COP_Branch; -const TIL_UnaryOpcode UOP_Min = UOP_Minus; -const TIL_UnaryOpcode UOP_Max = UOP_LogicNot; -const TIL_BinaryOpcode BOP_Min = BOP_Add; -const TIL_BinaryOpcode BOP_Max = BOP_LogicOr; -const TIL_CastOpcode CAST_Min = CAST_none; -const TIL_CastOpcode CAST_Max = CAST_toInt; +const TIL_Opcode COP_Min = COP_Future; +const TIL_Opcode COP_Max = COP_Branch; +const TIL_UnaryOpcode UOP_Min = UOP_Minus; +const TIL_UnaryOpcode UOP_Max = UOP_LogicNot; +const TIL_BinaryOpcode BOP_Min = BOP_Add; +const TIL_BinaryOpcode BOP_Max = BOP_LogicOr; +const TIL_CastOpcode CAST_Min = CAST_none; +const TIL_CastOpcode CAST_Max = CAST_toInt; steakhal wrote: ```suggestion // clang-format off const TIL_Opcode COP_Min = COP_Future; const TIL_Opcode COP_Max = COP_Branch; const TIL_UnaryOpcode UOP_Min = UOP_Minus; const TIL_UnaryOpcode UOP_Max = UOP_LogicNot; const TIL_BinaryOpcode BOP_Min = BOP_Add; const TIL_BinaryOpcode BOP_Max = BOP_LogicOr; const TIL_CastOpcode CAST_Min = CAST_none; const TIL_CastOpcode CAST_Max = CAST_toInt; // clang-format on ``` This was previously nicely aligned. https://github.com/llvm/llvm-project/pull/82599 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [DRAFT][analyzer][NFC] clang-format our folders (PR #82599)
@@ -28,29 +28,29 @@ namespace clang { //===--===// namespace dataflow { - struct forward_analysis_tag {}; - struct backward_analysis_tag {}; +struct forward_analysis_tag {}; +struct backward_analysis_tag {}; } // end namespace dataflow //===--===// /// DataflowValues. Container class to store dataflow values for a CFG. //===--===// template + typename _AnalysisDirTag = dataflow::forward_analysis_tag> class DataflowValues { //======// // Type declarations. //======// public: - typedef typename ValueTypes::ValTy ValTy; - typedef typename ValueTypes::AnalysisDataTy AnalysisDataTy; - typedef _AnalysisDirTag AnalysisDirTag; - typedef llvm::DenseMap EdgeDataMapTy; - typedef llvm::DenseMap BlockDataMapTy; - typedef llvm::DenseMap StmtDataMapTy; + typedef typename ValueTypes::ValTy ValTy; + typedef typename ValueTypes::AnalysisDataTy AnalysisDataTy; + typedef _AnalysisDirTag AnalysisDirTag; + typedef llvm::DenseMap EdgeDataMapTy; + typedef llvm::DenseMap BlockDataMapTy; + typedef llvm::DenseMap StmtDataMapTy; steakhal wrote: This was aligned before. https://github.com/llvm/llvm-project/pull/82599 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [DRAFT][analyzer][NFC] clang-format our folders (PR #82599)
@@ -1,4 +1,5 @@ -//===--- PathDiagnosticConsumers.h - Path Diagnostic Clients --*- C++ -*-===// +//===--- PathDiagnosticConsumers.h - Path Diagnostic Clients --*- C++ +//-*-===// steakhal wrote: ```suggestion //===--- PathDiagnosticConsumers.h - Path Diagnostic Clients *- C++ -*-===// ``` Uhh, I hope I didn't miss similar cases. https://github.com/llvm/llvm-project/pull/82599 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [DRAFT][analyzer][NFC] clang-format our folders (PR #82599)
@@ -393,20 +385,19 @@ class ExprEngine { ProgramStateRef processAssume(ProgramStateRef state, SVal cond, bool assumption); - /// processRegionChanges - Called by ProgramStateManager whenever a change is made + /// processRegionChanges - Called by ProgramStateManager whenever a change is + /// made steakhal wrote: ```suggestion /// It's called by ProgramStateManager whenever a change is made. ``` https://github.com/llvm/llvm-project/pull/82599 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [DRAFT][analyzer][NFC] clang-format our folders (PR #82599)
@@ -48,17 +48,26 @@ class ConstructionContextItem { LLVM_DUMP_METHOD static StringRef getKindAsString(ItemKind K) { switch (K) { - case VariableKind:return "construct into local variable"; - case NewAllocatorKind:return "construct into new-allocator"; - case ReturnKind: return "construct into return address"; - case MaterializationKind: return "materialize temporary"; - case TemporaryDestructorKind: return "destroy temporary"; - case ElidedDestructorKind:return "elide destructor"; - case ElidableConstructorKind: return "elide constructor"; - case ArgumentKind:return "construct into argument"; - case LambdaCaptureKind: -return "construct into lambda captured variable"; - case InitializerKind: return "construct into member variable"; +case VariableKind: + return "construct into local variable"; +case NewAllocatorKind: + return "construct into new-allocator"; +case ReturnKind: + return "construct into return address"; +case MaterializationKind: + return "materialize temporary"; +case TemporaryDestructorKind: + return "destroy temporary"; +case ElidedDestructorKind: + return "elide destructor"; +case ElidableConstructorKind: + return "elide constructor"; +case ArgumentKind: + return "construct into argument"; +case LambdaCaptureKind: + return "construct into lambda captured variable"; +case InitializerKind: + return "construct into member variable"; steakhal wrote: This hunk was aligned before, but I actually like the new format more. https://github.com/llvm/llvm-project/pull/82599 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [DRAFT][analyzer][NFC] clang-format our folders (PR #82599)
@@ -145,7 +146,8 @@ class CallDescription { return CD1.matchesAsWritten(CE); } - /// \copydoc clang::ento::CallDescription::matchesAnyAsWritten(const CallExpr &, const CallDescription &) + /// \copydoc clang::ento::CallDescription::matchesAnyAsWritten(const CallExpr + /// &, const CallDescription &) steakhal wrote: Same Doxygen problem here. https://github.com/llvm/llvm-project/pull/82599 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [DRAFT][analyzer][NFC] clang-format our folders (PR #82599)
https://github.com/steakhal edited https://github.com/llvm/llvm-project/pull/82599 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [DRAFT][analyzer][NFC] clang-format our folders (PR #82599)
@@ -898,18 +867,18 @@ class CFGBlock { size_t getIndexInCFG() const; - CFGElement front() const { return Elements.front(); } - CFGElement back()const { return Elements.back();} + CFGElement front() const { return Elements.front(); } + CFGElement back() const { return Elements.back(); } - iterator begin() { return Elements.begin(); } - iterator end() { return Elements.end(); } - const_iterator begin() const { return Elements.begin(); } - const_iterator end() const { return Elements.end(); } + iterator begin() { return Elements.begin(); } + iterator end() { return Elements.end(); } + const_iterator begin() const { return Elements.begin(); } + const_iterator end() const { return Elements.end(); } - reverse_iterator rbegin(){ return Elements.rbegin(); } - reverse_iterator rend() { return Elements.rend();} - const_reverse_iterator rbegin() const { return Elements.rbegin(); } - const_reverse_iterator rend()const { return Elements.rend();} + reverse_iterator rbegin() { return Elements.rbegin(); } + reverse_iterator rend() { return Elements.rend(); } + const_reverse_iterator rbegin() const { return Elements.rbegin(); } + const_reverse_iterator rend() const { return Elements.rend(); } steakhal wrote: These hunks were aligned before, but I don't think it's worth to keep it. There are some similar later, that I won't explicitly highlight. https://github.com/llvm/llvm-project/pull/82599 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [DRAFT][analyzer][NFC] clang-format our folders (PR #82599)
@@ -1481,22 +1449,24 @@ class Return : public Terminator { return Vs.reduceReturn(*this, Ne); } - template - typename C::CType compare(const Return *E, C ) const { + template typename C::CType compare(const Return *E, C ) const { return Cmp.compare(Retval, E->Retval); } private: - SExpr* Retval; + SExpr *Retval; }; -inline ArrayRef Terminator::successors() { +inline ArrayRef Terminator::successors() { switch (opcode()) { -case COP_Goto: return cast(this)->successors(); -case COP_Branch: return cast(this)->successors(); -case COP_Return: return cast(this)->successors(); -default: - return std::nullopt; + case COP_Goto: +return cast(this)->successors(); + case COP_Branch: +return cast(this)->successors(); + case COP_Return: +return cast(this)->successors(); + default: +return std::nullopt; steakhal wrote: This was aligned before, but I don't see much value keeping it. https://github.com/llvm/llvm-project/pull/82599 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [DRAFT][analyzer][NFC] clang-format our folders (PR #82599)
@@ -441,48 +419,79 @@ class PrettyPrinter { // Return the precedence of a given node, for use in pretty printing. unsigned precedence(const SExpr *E) { switch (E->opcode()) { - case COP_Future: return Prec_Atom; - case COP_Undefined: return Prec_Atom; - case COP_Wildcard: return Prec_Atom; - - case COP_Literal:return Prec_Atom; - case COP_LiteralPtr: return Prec_Atom; - case COP_Variable: return Prec_Atom; - case COP_Function: return Prec_Decl; - case COP_SFunction: return Prec_Decl; - case COP_Code: return Prec_Decl; - case COP_Field: return Prec_Decl; - - case COP_Apply: return Prec_Postfix; - case COP_SApply: return Prec_Postfix; - case COP_Project:return Prec_Postfix; - - case COP_Call: return Prec_Postfix; - case COP_Alloc: return Prec_Other; - case COP_Load: return Prec_Postfix; - case COP_Store: return Prec_Other; - case COP_ArrayIndex: return Prec_Postfix; - case COP_ArrayAdd: return Prec_Postfix; - - case COP_UnaryOp:return Prec_Unary; - case COP_BinaryOp: return Prec_Binary; - case COP_Cast: return Prec_Atom; - - case COP_SCFG: return Prec_Decl; - case COP_BasicBlock: return Prec_MAX; - case COP_Phi:return Prec_Atom; - case COP_Goto: return Prec_Atom; - case COP_Branch: return Prec_Atom; - case COP_Return: return Prec_Other; - - case COP_Identifier: return Prec_Atom; - case COP_IfThenElse: return Prec_Other; - case COP_Let:return Prec_Decl; +case COP_Future: + return Prec_Atom; +case COP_Undefined: + return Prec_Atom; +case COP_Wildcard: + return Prec_Atom; + +case COP_Literal: + return Prec_Atom; +case COP_LiteralPtr: + return Prec_Atom; +case COP_Variable: + return Prec_Atom; +case COP_Function: + return Prec_Decl; +case COP_SFunction: + return Prec_Decl; +case COP_Code: + return Prec_Decl; +case COP_Field: + return Prec_Decl; + +case COP_Apply: + return Prec_Postfix; +case COP_SApply: + return Prec_Postfix; +case COP_Project: + return Prec_Postfix; + +case COP_Call: + return Prec_Postfix; +case COP_Alloc: + return Prec_Other; +case COP_Load: + return Prec_Postfix; +case COP_Store: + return Prec_Other; +case COP_ArrayIndex: + return Prec_Postfix; +case COP_ArrayAdd: + return Prec_Postfix; + +case COP_UnaryOp: + return Prec_Unary; +case COP_BinaryOp: + return Prec_Binary; +case COP_Cast: + return Prec_Atom; + +case COP_SCFG: + return Prec_Decl; +case COP_BasicBlock: + return Prec_MAX; +case COP_Phi: + return Prec_Atom; +case COP_Goto: + return Prec_Atom; +case COP_Branch: + return Prec_Atom; +case COP_Return: + return Prec_Other; + +case COP_Identifier: + return Prec_Atom; +case COP_IfThenElse: + return Prec_Other; +case COP_Let: + return Prec_Decl; steakhal wrote: ```suggestion // clang-format off case COP_Future: return Prec_Atom; case COP_Undefined: return Prec_Atom; case COP_Wildcard: return Prec_Atom; case COP_Literal:return Prec_Atom; case COP_LiteralPtr: return Prec_Atom; case COP_Variable: return Prec_Atom; case COP_Function: return Prec_Decl; case COP_SFunction: return Prec_Decl; case COP_Code: return Prec_Decl; case COP_Field: return Prec_Decl; case COP_Apply: return Prec_Postfix; case COP_SApply: return Prec_Postfix; case COP_Project:return Prec_Postfix; case COP_Call: return Prec_Postfix; case COP_Alloc: return Prec_Other; case COP_Load: return Prec_Postfix; case COP_Store: return Prec_Other; case COP_ArrayIndex: return Prec_Postfix; case COP_ArrayAdd: return Prec_Postfix; case COP_UnaryOp:return Prec_Unary; case COP_BinaryOp: return Prec_Binary; case COP_Cast: return Prec_Atom; case COP_SCFG: return Prec_Decl; case COP_BasicBlock: return Prec_MAX; case COP_Phi:return Prec_Atom; case COP_Goto: return Prec_Atom; case COP_Branch: return Prec_Atom; case COP_Return: return Prec_Other; case COP_Identifier: return Prec_Atom; case COP_IfThenElse: return Prec_Other; case COP_Let:return Prec_Decl; // clang-format on ``` https://github.com/llvm/llvm-project/pull/82599 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [DRAFT][analyzer][NFC] clang-format our folders (PR #82599)
@@ -107,7 +107,8 @@ class CallDescription { return CD1.matches(Call); } - /// \copydoc clang::ento::CallDescription::matchesAny(const CallEvent &, const CallDescription &) + /// \copydoc clang::ento::CallDescription::matchesAny(const CallEvent &, const + /// CallDescription &) steakhal wrote: I'm not sure how to resolve this. I'd need to check how to resolve this for Doxygen. https://clang.llvm.org/doxygen/classclang_1_1ento_1_1CallDescription.html#ac6873d422bacd2cc5a9857b4f96248ff https://github.com/llvm/llvm-project/pull/82599 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [DRAFT][analyzer][NFC] clang-format our folders (PR #82599)
@@ -188,88 +187,79 @@ struct ValueType { inline ValueType::SizeType ValueType::getSizeType(unsigned nbytes) { switch (nbytes) { -case 1: return ST_8; -case 2: return ST_16; -case 4: return ST_32; -case 8: return ST_64; -case 16: return ST_128; -default: return ST_0; + case 1: +return ST_8; + case 2: +return ST_16; + case 4: +return ST_32; + case 8: +return ST_64; + case 16: +return ST_128; + default: +return ST_0; steakhal wrote: ```suggestion // clang-format off case 1: return ST_8; case 2: return ST_16; case 4: return ST_32; case 8: return ST_64; case 16: return ST_128; default: return ST_0; // clang-format on ``` This was previously nicely aligned. https://github.com/llvm/llvm-project/pull/82599 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [DRAFT][analyzer][NFC] clang-format our folders (PR #82599)
@@ -1,15 +1,16 @@ -//ProgramStateTrait.h - Partial implementations of ProgramStateTrait -*- C++ -*- +// ProgramStateTrait.h - Partial implementations of ProgramStateTrait -*- C++ +// -*- // -// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. -// See https://llvm.org/LICENSE.txt for license information. -// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// Part of the LLVM Project, under the Apache License v2.0 with LLVM +// Exceptions. See https://llvm.org/LICENSE.txt for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception // //===--===// // -// This file defines partial implementations of template specializations of -// the class ProgramStateTrait<>. ProgramStateTrait<> is used by ProgramState -// to implement set/get methods for manipulating a ProgramState's -// generic data map. +// This file defines partial implementations of template specializations of +// the class ProgramStateTrait<>. ProgramStateTrait<> is used by ProgramState +// to implement set/get methods for manipulating a ProgramState's +// generic data map. steakhal wrote: This comment header is just bad. I should come back to this. https://github.com/llvm/llvm-project/pull/82599 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [DRAFT][analyzer][NFC] clang-format our folders (PR #82599)
https://github.com/steakhal commented: Right now I don't have more time for this. I reached this part: https://github.com/llvm/llvm-project/pull/82599/files#diff-e06d50a75016837f80877b3aae594298eeead1f2260da82167e74289beca116dL2563 So far I haven't found anything critical. Only a handful of license headers break and maybe a few other tables looked better in the previous revision - but that's subjective. I plan to come back and continue this. https://github.com/llvm/llvm-project/pull/82599 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][StaticAnalyzer] Crash on loop unrolling mode (PR #82089)
@@ -226,6 +226,21 @@ static bool isPossiblyEscaped(ExplodedNode *N, const DeclRefExpr *DR) { return false; } } + +if (const SwitchStmt *SS = dyn_cast(S)) { + if (const CompoundStmt *CST = dyn_cast(SS->getBody())) { steakhal wrote: ```suggestion if (const auto *SS = dyn_cast(S)) { if (const auto *CST = dyn_cast(SS->getBody())) { ``` The type is already mentioned on the line. https://github.com/llvm/llvm-project/pull/82089 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][StaticAnalyzer] Crash on loop unrolling mode (PR #82089)
@@ -0,0 +1,11 @@ +// RUN: %clang_analyze_cc1 -analyzer-checker=core -analyzer-config unroll-loops=true -verify %s + +void test_escaping_on_var_before_switch_case_no_crash(int c) { + switch (c) { +int i; // expected error{{Reached root without finding the declaration of VD}} +case 0: { + for (i = 0; i < 16; i++) {} + break; +} + } +} steakhal wrote: It took me some time to realize that its not a verify expectation. BTW in this shape, this test fails. ```suggestion // RUN: %clang_analyze_cc1 -analyzer-checker=core -analyzer-config unroll-loops=true -verify %s // expected-no-diagnostics void test_escaping_on_var_before_switch_case_no_crash(int c) { // https://github.com/llvm/llvm-project/issues/68819 switch (c) { int i; // no-crash: The declaration of `i` is found here. case 0: { for (i = 0; i < 16; i++) {} break; } } } ``` I'd also suggest to rename the test file to highlight that this is only for loop unrolling. Or maybe even better, just append this to the end of `clang/test/Analysis/loop-unrolling.cpp`. https://github.com/llvm/llvm-project/pull/82089 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][StaticAnalyzer] Crash on loop unrolling mode (PR #82089)
https://github.com/steakhal requested changes to this pull request. Thanks for working on this. I think iterating the direct child nodes of the switch is fine. I can't think of a better way. https://github.com/llvm/llvm-project/pull/82089 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][StaticAnalyzer] Crash on loop unrolling mode (PR #82089)
@@ -226,6 +226,21 @@ static bool isPossiblyEscaped(ExplodedNode *N, const DeclRefExpr *DR) { return false; } } + +if (const SwitchStmt *SS = dyn_cast(S)) { + if (const CompoundStmt *CST = dyn_cast(SS->getBody())) { +for (const Stmt *CB : CST->body()) { + if (const DeclStmt *DST = dyn_cast(CB)) { +for (const Decl *D : DST->decls()) { + // Once we reach the declaration of the VD we can return. + if (D->getCanonicalDecl() == VD) +return false; +} + } steakhal wrote: This is duplicated from the previous check. Consider refactoring the code to hoist and name the common functionality. https://github.com/llvm/llvm-project/pull/82089 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][StaticAnalyzer] Crash on loop unrolling mode (PR #82089)
https://github.com/steakhal edited https://github.com/llvm/llvm-project/pull/82089 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][analyzer] Fix argument invalidations in StreamChecker. (PR #79470)
=?utf-8?q?Balázs_Kéri?= Message-ID: In-Reply-To: https://github.com/steakhal approved this pull request. Thanks for resolving my comments. FYI if I forget about a PR (that I promise to come back on the next day) - feel free to ping it or explicitly push the "request review" button. Wait for my collage to also have a look, as I believe he might be in context to review this change. @alejandro-alvarez-sonarsource https://github.com/llvm/llvm-project/pull/79470 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [analyzer] UnixAPIMisuseChecker Get O_CREAT from preprocessor (PR #81855)
Alejandro =?utf-8?q?Álvarez_Ayllón?Message-ID: In-Reply-To: https://github.com/steakhal closed https://github.com/llvm/llvm-project/pull/81855 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][analyzer] Change default value of checker option in unix.StdCLibraryFunctions. (PR #80457)
steakhal wrote: I'm excited to see this change. I've not reviewed this yet. https://github.com/llvm/llvm-project/pull/80457 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [analyzer] Model Microsoft "__assume" in the same way as clang "__builtin_assume" (PR #80456)
https://github.com/steakhal closed https://github.com/llvm/llvm-project/pull/80456 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [analyzer] Model Microsoft "__assume" in the same way as clang "__builtin_assume" (PR #80456)
steakhal wrote: Pushed manually to preserve the patch author. (I believe, "squash" would overwrite it.) Merged as ae354c5a45d319b3117c2822b8f6988461f3cb33. https://github.com/llvm/llvm-project/pull/80456 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] ae354c5 - [analyzer] Model Microsoft "__assume" in the same way as clang "__builtin_assume"
Author: Loïc Joly Date: 2024-02-05T17:02:24+01:00 New Revision: ae354c5a45d319b3117c2822b8f6988461f3cb33 URL: https://github.com/llvm/llvm-project/commit/ae354c5a45d319b3117c2822b8f6988461f3cb33 DIFF: https://github.com/llvm/llvm-project/commit/ae354c5a45d319b3117c2822b8f6988461f3cb33.diff LOG: [analyzer] Model Microsoft "__assume" in the same way as clang "__builtin_assume" Added: Modified: clang/lib/StaticAnalyzer/Checkers/BuiltinFunctionChecker.cpp clang/test/Analysis/builtin-functions.cpp Removed: diff --git a/clang/lib/StaticAnalyzer/Checkers/BuiltinFunctionChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/BuiltinFunctionChecker.cpp index 61521c259ca90a..01e46fa8591c07 100644 --- a/clang/lib/StaticAnalyzer/Checkers/BuiltinFunctionChecker.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/BuiltinFunctionChecker.cpp @@ -44,7 +44,8 @@ bool BuiltinFunctionChecker::evalCall(const CallEvent , default: return false; - case Builtin::BI__builtin_assume: { + case Builtin::BI__builtin_assume: + case Builtin::BI__assume: { assert (Call.getNumArgs() > 0); SVal Arg = Call.getArgSVal(0); if (Arg.isUndef()) diff --git a/clang/test/Analysis/builtin-functions.cpp b/clang/test/Analysis/builtin-functions.cpp index 37e522049b1748..8719193e405c4c 100644 --- a/clang/test/Analysis/builtin-functions.cpp +++ b/clang/test/Analysis/builtin-functions.cpp @@ -1,4 +1,5 @@ // RUN: %clang_analyze_cc1 -triple x86_64-apple-darwin10 -analyzer-checker=core,debug.ExprInspection %s -std=c++11 -verify +// RUN: %clang_analyze_cc1 -triple x86_64-pc-windows-msvc19.11.0 -fms-extensions -analyzer-checker=core,debug.ExprInspection %s -std=c++11 -verify void clang_analyzer_eval(bool); void clang_analyzer_warnIfReached(); @@ -65,6 +66,23 @@ void g(int i) { } } +#ifdef _WIN32 +namespace ms { +void f(int i) { + __assume(i < 10); + clang_analyzer_eval(i < 15); // expected-warning {{TRUE}} +} + +void g(int i) { + if (i > 5) { +__assume(i < 5); +clang_analyzer_warnIfReached(); // Assumtion contradicts constraints. +// We give up the analysis on this path. + } +} +} // namespace ms +#endif + void test_constant_p(void *ptr) { int i = 1; const int j = 2; ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[llvm] [clang] [clang-tools-extra] [analyzer] Support interestingness in ArrayBoundV2 (PR #78315)
=?utf-8?q?Don=C3=A1t?= Nagy , =?utf-8?q?Don=C3=A1t?= Nagy , =?utf-8?q?Don=C3=A1t?= Nagy , =?utf-8?q?Don=C3=A1t?= Nagy , =?utf-8?q?Don=C3=A1t?= Nagy , =?utf-8?q?Don=C3=A1t?= Nagy , =?utf-8?q?Don=C3=A1t?= Nagy , =?utf-8?q?Don=C3=A1t?= Nagy , =?utf-8?q?Don=C3=A1t?= Nagy , =?utf-8?q?Don=C3=A1t?= Nagy , =?utf-8?q?Don=C3=A1t?= Nagy , =?utf-8?q?Don=C3=A1t?= Nagy , =?utf-8?q?Don=C3=A1t?= Nagy , =?utf-8?q?Don=C3=A1t?= Nagy , =?utf-8?q?Don=C3=A1t?= Nagy Message-ID: In-Reply-To: https://github.com/steakhal approved this pull request. All good. Merge it. Thanks! https://github.com/llvm/llvm-project/pull/78315 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [llvm] [clang] [analyzer] Support interestingness in ArrayBoundV2 (PR #78315)
=?utf-8?q?Donát?= Nagy , =?utf-8?q?Donát?= Nagy , =?utf-8?q?Donát?= Nagy , =?utf-8?q?Donát?= Nagy , =?utf-8?q?Donát?= Nagy , =?utf-8?q?Donát?= Nagy , =?utf-8?q?Donát?= Nagy , =?utf-8?q?Donát?= Nagy , =?utf-8?q?Donát?= Nagy , =?utf-8?q?Donát?= Nagy , =?utf-8?q?Donát?= Nagy , =?utf-8?q?Donát?= Nagy , =?utf-8?q?Donát?= Nagy , =?utf-8?q?Donát?= Nagy Message-ID: In-Reply-To: @@ -381,66 +574,105 @@ void ArrayBoundCheckerV2::performCheck(const Expr *E, CheckerContext ) const { compareValueToThreshold(State, ByteOffset, *KnownSize, SVB); if (ExceedsUpperBound) { + // The offset may be invalid (>= Size)... if (!WithinUpperBound) { -// We know that the index definitely exceeds the upper bound. -if (isa(E) && isInAddressOf(E, C.getASTContext())) { - // ...but this is within an addressof expression, so we need to check - // for the exceptional case that `[size]` is valid. - auto [EqualsToThreshold, NotEqualToThreshold] = - compareValueToThreshold(ExceedsUpperBound, ByteOffset, *KnownSize, - SVB, /*CheckEquality=*/true); - if (EqualsToThreshold && !NotEqualToThreshold) { -// We are definitely in the exceptional case, so return early -// instead of reporting a bug. -C.addTransition(EqualsToThreshold); -return; - } +// ...and it cannot be within bounds, so report an error, unless we can +// definitely determine that this is an idiomatic `[size]` +// expression that calculates the past-the-end pointer. +if (isIdiomaticPastTheEndPtr(E, ExceedsUpperBound, ByteOffset, + *KnownSize, C)) { + C.addTransition(ExceedsUpperBound, SUR.createNoteTag(C)); + return; } + Messages Msgs = getExceedsMsgs(C.getASTContext(), Reg, ByteOffset, *KnownSize, Location); -reportOOB(C, ExceedsUpperBound, OOB_Exceeds, ByteOffset, Msgs); +reportOOB(C, ExceedsUpperBound, Msgs, ByteOffset, KnownSize); return; } + // ...and it can be valid as well... if (isTainted(State, ByteOffset)) { -// Both cases are possible, but the offset is tainted, so report. -std::string RegName = getRegionName(Reg); +// ...but it's tainted, so report an error. -// Diagnostic detail: "tainted offset" is always correct, but the -// common case is that 'idx' is tainted in 'arr[idx]' and then it's +// Diagnostic detail: saying "tainted offset" is always correct, but +// the common case is that 'idx' is tainted in 'arr[idx]' and then it's // nicer to say "tainted index". const char *OffsetName = "offset"; if (const auto *ASE = dyn_cast(E)) if (isTainted(State, ASE->getIdx(), C.getLocationContext())) OffsetName = "index"; Messages Msgs = getTaintMsgs(Reg, OffsetName); -reportOOB(C, ExceedsUpperBound, OOB_Taint, ByteOffset, Msgs); +reportOOB(C, ExceedsUpperBound, Msgs, ByteOffset, KnownSize, + /*IsTaintBug=*/true); return; } + // ...and it isn't tainted, so the checker will (optimistically) assume + // that the offset is in bounds and mention this in the note tag. + SUR.recordUpperBoundAssumption(*KnownSize); } +// Actually update the state. The "if" only fails in the extremely unlikely +// case when compareValueToThreshold returns {nullptr, nullptr} becasue +// evalBinOpNN fails to evaluate the less-than operator. if (WithinUpperBound) State = WithinUpperBound; } - C.addTransition(State); + // Add a transition, reporting the state updates that we accumulated. + C.addTransition(State, SUR.createNoteTag(C)); +} + +void ArrayBoundCheckerV2::markPartsInteresting(PathSensitiveBugReport , + ProgramStateRef ErrorState, + NonLoc Val, bool MarkTaint) { + if (SymbolRef Sym = Val.getAsSymbol()) { +// If the offset is a symbolic value, iterate over its "parts" with +// `SymExpr::symbols()` and mark each of them as interesting. +// For example, if the offset is `x*4 + y` then we put interestingness onto +// the SymSymExpr `x*4 + y`, the SymIntExpr `x*4` and the two data symbols +// `x` and `y`. +for (SymbolRef PartSym : Sym->symbols()) + BR.markInteresting(PartSym); + } + + if (MarkTaint) { +// If the issue that we're reporting depends on the taintedness of the +// offset, then put interestingness onto symbols that could be the origin +// of the taint. +for (SymbolRef Sym : getTaintedSymbols(ErrorState, Val)) + BR.markInteresting(Sym); + } } void ArrayBoundCheckerV2::reportOOB(CheckerContext , -
[clang] [llvm] [clang-tools-extra] [analyzer] Support interestingness in ArrayBoundV2 (PR #78315)
=?utf-8?q?Donát?= Nagy , =?utf-8?q?Donát?= Nagy , =?utf-8?q?Donát?= Nagy , =?utf-8?q?Donát?= Nagy , =?utf-8?q?Donát?= Nagy , =?utf-8?q?Donát?= Nagy , =?utf-8?q?Donát?= Nagy , =?utf-8?q?Donát?= Nagy , =?utf-8?q?Donát?= Nagy , =?utf-8?q?Donát?= Nagy , =?utf-8?q?Donát?= Nagy , =?utf-8?q?Donát?= Nagy , =?utf-8?q?Donát?= Nagy , =?utf-8?q?Donát?= Nagy Message-ID: In-Reply-To: @@ -381,66 +574,105 @@ void ArrayBoundCheckerV2::performCheck(const Expr *E, CheckerContext ) const { compareValueToThreshold(State, ByteOffset, *KnownSize, SVB); if (ExceedsUpperBound) { + // The offset may be invalid (>= Size)... if (!WithinUpperBound) { -// We know that the index definitely exceeds the upper bound. -if (isa(E) && isInAddressOf(E, C.getASTContext())) { - // ...but this is within an addressof expression, so we need to check - // for the exceptional case that `[size]` is valid. - auto [EqualsToThreshold, NotEqualToThreshold] = - compareValueToThreshold(ExceedsUpperBound, ByteOffset, *KnownSize, - SVB, /*CheckEquality=*/true); - if (EqualsToThreshold && !NotEqualToThreshold) { -// We are definitely in the exceptional case, so return early -// instead of reporting a bug. -C.addTransition(EqualsToThreshold); -return; - } +// ...and it cannot be within bounds, so report an error, unless we can +// definitely determine that this is an idiomatic `[size]` +// expression that calculates the past-the-end pointer. +if (isIdiomaticPastTheEndPtr(E, ExceedsUpperBound, ByteOffset, + *KnownSize, C)) { + C.addTransition(ExceedsUpperBound, SUR.createNoteTag(C)); + return; } + Messages Msgs = getExceedsMsgs(C.getASTContext(), Reg, ByteOffset, *KnownSize, Location); -reportOOB(C, ExceedsUpperBound, OOB_Exceeds, ByteOffset, Msgs); +reportOOB(C, ExceedsUpperBound, Msgs, ByteOffset, KnownSize); return; } + // ...and it can be valid as well... if (isTainted(State, ByteOffset)) { -// Both cases are possible, but the offset is tainted, so report. -std::string RegName = getRegionName(Reg); +// ...but it's tainted, so report an error. -// Diagnostic detail: "tainted offset" is always correct, but the -// common case is that 'idx' is tainted in 'arr[idx]' and then it's +// Diagnostic detail: saying "tainted offset" is always correct, but +// the common case is that 'idx' is tainted in 'arr[idx]' and then it's // nicer to say "tainted index". const char *OffsetName = "offset"; if (const auto *ASE = dyn_cast(E)) if (isTainted(State, ASE->getIdx(), C.getLocationContext())) OffsetName = "index"; Messages Msgs = getTaintMsgs(Reg, OffsetName); -reportOOB(C, ExceedsUpperBound, OOB_Taint, ByteOffset, Msgs); +reportOOB(C, ExceedsUpperBound, Msgs, ByteOffset, KnownSize, + /*IsTaintBug=*/true); return; } + // ...and it isn't tainted, so the checker will (optimistically) assume + // that the offset is in bounds and mention this in the note tag. + SUR.recordUpperBoundAssumption(*KnownSize); } +// Actually update the state. The "if" only fails in the extremely unlikely +// case when compareValueToThreshold returns {nullptr, nullptr} becasue +// evalBinOpNN fails to evaluate the less-than operator. if (WithinUpperBound) State = WithinUpperBound; } - C.addTransition(State); + // Add a transition, reporting the state updates that we accumulated. + C.addTransition(State, SUR.createNoteTag(C)); +} + +void ArrayBoundCheckerV2::markPartsInteresting(PathSensitiveBugReport , + ProgramStateRef ErrorState, + NonLoc Val, bool MarkTaint) { + if (SymbolRef Sym = Val.getAsSymbol()) { +// If the offset is a symbolic value, iterate over its "parts" with +// `SymExpr::symbols()` and mark each of them as interesting. +// For example, if the offset is `x*4 + y` then we put interestingness onto +// the SymSymExpr `x*4 + y`, the SymIntExpr `x*4` and the two data symbols +// `x` and `y`. +for (SymbolRef PartSym : Sym->symbols()) + BR.markInteresting(PartSym); + } + + if (MarkTaint) { +// If the issue that we're reporting depends on the taintedness of the +// offset, then put interestingness onto symbols that could be the origin +// of the taint. +for (SymbolRef Sym : getTaintedSymbols(ErrorState, Val)) + BR.markInteresting(Sym); + } steakhal wrote: This part seems redundant to me. In the previous loop, you already
[clang-tools-extra] [clang] [llvm] [analyzer] Support interestingness in ArrayBoundV2 (PR #78315)
=?utf-8?q?Don=C3=A1t?= Nagy , =?utf-8?q?Don=C3=A1t?= Nagy , =?utf-8?q?Don=C3=A1t?= Nagy , =?utf-8?q?Don=C3=A1t?= Nagy , =?utf-8?q?Don=C3=A1t?= Nagy , =?utf-8?q?Don=C3=A1t?= Nagy , =?utf-8?q?Don=C3=A1t?= Nagy , =?utf-8?q?Don=C3=A1t?= Nagy , =?utf-8?q?Don=C3=A1t?= Nagy , =?utf-8?q?Don=C3=A1t?= Nagy , =?utf-8?q?Don=C3=A1t?= Nagy , =?utf-8?q?Don=C3=A1t?= Nagy , =?utf-8?q?Don=C3=A1t?= Nagy , =?utf-8?q?Don=C3=A1t?= Nagy Message-ID: In-Reply-To: https://github.com/steakhal requested changes to this pull request. I only found one logic bump. Other than that, it's approved. https://github.com/llvm/llvm-project/pull/78315 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [llvm] [clang] [analyzer] Support interestingness in ArrayBoundV2 (PR #78315)
=?utf-8?q?Donát?= Nagy , =?utf-8?q?Donát?= Nagy , =?utf-8?q?Donát?= Nagy , =?utf-8?q?Donát?= Nagy , =?utf-8?q?Donát?= Nagy , =?utf-8?q?Donát?= Nagy , =?utf-8?q?Donát?= Nagy , =?utf-8?q?Donát?= Nagy , =?utf-8?q?Donát?= Nagy , =?utf-8?q?Donát?= Nagy , =?utf-8?q?Donát?= Nagy , =?utf-8?q?Donát?= Nagy , =?utf-8?q?Donát?= Nagy , =?utf-8?q?Donát?= Nagy Message-ID: In-Reply-To: @@ -318,17 +424,91 @@ static Messages getTaintMsgs(const SubRegion *Region, const char *OffsetName) { RegName, OffsetName)}; } -void ArrayBoundCheckerV2::performCheck(const Expr *E, CheckerContext ) const { - // NOTE: Instead of using ProgramState::assumeInBound(), we are prototyping - // some new logic here that reasons directly about memory region extents. - // Once that logic is more mature, we can bring it back to assumeInBound() - // for all clients to use. - // - // The algorithm we are using here for bounds checking is to see if the - // memory access is within the extent of the base region. Since we - // have some flexibility in defining the base region, we can achieve - // various levels of conservatism in our buffer overflow checking. +const NoteTag *StateUpdateReporter::createNoteTag(CheckerContext ) const { + // Don't create a note tag if we didn't assume anything: + if (!AssumedNonNegative && !AssumedUpperBound) +return nullptr; + + return C.getNoteTag([*this](PathSensitiveBugReport ) -> std::string { +return getMessage(BR); + }); +} + +std::string StateUpdateReporter::getMessage(PathSensitiveBugReport ) const { + bool ShouldReportNonNegative = AssumedNonNegative; + if (!providesInformationAboutInteresting(ByteOffsetVal, BR)) { +if (AssumedUpperBound && +providesInformationAboutInteresting(*AssumedUpperBound, BR)) { + // Even if the byte offset isn't interesting (e.g. it's a constant value), + // the assumption can still be interesting if it provides information + // about an interesting symbolic upper bound. + ShouldReportNonNegative = false; +} else { + // We don't have anything interesting, don't report the assumption. + return ""; +} steakhal wrote: ```suggestion } // We don't have anything interesting, don't report the assumption. return ""; ``` Return after else. https://github.com/llvm/llvm-project/pull/78315 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[llvm] [clang] [clang-tools-extra] [analyzer] Support interestingness in ArrayBoundV2 (PR #78315)
=?utf-8?q?Donát?= Nagy , =?utf-8?q?Donát?= Nagy , =?utf-8?q?Donát?= Nagy , =?utf-8?q?Donát?= Nagy , =?utf-8?q?Donát?= Nagy , =?utf-8?q?Donát?= Nagy , =?utf-8?q?Donát?= Nagy , =?utf-8?q?Donát?= Nagy , =?utf-8?q?Donát?= Nagy , =?utf-8?q?Donát?= Nagy , =?utf-8?q?Donát?= Nagy , =?utf-8?q?Donát?= Nagy , =?utf-8?q?Donát?= Nagy , =?utf-8?q?Donát?= Nagy Message-ID: In-Reply-To: https://github.com/steakhal edited https://github.com/llvm/llvm-project/pull/78315 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [analyzer] Model Microsoft "__assume" in the same way as clang "__builtin_assume" (PR #80456)
steakhal wrote: Thanks Donát! I'll wait for @Xazax-hun explicit approval to be sure everyone on board (who left remarks) are okay with the current content. https://github.com/llvm/llvm-project/pull/80456 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [analyzer] Model Microsoft "__assume" in the same way as clang "__builtin_assume" (PR #80456)
steakhal wrote: It turns out we already had a downstream patch, so I'll drop this one in favor of what we already had. Sorry about the confusion. This version is already in production for many years now. https://github.com/llvm/llvm-project/pull/80456 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [analyzer] Model Microsoft "__assume" in the same way as clang "__builtin_assume" (PR #80456)
https://github.com/steakhal edited https://github.com/llvm/llvm-project/pull/80456 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [analyzer] Teach analzer about ms __analyzer_assume(bool) and friends (PR #80456)
https://github.com/steakhal updated https://github.com/llvm/llvm-project/pull/80456 >From 3a11db7ce1e91daacb86e183e7137db7a6101c9b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Lo=C3=AFc=20Joly?= Date: Tue, 9 Aug 2022 23:21:18 +0200 Subject: [PATCH] [analyzer] Model Microsoft "__assume" in the same way as clang "__builtin_assume" --- .../Checkers/BuiltinFunctionChecker.cpp| 3 ++- clang/test/Analysis/builtin-functions.cpp | 18 ++ 2 files changed, 20 insertions(+), 1 deletion(-) diff --git a/clang/lib/StaticAnalyzer/Checkers/BuiltinFunctionChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/BuiltinFunctionChecker.cpp index 61521c259ca90..01e46fa8591c0 100644 --- a/clang/lib/StaticAnalyzer/Checkers/BuiltinFunctionChecker.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/BuiltinFunctionChecker.cpp @@ -44,7 +44,8 @@ bool BuiltinFunctionChecker::evalCall(const CallEvent , default: return false; - case Builtin::BI__builtin_assume: { + case Builtin::BI__builtin_assume: + case Builtin::BI__assume: { assert (Call.getNumArgs() > 0); SVal Arg = Call.getArgSVal(0); if (Arg.isUndef()) diff --git a/clang/test/Analysis/builtin-functions.cpp b/clang/test/Analysis/builtin-functions.cpp index 37e522049b174..8719193e405c4 100644 --- a/clang/test/Analysis/builtin-functions.cpp +++ b/clang/test/Analysis/builtin-functions.cpp @@ -1,4 +1,5 @@ // RUN: %clang_analyze_cc1 -triple x86_64-apple-darwin10 -analyzer-checker=core,debug.ExprInspection %s -std=c++11 -verify +// RUN: %clang_analyze_cc1 -triple x86_64-pc-windows-msvc19.11.0 -fms-extensions -analyzer-checker=core,debug.ExprInspection %s -std=c++11 -verify void clang_analyzer_eval(bool); void clang_analyzer_warnIfReached(); @@ -65,6 +66,23 @@ void g(int i) { } } +#ifdef _WIN32 +namespace ms { +void f(int i) { + __assume(i < 10); + clang_analyzer_eval(i < 15); // expected-warning {{TRUE}} +} + +void g(int i) { + if (i > 5) { +__assume(i < 5); +clang_analyzer_warnIfReached(); // Assumtion contradicts constraints. +// We give up the analysis on this path. + } +} +} // namespace ms +#endif + void test_constant_p(void *ptr) { int i = 1; const int j = 2; ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [analyzer] Teach analzer about ms __analyzer_assume(bool) and friends (PR #80456)
steakhal wrote: > The code LGTM with some minor remarks. (Disclaimer: I'm not familiar with > these MS functions.) > > I'm not sure whether these "builtin by Microsoft" functions are in scope for > "our" BuiltinFunctionChecker which previously only checked functions that are > recognized as `Builtin::BI__something` by Clang. (However, I can believe that > there is no better place for them and I don't think that they would cause > problems here.) For me, this file would be where I would look to see how `__assume` is eval called. This is actually an unknown function in regular mode, but with `-fms-extensions` it's actually defined. Check the AST for this [example](https://godbolt.org/z/1xdbP8scx). So, techniquely, it's not a builtin, but feels like it. About the remarks, yes, let's do [Clean As You Code](https://docs.sonarsource.com/sonarqube/latest/user-guide/clean-as-you-code/#what-is-clean-as-you-code)! I'm all in. > Hey! > > Thanks for looking into this! > > Did you actually encounter this call in the wild? The reason I ask, because > their definition looks like this in the current version of `sal.h`: > > ``` > #ifndef __analysis_assume // [ > #ifdef _PREFAST_ // [ > #define __analysis_assume(expr) __assume(expr) > #else // ][ > #define __analysis_assume(expr) > #endif // ] > #endif // ] > > #ifndef _Analysis_assume_ // [ > #ifdef _PREFAST_ // [ > #define _Analysis_assume_(expr) __assume(expr) > #else // ][ > #define _Analysis_assume_(expr) > #endif // ] > #endif // ] > ``` > > The basic idea is, when MSVC's analyzer is invoked, `_PREFAST_` is defined, > and these macros should expand to `__assume(expr)`. This makes me a bit > surprised if the original names are present in the preprocessed code. > > There is no difference between `__analysis_assume` and `_Analysis_assume_`. > The former is following the naming conventions in SAL 1, the latter is > following the conventions of SAL 2. The latter is preferred in user code, but > MSVC still supports both spellings. I think I've seen FPs on ChakraCore and on the DotnetRuntime and on other Windows-related projects defining some of their assert macros by wrapping `__analysis_assume`. I can't exactly recall ATM where exactly I've seen that, but I'll do a measurment to confirm that this PR actually improves the situation. Anyways, it turns out the FP I wanted to fix actually expands into using `__builtin_trap()`, which is still not modeled in CSA :face_with_spiral_eyes: I'll create a separate PR for handling that. I think this PR stands on it's own, and improves the situation - (I'll check and come back of course). I hope `__assume()` takes a single parameter (of any type) and returns `void` though. Sorry for not doing my homework in the beginning, and thanks for catching that! I though it's gonna be an easy one and I fire off a PR on my tea-break. https://github.com/llvm/llvm-project/pull/80456 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [analyzer] Teach analzer about ms __analyzer_assume(bool) and friends (PR #80456)
https://github.com/steakhal updated https://github.com/llvm/llvm-project/pull/80456 >From 9065aec18b5b9c4d922b0650e709e71ed31b5a45 Mon Sep 17 00:00:00 2001 From: Balazs Benics Date: Fri, 2 Feb 2024 16:24:21 +0100 Subject: [PATCH 1/2] [analyzer] Teach analzer about ms __analyzer_assume(bool) and friends See the MS docs: https://learn.microsoft.com/en-us/windows-hardware/drivers/devtest/using-the--analysis-assume-function-to-suppress-false-defects https://learn.microsoft.com/en-us/cpp/code-quality/how-to-specify-additional-code-information-by-using-analysis-assume TBH, I don't really know what is the difference between the two APIs. --- .../Checkers/BuiltinFunctionChecker.cpp | 57 +-- clang/test/Analysis/builtin-functions.cpp | 22 +++ 2 files changed, 62 insertions(+), 17 deletions(-) diff --git a/clang/lib/StaticAnalyzer/Checkers/BuiltinFunctionChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/BuiltinFunctionChecker.cpp index 61521c259ca90..ea874c1529b3b 100644 --- a/clang/lib/StaticAnalyzer/Checkers/BuiltinFunctionChecker.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/BuiltinFunctionChecker.cpp @@ -14,6 +14,7 @@ #include "clang/StaticAnalyzer/Checkers/BuiltinCheckerRegistration.h" #include "clang/StaticAnalyzer/Core/Checker.h" #include "clang/StaticAnalyzer/Core/CheckerManager.h" +#include "clang/StaticAnalyzer/Core/PathSensitive/CallDescription.h" #include "clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h" #include "clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h" #include "clang/StaticAnalyzer/Core/PathSensitive/DynamicExtent.h" @@ -26,10 +27,41 @@ namespace { class BuiltinFunctionChecker : public Checker { public: bool evalCall(const CallEvent , CheckerContext ) const; + +private: + const CallDescriptionSet MicrosoftAnalysisAssume{ + {{"__analysis_assume"}, 1}, + {{"_Analysis_assume_"}, 1}, + }; + + void evalCallAssume(const CallEvent , CheckerContext ) const; }; } +void BuiltinFunctionChecker::evalCallAssume(const CallEvent , +CheckerContext ) const { + assert(Call.getNumArgs() > 0); + assert(Call.getResultType()->isVoidType()); + SVal Arg = Call.getArgSVal(0); + + if (Arg.isUndef()) +return; // Return true to model purity. + + ProgramStateRef State = C.getState(); + State = State->assume(Arg.castAs(), true); + + // FIXME: do we want to warn here? Not right now. The most reports might + // come from infeasible paths, thus being false positives. + if (!State) { +C.generateSink(C.getState(), C.getPredecessor()); +return; + } + + C.addTransition(State); + return; +} + bool BuiltinFunctionChecker::evalCall(const CallEvent , CheckerContext ) const { ProgramStateRef state = C.getState(); @@ -39,29 +71,20 @@ bool BuiltinFunctionChecker::evalCall(const CallEvent , const LocationContext *LCtx = C.getLocationContext(); const Expr *CE = Call.getOriginExpr(); + bool ReturnsVoid = Call.getResultType()->isVoidType(); + + if (MicrosoftAnalysisAssume.contains(Call) && ReturnsVoid) { +evalCallAssume(Call, C); +return true; + } switch (FD->getBuiltinID()) { default: return false; - case Builtin::BI__builtin_assume: { -assert (Call.getNumArgs() > 0); -SVal Arg = Call.getArgSVal(0); -if (Arg.isUndef()) - return true; // Return true to model purity. - -state = state->assume(Arg.castAs(), true); -// FIXME: do we want to warn here? Not right now. The most reports might -// come from infeasible paths, thus being false positives. -if (!state) { - C.generateSink(C.getState(), C.getPredecessor()); - return true; -} - -C.addTransition(state); + case Builtin::BI__builtin_assume: +evalCallAssume(Call, C); return true; - } - case Builtin::BI__builtin_unpredictable: case Builtin::BI__builtin_expect: case Builtin::BI__builtin_expect_with_probability: diff --git a/clang/test/Analysis/builtin-functions.cpp b/clang/test/Analysis/builtin-functions.cpp index 37e522049b174..4a26f82ffcb16 100644 --- a/clang/test/Analysis/builtin-functions.cpp +++ b/clang/test/Analysis/builtin-functions.cpp @@ -1,5 +1,7 @@ // RUN: %clang_analyze_cc1 -triple x86_64-apple-darwin10 -analyzer-checker=core,debug.ExprInspection %s -std=c++11 -verify +void __analysis_assume(bool); +void _Analysis_assume_(bool); void clang_analyzer_eval(bool); void clang_analyzer_warnIfReached(); @@ -82,3 +84,23 @@ void test_constant_p(void *ptr) { clang_analyzer_eval(__builtin_constant_p(k - 3) == 1); // expected-warning {{TRUE}} clang_analyzer_eval(__builtin_constant_p(ptr == 0)); // expected-warning {{FALSE}} } + +void test_ms_analysis_assume(int *p) { + __analysis_assume(p); + if (!p) { +clang_analyzer_warnIfReached(); // no-warning: dead code. + } + clang_analyzer_warnIfReached(); // expected-warning {{REACHABLE}} + __analysis_assume(false); +
[clang] [analyzer] Teach analzer about ms __analyzer_assume(bool) and friends (PR #80456)
steakhal wrote: Let me know if this is correct @Xazax-hun. You probably have more insights on these APIs than me ;) https://github.com/llvm/llvm-project/pull/80456 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [analyzer] Teach analzer about ms __analyzer_assume(bool) and friends (PR #80456)
https://github.com/steakhal created https://github.com/llvm/llvm-project/pull/80456 See the MS docs: https://learn.microsoft.com/en-us/windows-hardware/drivers/devtest/using-the--analysis-assume-function-to-suppress-false-defects https://learn.microsoft.com/en-us/cpp/code-quality/how-to-specify-additional-code-information-by-using-analysis-assume TBH, I don't really know what is the difference between the two APIs. >From 9065aec18b5b9c4d922b0650e709e71ed31b5a45 Mon Sep 17 00:00:00 2001 From: Balazs Benics Date: Fri, 2 Feb 2024 16:24:21 +0100 Subject: [PATCH] [analyzer] Teach analzer about ms __analyzer_assume(bool) and friends See the MS docs: https://learn.microsoft.com/en-us/windows-hardware/drivers/devtest/using-the--analysis-assume-function-to-suppress-false-defects https://learn.microsoft.com/en-us/cpp/code-quality/how-to-specify-additional-code-information-by-using-analysis-assume TBH, I don't really know what is the difference between the two APIs. --- .../Checkers/BuiltinFunctionChecker.cpp | 57 +-- clang/test/Analysis/builtin-functions.cpp | 22 +++ 2 files changed, 62 insertions(+), 17 deletions(-) diff --git a/clang/lib/StaticAnalyzer/Checkers/BuiltinFunctionChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/BuiltinFunctionChecker.cpp index 61521c259ca90..ea874c1529b3b 100644 --- a/clang/lib/StaticAnalyzer/Checkers/BuiltinFunctionChecker.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/BuiltinFunctionChecker.cpp @@ -14,6 +14,7 @@ #include "clang/StaticAnalyzer/Checkers/BuiltinCheckerRegistration.h" #include "clang/StaticAnalyzer/Core/Checker.h" #include "clang/StaticAnalyzer/Core/CheckerManager.h" +#include "clang/StaticAnalyzer/Core/PathSensitive/CallDescription.h" #include "clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h" #include "clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h" #include "clang/StaticAnalyzer/Core/PathSensitive/DynamicExtent.h" @@ -26,10 +27,41 @@ namespace { class BuiltinFunctionChecker : public Checker { public: bool evalCall(const CallEvent , CheckerContext ) const; + +private: + const CallDescriptionSet MicrosoftAnalysisAssume{ + {{"__analysis_assume"}, 1}, + {{"_Analysis_assume_"}, 1}, + }; + + void evalCallAssume(const CallEvent , CheckerContext ) const; }; } +void BuiltinFunctionChecker::evalCallAssume(const CallEvent , +CheckerContext ) const { + assert(Call.getNumArgs() > 0); + assert(Call.getResultType()->isVoidType()); + SVal Arg = Call.getArgSVal(0); + + if (Arg.isUndef()) +return; // Return true to model purity. + + ProgramStateRef State = C.getState(); + State = State->assume(Arg.castAs(), true); + + // FIXME: do we want to warn here? Not right now. The most reports might + // come from infeasible paths, thus being false positives. + if (!State) { +C.generateSink(C.getState(), C.getPredecessor()); +return; + } + + C.addTransition(State); + return; +} + bool BuiltinFunctionChecker::evalCall(const CallEvent , CheckerContext ) const { ProgramStateRef state = C.getState(); @@ -39,29 +71,20 @@ bool BuiltinFunctionChecker::evalCall(const CallEvent , const LocationContext *LCtx = C.getLocationContext(); const Expr *CE = Call.getOriginExpr(); + bool ReturnsVoid = Call.getResultType()->isVoidType(); + + if (MicrosoftAnalysisAssume.contains(Call) && ReturnsVoid) { +evalCallAssume(Call, C); +return true; + } switch (FD->getBuiltinID()) { default: return false; - case Builtin::BI__builtin_assume: { -assert (Call.getNumArgs() > 0); -SVal Arg = Call.getArgSVal(0); -if (Arg.isUndef()) - return true; // Return true to model purity. - -state = state->assume(Arg.castAs(), true); -// FIXME: do we want to warn here? Not right now. The most reports might -// come from infeasible paths, thus being false positives. -if (!state) { - C.generateSink(C.getState(), C.getPredecessor()); - return true; -} - -C.addTransition(state); + case Builtin::BI__builtin_assume: +evalCallAssume(Call, C); return true; - } - case Builtin::BI__builtin_unpredictable: case Builtin::BI__builtin_expect: case Builtin::BI__builtin_expect_with_probability: diff --git a/clang/test/Analysis/builtin-functions.cpp b/clang/test/Analysis/builtin-functions.cpp index 37e522049b174..4a26f82ffcb16 100644 --- a/clang/test/Analysis/builtin-functions.cpp +++ b/clang/test/Analysis/builtin-functions.cpp @@ -1,5 +1,7 @@ // RUN: %clang_analyze_cc1 -triple x86_64-apple-darwin10 -analyzer-checker=core,debug.ExprInspection %s -std=c++11 -verify +void __analysis_assume(bool); +void _Analysis_assume_(bool); void clang_analyzer_eval(bool); void clang_analyzer_warnIfReached(); @@ -82,3 +84,23 @@ void test_constant_p(void *ptr) { clang_analyzer_eval(__builtin_constant_p(k - 3) == 1); // expected-warning {{TRUE}}
[clang] Fix analyzer crash on 'StructuralValue' (PR #79764)
steakhal wrote: > Thanks! Thanks for the quick workaround! FYI Backport proposed in issue #79992, that refers to PR #79997 actually doing the backport. https://github.com/llvm/llvm-project/pull/79764 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Fix analyzer crash on 'StructuralValue' (PR #79764)
https://github.com/steakhal closed https://github.com/llvm/llvm-project/pull/79764 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Fix analyzer crash on 'StructuralValue' (PR #79764)
steakhal wrote: > Thanks! It would be great to get this landed as soon as possible to unbreak > trunk. (I believe we need it for the 18.x branch too?) I'll take care of the backport, after this PR is merged by @bolshakov-a https://github.com/llvm/llvm-project/pull/79764 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Fix analyzer crash on 'StructuralValue' (PR #79764)
steakhal wrote: > LGTM for the Static Analyzer. Actually, the other hunk also makes sense. LGTM. https://github.com/llvm/llvm-project/pull/79764 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Fix analyzer crash on 'StructuralValue' (PR #79764)
@@ -40,8 +40,12 @@ static const Expr *ignoreTransparentExprs(const Expr *E) { switch (E->getStmtClass()) { case Stmt::OpaqueValueExprClass: -E = cast(E)->getSourceExpr(); -break; +if (const clang::Expr *SE = cast(E)->getSourceExpr()) { + E = SE; + break; +} else { + return E; +} steakhal wrote: Makes sense. Consider this resolved. https://github.com/llvm/llvm-project/pull/79764 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Fix analyzer crash on 'StructuralValue' (PR #79764)
https://github.com/steakhal approved this pull request. LGTM for the Static Analyzer. Thanks for fixing this crash. https://github.com/llvm/llvm-project/pull/79764 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Fix analyzer crash on 'StructuralValue' (PR #79764)
https://github.com/steakhal approved this pull request. Approved with nits. This works around the crash. https://github.com/llvm/llvm-project/pull/79764 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Fix analyzer crash on 'StructuralValue' (PR #79764)
@@ -40,8 +40,12 @@ static const Expr *ignoreTransparentExprs(const Expr *E) { switch (E->getStmtClass()) { case Stmt::OpaqueValueExprClass: -E = cast(E)->getSourceExpr(); -break; +if (const clang::Expr *SE = cast(E)->getSourceExpr()) { + E = SE; + break; +} else { + return E; +} steakhal wrote: ```suggestion if (const auto *SE = cast(E)->getSourceExpr()) { E = SE; break; } return E; ``` Fix `else-after-break`. Use `auto` after `cast<>`. https://github.com/llvm/llvm-project/pull/79764 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Fix analyzer crash on 'StructuralValue' (PR #79764)
https://github.com/steakhal edited https://github.com/llvm/llvm-project/pull/79764 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][analyzer] Fix argument invalidations in StreamChecker. (PR #79470)
@@ -544,6 +545,21 @@ const ExplodedNode *StreamChecker::getAcquisitionSite(const ExplodedNode *N, return nullptr; } +static ProgramStateRef +escapeArgs(ProgramStateRef State, CheckerContext , const CallEvent , + const SmallVector ) { + const auto *CE = Call.getOriginExpr(); + + SmallVector EscapingVals; + EscapingVals.reserve(EscapingArgs.size()); + for (auto EscArgIdx : EscapingArgs) +EscapingVals.push_back(Call.getArgSVal(EscArgIdx)); + State = State->invalidateRegions(EscapingVals, CE, C.blockCount(), + C.getLocationContext(), + /*CausesPointerEscape=*/false); steakhal wrote: Yup, have a look at `RegionStoreManager::invalidateRegions`. Depending on the nature of the call, invalidates system or every global variables. We don't really care about it, so let's just not pass the `Call` here. https://github.com/llvm/llvm-project/pull/79470 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [analyzer] Unbreak [[clang::suppress]] on checkers without decl-with-issue. (PR #79398)
https://github.com/steakhal approved this pull request. https://github.com/llvm/llvm-project/pull/79398 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clang] [clang][NFC] Refactor `CXXNewExpr::InitializationStyle` (re-land) (PR #71417)
steakhal wrote: Thank you all participating, and especially for @Endilll committing the fix as cc3fd1974696a792ba70ba670ed761937cd0735c. Consider my [issue](https://github.com/llvm/llvm-project/pull/71417#issuecomment-1897925793) resolved. :) https://github.com/llvm/llvm-project/pull/71417 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [lldb] [clang-tools-extra] [c++20] P1907R1: Support for generalized non-type template arguments of scalar type. (PR #78041)
steakhal wrote: FYI this caused a crash in the Static Analyzer, tracked here: #79575 We will (well, probably I will) look into this to see what could be done about it to workaround/fix the crash for clang-18. https://github.com/llvm/llvm-project/pull/78041 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][analyzer] Add missing stream related functions to StdLibraryFunctionsChecker. (PR #76979)
=?utf-8?q?Balázs_Kéri?= , =?utf-8?q?Balázs_Kéri?= , =?utf-8?q?Balázs_Kéri?= , =?utf-8?q?Balázs_Kéri?= , =?utf-8?q?Balázs_Kéri?= Message-ID: In-Reply-To: @@ -1385,14 +1385,16 @@ Improvements - Improved the ``unix.StdCLibraryFunctions`` checker by modeling more - functions like ``send``, ``recv``, ``readlink``, ``fflush``, ``mkdtemp``, - ``getcwd`` and ``errno`` behavior. + functions like ``send``, ``recv``, ``readlink``, ``fgetc``, ``fgets``, + ``fputc``, ``fputs``, ``fflush``, ``mkdtemp``,``getcwd`` and + ``errno`` behavior. steakhal wrote: Given that `release/18.x` has branched off, I think we are better off not touching the release notes. I advocate for only keeping the release notes up-to-date right before branching for a release (basically what we did), but not touching it for the rest of the time. By nature, release notes are frequently touched, and even if our stuff does not change, the diff context may. This can cause inconveniences for downstream users for reverting or backporting patches. And they don't really bring a lot of benefit, as I'd need to go over the changes prior a release anyways - just to be sure all important changes were mentioned. On that note, I kinda regret that I wanted a full list of PRs for the `StdCLibraryFunctions` checker, as it got bloated quite a bit over the last month. I wasn't expecting that much of a motion in this area TBH. https://github.com/llvm/llvm-project/pull/76979 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [clang-tools-extra] [analyzer] Support interestingness in ArrayBoundV2 (PR #78315)
=?utf-8?q?Donát?= Nagy , =?utf-8?q?Donát?= Nagy , =?utf-8?q?Donát?= Nagy , =?utf-8?q?Donát?= Nagy , =?utf-8?q?Donát?= Nagy , =?utf-8?q?Donát?= Nagy Message-ID: In-Reply-To: @@ -245,10 +265,15 @@ int *nothingIsCertain(int x, int y) { if (x >= 2) return 0; int *mem = (int*)malloc(x); + + // TODO: The next line is a temporary hack; see 'symbolicExtent()' for details. + (void)x; + steakhal wrote: These hacks are sort of well known. Just put these at the very end of the test-case. That's usually the canonical form of this hack. A comment like `Keep constraints alive.` is fine. Same applies for similar "hacks" in the tests of this PR. https://github.com/llvm/llvm-project/pull/78315 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clang] [llvm] [analyzer] Support interestingness in ArrayBoundV2 (PR #78315)
=?utf-8?q?Donát?= Nagy , =?utf-8?q?Donát?= Nagy , =?utf-8?q?Donát?= Nagy , =?utf-8?q?Donát?= Nagy , =?utf-8?q?Donát?= Nagy , =?utf-8?q?Donát?= Nagy Message-ID: In-Reply-To: https://github.com/steakhal edited https://github.com/llvm/llvm-project/pull/78315 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [llvm] [clang] [analyzer] Support interestingness in ArrayBoundV2 (PR #78315)
=?utf-8?q?Donát?= Nagy , =?utf-8?q?Donát?= Nagy , =?utf-8?q?Donát?= Nagy , =?utf-8?q?Donát?= Nagy , =?utf-8?q?Donát?= Nagy , =?utf-8?q?Donát?= Nagy Message-ID: In-Reply-To: @@ -221,18 +221,38 @@ int allocaRegion(void) { return *mem; } -int *unknownExtent(int arg) { - if (arg >= 2) +int *symbolicExtent(int arg) { + // expected-note@+2 {{Assuming 'arg' is < 5}} + // expected-note@+1 {{Taking false branch}} + if (arg >= 5) return 0; int *mem = (int*)malloc(arg); + + // TODO: without the following reference to 'arg', the analyzer would discard + // the range information about (the symbolic value of) 'arg'. This is + // incorrect because while the variable itself is inaccessible, it becomes + // the symbolic extent of 'mem', so we still want to reason about its + // potential values. + (void)arg; + mem[8] = -2; - // FIXME: this should produce - // {{Out of bound access to memory after the end of the heap area}} - // {{Access of 'int' element in the heap area at index 8}} + // expected-warning@-1 {{Out of bound access to memory after the end of the heap area}} + // expected-note@-2 {{Access of 'int' element in the heap area at index 8}} return mem; } -void unknownIndex(int arg) { +int *symbolicExtentDiscardedRangeInfo(int arg) { + // This is a copy of the case 'symbolicExtent' without the '(void)arg' hack. + // TODO: if the analyzer can detect the out-of-bounds access within this TC, + // then remove this TC and the `(void)arg` hack from `symbolicExtent`. steakhal wrote: I'd prefer to expand `TC`, it's not that longer. https://github.com/llvm/llvm-project/pull/78315 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-tools-extra] [llvm] [analyzer] Support interestingness in ArrayBoundV2 (PR #78315)
=?utf-8?q?Donát?= Nagy , =?utf-8?q?Donát?= Nagy , =?utf-8?q?Donát?= Nagy , =?utf-8?q?Donát?= Nagy , =?utf-8?q?Donát?= Nagy , =?utf-8?q?Donát?= Nagy Message-ID: In-Reply-To: https://github.com/steakhal commented: > Ouch, that seems to be a nasty issue. Thanks for doing the review and I hope > that you'll be able to share it eventually :) > > (If there is no clean resolution, feel free to dump a semi-formatted > copypaste / "Save As..." html snapshot of the review that you prepared. As > long as your suggestions are readable, they'll be helpful.) I waited a couple days, but I still get the same permission problem. I sent you a private email having the PDF exported view I had on reviewable.io I'm sorry about this inconvenience. https://github.com/llvm/llvm-project/pull/78315 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][analyzer] Fix argument invalidations in StreamChecker. (PR #79470)
@@ -0,0 +1,133 @@ +// RUN: %clang_analyze_cc1 -verify %s \ +// RUN: -analyzer-checker=core \ +// RUN: -analyzer-checker=alpha.unix.Stream \ +// RUN: -analyzer-checker=debug.StreamTester \ +// RUN: -analyzer-checker=debug.ExprInspection + +#include "Inputs/system-header-simulator.h" + +void clang_analyzer_eval(int); +void clang_analyzer_dump(int); +void clang_analyzer_warnIfReached(void); +void StreamTesterChecker_make_feof_stream(FILE *); +void StreamTesterChecker_make_ferror_stream(FILE *); + +void test_fread(void) { + FILE *F = fopen("file", "r+"); + if (!F) +return; + + char Buf[3] = {10, 10, 10}; + fread(Buf, 1, 3, F); + // this check applies to succes and failure + clang_analyzer_dump(Buf[0]); // expected-warning {{conj_$}} Should not preserve the previous value, thus should not be 10. + clang_analyzer_dump(Buf[2]); // expected-warning {{conj_$}} + if (feof(F)) { steakhal wrote: In this, and the in the rest of the test, could we have a branch for testing if the api-call encountered an error? E.g. when `ferror(F)==true`. On that path, we should still invalidate the buffer. https://github.com/llvm/llvm-project/pull/79470 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][analyzer] Fix argument invalidations in StreamChecker. (PR #79470)
@@ -544,6 +545,21 @@ const ExplodedNode *StreamChecker::getAcquisitionSite(const ExplodedNode *N, return nullptr; } +static ProgramStateRef +escapeArgs(ProgramStateRef State, CheckerContext , const CallEvent , + const SmallVector ) { + const auto *CE = Call.getOriginExpr(); + + SmallVector EscapingVals; + EscapingVals.reserve(EscapingArgs.size()); + for (auto EscArgIdx : EscapingArgs) +EscapingVals.push_back(Call.getArgSVal(EscArgIdx)); + State = State->invalidateRegions(EscapingVals, CE, C.blockCount(), + C.getLocationContext(), + /*CausesPointerEscape=*/false); steakhal wrote: ```suggestion State = State->invalidateRegions(EscapingVals, CE, C.blockCount(), C.getLocationContext(), /*CausesPointerEscape=*/false, /*InvalidatedSymbols=*/nullptr, ); ``` I can't recall now what difference it makes to pass the `Call` to this API, but given that we have one, why not? https://github.com/llvm/llvm-project/pull/79470 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][analyzer] Fix argument invalidations in StreamChecker. (PR #79470)
@@ -0,0 +1,133 @@ +// RUN: %clang_analyze_cc1 -verify %s \ +// RUN: -analyzer-checker=core \ +// RUN: -analyzer-checker=alpha.unix.Stream \ +// RUN: -analyzer-checker=debug.StreamTester \ +// RUN: -analyzer-checker=debug.ExprInspection + +#include "Inputs/system-header-simulator.h" + +void clang_analyzer_eval(int); +void clang_analyzer_dump(int); +void clang_analyzer_warnIfReached(void); +void StreamTesterChecker_make_feof_stream(FILE *); +void StreamTesterChecker_make_ferror_stream(FILE *); + +void test_fread(void) { + FILE *F = fopen("file", "r+"); + if (!F) +return; + + char Buf[3] = {10, 10, 10}; + fread(Buf, 1, 3, F); + // this check applies to succes and failure + clang_analyzer_dump(Buf[0]); // expected-warning {{conj_$}} Should not preserve the previous value, thus should not be 10. + clang_analyzer_dump(Buf[2]); // expected-warning {{conj_$}} + if (feof(F)) { +char Buf1[3] = {10, 10, 10}; +fread(Buf1, 1, 3, F); // expected-warning {{is in EOF state}} +clang_analyzer_dump(Buf1[0]); // expected-warning {{10 S32b}} +clang_analyzer_dump(Buf1[2]); // expected-warning {{10 S32b}} + } + + fclose(F); +} + +void test_fwrite(void) { + FILE *F = fopen("file", "r+"); + if (!F) +return; + + char Buf[3] = {10, 10, 10}; + fwrite(Buf, 1, 3, F); + // this check applies to succes and failure steakhal wrote: ```suggestion // This check applies to success and failure. ``` As per llvm style, capitalize and punctuate comments. `sed "s/this check applies to succes and failure/This check applies to success and failure."` There was also a typo `sed "s/succes/success/g"` this file. https://github.com/llvm/llvm-project/pull/79470 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][analyzer] Fix argument invalidations in StreamChecker. (PR #79470)
@@ -0,0 +1,133 @@ +// RUN: %clang_analyze_cc1 -verify %s \ +// RUN: -analyzer-checker=core \ +// RUN: -analyzer-checker=alpha.unix.Stream \ +// RUN: -analyzer-checker=debug.StreamTester \ +// RUN: -analyzer-checker=debug.ExprInspection + +#include "Inputs/system-header-simulator.h" + +void clang_analyzer_eval(int); +void clang_analyzer_dump(int); +void clang_analyzer_warnIfReached(void); +void StreamTesterChecker_make_feof_stream(FILE *); +void StreamTesterChecker_make_ferror_stream(FILE *); + +void test_fread(void) { + FILE *F = fopen("file", "r+"); + if (!F) +return; + + char Buf[3] = {10, 10, 10}; + fread(Buf, 1, 3, F); + // this check applies to succes and failure + clang_analyzer_dump(Buf[0]); // expected-warning {{conj_$}} Should not preserve the previous value, thus should not be 10. + clang_analyzer_dump(Buf[2]); // expected-warning {{conj_$}} + if (feof(F)) { +char Buf1[3] = {10, 10, 10}; +fread(Buf1, 1, 3, F); // expected-warning {{is in EOF state}} +clang_analyzer_dump(Buf1[0]); // expected-warning {{10 S32b}} +clang_analyzer_dump(Buf1[2]); // expected-warning {{10 S32b}} + } + + fclose(F); +} + +void test_fwrite(void) { + FILE *F = fopen("file", "r+"); + if (!F) +return; + + char Buf[3] = {10, 10, 10}; + fwrite(Buf, 1, 3, F); + // this check applies to succes and failure + clang_analyzer_dump(Buf[0]); // expected-warning {{10 S32b}} + clang_analyzer_dump(Buf[2]); // expected-warning {{10 S32b}} + + fclose(F); +} + +void test_fgets() { + FILE *F = tmpfile(); + if (!F) +return; + + char Buf[3] = {10, 10, 10}; + fgets(Buf, 3, F); + // this check applies to succes and failure + clang_analyzer_dump(Buf[0]); // expected-warning {{conj_$}} Should not preserve the previous value, thus should not be 10. + clang_analyzer_dump(Buf[2]); // expected-warning {{conj_$}} + if (feof(F)) { +char Buf1[3] = {10, 10, 10}; +fgets(Buf1, 3, F); // expected-warning {{is in EOF state}} +clang_analyzer_dump(Buf1[0]); // expected-warning {{10 S32b}} +clang_analyzer_dump(Buf1[2]); // expected-warning {{10 S32b}} + } + + fclose(F); +} + +void test_fputs() { + FILE *F = tmpfile(); + if (!F) +return; + + char *Buf = "aaa"; + fputs(Buf, F); + // this check applies to succes and failure + clang_analyzer_dump(Buf[0]); // expected-warning {{97 S32b}} + clang_analyzer_dump(Buf[2]); // expected-warning {{97 S32b}} + clang_analyzer_dump(Buf[3]); // expected-warning {{0 S32b}} + + fclose(F); +} + +void test_fscanf() { + FILE *F = tmpfile(); + if (!F) +return; + + int a = 1; + unsigned b; + int Ret = fscanf(F, "%d %u", , ); + if (Ret >= 0) { +// FIXME: return value +clang_analyzer_dump(a); // expected-warning {{conj_$}} +clang_analyzer_dump(b); // expected-warning {{conj_$}} + } else { +clang_analyzer_dump(a); // expected-warning {{1 S32b}} +clang_analyzer_dump(b); // expected-warning {{uninitialized value}} + } + fclose(F); +} + +void test_getdelim(char *P, size_t Sz) { + FILE *F = tmpfile(); + if (!F) +return; + + char *P1 = P; + size_t Sz1 = Sz; + ssize_t Ret = getdelim(, , '\t', F); + clang_analyzer_eval(P == P1); // expected-warning {{FALSE}} \ +// expected-warning {{TRUE}} + clang_analyzer_eval(Sz == Sz1); // expected-warning {{FALSE}} \ + // expected-warning {{TRUE}} steakhal wrote: I don't think this is too useful this way. We can't distinguish the failure and success branches. Lets have different branches depending on `Ret`, to make it clear. https://github.com/llvm/llvm-project/pull/79470 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][analyzer] Fix argument invalidations in StreamChecker. (PR #79470)
@@ -544,6 +545,21 @@ const ExplodedNode *StreamChecker::getAcquisitionSite(const ExplodedNode *N, return nullptr; } +static ProgramStateRef +escapeArgs(ProgramStateRef State, CheckerContext , const CallEvent , + const SmallVector ) { steakhal wrote: ```suggestion escapeArgs(ProgramStateRef State, CheckerContext , const CallEvent , ArrayRef EscapingArgs) { ``` Views should be preferred for input parameters. https://github.com/llvm/llvm-project/pull/79470 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][analyzer] Fix argument invalidations in StreamChecker. (PR #79470)
@@ -0,0 +1,133 @@ +// RUN: %clang_analyze_cc1 -verify %s \ +// RUN: -analyzer-checker=core \ +// RUN: -analyzer-checker=alpha.unix.Stream \ +// RUN: -analyzer-checker=debug.StreamTester \ +// RUN: -analyzer-checker=debug.ExprInspection + +#include "Inputs/system-header-simulator.h" + +void clang_analyzer_eval(int); +void clang_analyzer_dump(int); +void clang_analyzer_warnIfReached(void); +void StreamTesterChecker_make_feof_stream(FILE *); +void StreamTesterChecker_make_ferror_stream(FILE *); + +void test_fread(void) { + FILE *F = fopen("file", "r+"); + if (!F) +return; + + char Buf[3] = {10, 10, 10}; + fread(Buf, 1, 3, F); + // this check applies to succes and failure + clang_analyzer_dump(Buf[0]); // expected-warning {{conj_$}} Should not preserve the previous value, thus should not be 10. + clang_analyzer_dump(Buf[2]); // expected-warning {{conj_$}} + if (feof(F)) { +char Buf1[3] = {10, 10, 10}; +fread(Buf1, 1, 3, F); // expected-warning {{is in EOF state}} +clang_analyzer_dump(Buf1[0]); // expected-warning {{10 S32b}} +clang_analyzer_dump(Buf1[2]); // expected-warning {{10 S32b}} + } + + fclose(F); +} + +void test_fwrite(void) { + FILE *F = fopen("file", "r+"); + if (!F) +return; + + char Buf[3] = {10, 10, 10}; + fwrite(Buf, 1, 3, F); + // this check applies to succes and failure + clang_analyzer_dump(Buf[0]); // expected-warning {{10 S32b}} + clang_analyzer_dump(Buf[2]); // expected-warning {{10 S32b}} + + fclose(F); +} + +void test_fgets() { + FILE *F = tmpfile(); + if (!F) +return; + + char Buf[3] = {10, 10, 10}; + fgets(Buf, 3, F); + // this check applies to succes and failure + clang_analyzer_dump(Buf[0]); // expected-warning {{conj_$}} Should not preserve the previous value, thus should not be 10. + clang_analyzer_dump(Buf[2]); // expected-warning {{conj_$}} + if (feof(F)) { +char Buf1[3] = {10, 10, 10}; +fgets(Buf1, 3, F); // expected-warning {{is in EOF state}} +clang_analyzer_dump(Buf1[0]); // expected-warning {{10 S32b}} +clang_analyzer_dump(Buf1[2]); // expected-warning {{10 S32b}} + } + + fclose(F); +} + +void test_fputs() { + FILE *F = tmpfile(); + if (!F) +return; + + char *Buf = "aaa"; + fputs(Buf, F); + // this check applies to succes and failure + clang_analyzer_dump(Buf[0]); // expected-warning {{97 S32b}} + clang_analyzer_dump(Buf[2]); // expected-warning {{97 S32b}} + clang_analyzer_dump(Buf[3]); // expected-warning {{0 S32b}} + + fclose(F); +} + +void test_fscanf() { + FILE *F = tmpfile(); + if (!F) +return; + + int a = 1; + unsigned b; + int Ret = fscanf(F, "%d %u", , ); + if (Ret >= 0) { +// FIXME: return value +clang_analyzer_dump(a); // expected-warning {{conj_$}} steakhal wrote: ```suggestion clang_analyzer_dump(a); // expected-warning {{conj_$}} clang_analyzer_eval(Ret > 2); // expected-warning {{FALSE}} expected-warning {{TRUE}} FIXME: should be only FALSE. ``` Let's take the opportunity for adding an expected (but failing) assumption here. May be fixed in the future. https://github.com/llvm/llvm-project/pull/79470 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][analyzer] Fix argument invalidations in StreamChecker. (PR #79470)
https://github.com/steakhal commented: I like what you do in this patch. I only have a couple nits. That's it. Tomorrow, I'll check if there are any other APIs that we should test; but seems complete at first glance. https://github.com/llvm/llvm-project/pull/79470 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][analyzer] Fix argument invalidations in StreamChecker. (PR #79470)
https://github.com/steakhal edited https://github.com/llvm/llvm-project/pull/79470 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [analyzer] Avoid a crash in a debug printout function (PR #79446)
https://github.com/steakhal approved this pull request. https://github.com/llvm/llvm-project/pull/79446 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [analyzer] Avoid a crash in a debug printout function (PR #79446)
https://github.com/steakhal requested changes to this pull request. https://github.com/llvm/llvm-project/pull/79446 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [analyzer] Avoid a crash in a debug printout function (PR #79446)
@@ -3270,8 +3270,12 @@ void RangeConstraintManager::printJson(raw_ostream , ProgramStateRef State, void RangeConstraintManager::printValue(raw_ostream , ProgramStateRef State, SymbolRef Sym) { const RangeSet RS = getRange(State, Sym); - Out << RS.getBitWidth() << (RS.isUnsigned() ? "u:" : "s:"); - RS.dump(Out); + if (RS.isEmpty()) { +Out << ""; + } else { +Out << RS.getBitWidth() << (RS.isUnsigned() ? "u:" : "s:"); +RS.dump(Out); + } steakhal wrote: ```suggestion if (RS.isEmpty()) { Out << ""; return; } Out << RS.getBitWidth() << (RS.isUnsigned() ? "u:" : "s:"); RS.dump(Out); ``` https://github.com/llvm/llvm-project/pull/79446 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [analyzer] Avoid a crash in a debug printout function (PR #79446)
https://github.com/steakhal edited https://github.com/llvm/llvm-project/pull/79446 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [analyzer] Avoid a crash in a debug printout function (PR #79446)
steakhal wrote: When is it possible to have an empty range set as a constraint? https://github.com/llvm/llvm-project/pull/79446 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][analyzer] Simplify code of StreamChecker (NFC). (PR #79312)
steakhal wrote: > I'm seconding the suggestions of @steakhal, and in particular I agree with > > > I'd also advise against using more callables bundled with CallDescriptions. > > They make debugging code more difficult, as the control-flow would become > > also data-dependent. I'd suggest other ways to generalize. > > Instead of creating this shared framework that calls different callbacks > depending on the checked function, consider keeping separate top-level > evaluator functions that rely on a shared set of smaller tool-like helper > functions. One more not about composability. In general, `addTransition` calls don't compose, thus limits reusability. For example, `preReadWrite` should be reusable from other precondition handlers, such that the new one can just call `preReadWrite` and then do some other more specialized checks. However, once a handler calls `addTransition` we are done. We no longer can simply wrap it, aka. compose with other functionalities. Consequently, I'd suggest to have some common pattern, such as return state pointers, or NULL if an error was reported. And leave the `addTransition` to the caller - if they chose to call it. `eval*` handlers are more difficult to compose, given that they also need to bind the return value and apply their sideffects, but I believe we could achieve similar objectives there too (thus, have refined, composable handlers, combined in useful ways). https://github.com/llvm/llvm-project/pull/79312 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [analyzer] Unbreak [[clang::suppress]] on checkers without decl-with-issue. (PR #79398)
https://github.com/steakhal edited https://github.com/llvm/llvm-project/pull/79398 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [analyzer] Unbreak [[clang::suppress]] on checkers without decl-with-issue. (PR #79398)
@@ -44,7 +47,9 @@ class BugSuppression { using CachedRanges = llvm::SmallVector; - llvm::DenseMap CachedSuppressionLocations; + llvm::DenseMap CachedSuppressionLocations{}; + + ASTContext steakhal wrote: ```suggestion llvm::DenseMap CachedSuppressionLocations; const ASTContext ``` https://github.com/llvm/llvm-project/pull/79398 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [analyzer] Unbreak [[clang::suppress]] on checkers without decl-with-issue. (PR #79398)
@@ -27,6 +28,8 @@ class PathDiagnosticLocation; class BugSuppression { public: + BugSuppression(ASTContext ) : ACtx(ACtx) {} steakhal wrote: ```suggestion explicit BugSuppression(const ASTContext ) : ACtx(ACtx) {} ``` https://github.com/llvm/llvm-project/pull/79398 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [analyzer] Unbreak [[clang::suppress]] on checkers without decl-with-issue. (PR #79398)
https://github.com/steakhal commented: I only have minor nits. No objections. https://github.com/llvm/llvm-project/pull/79398 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][analyzer] Improve modeling of 'popen' and 'pclose' in StdLibraryFunctionsChecker (PR #78895)
https://github.com/steakhal approved this pull request. https://github.com/llvm/llvm-project/pull/78895 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][analyzer] Improve modeling of 'popen' and 'pclose' in StdLibraryFunctionsChecker (PR #78895)
@@ -2211,6 +2221,15 @@ void StdLibraryFunctionsChecker::initFunctionSummaries( ErrnoNEZeroIrrelevant, GenericFailureMsg) .ArgConstraint(NotNull(ArgNo(0; +// int pclose(FILE *stream); +addToFunctionSummaryMap( +"pclose", Signature(ArgTypes{FilePtrTy}, RetType{IntTy}), +Summary(NoEvalCall) +.Case({ReturnValueCondition(WithinRange, {{0, IntMax}})}, + ErrnoMustNotBeChecked, GenericSuccessMsg) +.Case(ReturnsMinusOne, ErrnoNEZeroIrrelevant, GenericFailureMsg) +.ArgConstraint(NotNull(ArgNo(0; + steakhal wrote: Okay. Thanks for digging into this. Consider this thread resolved. https://github.com/llvm/llvm-project/pull/78895 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][analyzer] Improve modeling of 'popen' and 'pclose' in StdLibraryFunctionsChecker (PR #78895)
@@ -2211,6 +2221,15 @@ void StdLibraryFunctionsChecker::initFunctionSummaries( ErrnoNEZeroIrrelevant, GenericFailureMsg) .ArgConstraint(NotNull(ArgNo(0; +// int pclose(FILE *stream); +addToFunctionSummaryMap( +"pclose", Signature(ArgTypes{FilePtrTy}, RetType{IntTy}), +Summary(NoEvalCall) +.Case({ReturnValueCondition(WithinRange, {{0, IntMax}})}, + ErrnoMustNotBeChecked, GenericSuccessMsg) +.Case(ReturnsMinusOne, ErrnoNEZeroIrrelevant, GenericFailureMsg) +.ArgConstraint(NotNull(ArgNo(0; + steakhal wrote: Quoting manpage: `on success, returns the exit status of the command`, consequently, this might be negative on success. I believe the manpage for `exit(3)` should tell us what values can we expect as exit codes. > The exit() function causes normal process termination and the least significant byte of status (i.e., status & 0xFF) is returned to the parent (see [wait(2)](https://man7.org/linux/man-pages/man2/wait.2.html)). So, I believe any value could be an exit code, that is representable by a signed char (if CHAR_BITS == 8, then [-128,127]) Also add tests for the range for the result of the `pclose`. https://github.com/llvm/llvm-project/pull/78895 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][analyzer] Improve modeling of 'popen' and 'pclose' in StdLibraryFunctionsChecker (PR #78895)
https://github.com/steakhal edited https://github.com/llvm/llvm-project/pull/78895 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][analyzer] Improve modeling of 'popen' and 'pclose' in StdLibraryFunctionsChecker (PR #78895)
https://github.com/steakhal requested changes to this pull request. https://github.com/llvm/llvm-project/pull/78895 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits