REGISTER_TRAIT_WITH_PROGRAMSTATE(InitializedTemporaries,
InitializedTemporariesMap)
+typedef llvm::ImmutableMap<std::pair<const MaterializeTemporaryExpr *,
+ const StackFrameContext *>,
+ const CXXTempObjectRegion *>
+ TemporaryMaterializationMap;
+
+// Keeps track of temporaries that will need to be materialized later.
+// The StackFrameContext assures that nested calls due to inlined recursive
+// functions do not interfere.
+REGISTER_TRAIT_WITH_PROGRAMSTATE(TemporaryMaterializations,
+ TemporaryMaterializationMap)
+
typedef llvm::ImmutableMap<std::pair<const CXXNewExpr *,
const LocationContext *>,
SVal>
const Expr *Init = InitWithAdjustments->skipRValueSubobjectAdjustments(
CommaLHSs, Adjustments);
+ // Take the region for Init, i.e. for the whole object. If we do not remember
+ // the region in which the object originally was constructed, come up with
+ // a new temporary region out of thin air and copy the contents of the object
+ // (which are currently present in the Environment, because Init is an rvalue)
+ // into that region. This is not correct, but it is better than nothing.
+ bool FoundOriginalMaterializationRegion = false;
const TypedValueRegion *TR = nullptr;
if (const MaterializeTemporaryExpr *MT =
dyn_cast<MaterializeTemporaryExpr>(Result)) {
- StorageDuration SD = MT->getStorageDuration();
- // If this object is bound to a reference with static storage duration, we
- // put it in a different region to prevent "address leakage" warnings.
- if (SD == SD_Static || SD == SD_Thread)
- TR = MRMgr.getCXXStaticTempObjectRegion(Init);
- }
- if (!TR)
+ auto Key = std::make_pair(MT, LC->getCurrentStackFrame());
+ if (const CXXTempObjectRegion *const *TRPtr =
+ State->get<TemporaryMaterializations>(Key)) {
+ FoundOriginalMaterializationRegion = true;
+ TR = *TRPtr;
+ assert(TR);
+ State = State->remove<TemporaryMaterializations>(Key);
+ } else {
+ StorageDuration SD = MT->getStorageDuration();
+ // If this object is bound to a reference with static storage duration, we
+ // put it in a different region to prevent "address leakage" warnings.
+ if (SD == SD_Static || SD == SD_Thread) {
+ TR = MRMgr.getCXXStaticTempObjectRegion(Init);
+ } else {
+ TR = MRMgr.getCXXTempObjectRegion(Init, LC);
+ }
+ }
+ } else {
TR = MRMgr.getCXXTempObjectRegion(Init, LC);
+ }
SVal Reg = loc::MemRegionVal(TR);
SVal BaseReg = Reg;
}
}
- // 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());
+ if (!FoundOriginalMaterializationRegion) {
+ // 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);
}
- 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
// correctly in case (Result == Init).
State = State->BindExpr(Result, LC, Reg);
- // Notify checkers once for two bindLoc()s.
- State = processRegionChange(State, TR, LC);
+ if (!FoundOriginalMaterializationRegion) {
+ // Notify checkers once for two bindLoc()s.
+ State = processRegionChange(State, TR, LC);
+ }
return State;
}
return true;
}
+ProgramStateRef ExprEngine::addTemporaryMaterialization(
+ ProgramStateRef State, const MaterializeTemporaryExpr *MTE,
+ const LocationContext *LC, const CXXTempObjectRegion *R) {
+ const auto &Key = std::make_pair(MTE, LC->getCurrentStackFrame());
+ assert(!State->contains<TemporaryMaterializations>(Key));
+ return State->set<TemporaryMaterializations>(Key, R);
+}
+
+bool ExprEngine::areTemporaryMaterializationsClear(
+ ProgramStateRef State, const LocationContext *FromLC,
+ const LocationContext *ToLC) {
+ const LocationContext *LC = FromLC;
+ while (LC != ToLC) {
+ assert(LC && "ToLC must be a parent of FromLC!");
+ for (auto I : State->get<TemporaryMaterializations>())
+ if (I.first.second == LC)
+ return false;
+
+ LC = LC->getParent();
+ }
+ return true;
+}
+
ProgramStateRef
ExprEngine::setCXXNewAllocatorValue(ProgramStateRef State,
const CXXNewExpr *CNE,
}
}
+static void printTemporaryMaterializationsForContext(
+ raw_ostream &Out, ProgramStateRef State, const char *NL, const char *Sep,
+ const LocationContext *LC) {
+ PrintingPolicy PP =
+ LC->getAnalysisDeclContext()->getASTContext().getPrintingPolicy();
+ for (auto I : State->get<TemporaryMaterializations>()) {
+ std::pair<const MaterializeTemporaryExpr *, const LocationContext *> Key =
+ I.first;
+ const MemRegion *Value = I.second;
+ if (Key.second != LC)
+ continue;
+ Out << '(' << Key.second << ',' << Key.first << ") ";
+ Key.first->printPretty(Out, nullptr, PP);
+ assert(Value);
+ Out << " : " << Value << NL;
+ }
+}
+
static void printCXXNewAllocatorValuesForContext(raw_ostream &Out,
ProgramStateRef State,
const char *NL,
});
}
+ if (!State->get<TemporaryMaterializations>().isEmpty()) {
+ Out << Sep << "Temporaries to be materialized:" << NL;
+
+ LCtx->dumpStack(Out, "", NL, Sep, [&](const LocationContext *LC) {
+ printTemporaryMaterializationsForContext(Out, State, NL, Sep, LC);
+ });
+ }
+
if (!State->get<CXXNewAllocatorValues>().isEmpty()) {
Out << Sep << "operator new() allocator return values:" << NL;
if (I.second)
SymReaper.markLive(I.second);
+ for (auto I : CleanedState->get<TemporaryMaterializations>())
+ if (I.second)
+ SymReaper.markLive(I.second);
+
for (auto I : CleanedState->get<CXXNewAllocatorValues>()) {
if (SymbolRef Sym = I.second.getAsSymbol())
SymReaper.markLive(Sym);
Pred->getStackFrame()->getParent()));
// FIXME: We currently assert that temporaries are clear, as lifetime extended
- // temporaries are not modelled correctly. When we materialize the temporary,
- // we do createTemporaryRegionIfNeeded(), and the region changes, and also
- // the respective destructor becomes automatic from temporary.
- // So for now clean up the state manually before asserting. Ideally, the code
- // above the assertion should go away, but the assertion should remain.
+ // temporaries are not always modelled correctly. In some cases when we
+ // materialize the temporary, we do createTemporaryRegionIfNeeded(), and
+ // the region changes, and also the respective destructor becomes automatic
+ // from temporary. So for now clean up the state manually before asserting.
+ // Ideally, the code above the assertion should go away, but the assertion
+ // should remain.
{
ExplodedNodeSet CleanUpTemporaries;
NodeBuilder Bldr(Pred, CleanUpTemporaries, BC);
assert(areInitializedTemporariesClear(Pred->getState(),
Pred->getLocationContext(),
Pred->getStackFrame()->getParent()));
+ assert(areTemporaryMaterializationsClear(Pred->getState(),
+ Pred->getLocationContext(),
+ Pred->getStackFrame()->getParent()));
PrettyStackTraceLocationContext CrashInfo(Pred->getLocationContext());
StateMgr.EndPath(Pred->getState());
-// RUN: %clang_analyze_cc1 -Wno-unused -std=c++11 -analyzer-checker=debug.ExprInspection -verify %s
+// RUN: %clang_analyze_cc1 -Wno-unused -std=c++11 -analyzer-checker=core,debug.ExprInspection -verify %s
+// RUN: %clang_analyze_cc1 -Wno-unused -std=c++11 -analyzer-checker=core,debug.ExprInspection -analyzer-config cfg-temporary-dtors=true -DTEMPORARIES -verify %s
void clang_analyzer_eval(bool);
const int &y = A().j[1]; // no-crash
const int &z = (A().j[1], A().j[0]); // no-crash
- // FIXME: All of these should be TRUE, but constructors aren't inlined.
- clang_analyzer_eval(x == 1); // expected-warning{{UNKNOWN}}
- clang_analyzer_eval(y == 3); // expected-warning{{UNKNOWN}}
- clang_analyzer_eval(z == 2); // expected-warning{{UNKNOWN}}
+ clang_analyzer_eval(x == 1);
+ clang_analyzer_eval(y == 3);
+ clang_analyzer_eval(z == 2);
+#ifdef TEMPORARIES
+ // expected-warning@-4{{TRUE}}
+ // expected-warning@-4{{TRUE}}
+ // expected-warning@-4{{TRUE}}
+#else
+ // expected-warning@-8{{UNKNOWN}}
+ // expected-warning@-8{{UNKNOWN}}
+ // expected-warning@-8{{UNKNOWN}}
+#endif
}
} // end namespace pr19539_crash_on_destroying_an_integer
+
+namespace maintain_original_object_address_on_lifetime_extension {
+class C {
+ C **after, **before;
+
+public:
+ bool x;
+
+ C(bool x, C **after, C **before) : x(x), after(after), before(before) {
+ *before = this;
+ }
+
+ // Don't track copies in our tests.
+ C(const C &c) : x(c.x), after(nullptr), before(nullptr) {}
+
+ ~C() { if (after) *after = this; }
+
+ operator bool() const { return x; }
+};
+
+void f1() {
+ C *after, *before;
+ {
+ const C &c = C(true, &after, &before);
+ }
+ clang_analyzer_eval(after == before);
+#ifdef TEMPORARIES
+ // expected-warning@-2{{TRUE}}
+#else
+ // expected-warning@-4{{UNKNOWN}}
+#endif
+}
+
+void f2() {
+ C *after, *before;
+ C c = C(1, &after, &before);
+ clang_analyzer_eval(after == before);
+#ifdef TEMPORARIES
+ // expected-warning@-2{{TRUE}}
+#else
+ // expected-warning@-4{{UNKNOWN}}
+#endif
+}
+
+void f3(bool coin) {
+ C *after, *before;
+ {
+ const C &c = coin ? C(true, &after, &before) : C(false, &after, &before);
+ }
+ clang_analyzer_eval(after == before);
+#ifdef TEMPORARIES
+ // expected-warning@-2{{TRUE}}
+#else
+ // expected-warning@-4{{UNKNOWN}}
+#endif
+}
+
+void f4(bool coin) {
+ C *after, *before;
+ {
+ // no-crash
+ const C &c = C(coin, &after, &before) ?: C(false, &after, &before);
+ }
+ // FIXME: Add support for lifetime extension through binary conditional
+ // operator. Ideally also add support for the binary conditional operator in
+ // C++. Because for now it calls the constructor for the condition twice.
+ if (coin) {
+ clang_analyzer_eval(after == before);
+#ifdef TEMPORARIES
+ // expected-warning@-2{{The left operand of '==' is a garbage value}}
+#else
+ // expected-warning@-4{{UNKNOWN}}
+#endif
+ } else {
+ clang_analyzer_eval(after == before);
+#ifdef TEMPORARIES
+ // Seems to work at the moment, but also seems accidental.
+ // Feel free to break.
+ // expected-warning@-4{{TRUE}}
+#else
+ // expected-warning@-6{{UNKNOWN}}
+#endif
+ }
+}
+
+void f5() {
+ C *after, *before;
+ {
+ const bool &x = C(true, &after, &before).x; // no-crash
+ }
+ // FIXME: Should be TRUE. Should not warn about garbage value.
+ clang_analyzer_eval(after == before);
+#ifdef TEMPORARIES
+ // expected-warning@-2{{The left operand of '==' is a garbage value}}
+#else
+ // expected-warning@-4{{UNKNOWN}}
+#endif
+}
+} // end namespace maintain_original_object_address_on_lifetime_extension
+
+namespace maintain_original_object_address_on_move {
+class C {
+ int *x;
+
+public:
+ C() : x(nullptr) {}
+ C(int *x) : x(x) {}
+ C(const C &c) = delete;
+ C(C &&c) : x(c.x) { c.x = nullptr; }
+ C &operator=(C &&c) {
+ x = c.x;
+ c.x = nullptr;
+ return *this;
+ }
+ ~C() {
+ // This was triggering the division by zero warning in f1() and f2():
+ // Because move-elision materialization was incorrectly causing the object
+ // to be relocated from one address to another before move, but destructor
+ // was operating on the old address, it was still thinking that 'x' is set.
+ if (x)
+ *x = 0;
+ }
+};
+
+void f1() {
+ int x = 1;
+ // &x is replaced with nullptr in move-constructor before the temporary dies.
+ C c = C(&x);
+ // Hence x was not set to 0 yet.
+ 1 / x; // no-warning
+}
+void f2() {
+ int x = 1;
+ C c;
+ // &x is replaced with nullptr in move-assignment before the temporary dies.
+ c = C(&x);
+ // Hence x was not set to 0 yet.
+ 1 / x; // no-warning
+}
+} // end namespace maintain_original_object_address_on_move
projects / platform / upstream / llvm.git / commitdiff