Skip to content

Commit 6804623

Browse files
committed
Fix some nullptr dereferences
Turns out we were getting away with dereferencing `nullptr` in a few cases as `walk` would use `nullptr` to indicate that the walk should be stopped, and luckily Clang didn't optimize it to something broken. This commit is fairly defensive and sprinkles some null checks for calls to `walk` directly on a body of a function or top-level code decl.
1 parent 8d71067 commit 6804623

8 files changed

+25
-12
lines changed

lib/IDE/SourceEntityWalker.cpp

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -274,10 +274,14 @@ std::pair<bool, Stmt *> SemaAnnotator::walkToStmtPre(Stmt *S) {
274274
// Since 'DeferStmt::getTempDecl()' is marked as implicit, we manually
275275
// walk into the body.
276276
if (auto *FD = DeferS->getTempDecl()) {
277-
auto *RetS = FD->getBody()->walk(*this);
277+
auto *Body = FD->getBody();
278+
if (!Body)
279+
return Action::Stop();
280+
281+
auto *RetS = Body->walk(*this);
278282
if (!RetS)
279283
return { false, nullptr };
280-
assert(RetS == FD->getBody());
284+
assert(RetS == Body);
281285
}
282286
bool Continue = SEWalker.walkToStmtPost(DeferS);
283287
if (!Continue)

lib/IDE/SyntaxModel.cpp

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -854,9 +854,11 @@ std::pair<bool, Stmt *> ModelASTWalker::walkToStmtPre(Stmt *S) {
854854
// Since 'DeferStmt::getTempDecl()' is marked as implicit, we manually walk
855855
// into the body.
856856
if (auto *FD = DeferS->getTempDecl()) {
857-
auto *RetS = FD->getBody()->walk(*this);
858-
assert(RetS == FD->getBody());
859-
(void)RetS;
857+
if (auto *Body = FD->getBody()) {
858+
auto *RetS = Body->walk(*this);
859+
assert(RetS == Body);
860+
(void)RetS;
861+
}
860862
walkToStmtPost(DeferS);
861863
}
862864
// Already walked children.

lib/Sema/CSDiagnostics.cpp

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1595,7 +1595,8 @@ bool MissingOptionalUnwrapFailure::diagnoseAsError() {
15951595
AbstractFunctionDecl *AFD = nullptr;
15961596
if ((AFD = dyn_cast<AbstractFunctionDecl>(varDecl->getDeclContext()))) {
15971597
auto checker = VarDeclMultipleReferencesChecker(getDC(), varDecl);
1598-
AFD->getBody()->walk(checker);
1598+
if (auto *body = AFD->getBody())
1599+
body->walk(checker);
15991600
singleUse = checker.referencesCount() == 1;
16001601
}
16011602

lib/Sema/TypeCheckConcurrency.cpp

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3073,7 +3073,8 @@ bool ActorIsolationChecker::mayExecuteConcurrentlyWith(
30733073

30743074
void swift::checkTopLevelActorIsolation(TopLevelCodeDecl *decl) {
30753075
ActorIsolationChecker checker(decl);
3076-
decl->getBody()->walk(checker);
3076+
if (auto *body = decl->getBody())
3077+
body->walk(checker);
30773078
}
30783079

30793080
void swift::checkFunctionActorIsolation(AbstractFunctionDecl *decl) {

lib/Sema/TypeCheckDecl.cpp

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -582,7 +582,8 @@ BodyInitKindRequest::evaluate(Evaluator &evaluator,
582582

583583
auto &ctx = decl->getASTContext();
584584
FindReferenceToInitializer finder(decl, ctx);
585-
decl->getBody()->walk(finder);
585+
if (auto *body = decl->getBody())
586+
body->walk(finder);
586587

587588
// get the kind out of the finder.
588589
auto Kind = finder.Kind;

lib/Sema/TypeCheckEffects.cpp

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2906,8 +2906,10 @@ void TypeChecker::checkTopLevelEffects(TopLevelCodeDecl *code) {
29062906
if (ctx.LangOpts.EnableThrowWithoutTry)
29072907
checker.setTopLevelThrowWithoutTry();
29082908

2909-
code->getBody()->walk(checker);
2910-
code->getBody()->walk(LocalFunctionEffectsChecker());
2909+
if (auto *body = code->getBody()) {
2910+
body->walk(checker);
2911+
body->walk(LocalFunctionEffectsChecker());
2912+
}
29112913
}
29122914

29132915
void TypeChecker::checkFunctionEffects(AbstractFunctionDecl *fn) {

lib/Sema/TypeCheckStmt.cpp

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -205,7 +205,8 @@ void TypeChecker::contextualizeTopLevelCode(TopLevelCodeDecl *TLCD) {
205205
auto &Context = TLCD->DeclContext::getASTContext();
206206
unsigned nextDiscriminator = Context.NextAutoClosureDiscriminator;
207207
ContextualizeClosures CC(TLCD, nextDiscriminator);
208-
TLCD->getBody()->walk(CC);
208+
if (auto *body = TLCD->getBody())
209+
body->walk(CC);
209210
assert(nextDiscriminator == Context.NextAutoClosureDiscriminator &&
210211
"reentrant/concurrent invocation of contextualizeTopLevelCode?");
211212
Context.NextAutoClosureDiscriminator = CC.NextDiscriminator;

lib/Sema/TypeCheckStorage.cpp

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3584,7 +3584,8 @@ bool SimpleDidSetRequest::evaluate(Evaluator &evaluator,
35843584
// If we find a reference to the implicit 'oldValue' parameter, then it is
35853585
// not a "simple" didSet because we need to fetch it.
35863586
auto walker = OldValueFinder(param);
3587-
decl->getTypecheckedBody()->walk(walker);
3587+
if (auto *body = decl->getTypecheckedBody())
3588+
body->walk(walker);
35883589
auto hasOldValueRef = walker.didFindOldValueRef();
35893590
if (!hasOldValueRef) {
35903591
// If the body does not refer to implicit 'oldValue', it means we can

0 commit comments

Comments
 (0)