Skip to content

Commit abfa152

Browse files
jensjohaCommit Queue
authored andcommitted
[kernel/DDC] DDC shouldn't crash when trying to translate line/column to offset for expresion compilation
Follow-up to https://dart-review.googlesource.com/c/sdk/+/446042. Change-Id: Icfbaa763a6d6089a74f62088602c07c478520741 Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/446420 Reviewed-by: Johnni Winther <[email protected]> Commit-Queue: Jens Johansen <[email protected]> Reviewed-by: Nicholas Shahan <[email protected]>
1 parent 3369772 commit abfa152

File tree

3 files changed

+126
-2
lines changed

3 files changed

+126
-2
lines changed

pkg/dev_compiler/lib/src/kernel/expression_compiler.dart

Lines changed: 17 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -353,7 +353,14 @@ class ExpressionCompiler {
353353

354354
// TODO(jensj): Eventually make the scriptUri required.
355355
if (scriptFileUri != null) {
356-
final offset = _component.getOffset(scriptFileUri, line, column);
356+
final offset = _component.getOffsetNoThrow(scriptFileUri, line, column);
357+
if (offset < 0) {
358+
_log(
359+
'Got offset negative offset ($offset) for '
360+
'$scriptFileUri:$line:$column',
361+
);
362+
return DartScope(library, null, null, null, null, {}, []);
363+
}
357364
final scope = DartScopeBuilder2.findScopeFromOffset(
358365
library,
359366
scriptFileUri,
@@ -388,11 +395,19 @@ class ExpressionCompiler {
388395

389396
final foundScopes = <DartScope>[];
390397
for (final fileUri in fileUris) {
391-
final offset = _component.getOffset(fileUri, line, column);
398+
final offset = _component.getOffsetNoThrow(fileUri, line, column);
399+
if (offset < 0) continue;
392400
foundScopes.add(
393401
DartScopeBuilder2.findScopeFromOffset(library, fileUri, offset),
394402
);
395403
}
404+
if (foundScopes.isEmpty) {
405+
_log(
406+
'Got only negative offsets for library/parts ${library.fileUri} '
407+
'at $line:$column.',
408+
);
409+
return DartScope(library, null, null, null, null, {}, []);
410+
}
396411
if (foundScopes.length == 1) {
397412
return foundScopes[0];
398413
}

pkg/dev_compiler/test/expression_compiler/expression_compiler_worker_shared.dart

Lines changed: 82 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -217,6 +217,88 @@ void runExpressionCompilationTests(ExpressionCompilerWorkerTestDriver driver) {
217217
);
218218
});
219219

220+
test('does not crash on line 0 in a library', () {
221+
driver.requestController.add({
222+
'command': 'UpdateDeps',
223+
'inputs': driver.inputs,
224+
});
225+
226+
driver.requestController.add({
227+
'command': 'CompileExpression',
228+
'expression': '1 + 1',
229+
'line': 0,
230+
'column': 0,
231+
'jsModules': {},
232+
'jsScope': {},
233+
'libraryUri': driver.config.getModule('testModule').libraryUris.first,
234+
'moduleName': driver.config.getModule('testModule').moduleName,
235+
});
236+
237+
expect(
238+
driver.responseController.stream,
239+
emitsInOrder([
240+
equals({'succeeded': true}),
241+
equals({
242+
'succeeded': true,
243+
'errors': isEmpty,
244+
'warnings': isEmpty,
245+
'infos': isEmpty,
246+
'compiledProcedure': contains('1 + 1;'),
247+
}),
248+
]),
249+
);
250+
});
251+
252+
test('does not crash on line thats too high in a library', () {
253+
driver.requestController.add({
254+
'command': 'UpdateDeps',
255+
'inputs': driver.inputs,
256+
});
257+
258+
driver.requestController.add({
259+
'command': 'CompileExpression',
260+
'expression': '1 + 1',
261+
'line': 10000000,
262+
'column': 1,
263+
'jsModules': {},
264+
'jsScope': {},
265+
'libraryUri': driver.config.getModule('testModule').libraryUris.first,
266+
'moduleName': driver.config.getModule('testModule').moduleName,
267+
});
268+
269+
driver.requestController.add({
270+
'command': 'CompileExpression',
271+
'expression': '2 + 2',
272+
'line': 1,
273+
'column': 10000000,
274+
'jsModules': {},
275+
'jsScope': {},
276+
'libraryUri': driver.config.getModule('testModule').libraryUris.first,
277+
'moduleName': driver.config.getModule('testModule').moduleName,
278+
});
279+
280+
expect(
281+
driver.responseController.stream,
282+
emitsInOrder([
283+
equals({'succeeded': true}),
284+
equals({
285+
'succeeded': true,
286+
'errors': isEmpty,
287+
'warnings': isEmpty,
288+
'infos': isEmpty,
289+
'compiledProcedure': contains('1 + 1;'),
290+
}),
291+
equals({
292+
'succeeded': true,
293+
'errors': isEmpty,
294+
'warnings': isEmpty,
295+
'infos': isEmpty,
296+
'compiledProcedure': contains('2 + 2;'),
297+
}),
298+
]),
299+
);
300+
});
301+
220302
test('can compile expressions in a library', () {
221303
driver.requestController.add({
222304
'command': 'UpdateDeps',

pkg/kernel/lib/src/ast/components.dart

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -159,6 +159,14 @@ class Component extends TreeNode {
159159
return uriToSource[file]?.getOffset(line, column) ?? -1;
160160
}
161161

162+
/// Translates line and column numbers to an offset in the given file.
163+
///
164+
/// Returns offset of the line and column in the file, or -1 if the
165+
/// source is not available, has no lines, or the line is out of range.
166+
int getOffsetNoThrow(Uri file, int line, int column) {
167+
return uriToSource[file]?.getOffsetNoThrow(line, column) ?? -1;
168+
}
169+
162170
void addMetadataRepository(MetadataRepository repository) {
163171
metadata[repository.tag] = repository;
164172
}
@@ -291,6 +299,25 @@ class Source {
291299
RangeError.checkValueInInterval(offset, 0, lineStarts.last, 'offset');
292300
return offset;
293301
}
302+
303+
/// Translates 1-based line and column numbers to an offset in the given file
304+
///
305+
/// Returns offset of the line and column in the file, or -1 if the source
306+
/// has no lines or the input is out of range.
307+
int getOffsetNoThrow(int line, int column) {
308+
List<int>? lineStarts = this.lineStarts;
309+
if (lineStarts == null || lineStarts.isEmpty) {
310+
return -1;
311+
}
312+
if (line < 1 || line > lineStarts.length) {
313+
return -1;
314+
}
315+
int offset = lineStarts[line - 1] + column - 1;
316+
if (offset < 0 || offset > lineStarts.last) {
317+
return -1;
318+
}
319+
return offset;
320+
}
294321
}
295322

296323
abstract class MetadataRepository<T> {

0 commit comments

Comments
 (0)