A good start! Many spot comments.
================
Comment at: lib/StaticAnalyzer/Core/ExprEngineCXX.cpp:150-163
@@ -149,2 +149,16 @@
}
+ // Or a constructor for a temporary variable bound to a static
+ // reference?
+ } else if (isa<ImplicitCastExpr>(StmtElem->getStmt()) &&
+ currStmtIdx + 2 < B->size()) {
+ CFGElement NNext = (*B)[currStmtIdx+2];
+ if (Optional<CFGStmt> NStmtElem = NNext.getAs<CFGStmt>()) {
+ if (const MaterializeTemporaryExpr *MT =
+ dyn_cast<MaterializeTemporaryExpr>(NStmtElem->getStmt())) {
+ if (MT->getStorageDuration() == SD_Static) {
+ MemRegionManager &MRMgr = getSValBuilder().getRegionManager();
+ Target = MRMgr.getCXXExtendedTempObjectRegion(CE);
+ }
+ }
+ }
}
----------------
The `currStmtIdx + 2` seems really brittle. We use that for local variables
because we synthesize DeclStmts that don't appear in the parent map, and
because at one point I was concerned about the overhead of ParentMap lookup.
In this case, though, there may be an arbitrary number of layers between the
CXXTemporaryObjectExpr and the enclosing MaterializeTemporaryExpr.
For now, it might be easier to just tweak CreateCXXTemporaryObject to not reuse
the temp region if the storage duration is different.
================
Comment at: lib/StaticAnalyzer/Core/MemRegion.cpp:868
@@ +867,3 @@
+const CXXTempObjectRegion *
+MemRegionManager::getCXXExtendedTempObjectRegion(Expr const *Ex) {
+ return getSubRegion<CXXTempObjectRegion>(
----------------
Not sure about this name. How about `getStaticTempObjectRegion`?
Also, `const Expr *Ex`, please. I don't know why the other one has it reversed.
(I know they're both correct, but const-first is by far the convention in
Clang.)
================
Comment at: lib/StaticAnalyzer/Core/ExprEngine.cpp:214
@@ +213,3 @@
+ dyn_cast<MaterializeTemporaryExpr>(Result)) {
+ if (MT->getStorageDuration() == SD_Static)
+ TR = MRMgr.getCXXExtendedTempObjectRegion(Inner);
----------------
What about SD_Thread? The analyzer currently assumes non-parallel execution, so
SD_Thread could behave like SD_Static for now.
================
Comment at: test/Analysis/stack-addr-ps.cpp:23-25
@@ -22,1 +22,5 @@
+void g4() {
+ static const int &x = 3; // no warning
+}
+
----------------
Please add several more tests, and actually test that we inline the constructor
and store the right values into the temporary reference. In this case there
isn't a constructor at all, so you're only testing one of your modifications.
http://llvm-reviews.chandlerc.com/D1133
BRANCH
master
ARCANIST PROJECT
clang
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
