From abfa152cfc7e72e5e00fc4f015cb8400fc22d90a Mon Sep 17 00:00:00 2001 From: Jens Johansen Date: Sun, 24 Aug 2025 23:50:07 -0700 Subject: [PATCH] [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 Commit-Queue: Jens Johansen Reviewed-by: Nicholas Shahan --- .../lib/src/kernel/expression_compiler.dart | 19 ++++- .../expression_compiler_worker_shared.dart | 82 +++++++++++++++++++ pkg/kernel/lib/src/ast/components.dart | 27 ++++++ 3 files changed, 126 insertions(+), 2 deletions(-) diff --git a/pkg/dev_compiler/lib/src/kernel/expression_compiler.dart b/pkg/dev_compiler/lib/src/kernel/expression_compiler.dart index f02e26ffbfa..b17aff60c41 100644 --- a/pkg/dev_compiler/lib/src/kernel/expression_compiler.dart +++ b/pkg/dev_compiler/lib/src/kernel/expression_compiler.dart @@ -353,7 +353,14 @@ class ExpressionCompiler { // TODO(jensj): Eventually make the scriptUri required. if (scriptFileUri != null) { - final offset = _component.getOffset(scriptFileUri, line, column); + final offset = _component.getOffsetNoThrow(scriptFileUri, line, column); + if (offset < 0) { + _log( + 'Got offset negative offset ($offset) for ' + '$scriptFileUri:$line:$column', + ); + return DartScope(library, null, null, null, null, {}, []); + } final scope = DartScopeBuilder2.findScopeFromOffset( library, scriptFileUri, @@ -388,11 +395,19 @@ class ExpressionCompiler { final foundScopes = []; for (final fileUri in fileUris) { - final offset = _component.getOffset(fileUri, line, column); + final offset = _component.getOffsetNoThrow(fileUri, line, column); + if (offset < 0) continue; foundScopes.add( DartScopeBuilder2.findScopeFromOffset(library, fileUri, offset), ); } + if (foundScopes.isEmpty) { + _log( + 'Got only negative offsets for library/parts ${library.fileUri} ' + 'at $line:$column.', + ); + return DartScope(library, null, null, null, null, {}, []); + } if (foundScopes.length == 1) { return foundScopes[0]; } diff --git a/pkg/dev_compiler/test/expression_compiler/expression_compiler_worker_shared.dart b/pkg/dev_compiler/test/expression_compiler/expression_compiler_worker_shared.dart index baa3c9bf164..56d4c538453 100644 --- a/pkg/dev_compiler/test/expression_compiler/expression_compiler_worker_shared.dart +++ b/pkg/dev_compiler/test/expression_compiler/expression_compiler_worker_shared.dart @@ -217,6 +217,88 @@ void runExpressionCompilationTests(ExpressionCompilerWorkerTestDriver driver) { ); }); + test('does not crash on line 0 in a library', () { + driver.requestController.add({ + 'command': 'UpdateDeps', + 'inputs': driver.inputs, + }); + + driver.requestController.add({ + 'command': 'CompileExpression', + 'expression': '1 + 1', + 'line': 0, + 'column': 0, + 'jsModules': {}, + 'jsScope': {}, + 'libraryUri': driver.config.getModule('testModule').libraryUris.first, + 'moduleName': driver.config.getModule('testModule').moduleName, + }); + + expect( + driver.responseController.stream, + emitsInOrder([ + equals({'succeeded': true}), + equals({ + 'succeeded': true, + 'errors': isEmpty, + 'warnings': isEmpty, + 'infos': isEmpty, + 'compiledProcedure': contains('1 + 1;'), + }), + ]), + ); + }); + + test('does not crash on line thats too high in a library', () { + driver.requestController.add({ + 'command': 'UpdateDeps', + 'inputs': driver.inputs, + }); + + driver.requestController.add({ + 'command': 'CompileExpression', + 'expression': '1 + 1', + 'line': 10000000, + 'column': 1, + 'jsModules': {}, + 'jsScope': {}, + 'libraryUri': driver.config.getModule('testModule').libraryUris.first, + 'moduleName': driver.config.getModule('testModule').moduleName, + }); + + driver.requestController.add({ + 'command': 'CompileExpression', + 'expression': '2 + 2', + 'line': 1, + 'column': 10000000, + 'jsModules': {}, + 'jsScope': {}, + 'libraryUri': driver.config.getModule('testModule').libraryUris.first, + 'moduleName': driver.config.getModule('testModule').moduleName, + }); + + expect( + driver.responseController.stream, + emitsInOrder([ + equals({'succeeded': true}), + equals({ + 'succeeded': true, + 'errors': isEmpty, + 'warnings': isEmpty, + 'infos': isEmpty, + 'compiledProcedure': contains('1 + 1;'), + }), + equals({ + 'succeeded': true, + 'errors': isEmpty, + 'warnings': isEmpty, + 'infos': isEmpty, + 'compiledProcedure': contains('2 + 2;'), + }), + ]), + ); + }); + test('can compile expressions in a library', () { driver.requestController.add({ 'command': 'UpdateDeps', diff --git a/pkg/kernel/lib/src/ast/components.dart b/pkg/kernel/lib/src/ast/components.dart index 17df623ab37..770b6775325 100644 --- a/pkg/kernel/lib/src/ast/components.dart +++ b/pkg/kernel/lib/src/ast/components.dart @@ -159,6 +159,14 @@ class Component extends TreeNode { return uriToSource[file]?.getOffset(line, column) ?? -1; } + /// Translates line and column numbers to an offset in the given file. + /// + /// Returns offset of the line and column in the file, or -1 if the + /// source is not available, has no lines, or the line is out of range. + int getOffsetNoThrow(Uri file, int line, int column) { + return uriToSource[file]?.getOffsetNoThrow(line, column) ?? -1; + } + void addMetadataRepository(MetadataRepository repository) { metadata[repository.tag] = repository; } @@ -291,6 +299,25 @@ class Source { RangeError.checkValueInInterval(offset, 0, lineStarts.last, 'offset'); return offset; } + + /// Translates 1-based line and column numbers to an offset in the given file + /// + /// Returns offset of the line and column in the file, or -1 if the source + /// has no lines or the input is out of range. + int getOffsetNoThrow(int line, int column) { + List? lineStarts = this.lineStarts; + if (lineStarts == null || lineStarts.isEmpty) { + return -1; + } + if (line < 1 || line > lineStarts.length) { + return -1; + } + int offset = lineStarts[line - 1] + column - 1; + if (offset < 0 || offset > lineStarts.last) { + return -1; + } + return offset; + } } abstract class MetadataRepository {