Skip to content

Commit

Permalink
[analyzer] Differentiate lifetime extended temporaries
Browse files Browse the repository at this point in the history
This patch introduces a new `CXXLifetimeExtendedObjectRegion` as a representation
of the memory for the temporary object that is lifetime extended by the reference
to which they are bound.

This separation provides an ability to detect the use of dangling pointers
(either binding or dereference) in a robust manner.
For example, the `ref` is conditionally dangling in the following example:
```
template<typename T>
T const& select(bool cond, T const& t, T const& u) { return cond ? t : u; }

int const& le = Composite{}.x;
auto&& ref = select(cond, le, 10);
```
Before the change, regardless of the value of `cond`, the `select()` call would
have returned a `temp_object` region.
With the proposed change we would produce a (non-dangling) `lifetime_extended_object`
region with lifetime bound to `le` or a `temp_object` region for the dangling case.

We believe that such separation is desired, as such lifetime extended temporaries
are closer to the variables. For example, they may have a static storage duration
(this patch removes a static temporary region, which was an abomination).
We also think that alternative approaches are not viable.

While for some cases it may be possible to determine if the region is lifetime
extended by searching the parents of the initializer expr, this quickly becomes
complex in the presence of the conditions operators like this one:
```
Composite cc;
// Ternary produces prvalue 'int' which is extended, as branches differ in value category
auto&& x = cond ? Composite{}.x : cc.x;

// Ternary produces xvalue, and extends the Composite object
auto&& y = cond ? Composite{}.x : std::move(cc).x;
```

Finally, the lifetime of the `CXXLifetimeExtendedObjectRegion` is tied to the lifetime of
the corresponding variables, however, the "liveness" (or reachability) of the extending
variable does not imply the reachability of all symbols in the region.
In conclusion `CXXLifetimeExtendedObjectRegion`, in contrast to `VarRegions`, does not
need any special handling in `SymReaper`.

RFC: https://discourse.llvm.org/t/rfc-detecting-uses-of-dangling-references/70731

Reviewed By: xazax.hun

Differential Revision: https://reviews.llvm.org/D151325
  • Loading branch information
