Skip to content

Commit f48bd90

Browse files
committed
[ast] Fix a series of potential nullptr violations in CaptureInfo.
Specifically, CaptureInfo previously as a pattern in CaptureInfo.cpp would attempt to ascertain if a CapturedValue was a local capture by checking the CapturedValue's decl. Unfortunately, when we have captured dynamic self metadata, we do not even have a getDecl() and should return false. I fixed this by adding a new API to CaptureValue that checks if it has a decl before checking if the decl is a local capture. This avoids this problem.
1 parent 2f353b8 commit f48bd90

File tree

3 files changed

+47
-5
lines changed

3 files changed

+47
-5
lines changed

include/swift/AST/CaptureInfo.h

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -100,6 +100,12 @@ class CapturedValue {
100100
return Value.getPointer().is<OpaqueValueExpr *>();
101101
}
102102

103+
/// Returns true if this captured value has a local capture.
104+
///
105+
/// NOTE: This implies that the value is not dynamic self metadata, since
106+
/// values with decls are the only values that are able to be local captures.
107+
bool isLocalCapture() const;
108+
103109
CapturedValue mergeFlags(CapturedValue cv) {
104110
assert(Value.getPointer() == cv.Value.getPointer() &&
105111
"merging flags on two different value decls");

lib/AST/CaptureInfo.cpp

Lines changed: 20 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,10 @@
1717

1818
using namespace swift;
1919

20+
//===----------------------------------------------------------------------===//
21+
// MARK: CaptureInfo
22+
//===----------------------------------------------------------------------===//
23+
2024
CaptureInfo::CaptureInfo(ASTContext &ctx, ArrayRef<CapturedValue> captures,
2125
DynamicSelfType *dynamicSelf,
2226
OpaqueValueExpr *opaqueValue,
@@ -56,9 +60,10 @@ CaptureInfo CaptureInfo::empty() {
5660
}
5761

5862
bool CaptureInfo::hasLocalCaptures() const {
59-
for (auto capture : getCaptures())
60-
if (capture.getDecl()->isLocalCapture())
63+
for (auto capture : getCaptures()) {
64+
if (capture.isLocalCapture())
6165
return true;
66+
}
6267
return false;
6368
}
6469

@@ -71,7 +76,7 @@ getLocalCaptures(SmallVectorImpl<CapturedValue> &Result) const {
7176

7277
// Filter out global variables.
7378
for (auto capture : getCaptures()) {
74-
if (!capture.getDecl()->isLocalCapture())
79+
if (!capture.isLocalCapture())
7580
continue;
7681

7782
Result.push_back(capture);
@@ -83,10 +88,12 @@ VarDecl *CaptureInfo::getIsolatedParamCapture() const {
8388
return nullptr;
8489

8590
for (const auto &capture : getCaptures()) {
86-
if (!capture.getDecl()->isLocalCapture())
91+
// Check for dynamic self metadata before checking if we have a local
92+
// capture since dynamic self metadata doesn't have a decl.
93+
if (capture.isDynamicSelfMetadata())
8794
continue;
8895

89-
if (capture.isDynamicSelfMetadata())
96+
if (!capture.isLocalCapture())
9097
continue;
9198

9299
// If we captured an isolated parameter, return it.
@@ -134,3 +141,11 @@ void CaptureInfo::print(raw_ostream &OS) const {
134141
OS << ')';
135142
}
136143

144+
//===----------------------------------------------------------------------===//
145+
// MARK: CapturedValue
146+
//===----------------------------------------------------------------------===//
147+
148+
bool CapturedValue::isLocalCapture() const {
149+
auto *decl = Value.getPointer().dyn_cast<ValueDecl *>();
150+
return decl && decl->isLocalCapture();
151+
}
Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
1+
// RUN: %target-swift-frontend -o - -emit-silgen %s
2+
3+
// REQUIRES: concurrency
4+
5+
// READ THIS! This test is meant to test invariants around CaptureInfo usage in
6+
// SILGen. Only add examples to this if we hit a crasher in capture info and the
7+
// example makes sure we do not crash again.
8+
9+
class GetIsolatedParamTest {
10+
public static var rootType: Any.Type? { nil }
11+
}
12+
13+
extension GetIsolatedParamTest : CustomDebugStringConvertible {
14+
public var debugDescription: String {
15+
let description = "\\\(String(describing: Self.rootType!))"
16+
let x: Any.Type? = nil
17+
// The error is triggered by the ?? autoclosure.
18+
var valueType: Any.Type? = x ?? Self.rootType
19+
return description
20+
}
21+
}

0 commit comments

Comments
 (0)