Skip to content

Commit

Permalink
Revert "[analyzer] Fix memory space of static locals seen from nested…
Browse files Browse the repository at this point in the history
… blocks."

This reverts commit r292800.

It is causing null pointer dereference false positives when a block that
captures a static local is evaluated at the top level.

llvm-svn: 292874
  • Loading branch information
devincoughlin committed Jan 24, 2017
1 parent 9028f05 commit e1a5630
Show file tree
Hide file tree
Showing 2 changed files with 19 additions and 42 deletions.
54 changes: 19 additions & 35 deletions clang/lib/StaticAnalyzer/Core/MemRegion.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -776,22 +776,6 @@ getStackOrCaptureRegionForDeclContext(const LocationContext *LC,
return (const StackFrameContext *)nullptr;
}

static CanQualType getBlockPointerType(const BlockDecl *BD, ASTContext &C) {
// FIXME: The fallback type here is totally bogus -- though it should
// never be queried, it will prevent uniquing with the real
// BlockCodeRegion. Ideally we'd fix the AST so that we always had a
// signature.
QualType T;
if (const TypeSourceInfo *TSI = BD->getSignatureAsWritten())
T = TSI->getType();
if (T.isNull())
T = C.VoidTy;
if (!T->getAs<FunctionType>())
T = C.getFunctionNoProtoType(T);
T = C.getBlockPointerType(T);
return C.getCanonicalType(T);
}

const VarRegion* MemRegionManager::getVarRegion(const VarDecl *D,
const LocationContext *LC) {
const MemRegion *sReg = nullptr;
Expand Down Expand Up @@ -819,7 +803,7 @@ const VarRegion* MemRegionManager::getVarRegion(const VarDecl *D,
sReg = getGlobalsRegion();
}

// Finally handle locals.
// Finally handle static locals.
} else {
// FIXME: Once we implement scope handling, we will need to properly lookup
// 'D' to the proper LocationContext.
Expand All @@ -832,22 +816,9 @@ const VarRegion* MemRegionManager::getVarRegion(const VarDecl *D,

const StackFrameContext *STC = V.get<const StackFrameContext*>();

if (!STC) {
if (D->isStaticLocal()) {
const CodeTextRegion *fReg = nullptr;
if (const auto *ND = dyn_cast<NamedDecl>(DC))
fReg = getFunctionCodeRegion(ND);
else if (const auto *BD = dyn_cast<BlockDecl>(DC))
fReg = getBlockCodeRegion(BD, getBlockPointerType(BD, getContext()),
LC->getAnalysisDeclContext());
assert(fReg && "Unable to determine code region for a static local!");
sReg = getGlobalsRegion(MemRegion::StaticGlobalSpaceRegionKind, fReg);
} else {
// We're looking at a block-captured local variable, which may be either
// still local, or already moved to the heap. So we're not sure.
sReg = getUnknownRegion();
}
} else {
if (!STC)
sReg = getUnknownRegion();
else {
if (D->hasLocalStorage()) {
sReg = isa<ParmVarDecl>(D) || isa<ImplicitParamDecl>(D)
? static_cast<const MemRegion*>(getStackArgumentsRegion(STC))
Expand All @@ -860,9 +831,22 @@ const VarRegion* MemRegionManager::getVarRegion(const VarDecl *D,
sReg = getGlobalsRegion(MemRegion::StaticGlobalSpaceRegionKind,
getFunctionCodeRegion(cast<NamedDecl>(STCD)));
else if (const BlockDecl *BD = dyn_cast<BlockDecl>(STCD)) {
// FIXME: The fallback type here is totally bogus -- though it should
// never be queried, it will prevent uniquing with the real
// BlockCodeRegion. Ideally we'd fix the AST so that we always had a
// signature.
QualType T;
if (const TypeSourceInfo *TSI = BD->getSignatureAsWritten())
T = TSI->getType();
if (T.isNull())
T = getContext().VoidTy;
if (!T->getAs<FunctionType>())
T = getContext().getFunctionNoProtoType(T);
T = getContext().getBlockPointerType(T);

const BlockCodeRegion *BTR =
getBlockCodeRegion(BD, getBlockPointerType(BD, getContext()),
STC->getAnalysisDeclContext());
getBlockCodeRegion(BD, C.getCanonicalType(T),
STC->getAnalysisDeclContext());
sReg = getGlobalsRegion(MemRegion::StaticGlobalSpaceRegionKind,
BTR);
}
Expand Down
7 changes: 0 additions & 7 deletions clang/test/Analysis/dispatch-once.m
Original file line number Diff line number Diff line change
Expand Up @@ -107,10 +107,3 @@ void test_block_var_from_outside_block() {
};
dispatch_once(&once, ^{}); // expected-warning{{Call to 'dispatch_once' uses the block variable 'once' for the predicate value.}}
}

void test_static_var_from_outside_block() {
static dispatch_once_t once;
^{
dispatch_once(&once, ^{}); // no-warning
};
}

0 comments on commit e1a5630

Please sign in to comment.