Tomasz Kamiński committed Jul 5, 2023
1 parent b89b3cd commit feafbb9
Show file tree
Hide file tree
Showing 11 changed files with 326 additions and 28 deletions.
63 changes: 56 additions & 7 deletions clang/include/clang/StaticAnalyzer/Core/PathSensitive/MemRegion.h
Original file line number Diff line number Diff line change
Expand Up @@ -1231,8 +1231,7 @@ class CXXTempObjectRegion : public TypedValueRegion {
CXXTempObjectRegion(Expr const *E, MemSpaceRegion const *sReg)
: TypedValueRegion(sReg, CXXTempObjectRegionKind), Ex(E) {
assert(E);
assert(isa<StackLocalsSpaceRegion>(sReg) ||
isa<GlobalInternalSpaceRegion>(sReg));
assert(isa<StackLocalsSpaceRegion>(sReg));
}

static void ProfileRegion(llvm::FoldingSetNodeID &ID,
Expand All @@ -1242,6 +1241,9 @@ class CXXTempObjectRegion : public TypedValueRegion {
LLVM_ATTRIBUTE_RETURNS_NONNULL
const Expr *getExpr() const { return Ex; }

LLVM_ATTRIBUTE_RETURNS_NONNULL
const StackFrameContext *getStackFrame() const;

QualType getValueType() const override { return Ex->getType(); }

void dumpToStream(raw_ostream &os) const override;
Expand All @@ -1253,6 +1255,45 @@ class CXXTempObjectRegion : public TypedValueRegion {
}
};

// C++ temporary object that have lifetime extended to lifetime of the
// variable. Usually they represent temporary bounds to reference variables.
class CXXLifetimeExtendedObjectRegion : public TypedValueRegion {
friend class MemRegionManager;

Expr const *Ex;
ValueDecl const *ExD;

CXXLifetimeExtendedObjectRegion(Expr const *E, ValueDecl const *D,
MemSpaceRegion const *sReg)
: TypedValueRegion(sReg, CXXLifetimeExtendedObjectRegionKind), Ex(E),
ExD(D) {
assert(E);
assert(D);
assert((isa<StackLocalsSpaceRegion, GlobalInternalSpaceRegion>(sReg)));
}

static void ProfileRegion(llvm::FoldingSetNodeID &ID, Expr const *E,
ValueDecl const *D, const MemRegion *sReg);

public:
LLVM_ATTRIBUTE_RETURNS_NONNULL
const Expr *getExpr() const { return Ex; }
LLVM_ATTRIBUTE_RETURNS_NONNULL
const ValueDecl *getExtendingDecl() const { return ExD; }
/// It might return null.
const StackFrameContext *getStackFrame() const;

QualType getValueType() const override { return Ex->getType(); }

void dumpToStream(raw_ostream &os) const override;

void Profile(llvm::FoldingSetNodeID &ID) const override;

static bool classof(const MemRegion *R) {
return R->getKind() == CXXLifetimeExtendedObjectRegionKind;
}
};

// CXXBaseObjectRegion represents a base object within a C++ object. It is
// identified by the base class declaration and the region of its parent object.
class CXXBaseObjectRegion : public TypedValueRegion {
Expand Down Expand Up @@ -1485,6 +1526,19 @@ class MemRegionManager {
const CXXTempObjectRegion *getCXXTempObjectRegion(Expr const *Ex,
LocationContext const *LC);

/// Create a CXXLifetimeExtendedObjectRegion for temporaries which are
/// lifetime-extended by local references.
const CXXLifetimeExtendedObjectRegion *
getCXXLifetimeExtendedObjectRegion(Expr const *Ex, ValueDecl const *VD,
LocationContext const *LC);

/// Create a CXXLifetimeExtendedObjectRegion for temporaries which are
/// lifetime-extended by *static* references.
/// This differs from \ref getCXXLifetimeExtendedObjectRegion(Expr const *,
/// ValueDecl const *, LocationContext const *) in the super-region used.
const CXXLifetimeExtendedObjectRegion *
getCXXStaticLifetimeExtendedObjectRegion(const Expr *Ex, ValueDecl const *VD);

/// Create a CXXBaseObjectRegion with the given base class for region
/// \p Super.
///
Expand Down Expand Up @@ -1523,11 +1577,6 @@ class MemRegionManager {
const LocationContext *lc,
unsigned blockCount);

/// Create a CXXTempObjectRegion for temporaries which are lifetime-extended
/// by static references. This differs from getCXXTempObjectRegion in the
/// super-region used.
const CXXTempObjectRegion *getCXXStaticTempObjectRegion(const Expr *Ex);

private:
template <typename RegionTy, typename SuperTy,
typename Arg1Ty>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,7 @@ ABSTRACT_REGION(SubRegion, MemRegion)
REGION(CXXBaseObjectRegion, TypedValueRegion)
REGION(CXXDerivedObjectRegion, TypedValueRegion)
REGION(CXXTempObjectRegion, TypedValueRegion)
REGION(CXXLifetimeExtendedObjectRegion, TypedValueRegion)
REGION(CXXThisRegion, TypedValueRegion)
ABSTRACT_REGION(DeclRegion, TypedValueRegion)
REGION(FieldRegion, DeclRegion)
Expand Down
5 changes: 3 additions & 2 deletions clang/lib/StaticAnalyzer/Checkers/MoveChecker.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -552,8 +552,9 @@ MoveChecker::classifyObject(const MemRegion *MR,
// For the purposes of this checker, we classify move-safe STL types
// as not-"STL" types, because that's how the checker treats them.
MR = unwrapRValueReferenceIndirection(MR);
bool IsLocal = isa_and_nonnull<VarRegion>(MR) &&
isa<StackSpaceRegion>(MR->getMemorySpace());
bool IsLocal =
isa_and_nonnull<VarRegion, CXXLifetimeExtendedObjectRegion>(MR) &&
isa<StackSpaceRegion>(MR->getMemorySpace());

if (!RD || !RD->getDeclContext()->isStdNamespace())
return { IsLocal, SK_NonStd };
Expand Down
10 changes: 9 additions & 1 deletion clang/lib/StaticAnalyzer/Checkers/StackAddrEscapeChecker.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -96,6 +96,14 @@ SourceRange StackAddrEscapeChecker::genName(raw_ostream &os, const MemRegion *R,
os << "stack memory associated with local variable '" << VR->getString()
<< '\'';
range = VR->getDecl()->getSourceRange();
} else if (const auto *LER = dyn_cast<CXXLifetimeExtendedObjectRegion>(R)) {
QualType Ty = LER->getValueType().getLocalUnqualifiedType();
os << "stack memory associated with temporary object of type '";
Ty.print(os, Ctx.getPrintingPolicy());
os << "' lifetime extended by local variable";
if (const IdentifierInfo *ID = LER->getExtendingDecl()->getIdentifier())
os << " '" << ID->getName() << '\'';
range = LER->getExpr()->getSourceRange();
} else if (const auto *TOR = dyn_cast<CXXTempObjectRegion>(R)) {
QualType Ty = TOR->getValueType().getLocalUnqualifiedType();
os << "stack memory associated with temporary object of type '";
Expand Down Expand Up @@ -376,7 +384,7 @@ void StackAddrEscapeChecker::checkEndFunction(const ReturnStmt *RS,
llvm::raw_svector_ostream Out(Buf);
const SourceRange Range = genName(Out, Referred, Ctx.getASTContext());

if (isa<CXXTempObjectRegion>(Referrer)) {
if (isa<CXXTempObjectRegion, CXXLifetimeExtendedObjectRegion>(Referrer)) {
Out << " is still referred to by a temporary object on the stack "
<< CommonSuffix;
auto Report =
Expand Down
10 changes: 7 additions & 3 deletions clang/lib/StaticAnalyzer/Core/ExprEngine.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -386,15 +386,19 @@ ProgramStateRef ExprEngine::createTemporaryRegionIfNeeded(
State = finishObjectConstruction(State, MT, LC);
State = State->BindExpr(Result, LC, *V);
return State;
} else {
} else if (const ValueDecl *VD = MT->getExtendingDecl()) {
StorageDuration SD = MT->getStorageDuration();
assert(SD != SD_FullExpression);
// 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);
TR = MRMgr.getCXXStaticLifetimeExtendedObjectRegion(Init, VD);
} else {
TR = MRMgr.getCXXTempObjectRegion(Init, LC);
TR = MRMgr.getCXXLifetimeExtendedObjectRegion(Init, VD, LC);
}
} else {
assert(MT->getStorageDuration() == SD_FullExpression);
TR = MRMgr.getCXXTempObjectRegion(Init, LC);
}
} else {
TR = MRMgr.getCXXTempObjectRegion(Init, LC);
Expand Down
18 changes: 12 additions & 6 deletions clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -284,7 +284,8 @@ SVal ExprEngine::computeObjectUnderConstruction(
CallOpts.IsTemporaryCtorOrDtor = true;
if (MTE) {
if (const ValueDecl *VD = MTE->getExtendingDecl()) {
assert(MTE->getStorageDuration() != SD_FullExpression);
StorageDuration SD = MTE->getStorageDuration();
assert(SD != SD_FullExpression);
if (!VD->getType()->isReferenceType()) {
// We're lifetime-extended by a surrounding aggregate.
// Automatic destructors aren't quite working in this case
Expand All @@ -293,11 +294,15 @@ SVal ExprEngine::computeObjectUnderConstruction(
// the MaterializeTemporaryExpr?
CallOpts.IsTemporaryLifetimeExtendedViaAggregate = true;
}
}

if (MTE->getStorageDuration() == SD_Static ||
MTE->getStorageDuration() == SD_Thread)
return loc::MemRegionVal(MRMgr.getCXXStaticTempObjectRegion(E));
if (SD == SD_Static || SD == SD_Thread)
return loc::MemRegionVal(
MRMgr.getCXXStaticLifetimeExtendedObjectRegion(E, VD));

return loc::MemRegionVal(
MRMgr.getCXXLifetimeExtendedObjectRegion(E, VD, LCtx));
}
assert(MTE->getStorageDuration() == SD_FullExpression);
}

return loc::MemRegionVal(MRMgr.getCXXTempObjectRegion(E, LCtx));
Expand Down Expand Up @@ -799,7 +804,8 @@ void ExprEngine::handleConstructor(const Expr *E,
StmtNodeBuilder Bldr(DstEvaluated, DstEvaluatedPostProcessed, *currBldrCtx);
const AnalysisDeclContext *ADC = LCtx->getAnalysisDeclContext();
if (!ADC->getCFGBuildOptions().AddTemporaryDtors) {
if (llvm::isa_and_nonnull<CXXTempObjectRegion>(TargetRegion) &&
if (llvm::isa_and_nonnull<CXXTempObjectRegion,
CXXLifetimeExtendedObjectRegion>(TargetRegion) &&
cast<CXXConstructorDecl>(Call->getDecl())
->getParent()
->isAnyDestructorNoReturn()) {
Expand Down
61 changes: 55 additions & 6 deletions clang/lib/StaticAnalyzer/Core/MemRegion.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -158,6 +158,18 @@ const StackFrameContext *VarRegion::getStackFrame() const {
return SSR ? SSR->getStackFrame() : nullptr;
}

const StackFrameContext *
CXXLifetimeExtendedObjectRegion::getStackFrame() const {
const auto *SSR = dyn_cast<StackSpaceRegion>(getMemorySpace());
return SSR ? SSR->getStackFrame() : nullptr;
}

const StackFrameContext *CXXTempObjectRegion::getStackFrame() const {
assert(isa<StackSpaceRegion>(getMemorySpace()) &&
"A temporary object can only be allocated on the stack");
return cast<StackSpaceRegion>(getMemorySpace())->getStackFrame();
}

ObjCIvarRegion::ObjCIvarRegion(const ObjCIvarDecl *ivd, const SubRegion *sReg)
: DeclRegion(sReg, ObjCIvarRegionKind), IVD(ivd) {
assert(IVD);
Expand Down Expand Up @@ -389,6 +401,20 @@ void CXXTempObjectRegion::Profile(llvm::FoldingSetNodeID &ID) const {
ProfileRegion(ID, Ex, getSuperRegion());
}

void CXXLifetimeExtendedObjectRegion::ProfileRegion(llvm::FoldingSetNodeID &ID,
const Expr *E,
const ValueDecl *D,
const MemRegion *sReg) {
ID.AddPointer(E);
ID.AddPointer(D);
ID.AddPointer(sReg);
}

void CXXLifetimeExtendedObjectRegion::Profile(
llvm::FoldingSetNodeID &ID) const {
ProfileRegion(ID, Ex, ExD, getSuperRegion());
}

void CXXBaseObjectRegion::ProfileRegion(llvm::FoldingSetNodeID &ID,
const CXXRecordDecl *RD,
bool IsVirtual,
Expand Down Expand Up @@ -483,6 +509,16 @@ void CXXTempObjectRegion::dumpToStream(raw_ostream &os) const {
<< "S" << Ex->getID(getContext()) << '}';
}

void CXXLifetimeExtendedObjectRegion::dumpToStream(raw_ostream &os) const {
os << "lifetime_extended_object{" << getValueType() << ", ";
if (const IdentifierInfo *ID = ExD->getIdentifier())
os << ID->getName();
else
os << "D" << ExD->getID();
os << ", "
<< "S" << Ex->getID(getContext()) << '}';
}

void CXXBaseObjectRegion::dumpToStream(raw_ostream &os) const {
os << "Base{" << superRegion << ',' << getDecl()->getName() << '}';
}
Expand Down Expand Up @@ -743,6 +779,7 @@ DefinedOrUnknownSVal MemRegionManager::getStaticSize(const MemRegion *MR,
case MemRegion::CXXBaseObjectRegionKind:
case MemRegion::CXXDerivedObjectRegionKind:
case MemRegion::CXXTempObjectRegionKind:
case MemRegion::CXXLifetimeExtendedObjectRegionKind:
case MemRegion::CXXThisRegionKind:
case MemRegion::ObjCIvarRegionKind:
case MemRegion::NonParamVarRegionKind:
Expand Down Expand Up @@ -1097,12 +1134,6 @@ MemRegionManager::getBlockDataRegion(const BlockCodeRegion *BC,
return getSubRegion<BlockDataRegion>(BC, LC, blockCount, sReg);
}

const CXXTempObjectRegion *
MemRegionManager::getCXXStaticTempObjectRegion(const Expr *Ex) {
return getSubRegion<CXXTempObjectRegion>(
Ex, getGlobalsRegion(MemRegion::GlobalInternalSpaceRegionKind, nullptr));
}

const CompoundLiteralRegion*
MemRegionManager::getCompoundLiteralRegion(const CompoundLiteralExpr *CL,
const LocationContext *LC) {
Expand Down Expand Up @@ -1184,6 +1215,23 @@ MemRegionManager::getCXXTempObjectRegion(Expr const *E,
return getSubRegion<CXXTempObjectRegion>(E, getStackLocalsRegion(SFC));
}

const CXXLifetimeExtendedObjectRegion *
MemRegionManager::getCXXLifetimeExtendedObjectRegion(
const Expr *Ex, const ValueDecl *VD, const LocationContext *LC) {
const StackFrameContext *SFC = LC->getStackFrame();
assert(SFC);
return getSubRegion<CXXLifetimeExtendedObjectRegion>(
Ex, VD, getStackLocalsRegion(SFC));
}

const CXXLifetimeExtendedObjectRegion *
MemRegionManager::getCXXStaticLifetimeExtendedObjectRegion(
const Expr *Ex, const ValueDecl *VD) {
return getSubRegion<CXXLifetimeExtendedObjectRegion>(
Ex, VD,
getGlobalsRegion(MemRegion::GlobalInternalSpaceRegionKind, nullptr));
}

/// Checks whether \p BaseClass is a valid virtual or direct non-virtual base
/// class of the type of \p Super.
static bool isValidBaseClass(const CXXRecordDecl *BaseClass,
Expand Down Expand Up @@ -1457,6 +1505,7 @@ static RegionOffset calculateOffset(const MemRegion *R) {
case MemRegion::NonParamVarRegionKind:
case MemRegion::ParamVarRegionKind:
case MemRegion::CXXTempObjectRegionKind:
case MemRegion::CXXLifetimeExtendedObjectRegionKind:
// Usual base regions.
goto Finish;

Expand Down
1 change: 1 addition & 0 deletions clang/lib/StaticAnalyzer/Core/Store.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -144,6 +144,7 @@ std::optional<const MemRegion *> StoreManager::castRegion(const MemRegion *R,
case MemRegion::NonParamVarRegionKind:
case MemRegion::ParamVarRegionKind:
case MemRegion::CXXTempObjectRegionKind:
case MemRegion::CXXLifetimeExtendedObjectRegionKind:
case MemRegion::CXXBaseObjectRegionKind:
case MemRegion::CXXDerivedObjectRegionKind:
return MakeElementRegion(cast<SubRegion>(R), PointeeTy);
Expand Down

0 comments on commit feafbb9

Please sign in to comment.