Skip to content

Commit

Permalink
[analyzer] Improve valist checks and move it out from alpha state.
Browse files Browse the repository at this point in the history 
This patch makes the valist check more robust to the different AST variants on
different platforms and also fixes a FIXME.

Differential Revision: https://reviews.llvm.org/D30157

llvm-svn: 297153
  • Loading branch information
@Xazax-hun
Xazax-hun committed Mar 7, 2017
1 parent 8bb4087 commit dbf2790
Show file tree
Hide file tree
Showing 5 changed files with 202 additions and 100 deletions.
5 changes: 2 additions & 3 deletions clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
View file
Expand Up @@ -45,7 +45,6 @@ def CplusplusAlpha : Package<"cplusplus">, InPackage<Alpha>, Hidden;
def CplusplusOptIn : Package<"cplusplus">, InPackage<OptIn>;

def Valist : Package<"valist">;
def ValistAlpha : Package<"valist">, InPackage<Alpha>, Hidden;

def DeadCode : Package<"deadcode">;
def DeadCodeAlpha : Package<"deadcode">, InPackage<Alpha>, Hidden;
Expand Down Expand Up @@ -291,7 +290,7 @@ def IteratorPastEndChecker : Checker<"IteratorPastEnd">,
// Valist checkers.
//===----------------------------------------------------------------------===//

let ParentPackage = ValistAlpha in {
let ParentPackage = Valist in {

def UninitializedChecker : Checker<"Uninitialized">,
HelpText<"Check for usages of uninitialized (or already released) va_lists.">,
Expand All @@ -305,7 +304,7 @@ def CopyToSelfChecker : Checker<"CopyToSelf">,
HelpText<"Check for va_lists which are copied onto itself.">,
DescFile<"ValistChecker.cpp">;

} // end : "alpha.valist"
} // end : "valist"

//===----------------------------------------------------------------------===//
// Deadcode checkers.
Expand Down
89 changes: 65 additions & 24 deletions clang/lib/StaticAnalyzer/Checkers/ValistChecker.cpp
View file
Expand Up @@ -54,11 +54,11 @@ class ValistChecker : public Checker<check::PreCall, check::PreStmt<VAArgExpr>,
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;
Expand Down Expand Up @@ -138,14 +138,21 @@ void ValistChecker::checkPreCall(const CallEvent &Call,
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<InitializedVALists>(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";
Expand All @@ -155,13 +162,45 @@ void ValistChecker::checkPreCall(const CallEvent &Call,
}
}

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<CastExpr>(E)) {
QualType Ty = Cast->getType();
VaListModelledAsArray =
Ty->isPointerType() && Ty->getPointeeType()->isRecordType();
}
const MemRegion *Reg = SV.getAsRegion();
if (const auto *DeclReg = Reg->getAs<DeclRegion>()) {
if (isa<ParmVarDecl>(DeclReg->getDecl()))
Reg = C.getState()->getSVal(SV.castAs<Loc>()).getAsRegion();
}
IsSymbolic = Reg && Reg->getAs<SymbolicRegion>();
// Some VarRegion based VA lists reach here as ElementRegions.
const auto *EReg = dyn_cast_or_null<ElementRegion>(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<InitializedVALists>(VAList))
reportUninitializedAccess(
VAList, "va_arg() is called on an uninitialized va_list", C);
Expand All @@ -183,22 +222,13 @@ void ValistChecker::checkDeadSymbols(SymbolReaper &SR,
N);
}

const MemRegion *ValistChecker::getVAListAsRegion(SVal SV,
CheckerContext &C) const {
const MemRegion *Reg = SV.getAsRegion();
const auto *TReg = dyn_cast_or_null<TypedValueRegion>(Reg);
// Some VarRegion based VLAs reach here as ElementRegions.
const auto *EReg = dyn_cast_or_null<ElementRegion>(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;

Expand Down Expand Up @@ -252,7 +282,7 @@ void ValistChecker::reportLeakedVALists(const RegionVector &LeakedVALists,
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))
Expand All @@ -278,21 +308,25 @@ void ValistChecker::reportLeakedVALists(const RegionVector &LeakedVALists,

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};
if (ExplodedNode *N = C.addTransition(State))
reportLeakedVALists(LeakedVALists, "va_list",
" is copied onto itself", C, N, true);
return;
} else if (!State->contains<InitializedVALists>(Arg2)) {
} else if (!State->contains<InitializedVALists>(Arg2) && !Symbolic) {
if (State->contains<InitializedVALists>(VAList)) {
State = State->remove<InitializedVALists>(VAList);
RegionVector LeakedVALists{VAList};
Expand Down Expand Up @@ -321,10 +355,17 @@ void ValistChecker::checkVAListStartCall(const CallEvent &Call,

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<InitializedVALists>(VAList)) {
reportUninitializedAccess(
VAList, "va_end() is called on an uninitialized va_list", C);
Expand Down
40 changes: 40 additions & 0 deletions clang/test/Analysis/valist-uninitialized-no-undef.c
View file
@@ -0,0 +1,40 @@
// RUN: %clang_cc1 -triple x86_64-pc-linux-gnu -analyze -analyzer-checker=core,valist.Uninitialized,valist.CopyToSelf -analyzer-output=text -analyzer-store=region -verify %s

#include "Inputs/system-header-simulator-for-valist.h"

// This 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@-1{{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@-1{{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@-1{{Function 'vprintf' is called with an uninitialized va_list argument}}
// expected-note@-2{{Assuming 'isstring' is 0}}
// expected-note@-3{{'?' 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@-1{{Function 'vsprintf' is called with an uninitialized va_list argument}}
}

0 comments on commit dbf2790

Please sign in to comment.