Author: Balázs Kéri Date: 2025-12-17T09:46:24+01:00 New Revision: 5bfd57e7c9f83d56647a094e605cc15dce6b8d4c
URL: https://github.com/llvm/llvm-project/commit/5bfd57e7c9f83d56647a094e605cc15dce6b8d4c DIFF: https://github.com/llvm/llvm-project/commit/5bfd57e7c9f83d56647a094e605cc15dce6b8d4c.diff LOG: [clang][analyzer] CallAndMessage warnings at pointer to uninitialized struct (#164600) `CallAndMessageChecker` did have a warning for the case when pointer to uninitialized data is passed to a function when the argument type is pointer to const. This did not work for struct types. The check is improved to handle cases when struct with uninitialized data is passed to a function. Added: clang/test/Analysis/call-and-message-argpointeeinitializedness.cpp Modified: clang/include/clang/StaticAnalyzer/Checkers/Checkers.td clang/lib/StaticAnalyzer/Checkers/CallAndMessageChecker.cpp clang/test/Analysis/analyzer-config.c clang/test/Analysis/call-and-message.c Removed: ################################################################################ diff --git a/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td b/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td index ffae3b9310979..342e99fb0e878 100644 --- a/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td +++ b/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td @@ -179,6 +179,12 @@ def CallAndMessageChecker "Check whether the reciever in the message expression is " "undefined", "true", Released>, + CmdLineOption< + Boolean, "ArgPointeeInitializednessComplete", + "If set to true, treat a struct as initialized when all of its " + "members are initialized, otherwise when it has any initialized " + "member (used only at ArgPointeeInitializedness)", + "false", Released>, ]>, Documentation<HasDocumentation>; diff --git a/clang/lib/StaticAnalyzer/Checkers/CallAndMessageChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/CallAndMessageChecker.cpp index e71fe47bb8792..97acc34644c9c 100644 --- a/clang/lib/StaticAnalyzer/Checkers/CallAndMessageChecker.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/CallAndMessageChecker.cpp @@ -20,7 +20,9 @@ #include "clang/StaticAnalyzer/Core/CheckerManager.h" #include "clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h" #include "clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h" +#include "llvm/ADT/STLExtras.h" #include "llvm/ADT/StringExtras.h" +#include "llvm/Support/FormatVariadic.h" #include "llvm/Support/raw_ostream.h" using namespace clang; @@ -77,6 +79,10 @@ class CallAndMessageChecker bool ChecksEnabled[CK_NumCheckKinds] = {false}; + /// When checking a struct value for uninitialized data, should all the fields + /// be un-initialized or only find one uninitialized field. + bool StructInitializednessComplete = true; + void checkPreObjCMessage(const ObjCMethodCall &msg, CheckerContext &C) const; /// Fill in the return value that results from messaging nil based on the @@ -179,69 +185,23 @@ static void describeUninitializedArgumentInCall(const CallEvent &Call, } } -bool CallAndMessageChecker::uninitRefOrPointer( - CheckerContext &C, SVal V, SourceRange ArgRange, const Expr *ArgEx, - const BugType &BT, const ParmVarDecl *ParamDecl, int ArgumentNumber) const { - - // The pointee being uninitialized is a sign of code smell, not a bug, no need - // to sink here. - if (!ChecksEnabled[CK_ArgPointeeInitializedness]) - return false; - - // No parameter declaration available, i.e. variadic function argument. - if(!ParamDecl) - return false; - - // If parameter is declared as pointer to const in function declaration, - // then check if corresponding argument in function call is - // pointing to undefined symbol value (uninitialized memory). - SmallString<200> Buf; - llvm::raw_svector_ostream Os(Buf); - - if (ParamDecl->getType()->isPointerType()) { - Os << (ArgumentNumber + 1) << llvm::getOrdinalSuffix(ArgumentNumber + 1) - << " function call argument is a pointer to uninitialized value"; - } else if (ParamDecl->getType()->isReferenceType()) { - Os << (ArgumentNumber + 1) << llvm::getOrdinalSuffix(ArgumentNumber + 1) - << " function call argument is an uninitialized value"; - } else - return false; - - if(!ParamDecl->getType()->getPointeeType().isConstQualified()) - return false; - - if (const MemRegion *SValMemRegion = V.getAsRegion()) { - const ProgramStateRef State = C.getState(); - const SVal PSV = State->getSVal(SValMemRegion, C.getASTContext().CharTy); - if (PSV.isUndef()) { - if (ExplodedNode *N = C.generateErrorNode()) { - auto R = std::make_unique<PathSensitiveBugReport>(BT, Os.str(), N); - R->addRange(ArgRange); - if (ArgEx) - bugreporter::trackExpressionValue(N, ArgEx, *R); - - C.emitReport(std::move(R)); - } - return true; - } - } - return false; -} - namespace { class FindUninitializedField { public: - SmallVector<const FieldDecl *, 10> FieldChain; + using FieldChainTy = SmallVector<const FieldDecl *, 10>; + FieldChainTy FieldChain; private: StoreManager &StoreMgr; MemRegionManager &MrMgr; Store store; + bool FindNotUninitialized; public: FindUninitializedField(StoreManager &storeMgr, MemRegionManager &mrMgr, - Store s) - : StoreMgr(storeMgr), MrMgr(mrMgr), store(s) {} + Store s, bool FindNotUninitialized = false) + : StoreMgr(storeMgr), MrMgr(mrMgr), store(s), + FindNotUninitialized(FindNotUninitialized) {} bool Find(const TypedValueRegion *R) { QualType T = R->getValueType(); @@ -255,22 +215,114 @@ class FindUninitializedField { FieldChain.push_back(I); T = I->getType(); if (T->isStructureType()) { - if (Find(FR)) - return true; + if (FindNotUninitialized ? !Find(FR) : Find(FR)) + return !FindNotUninitialized; } else { SVal V = StoreMgr.getBinding(store, loc::MemRegionVal(FR)); - if (V.isUndef()) - return true; + if (FindNotUninitialized ? !V.isUndef() : V.isUndef()) + return !FindNotUninitialized; } FieldChain.pop_back(); } } - return false; + return FindNotUninitialized; } }; } // namespace +namespace llvm { +template <> struct format_provider<FindUninitializedField::FieldChainTy> { + static void format(const FindUninitializedField::FieldChainTy &V, + raw_ostream &Stream, StringRef Style) { + if (V.size() == 0) + return; + else if (V.size() == 1) + Stream << " (e.g., field: '" << *V[0] << "')"; + else { + Stream << " (e.g., via the field chain: '"; + interleave( + V, Stream, [&Stream](const FieldDecl *FD) { Stream << *FD; }, "."); + Stream << "')"; + } + } +}; +} // namespace llvm + +bool CallAndMessageChecker::uninitRefOrPointer( + CheckerContext &C, SVal V, SourceRange ArgRange, const Expr *ArgEx, + const BugType &BT, const ParmVarDecl *ParamDecl, int ArgumentNumber) const { + + if (!ChecksEnabled[CK_ArgPointeeInitializedness]) + return false; + + // No parameter declaration available, i.e. variadic function argument. + if (!ParamDecl) + return false; + + QualType ParamT = ParamDecl->getType(); + if (!ParamT->isPointerOrReferenceType()) + return false; + + QualType PointeeT = ParamT->getPointeeType(); + if (!PointeeT.isConstQualified()) + return false; + + const MemRegion *SValMemRegion = V.getAsRegion(); + if (!SValMemRegion) + return false; + + // If parameter is declared as pointer to const in function declaration, + // then check if corresponding argument in function call is + // pointing to undefined symbol value (uninitialized memory). + + const ProgramStateRef State = C.getState(); + if (PointeeT->isVoidType()) + PointeeT = C.getASTContext().CharTy; + const SVal PointeeV = State->getSVal(SValMemRegion, PointeeT); + + if (PointeeV.isUndef()) { + if (ExplodedNode *N = C.generateErrorNode()) { + std::string Msg = llvm::formatv( + "{0}{1} function call argument is {2} uninitialized value", + ArgumentNumber + 1, llvm::getOrdinalSuffix(ArgumentNumber + 1), + ParamT->isPointerType() ? "a pointer to" : "an"); + auto R = std::make_unique<PathSensitiveBugReport>(BT, Msg, N); + R->addRange(ArgRange); + if (ArgEx) + bugreporter::trackExpressionValue(N, ArgEx, *R); + + C.emitReport(std::move(R)); + } + return true; + } + + if (auto LV = PointeeV.getAs<nonloc::LazyCompoundVal>()) { + const LazyCompoundValData *D = LV->getCVData(); + FindUninitializedField F(C.getState()->getStateManager().getStoreManager(), + C.getSValBuilder().getRegionManager(), + D->getStore(), StructInitializednessComplete); + + if (F.Find(D->getRegion())) { + if (ExplodedNode *N = C.generateErrorNode()) { + std::string Msg = llvm::formatv( + "{0}{1} function call argument {2} an uninitialized value{3}", + (ArgumentNumber + 1), llvm::getOrdinalSuffix(ArgumentNumber + 1), + ParamT->isPointerType() ? "points to" : "references", F.FieldChain); + auto R = std::make_unique<PathSensitiveBugReport>(BT, Msg, N); + R->addRange(ArgRange); + if (ArgEx) + bugreporter::trackExpressionValue(N, ArgEx, *R); + + C.emitReport(std::move(R)); + } + return true; + } + } + + return false; +} + bool CallAndMessageChecker::PreVisitProcessArg( CheckerContext &C, SVal V, SourceRange ArgRange, const Expr *ArgEx, int ArgumentNumber, bool CheckUninitFields, const CallEvent &Call, @@ -313,27 +365,12 @@ bool CallAndMessageChecker::PreVisitProcessArg( return true; } if (ExplodedNode *N = C.generateErrorNode()) { - SmallString<512> Str; - llvm::raw_svector_ostream os(Str); - os << "Passed-by-value struct argument contains uninitialized data"; - - if (F.FieldChain.size() == 1) - os << " (e.g., field: '" << *F.FieldChain[0] << "')"; - else { - os << " (e.g., via the field chain: '"; - bool first = true; - for (const FieldDecl *FD : F.FieldChain) { - if (first) - first = false; - else - os << '.'; - os << *FD; - } - os << "')"; - } + std::string Msg = llvm::formatv( + "Passed-by-value struct argument contains uninitialized data{0}", + F.FieldChain); // Generate a report for this bug. - auto R = std::make_unique<PathSensitiveBugReport>(BT, os.str(), N); + auto R = std::make_unique<PathSensitiveBugReport>(BT, Msg, N); R->addRange(ArgRange); if (ArgEx) @@ -689,6 +726,10 @@ void ento::registerCallAndMessageChecker(CheckerManager &Mgr) { QUERY_CHECKER_OPTION(ArgPointeeInitializedness) QUERY_CHECKER_OPTION(NilReceiver) QUERY_CHECKER_OPTION(UndefReceiver) + + Chk->StructInitializednessComplete = + Mgr.getAnalyzerOptions().getCheckerBooleanOption( + Mgr.getCurrentCheckerName(), "ArgPointeeInitializednessComplete"); } bool ento::shouldRegisterCallAndMessageChecker(const CheckerManager &) { diff --git a/clang/test/Analysis/analyzer-config.c b/clang/test/Analysis/analyzer-config.c index 7936273415ad4..4e1f5336a9040 100644 --- a/clang/test/Analysis/analyzer-config.c +++ b/clang/test/Analysis/analyzer-config.c @@ -31,6 +31,7 @@ // CHECK-NEXT: core.BitwiseShift:Pedantic = false // CHECK-NEXT: core.CallAndMessage:ArgInitializedness = true // CHECK-NEXT: core.CallAndMessage:ArgPointeeInitializedness = false +// CHECK-NEXT: core.CallAndMessage:ArgPointeeInitializednessComplete = false // CHECK-NEXT: core.CallAndMessage:CXXDeallocationArg = true // CHECK-NEXT: core.CallAndMessage:CXXThisMethodCall = true // CHECK-NEXT: core.CallAndMessage:FunctionPointer = true diff --git a/clang/test/Analysis/call-and-message-argpointeeinitializedness.cpp b/clang/test/Analysis/call-and-message-argpointeeinitializedness.cpp new file mode 100644 index 0000000000000..bf84f8d88fe95 --- /dev/null +++ b/clang/test/Analysis/call-and-message-argpointeeinitializedness.cpp @@ -0,0 +1,98 @@ +// RUN: %clang_analyze_cc1 %s -verify=initializedness-complete \ +// RUN: -analyzer-checker=core \ +// RUN: -analyzer-config core.CallAndMessage:ArgPointeeInitializedness=true \ +// RUN: -analyzer-config core.CallAndMessage:ArgPointeeInitializednessComplete=true \ +// RUN: -analyzer-config core.CallAndMessage:ArgInitializedness=false + +// RUN: %clang_analyze_cc1 %s -verify=initializedness-partial \ +// RUN: -analyzer-checker=core \ +// RUN: -analyzer-config core.CallAndMessage:ArgPointeeInitializedness=true \ +// RUN: -analyzer-config core.CallAndMessage:ArgInitializedness=false + +struct S1 { + char c; +}; + +struct S { + int a; + S1 b; +}; + +S GlobalS; + +void doStuffP(const S *); +void doStuffR(const S &); + +void uninit_val_p() { + S s; + doStuffP(&s); // initializedness-partial-warning{{1st function call argument points to an uninitialized value (e.g., field: 'a')}} \ + // initializedness-complete-warning{{1st function call argument points to an uninitialized value}} +} + +void uninit_val_r() { + S s; + doStuffR(s); // initializedness-partial-warning{{1st function call argument references an uninitialized value (e.g., field: 'a')}} \ + // initializedness-complete-warning{{1st function call argument references an uninitialized value}} +} + +S *uninit_new() { + S *s = new S; + doStuffP(s); // initializedness-partial-warning{{1st function call argument points to an uninitialized value (e.g., field: 'a')}} \ + // initializedness-complete-warning{{1st function call argument points to an uninitialized value}} + return s; +} + +void uninit_ctr() { + S s = S(); + doStuffP(&s); +} + +void uninit_init() { + S s{}; + doStuffP(&s); +} + +void uninit_init_val() { + S s{1, {2}}; + doStuffP(&s); +} + +void uninit_parm_ptr(S *s) { + doStuffP(s); +} + +void uninit_parm_val(S s) { + doStuffP(&s); +} + +void uninit_parm_ref(S &s) { + doStuffP(&s); +} + +void init_val() { + S s; + s.a = 1; + s.b.c = 1; + doStuffP(&s); +} + +void uninit_global() { + doStuffP(&GlobalS); +} + +void uninit_static() { + static S s; + doStuffP(&s); +} + +void uninit_val_partial_1() { + S s; + s.a = 1; + doStuffR(s); // initializedness-partial-warning{{1st function call argument references an uninitialized value (e.g., via the field chain: 'b.c')}} +} + +void uninit_val_partial_2() { + S s; + s.b.c = 1; + doStuffR(s); // initializedness-partial-warning{{1st function call argument references an uninitialized value (e.g., field: 'a')}} +} diff --git a/clang/test/Analysis/call-and-message.c b/clang/test/Analysis/call-and-message.c index ade51145e2a93..5f34635123426 100644 --- a/clang/test/Analysis/call-and-message.c +++ b/clang/test/Analysis/call-and-message.c @@ -24,6 +24,34 @@ void pointee_uninit(void) { doStuff_pointerToConstInt(p); // expected-warning{{1st function call argument is a pointer to uninitialized value [core.CallAndMessage]}} } +typedef struct S { + int a; + short b; +} S; + +void doStuff_pointerToConstStruct(const S *s){}; +void pointee_uninit_struct(void) { + S s; + S *p = &s; + doStuff_pointerToConstStruct(p); // expected-warning{{1st function call argument points to an uninitialized value (e.g., field: 'a') [core.CallAndMessage]}} +} +void pointee_uninit_struct_1(void) { + S s; + s.a = 2; + doStuff_pointerToConstStruct(&s); // expected-warning{{1st function call argument points to an uninitialized value (e.g., field: 'b') [core.CallAndMessage]}} +} +void pointee_uninit_struct_2(void) { + S s = {}; + doStuff_pointerToConstStruct(&s); +} +void pointee_uninit_struct_3(S *s) { + doStuff_pointerToConstStruct(s); +} +void pointee_uninit_struct_4(void) { + S s = {1, 2}; + doStuff_pointerToConstStruct(&s); +} + // TODO: If this hash ever changes, turn // core.CallAndMessage:ArgPointeeInitializedness from a checker option into a // checker, as described in the CallAndMessage comments! _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
