Index: lib/StaticAnalyzer/Checkers/ValistChecker.cpp =================================================================== --- lib/StaticAnalyzer/Checkers/ValistChecker.cpp +++ lib/StaticAnalyzer/Checkers/ValistChecker.cpp @@ -54,11 +54,11 @@ void checkDeadSymbols(SymbolReaper &SR, CheckerContext &C) const; private: - const MemRegion *getVAListAsRegion(SVal SV, CheckerContext &C) const; + const MemRegion *getVAListAsRegion(SVal SV, const Expr *VAExpr, + bool &IsSymbolic, CheckerContext &C) const; StringRef getVariableNameFromRegion(const MemRegion *Reg) const; const ExplodedNode *getStartCallSite(const ExplodedNode *N, - const MemRegion *Reg, - CheckerContext &C) const; + const MemRegion *Reg) const; void reportUninitializedAccess(const MemRegion *VAList, StringRef Msg, CheckerContext &C) const; @@ -138,14 +138,21 @@ for (auto FuncInfo : VAListAccepters) { if (!Call.isCalled(FuncInfo.Func)) continue; + bool Symbolic; const MemRegion *VAList = - getVAListAsRegion(Call.getArgSVal(FuncInfo.VAListPos), C); + getVAListAsRegion(Call.getArgSVal(FuncInfo.VAListPos), + Call.getArgExpr(FuncInfo.VAListPos), Symbolic, C); if (!VAList) return; if (C.getState()->contains(VAList)) return; + // We did not see va_start call, but the source of the region is unknown. + // Be conservative and assume the best. + if (Symbolic) + return; + SmallString<80> Errmsg("Function '"); Errmsg += FuncInfo.Func.getFunctionName(); Errmsg += "' is called with an uninitialized va_list argument"; @@ -155,13 +162,45 @@ } } +const MemRegion *ValistChecker::getVAListAsRegion(SVal SV, const Expr *E, + bool &IsSymbolic, + CheckerContext &C) const { + // FIXME: on some platforms CallAndMessage checker finds some instances of + // the uninitialized va_list usages. CallAndMessage checker is disabled in + // the tests so they can verify platform independently those issues. As a + // side effect, this check is required here. + if (SV.isUnknownOrUndef()) + return nullptr; + // TODO: In the future this should be abstracted away by the analyzer. + bool VaListModelledAsArray = false; + if (const auto *Cast = dyn_cast(E)) { + QualType Ty = Cast->getType(); + VaListModelledAsArray = + Ty->isPointerType() && Ty->getPointeeType()->isRecordType(); + } + const MemRegion *Reg = SV.getAsRegion(); + if (const auto *DeclReg = Reg->getAs()) { + if (isa(DeclReg->getDecl())) + Reg = C.getState()->getSVal(SV.castAs()).getAsRegion(); + } + IsSymbolic = Reg && Reg->getAs(); + // Some VarRegion based VA lists reach here as ElementRegions. + const auto *EReg = dyn_cast_or_null(Reg); + return EReg && VaListModelledAsArray ? EReg->getSuperRegion() : Reg; +} + void ValistChecker::checkPreStmt(const VAArgExpr *VAA, CheckerContext &C) const { ProgramStateRef State = C.getState(); - SVal VAListSVal = State->getSVal(VAA->getSubExpr(), C.getLocationContext()); - const MemRegion *VAList = getVAListAsRegion(VAListSVal, C); + const Expr *VASubExpr = VAA->getSubExpr(); + SVal VAListSVal = State->getSVal(VASubExpr, C.getLocationContext()); + bool Symbolic; + const MemRegion *VAList = + getVAListAsRegion(VAListSVal, VASubExpr, Symbolic, C); if (!VAList) return; + if (Symbolic) + return; if (!State->contains(VAList)) reportUninitializedAccess( VAList, "va_arg() is called on an uninitialized va_list", C); @@ -183,22 +222,13 @@ N); } -const MemRegion *ValistChecker::getVAListAsRegion(SVal SV, - CheckerContext &C) const { - const MemRegion *Reg = SV.getAsRegion(); - const auto *TReg = dyn_cast_or_null(Reg); - // Some VarRegion based VLAs reach here as ElementRegions. - const auto *EReg = dyn_cast_or_null(TReg); - return EReg ? EReg->getSuperRegion() : TReg; -} - // This function traverses the exploded graph backwards and finds the node where // the va_list is initialized. That node is used for uniquing the bug paths. // It is not likely that there are several different va_lists that belongs to // different stack frames, so that case is not yet handled. -const ExplodedNode *ValistChecker::getStartCallSite(const ExplodedNode *N, - const MemRegion *Reg, - CheckerContext &C) const { +const ExplodedNode * +ValistChecker::getStartCallSite(const ExplodedNode *N, + const MemRegion *Reg) const { const LocationContext *LeakContext = N->getLocationContext(); const ExplodedNode *StartCallNode = N; @@ -252,7 +282,7 @@ BT_leakedvalist->setSuppressOnSink(true); } - const ExplodedNode *StartNode = getStartCallSite(N, Reg, C); + const ExplodedNode *StartNode = getStartCallSite(N, Reg); PathDiagnosticLocation LocUsedForUniqueing; if (const Stmt *StartCallStmt = PathDiagnosticLocation::getStmt(StartNode)) @@ -278,13 +308,17 @@ void ValistChecker::checkVAListStartCall(const CallEvent &Call, CheckerContext &C, bool IsCopy) const { - const MemRegion *VAList = getVAListAsRegion(Call.getArgSVal(0), C); - ProgramStateRef State = C.getState(); + bool Symbolic; + const MemRegion *VAList = + getVAListAsRegion(Call.getArgSVal(0), Call.getArgExpr(0), Symbolic, C); if (!VAList) return; + ProgramStateRef State = C.getState(); + if (IsCopy) { - const MemRegion *Arg2 = getVAListAsRegion(Call.getArgSVal(1), C); + const MemRegion *Arg2 = + getVAListAsRegion(Call.getArgSVal(1), Call.getArgExpr(1), Symbolic, C); if (Arg2) { if (ChecksEnabled[CK_CopyToSelf] && VAList == Arg2) { RegionVector LeakedVALists{VAList}; @@ -292,7 +326,7 @@ reportLeakedVALists(LeakedVALists, "va_list", " is copied onto itself", C, N, true); return; - } else if (!State->contains(Arg2)) { + } else if (!State->contains(Arg2) && !Symbolic) { if (State->contains(VAList)) { State = State->remove(VAList); RegionVector LeakedVALists{VAList}; @@ -321,10 +355,17 @@ void ValistChecker::checkVAListEndCall(const CallEvent &Call, CheckerContext &C) const { - const MemRegion *VAList = getVAListAsRegion(Call.getArgSVal(0), C); + bool Symbolic; + const MemRegion *VAList = + getVAListAsRegion(Call.getArgSVal(0), Call.getArgExpr(0), Symbolic, C); if (!VAList) return; + // We did not see va_start call, but the source of the region is unknown. + // Be conservative and assume the best. + if (Symbolic) + return; + if (!C.getState()->contains(VAList)) { reportUninitializedAccess( VAList, "va_end() is called on an uninitialized va_list", C); Index: test/Analysis/valist-uninitialized-no-undef.c =================================================================== --- /dev/null +++ test/Analysis/valist-uninitialized-no-undef.c @@ -0,0 +1,34 @@ +// RUN: %clang_cc1 -triple x86_64-pc-linux-gnu -analyze -analyzer-checker=core,alpha.valist.Uninitialized,alpha.valist.CopyToSelf -analyzer-output=text -analyzer-store=region -verify %s + +#include "Inputs/system-header-simulator-for-valist.h" + +// This is the same function as the previous one, but it is called in call_inlined_uses_arg(), +// and the warning is generated during the analysis of call_inlined_uses_arg(). +void inlined_uses_arg(va_list arg) { + (void)va_arg(arg, int); //expected-warning{{va_arg() is called on an uninitialized va_list}} expected-note{{va_arg() is called on an uninitialized va_list}} +} + +void call_inlined_uses_arg(int fst, ...) { + va_list va; + inlined_uses_arg(va); // expected-note{{Calling 'inlined_uses_arg'}} +} + +void f6(va_list *fst, ...) { + va_start(*fst, fst); + // FIXME: There should be no warning for this. + (void)va_arg(*fst, int); // expected-warning{{va_arg() is called on an uninitialized va_list}} expected-note{{va_arg() is called on an uninitialized va_list}} + va_end(*fst); +} + +void call_vprintf_bad(int isstring, ...) { + va_list va; + vprintf(isstring ? "%s" : "%d", va); //expected-warning{{Function 'vprintf' is called with an uninitialized va_list argument}} expected-note{{Function 'vprintf' is called with an uninitialized va_list argument}} expected-note{{Assuming 'isstring' is 0}} expected-note{{'?' condition is false}} +} + +void call_vsprintf_bad(char *buffer, ...) { + va_list va; + va_start(va, buffer); // expected-note{{Initialized va_list}} + va_end(va); // expected-note{{Ended va_list}} + vsprintf(buffer, "%s %d %d %lf %03d", va); //expected-warning{{Function 'vsprintf' is called with an uninitialized va_list argument}} expected-note{{Function 'vsprintf' is called with an uninitialized va_list argument}} +} + Index: test/Analysis/valist-uninitialized.c =================================================================== --- test/Analysis/valist-uninitialized.c +++ test/Analysis/valist-uninitialized.c @@ -1,4 +1,5 @@ -// RUN: %clang_cc1 -triple x86_64-pc-linux-gnu -analyze -analyzer-checker=core,alpha.valist.Uninitialized,alpha.valist.CopyToSelf -analyzer-output=text -analyzer-store=region -verify %s +// RUN: %clang_cc1 -triple hexagon-unknown-linux -analyze -analyzer-checker=core,alpha.valist.Uninitialized,alpha.valist.CopyToSelf -analyzer-disable-checker=core.CallAndMessage -analyzer-output=text -analyzer-store=region -verify %s +// RUN: %clang_cc1 -triple x86_64-pc-linux-gnu -analyze -analyzer-checker=core,alpha.valist.Uninitialized,alpha.valist.CopyToSelf -analyzer-disable-checker=core.CallAndMessage -analyzer-output=text -analyzer-store=region -verify %s #include "Inputs/system-header-simulator-for-valist.h" @@ -38,13 +39,6 @@ va_end(fst); } // no-warning -//FIXME: this should not cause a warning -void f6(va_list *fst, ...) { - va_start(*fst, fst); - (void)va_arg(*fst, int); //expected-warning{{va_arg() is called on an uninitialized va_list}} expected-note{{va_arg() is called on an uninitialized va_list}} - va_end(*fst); -} - void f7(int *fst, ...) { va_list x; va_list *y = &x; @@ -141,17 +135,6 @@ (void)va_arg(arg, int); } //no-warning -// This is the same function as the previous one, but it is called in call_uses_arg2(), -// and the warning is generated during the analysis of call_uses_arg2(). -void inlined_uses_arg(va_list arg) { - (void)va_arg(arg, int); //expected-warning{{va_arg() is called on an uninitialized va_list}} expected-note{{va_arg() is called on an uninitialized va_list}} -} - -void call_inlined_uses_arg(int fst, ...) { - va_list va; - inlined_uses_arg(va); // expected-note{{Calling 'inlined_uses_arg'}} -} - void call_vprintf_ok(int isstring, ...) { va_list va; va_start(va, isstring); @@ -159,20 +142,24 @@ va_end(va); } //no-warning -void call_vprintf_bad(int isstring, ...) { +void call_some_other_func(int n, ...) { va_list va; - vprintf(isstring ? "%s" : "%d", va); //expected-warning{{Function 'vprintf' is called with an uninitialized va_list argument}} expected-note{{Function 'vprintf' is called with an uninitialized va_list argument}} expected-note{{Assuming 'isstring' is 0}} expected-note{{'?' condition is false}} -} + some_library_function(n, va); +} //no-warning -void call_vsprintf_bad(char *buffer, ...) { - va_list va; - va_start(va, buffer); // expected-note{{Initialized va_list}} - va_end(va); // expected-note{{Ended va_list}} - vsprintf(buffer, "%s %d %d %lf %03d", va); //expected-warning{{Function 'vsprintf' is called with an uninitialized va_list argument}} expected-note{{Function 'vsprintf' is called with an uninitialized va_list argument}} +void inlined_uses_arg_good(va_list arg) { + (void)va_arg(arg, int); } -void call_some_other_func(int n, ...) { +void call_inlined_uses_arg_good(int fst, ...) { va_list va; - some_library_function(n, va); -} //no-warning + va_start(va, fst); + inlined_uses_arg_good(va); + va_end(va); +} +void va_copy_test(va_list arg) { + va_list dst; + va_copy(dst, arg); + va_end(dst); +} Index: test/Analysis/valist-unterminated.c =================================================================== --- test/Analysis/valist-unterminated.c +++ test/Analysis/valist-unterminated.c @@ -1,3 +1,4 @@ +// RUN: %clang_cc1 -triple hexagon-unknown-linux -analyze -analyzer-checker=core,alpha.valist.Unterminated,alpha.valist.CopyToSelf -analyzer-output=text -analyzer-store=region -verify %s // RUN: %clang_cc1 -triple x86_64-pc-linux-gnu -analyze -analyzer-checker=core,alpha.valist.Unterminated,alpha.valist.CopyToSelf -analyzer-output=text -analyzer-store=region -verify %s #include "Inputs/system-header-simulator-for-valist.h" @@ -28,9 +29,8 @@ } void f5(va_list fst, ...) { - va_start(fst, fst); - //FIXME: this should cause a warning -} // no-warning + va_start(fst, fst); // expected-note{{Initialized va_list}} +} // expected-warning{{Initialized va_list}} expected-note{{Initialized va_list}} void f6(va_list *fst, ...) { va_start(*fst, fst); // expected-note{{Initialized va_list}}