Skip to content

Commit 3bffa79

Browse files
ahoppenahoppen
committed
[SourceKit] Run SwiftParser on a deep stack for the related identifiers request
We ran SwiftParser in `handlePrimaryAST` for related identifiers. That function is called on a dispatch queue with reduced stack size and could cause the parser to stack overflow if the program is very nested. Run `handlePrimaryAST` on a deep stack for this request to fix the issue. rdar://129960285
1 parent 3ee8272 commit 3bffa79

File tree

4 files changed

+320
-6
lines changed

4 files changed

+320
-6
lines changed

‎test/SourceKit/RelatedIdents/run-parser-with-large-stack.swift‎

Lines changed: 303 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,303 @@
1+
// This test used to overflow the stack because SwiftParser was run on a background thread with reduced stack size
2+
// RUN: %sourcekitd-test -req=related-idents -pos=%(line + 1):5 %s -- %s
3+
let x = 1
4+
5+
if true {
6+
if true {
7+
if true {
8+
if true {
9+
if true {
10+
if true {
11+
if true {
12+
if true {
13+
if true {
14+
if true {
15+
if true {
16+
if true {
17+
if true {
18+
if true {
19+
if true {
20+
if true {
21+
if true {
22+
if true {
23+
if true {
24+
if true {
25+
if true {
26+
if true {
27+
if true {
28+
if true {
29+
if true {
30+
if true {
31+
if true {
32+
if true {
33+
if true {
34+
if true {
35+
if true {
36+
if true {
37+
if true {
38+
if true {
39+
if true {
40+
if true {
41+
if true {
42+
if true {
43+
if true {
44+
if true {
45+
if true {
46+
if true {
47+
if true {
48+
if true {
49+
if true {
50+
if true {
51+
if true {
52+
if true {
53+
if true {
54+
if true {
55+
if true {
56+
if true {
57+
if true {
58+
if true {
59+
if true {
60+
if true {
61+
if true {
62+
if true {
63+
if true {
64+
if true {
65+
if true {
66+
if true {
67+
if true {
68+
if true {
69+
if true {
70+
if true {
71+
if true {
72+
if true {
73+
if true {
74+
if true {
75+
if true {
76+
if true {
77+
if true {
78+
if true {
79+
if true {
80+
if true {
81+
if true {
82+
if true {
83+
if true {
84+
if true {
85+
if true {
86+
if true {
87+
if true {
88+
if true {
89+
if true {
90+
if true {
91+
if true {
92+
if true {
93+
if true {
94+
if true {
95+
if true {
96+
if true {
97+
if true {
98+
if true {
99+
if true {
100+
if true {
101+
if true {
102+
if true {
103+
if true {
104+
if true {
105+
if true {
106+
if true {
107+
if true {
108+
if true {
109+
if true {
110+
if true {
111+
if true {
112+
if true {
113+
if true {
114+
if true {
115+
if true {
116+
if true {
117+
if true {
118+
if true {
119+
if true {
120+
if true {
121+
if true {
122+
if true {
123+
if true {
124+
if true {
125+
if true {
126+
if true {
127+
if true {
128+
if true {
129+
if true {
130+
if true {
131+
if true {
132+
if true {
133+
if true {
134+
if true {
135+
if true {
136+
if true {
137+
if true {
138+
if true {
139+
if true {
140+
if true {
141+
if true {
142+
if true {
143+
if true {
144+
if true {
145+
if true {
146+
if true {
147+
if true {
148+
if true {
149+
if true {
150+
if true {
151+
if true {
152+
if true {
153+
if true {
154+
if true {
155+
if true {
156+
if true {
157+
if true {
158+
if true {
159+
if true {
160+
if true {
161+
if true {
162+
if true {
163+
if true {
164+
if true {
165+
if true {
166+
if true {
167+
if true {
168+
if true {
169+
if true {
170+
if true {
171+
if true {
172+
if true {
173+
if true {
174+
if true {
175+
if true {
176+
if true {
177+
if true {
178+
if true {
179+
if true {
180+
if true {
181+
if true {
182+
if true {
183+
if true {
184+
if true {
185+
if true {
186+
if true {
187+
if true {
188+
if true {
189+
if true {
190+
if true {
191+
if true {
192+
if true {
193+
if true {
194+
if true {
195+
if true {
196+
if true {
197+
if true {
198+
if true {
199+
if true {
200+
if true {
201+
if true {
202+
if true {
203+
if true {
204+
if true {
205+
if true {
206+
if true {
207+
if true {
208+
if true {
209+
if true {
210+
if true {
211+
if true {
212+
if true {
213+
if true {
214+
if true {
215+
if true {
216+
if true {
217+
if true {
218+
if true {
219+
if true {
220+
if true {
221+
if true {
222+
if true {
223+
if true {
224+
if true {
225+
if true {
226+
if true {
227+
if true {
228+
if true {
229+
if true {
230+
if true {
231+
if true {
232+
if true {
233+
if true {
234+
if true {
235+
if true {
236+
if true {
237+
if true {
238+
if true {
239+
if true {
240+
if true {
241+
if true {
242+
if true {
243+
if true {
244+
if true {
245+
if true {
246+
if true {
247+
if true {
248+
if true {
249+
if true {
250+
if true {
251+
if true {
252+
if true {
253+
if true {
254+
if true {
255+
if true {
256+
if true {
257+
if true {
258+
if true {
259+
if true {
260+
if true {
261+
if true {
262+
if true {
263+
if true {
264+
if true {
265+
if true {
266+
if true {
267+
if true {
268+
if true {
269+
if true {
270+
if true {
271+
if true {
272+
if true {
273+
if true {
274+
if true {
275+
if true {
276+
if true {
277+
if true {
278+
if true {
279+
if true {
280+
if true {
281+
if true {
282+
if true {
283+
if true {
284+
if true {
285+
if true {
286+
if true {
287+
if true {
288+
if true {
289+
if true {
290+
if true {
291+
if true {
292+
if true {
293+
if true {
294+
if true {
295+
if true {
296+
if true {
297+
if true {
298+
if true {
299+
if true {
300+
if true {
301+
if true {
302+
if true {
303+
if true {

‎tools/SourceKit/lib/SwiftLang/SwiftASTManager.cpp‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -174,13 +174,13 @@ namespace SourceKit {
174174
void ASTUnit::Implementation::consumeAsync(SwiftASTConsumerRef ConsumerRef,
175175
ASTUnitRef ASTRef) {
176176
#if defined(_WIN32)
177-
// Windows uses more up for stack space (why?) than macOS/Linux which
178-
// causes stack overflows in a dispatch thread with 64k stack. Passing
179-
// useDeepStack=true means it's given a _beginthreadex thread with an 8MB
180-
// stack.
181-
bool useDeepStack = true;
177+
// Windows uses more up for stack space (why?) than macOS/Linux which
178+
// causes stack overflows in a dispatch thread with 64k stack. Passing
179+
// useDeepStack=true means it's given a _beginthreadex thread with an 8MB
180+
// stack.
181+
bool useDeepStack = true;
182182
#else
183-
bool useDeepStack = false;
183+
bool useDeepStack = ConsumerRef->requiresDeepStack();
184184
#endif
185185
Queue.dispatch([ASTRef, ConsumerRef]{
186186
SwiftASTConsumer &ASTConsumer = *ConsumerRef;

‎tools/SourceKit/lib/SwiftLang/SwiftASTManager.h‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -153,6 +153,15 @@ class SwiftASTConsumer : public std::enable_shared_from_this<SwiftASTConsumer> {
153153
public:
154154
virtual ~SwiftASTConsumer() { }
155155

156+
/// Whether `handlePrimaryAST` should be executed with the same stack size as
157+
/// the main thread.
158+
///
159+
/// By default, it is assumed that `handlePrimaryAST` does not do a lot of
160+
/// work and it is sufficient to run it on a background thread's stack with
161+
/// reduced size. Set this to `true` if the consumer can perform additional
162+
/// work that might require more stack size, such as invoking SwiftParser.
163+
virtual bool requiresDeepStack() { return false; }
164+
156165
// MARK: Cancellation
157166

158167
/// The result of this consumer is no longer of interest to the SourceKit

‎tools/SourceKit/lib/SwiftLang/SwiftSourceDocInfo.cpp‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2537,6 +2537,8 @@ void SwiftLangSupport::findRelatedIdentifiersInFile(
25372537
std::function<void(const RequestResult<RelatedIdentsResult> &)> Receiver;
25382538
SwiftInvocationRef Invok;
25392539

2540+
bool requiresDeepStack() override { return true; }
2541+
25402542
#if SWIFT_BUILD_SWIFT_SYNTAX
25412543
// FIXME: Don't silently eat errors here.
25422544
RelatedIdentsResult getRelatedIdents(SourceFile *SrcFile,

0 commit comments

Comments
 (0)