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; diff --git a/clang/lib/StaticAnalyzer/Checkers/CallAndMessageChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/CallAndMessageChecker.cpp index e71fe47bb8792..606db37318732 100644 --- a/clang/lib/StaticAnalyzer/Checkers/CallAndMessageChecker.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/CallAndMessageChecker.cpp @@ -21,6 +21,7 @@ #include "clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h" #include "clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h" #include "llvm/ADT/StringExtras.h" +#include "llvm/Support/FormatVariadic.h" #include "llvm/Support/raw_ostream.h" using namespace clang; @@ -77,6 +78,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 +184,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(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 FieldChain; + using FieldChainTy = SmallVector; + 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 +214,120 @@ 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 { + 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: '"; + bool First = true; + for (const FieldDecl *FD : V) { + if (First) + First = false; + else + Stream << '.'; + 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(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()) { + 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(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 +370,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(BT, os.str(), N); + auto R = std::make_unique(BT, Msg, N); R->addRange(ArgRange); if (ArgEx) @@ -689,6 +731,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!