Merge remote-tracking branch 'origin/swift-4.0-branch' into stable
diff --git a/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h b/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h
index fa7769b..1a20074 100644
--- a/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h
+++ b/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h
@@ -621,16 +621,16 @@
void performTrivialCopy(NodeBuilder &Bldr, ExplodedNode *Pred,
const CallEvent &Call);
- /// If the value of the given expression is a NonLoc, copy it into a new
- /// temporary object region, and replace the value of the expression with
- /// that.
+ /// If the value of the given expression \p InitWithAdjustments is a NonLoc,
+ /// copy it into a new temporary object region, and replace the value of the
+ /// expression with that.
///
- /// If \p ResultE is provided, the new region will be bound to this expression
- /// instead of \p E.
+ /// If \p Result is provided, the new region will be bound to this expression
+ /// instead of \p InitWithAdjustments.
ProgramStateRef createTemporaryRegionIfNeeded(ProgramStateRef State,
const LocationContext *LC,
- const Expr *E,
- const Expr *ResultE = nullptr);
+ const Expr *InitWithAdjustments,
+ const Expr *Result = nullptr);
/// For a DeclStmt or CXXInitCtorInitializer, walk backward in the current CFG
/// block to find the constructor expression that directly constructed into
diff --git a/lib/StaticAnalyzer/Checkers/AnalysisOrderChecker.cpp b/lib/StaticAnalyzer/Checkers/AnalysisOrderChecker.cpp
index e6592a2..90d5c0e 100644
--- a/lib/StaticAnalyzer/Checkers/AnalysisOrderChecker.cpp
+++ b/lib/StaticAnalyzer/Checkers/AnalysisOrderChecker.cpp
@@ -24,16 +24,29 @@
namespace {
-class AnalysisOrderChecker : public Checker< check::PreStmt<CastExpr>,
- check::PostStmt<CastExpr>,
- check::PreStmt<ArraySubscriptExpr>,
- check::PostStmt<ArraySubscriptExpr>> {
- bool isCallbackEnabled(CheckerContext &C, StringRef CallbackName) const {
- AnalyzerOptions &Opts = C.getAnalysisManager().getAnalyzerOptions();
+class AnalysisOrderChecker
+ : public Checker<check::PreStmt<CastExpr>,
+ check::PostStmt<CastExpr>,
+ check::PreStmt<ArraySubscriptExpr>,
+ check::PostStmt<ArraySubscriptExpr>,
+ check::Bind,
+ check::RegionChanges> {
+ bool isCallbackEnabled(AnalyzerOptions &Opts, StringRef CallbackName) const {
return Opts.getBooleanOption("*", false, this) ||
Opts.getBooleanOption(CallbackName, false, this);
}
+ bool isCallbackEnabled(CheckerContext &C, StringRef CallbackName) const {
+ AnalyzerOptions &Opts = C.getAnalysisManager().getAnalyzerOptions();
+ return isCallbackEnabled(Opts, CallbackName);
+ }
+
+ bool isCallbackEnabled(ProgramStateRef State, StringRef CallbackName) const {
+ AnalyzerOptions &Opts = State->getStateManager().getOwningEngine()
+ ->getAnalysisManager().getAnalyzerOptions();
+ return isCallbackEnabled(Opts, CallbackName);
+ }
+
public:
void checkPreStmt(const CastExpr *CE, CheckerContext &C) const {
if (isCallbackEnabled(C, "PreStmtCastExpr"))
@@ -47,17 +60,35 @@
<< ")\n";
}
- void checkPreStmt(const ArraySubscriptExpr *SubExpr, CheckerContext &C) const {
+ void checkPreStmt(const ArraySubscriptExpr *SubExpr,
+ CheckerContext &C) const {
if (isCallbackEnabled(C, "PreStmtArraySubscriptExpr"))
llvm::errs() << "PreStmt<ArraySubscriptExpr>\n";
}
- void checkPostStmt(const ArraySubscriptExpr *SubExpr, CheckerContext &C) const {
+ void checkPostStmt(const ArraySubscriptExpr *SubExpr,
+ CheckerContext &C) const {
if (isCallbackEnabled(C, "PostStmtArraySubscriptExpr"))
llvm::errs() << "PostStmt<ArraySubscriptExpr>\n";
}
+
+ void checkBind(SVal Loc, SVal Val, const Stmt *S, CheckerContext &C) const {
+ if (isCallbackEnabled(C, "Bind"))
+ llvm::errs() << "Bind\n";
+ }
+
+ ProgramStateRef
+ checkRegionChanges(ProgramStateRef State,
+ const InvalidatedSymbols *Invalidated,
+ ArrayRef<const MemRegion *> ExplicitRegions,
+ ArrayRef<const MemRegion *> Regions,
+ const LocationContext *LCtx, const CallEvent *Call) const {
+ if (isCallbackEnabled(State, "RegionChanges"))
+ llvm::errs() << "RegionChanges\n";
+ return State;
+ }
};
-}
+} // end anonymous namespace
//===----------------------------------------------------------------------===//
// Registration.
diff --git a/lib/StaticAnalyzer/Core/AnalyzerOptions.cpp b/lib/StaticAnalyzer/Core/AnalyzerOptions.cpp
index 1542263..45ef612 100644
--- a/lib/StaticAnalyzer/Core/AnalyzerOptions.cpp
+++ b/lib/StaticAnalyzer/Core/AnalyzerOptions.cpp
@@ -230,7 +230,7 @@
bool AnalyzerOptions::shouldSuppressFromCXXStandardLibrary() {
return getBooleanOption(SuppressFromCXXStandardLibrary,
"suppress-c++-stdlib",
- /* Default = */ false);
+ /* Default = */ true);
}
bool AnalyzerOptions::shouldReportIssuesInMainSourceFile() {
diff --git a/lib/StaticAnalyzer/Core/ExprEngine.cpp b/lib/StaticAnalyzer/Core/ExprEngine.cpp
index 3509928..cb9c1a2 100644
--- a/lib/StaticAnalyzer/Core/ExprEngine.cpp
+++ b/lib/StaticAnalyzer/Core/ExprEngine.cpp
@@ -182,19 +182,25 @@
ProgramStateRef
ExprEngine::createTemporaryRegionIfNeeded(ProgramStateRef State,
const LocationContext *LC,
- const Expr *Ex,
+ const Expr *InitWithAdjustments,
const Expr *Result) {
- SVal V = State->getSVal(Ex, LC);
+ // FIXME: This function is a hack that works around the quirky AST
+ // we're often having with respect to C++ temporaries. If only we modelled
+ // the actual execution order of statements properly in the CFG,
+ // all the hassle with adjustments would not be necessary,
+ // and perhaps the whole function would be removed.
+ SVal InitValWithAdjustments = State->getSVal(InitWithAdjustments, LC);
if (!Result) {
// If we don't have an explicit result expression, we're in "if needed"
// mode. Only create a region if the current value is a NonLoc.
- if (!V.getAs<NonLoc>())
+ if (!InitValWithAdjustments.getAs<NonLoc>())
return State;
- Result = Ex;
+ Result = InitWithAdjustments;
} else {
// We need to create a region no matter what. For sanity, make sure we don't
// try to stuff a Loc into a non-pointer temporary region.
- assert(!V.getAs<Loc>() || Loc::isLocType(Result->getType()) ||
+ assert(!InitValWithAdjustments.getAs<Loc>() ||
+ Loc::isLocType(Result->getType()) ||
Result->getType()->isMemberPointerType());
}
@@ -226,7 +232,8 @@
SmallVector<const Expr *, 2> CommaLHSs;
SmallVector<SubobjectAdjustment, 2> Adjustments;
- const Expr *Init = Ex->skipRValueSubobjectAdjustments(CommaLHSs, Adjustments);
+ const Expr *Init = InitWithAdjustments->skipRValueSubobjectAdjustments(
+ CommaLHSs, Adjustments);
const TypedValueRegion *TR = nullptr;
if (const MaterializeTemporaryExpr *MT =
@@ -241,6 +248,7 @@
TR = MRMgr.getCXXTempObjectRegion(Init, LC);
SVal Reg = loc::MemRegionVal(TR);
+ SVal BaseReg = Reg;
// Make the necessary adjustments to obtain the sub-object.
for (auto I = Adjustments.rbegin(), E = Adjustments.rend(); I != E; ++I) {
@@ -254,19 +262,47 @@
break;
case SubobjectAdjustment::MemberPointerAdjustment:
// FIXME: Unimplemented.
- State->bindDefault(Reg, UnknownVal(), LC);
+ State = State->bindDefault(Reg, UnknownVal(), LC);
return State;
}
}
- // Try to recover some path sensitivity in case we couldn't compute the value.
- if (V.isUnknown())
- V = getSValBuilder().conjureSymbolVal(Result, LC, TR->getValueType(),
- currBldrCtx->blockCount());
- // Bind the value of the expression to the sub-object region, and then bind
- // the sub-object region to our expression.
- State = State->bindLoc(Reg, V, LC);
+ // What remains is to copy the value of the object to the new region.
+ // FIXME: In other words, what we should always do is copy value of the
+ // Init expression (which corresponds to the bigger object) to the whole
+ // temporary region TR. However, this value is often no longer present
+ // in the Environment. If it has disappeared, we instead invalidate TR.
+ // Still, what we can do is assign the value of expression Ex (which
+ // corresponds to the sub-object) to the TR's sub-region Reg. At least,
+ // values inside Reg would be correct.
+ SVal InitVal = State->getSVal(Init, LC);
+ if (InitVal.isUnknown()) {
+ InitVal = getSValBuilder().conjureSymbolVal(Result, LC, Init->getType(),
+ currBldrCtx->blockCount());
+ State = State->bindLoc(BaseReg.castAs<Loc>(), InitVal, LC, false);
+
+ // Then we'd need to take the value that certainly exists and bind it over.
+ if (InitValWithAdjustments.isUnknown()) {
+ // Try to recover some path sensitivity in case we couldn't
+ // compute the value.
+ InitValWithAdjustments = getSValBuilder().conjureSymbolVal(
+ Result, LC, InitWithAdjustments->getType(),
+ currBldrCtx->blockCount());
+ }
+ State =
+ State->bindLoc(Reg.castAs<Loc>(), InitValWithAdjustments, LC, false);
+ } else {
+ State = State->bindLoc(BaseReg.castAs<Loc>(), InitVal, LC, false);
+ }
+
+ // The result expression would now point to the correct sub-region of the
+ // newly created temporary region. Do this last in order to getSVal of Init
+ // correctly in case (Result == Init).
State = State->BindExpr(Result, LC, Reg);
+
+ // Notify checkers once for two bindLoc()s.
+ State = processRegionChange(State, TR, LC);
+
return State;
}
diff --git a/lib/StaticAnalyzer/Core/ExprEngineC.cpp b/lib/StaticAnalyzer/Core/ExprEngineC.cpp
index ea4ff91..8f720a2 100644
--- a/lib/StaticAnalyzer/Core/ExprEngineC.cpp
+++ b/lib/StaticAnalyzer/Core/ExprEngineC.cpp
@@ -1054,7 +1054,7 @@
// Conjure a new symbol if necessary to recover precision.
if (Result.isUnknown()){
DefinedOrUnknownSVal SymVal =
- svalBuilder.conjureSymbolVal(nullptr, Ex, LCtx,
+ svalBuilder.conjureSymbolVal(nullptr, U, LCtx,
currBldrCtx->blockCount());
Result = SymVal;
diff --git a/lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp b/lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp
index 28b43dd..3598416 100644
--- a/lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp
+++ b/lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp
@@ -362,6 +362,9 @@
resultTy);
case nonloc::ConcreteIntKind: {
// Transform the integer into a location and compare.
+ // FIXME: This only makes sense for comparisons. If we want to, say,
+ // add 1 to a LocAsInteger, we'd better unpack the Loc and add to it,
+ // then pack it back into a LocAsInteger.
llvm::APSInt i = rhs.castAs<nonloc::ConcreteInt>().getValue();
BasicVals.getAPSIntType(Context.VoidPtrTy).apply(i);
return evalBinOpLL(state, op, lhsL, makeLoc(i), resultTy);
@@ -942,6 +945,8 @@
rhs = convertToArrayIndex(rhs).castAs<NonLoc>();
SVal index = UnknownVal();
const MemRegion *superR = nullptr;
+ // We need to know the type of the pointer in order to add an integer to it.
+ // Depending on the type, different amount of bytes is added.
QualType elementType;
if (const ElementRegion *elemReg = dyn_cast<ElementRegion>(region)) {
@@ -955,6 +960,10 @@
assert(op == BO_Add || op == BO_Sub);
index = (op == BO_Add) ? rhs : evalMinus(rhs);
superR = region;
+ // TODO: Is this actually reliable? Maybe improving our MemRegion
+ // hierarchy to provide typed regions for all non-void pointers would be
+ // better. For instance, we cannot extend this towards LocAsInteger
+ // operations, where result type of the expression is integer.
if (resultTy->isAnyPointerType())
elementType = resultTy->getPointeeType();
}
diff --git a/test/Analysis/casts.c b/test/Analysis/casts.c
index b5e6e27..37e0da7 100644
--- a/test/Analysis/casts.c
+++ b/test/Analysis/casts.c
@@ -118,3 +118,8 @@
extern float globalFloat;
clang_analyzer_eval(globalFloat); // expected-warning{{UNKNOWN}}
}
+
+void locAsIntegerCasts(void *p) {
+ int x = (int) p;
+ clang_analyzer_eval(++x < 10); // no-crash // expected-warning{{UNKNOWN}}
+}
diff --git a/test/Analysis/diagnostics/explicit-suppression.cpp b/test/Analysis/diagnostics/explicit-suppression.cpp
index d36def2..c1cd948 100644
--- a/test/Analysis/diagnostics/explicit-suppression.cpp
+++ b/test/Analysis/diagnostics/explicit-suppression.cpp
@@ -1,5 +1,6 @@
// RUN: %clang_cc1 -analyze -analyzer-checker=core,debug.ExprInspection -analyzer-config suppress-c++-stdlib=false -verify %s
// RUN: %clang_cc1 -analyze -analyzer-checker=core,debug.ExprInspection -analyzer-config suppress-c++-stdlib=true -DSUPPRESSED=1 -verify %s
+// RUN: %clang_cc1 -analyze -analyzer-checker=core,debug.ExprInspection -DSUPPRESSED=1 -verify %s
#ifdef SUPPRESSED
// expected-no-diagnostics
diff --git a/test/Analysis/temporaries-callback-order.cpp b/test/Analysis/temporaries-callback-order.cpp
new file mode 100644
index 0000000..df916cc
--- /dev/null
+++ b/test/Analysis/temporaries-callback-order.cpp
@@ -0,0 +1,36 @@
+// RUN: %clang_cc1 -analyze -analyzer-checker=debug.AnalysisOrder -analyzer-config debug.AnalysisOrder:Bind=true -analyzer-config debug.AnalysisOrder:RegionChanges=true %s 2>&1 | FileCheck %s
+
+struct Super {
+ virtual void m();
+};
+struct Sub : Super {
+ virtual void m() {}
+};
+
+void testTemporaries() {
+ // This triggers RegionChanges twice:
+ // - Once for zero-initialization of the structure.
+ // - Once for creating a temporary region and copying the structure there.
+ // FIXME: This code shouldn't really produce the extra temporary, however
+ // that's how we behave for now.
+ Sub().m();
+}
+
+void seeIfCheckBindWorks() {
+ // This should trigger checkBind. The rest of the code shouldn't.
+ // This also triggers checkRegionChanges after that.
+ // Note that this function is analyzed first, so the messages would be on top.
+ int x = 1;
+}
+
+// seeIfCheckBindWorks():
+// CHECK: Bind
+// CHECK-NEXT: RegionChanges
+
+// testTemporaries():
+// CHECK-NEXT: RegionChanges
+// CHECK-NEXT: RegionChanges
+
+// Make sure there's no further output.
+// CHECK-NOT: Bind
+// CHECK-NOT: RegionChanges
diff --git a/test/Analysis/temporaries.cpp b/test/Analysis/temporaries.cpp
index cc39201..d19efac 100644
--- a/test/Analysis/temporaries.cpp
+++ b/test/Analysis/temporaries.cpp
@@ -503,3 +503,30 @@
});
}
}
+
+namespace CopyToTemporaryCorrectly {
+class Super {
+public:
+ void m() {
+ mImpl();
+ }
+ virtual void mImpl() = 0;
+};
+class Sub : public Super {
+public:
+ Sub(const int &p) : j(p) {}
+ virtual void mImpl() override {
+ // Used to be undefined pointer dereference because we didn't copy
+ // the subclass data (j) to the temporary object properly.
+ (void)(j + 1); // no-warning
+ if (j != 22) {
+ clang_analyzer_warnIfReached(); // no-warning
+ }
+ }
+ const int &j;
+};
+void run() {
+ int i = 22;
+ Sub(i).m();
+}
+}
