xazax.hun created this revision.
This patch makes the valist check more robust to the different kind of ASTs
that are generated on different platforms:
Generated on x86_64-pc-linux-gnu:
|-TypedefDecl 0x1d07a40 <<invalid sloc>> <invalid sloc> implicit
__builtin_ms_va_list 'char *'
| `-PointerType 0x1d07a00 'char *'
| `-BuiltinType 0x1d071b0 'char'
|-TypedefDecl 0x1d07d08 <<invalid sloc>> <invalid sloc> implicit referenced
__builtin_va_list 'struct __va_list_tag [1]'
| `-ConstantArrayType 0x1d07cb0 'struct __va_list_tag [1]' 1
| `-RecordType 0x1d07b20 'struct __va_list_tag'
| `-Record 0x1d07a90 '__va_list_tag'
Generated on hexagon-unknown-linux:
-TypedefDecl 0x6c47e0 <<invalid sloc>> <invalid sloc> implicit referenced
__builtin_va_list 'char *'
| `-PointerType 0x6c47a0 'char *'
| `-BuiltinType 0x6c4020 'char'
|-TypedefDecl 0x6c4860
</home/xazax/C++Modelling/llvm/tools/clang/test/Analysis/Inputs/system-header-simulator-for-valist.h:13:1,
col:27> col:27 referenced va_list '__builtin_va_list':'char *'
| `-TypedefType 0x6c4830 '__builtin_va_list' sugar
| |-Typedef 0x6c47e0 '__builtin_va_list'
| `-PointerType 0x6c47a0 'char *'
| `-BuiltinType 0x6c4020 'char'
This patch also manages to fix one of the FIXMEs in the tests.
Note that, some tests are only there for x86_64-pc-linux-gnu. The reason is
that, the analyzer manages to notice the uninitializedness of va_list on
hexagon-unknown-linux, so it generates a sink node before this check could be
triggered.
https://reviews.llvm.org/D30157
Files:
lib/StaticAnalyzer/Checkers/ValistChecker.cpp
test/Analysis/valist-uninitialized-no-undef.c
test/Analysis/valist-uninitialized.c
test/Analysis/valist-unterminated.c
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}}
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,36 +135,13 @@
(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);
vprintf(isstring ? "%s" : "%d", va);
va_end(va);
} //no-warning
-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}}
-}
-
void call_some_other_func(int n, ...) {
va_list va;
some_library_function(n, va);
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: 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<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";
@@ -155,13 +162,42 @@
}
}
+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.isUndef())
+ 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))
+ VaListModelledAsArray = Cast->getCastKind() == CK_ArrayToPointerDecay;
+ 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);
@@ -183,22 +219,13 @@
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;
@@ -252,7 +279,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 +305,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};
@@ -321,10 +352,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<InitializedVALists>(VAList)) {
reportUninitializedAccess(
VAList, "va_end() is called on an uninitialized va_list", C);
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
