From fe6ea5e39dab16a335a016e1155126b6b1cb14a4 Mon Sep 17 00:00:00 2001 From: Tess Strickland Date: Thu, 19 Feb 2026 03:00:23 -0800 Subject: [PATCH] [vm,dyn_modules] Rename DebugCheck to Nop and use after debugger() calls. Normally, bytecodes no longer in use would be renamed to Unused. However, in the case of DebugCheck, its only use was to call the debugger when single stepping, and since its original creation, the interpreter has been changed to call the debugger when single stepping on every instruction. Thus, DebugCheck instructions are effectively no-ops, only used as a distinct PC offset for source positions, and this CL changes their name to reflect this. This CL also changes the bytecode generator to detect uses of debugger() from dart:developer and to add a Nop after it, mimicking how StreamingFlowGraphBuilder recognizes uses of debugger() and adds a DebugStepCheck instruction afterwards. Doing this instead of just using asm.emitSourcePosition() at the end of visitStaticInvocation ensures that the source position isn't overwritten by the next emitted instruction. TEST=pkg/vm_service/test/set_sdk_library_debuggable_test Cq-Include-Trybots: luci.dart.try:vm-dyn-linux-debug-x64-try,vm-dyn-mac-debug-arm64-try Change-Id: Ie24bcea0b5aeb9e41d7765f25b1cd123bb2565b4 Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/480203 Reviewed-by: Alexander Markov Commit-Queue: Tess Strickland --- pkg/dart2bytecode/docs/bytecode.md | 7 ++-- pkg/dart2bytecode/lib/assembler.dart | 9 +++-- pkg/dart2bytecode/lib/bytecode_generator.dart | 35 +++++++++++++++---- pkg/dart2bytecode/lib/dbc.dart | 4 +-- runtime/vm/constants_kbc.h | 2 +- runtime/vm/interpreter.cc | 2 +- 6 files changed, 43 insertions(+), 16 deletions(-) diff --git a/pkg/dart2bytecode/docs/bytecode.md b/pkg/dart2bytecode/docs/bytecode.md index 071260001f9..ed201ee0559 100644 --- a/pkg/dart2bytecode/docs/bytecode.md +++ b/pkg/dart2bytecode/docs/bytecode.md @@ -1422,8 +1422,7 @@ SP[0] = SP[-1] SP[0] ? true : false Allocate closure object for closure function ConstantPool[D]. -#### DebugCheck - -No-op. Provides a point where debugger can stop executing code when single stepping -or when it hits a breakpoint. +#### Nop +No-op. Provides a unique PC offset to ensure an emitted source position is not +overwritten by a different source position emitted by the next instruction. diff --git a/pkg/dart2bytecode/lib/assembler.dart b/pkg/dart2bytecode/lib/assembler.dart index e28f10d257e..0436ea8d2f5 100644 --- a/pkg/dart2bytecode/lib/assembler.dart +++ b/pkg/dart2bytecode/lib/assembler.dart @@ -708,9 +708,14 @@ class BytecodeAssembler { } @pragma('vm:prefer-inline') - void emitDebugCheck() { + void emitNop() { + // Currently, the only reason the bytecode generator emits a no-op + // instruction is when the next instruction to emit is unknown (e.g., + // at the end of a visit method), but a source position must be + // emitted after the last known instruction and not overwritten by + // the next emitted instruction. emitSourcePosition(); - _emitInstruction0(Opcode.kDebugCheck); + _emitInstruction0(Opcode.kNop); } @pragma('vm:prefer-inline') diff --git a/pkg/dart2bytecode/lib/bytecode_generator.dart b/pkg/dart2bytecode/lib/bytecode_generator.dart index ffbe33c67a4..168181a7691 100644 --- a/pkg/dart2bytecode/lib/bytecode_generator.dart +++ b/pkg/dart2bytecode/lib/bytecode_generator.dart @@ -109,7 +109,8 @@ class BytecodeGenerator extends RecursiveVisitor { final Map astUriToSource; final List libraries; final Set extraLoadedLibraries; - final LibraryIndex ffiLibraryIndex; + late LibraryIndex ffiLibraryIndex; + late LibraryIndex developerLibraryIndex; late StringTable stringTable; late ObjectTable objectTable; late Component bytecodeComponent; @@ -147,6 +148,16 @@ class BytecodeGenerator extends RecursiveVisitor { bool isInDeeplyImmutableClass = false; + late final Set allLibraries = { + ...libraries, + ...extraLoadedLibraries + }; + + LibraryIndex getLibraryIndexFor(String library) => + coreTypes.index.containsLibrary(library) + ? coreTypes.index + : LibraryIndex.fromLibraries(allLibraries, [library]); + BytecodeGenerator( ast.Component component, CoreTypes coreTypes, @@ -178,14 +189,12 @@ class BytecodeGenerator extends RecursiveVisitor { {required this.libraries, required this.extraLoadedLibraries}) : recognizedMethods = new RecognizedMethods(staticTypeContext), - astUriToSource = component.uriToSource, - ffiLibraryIndex = coreTypes.index.containsLibrary('dart:ffi') - ? coreTypes.index - : LibraryIndex.fromLibraries( - {...libraries, ...extraLoadedLibraries}, const ['dart:ffi']) { + astUriToSource = component.uriToSource { bytecodeComponent = new Component(coreTypes); stringTable = bytecodeComponent.stringTable; objectTable = bytecodeComponent.objectTable; + ffiLibraryIndex = getLibraryIndexFor('dart:ffi'); + developerLibraryIndex = getLibraryIndexFor('dart:developer'); } @override @@ -1069,6 +1078,13 @@ class BytecodeGenerator extends RecursiveVisitor { ? ffiLibraryIndex.getTopLevelProcedure('dart:ffi', '_ffiCall') : null; + late Library? dartDeveloperLibrary = + developerLibraryIndex.tryGetLibrary('dart:developer'); + + late Procedure? debugger = (dartDeveloperLibrary != null) + ? developerLibraryIndex.getTopLevelProcedure('dart:developer', 'debugger') + : null; + late Procedure ensureDeeplyImmutable = libraryIndex.getTopLevelProcedure( 'dart:_internal', '_ensureDeeplyImmutable'); @@ -3574,6 +3590,13 @@ class BytecodeGenerator extends RecursiveVisitor { _genArguments(null, args); _genDirectCallWithArgs(target, args, isFactory: target.isFactory, node: node); + if (target == debugger) { + // The debugger needs a pause right after stepping out from the debugger + // function. Just using asm.emitSourcePosition() won't work here, because + // the next emitted instruction may have its own source position + // and thus would overwrite that one. + asm.emitNop(); + } } @override diff --git a/pkg/dart2bytecode/lib/dbc.dart b/pkg/dart2bytecode/lib/dbc.dart index 1bdac9c285e..3f2d184e2b7 100644 --- a/pkg/dart2bytecode/lib/dbc.dart +++ b/pkg/dart2bytecode/lib/dbc.dart @@ -25,7 +25,7 @@ enum Opcode { kCheckFunctionTypeArgs, kCheckFunctionTypeArgs_Wide, kCheckStack, - kDebugCheck, + kNop, kJumpIfUnchecked, kJumpIfUnchecked_Wide, @@ -294,7 +294,7 @@ const Map BytecodeFormats = const { Encoding.kAE, const [Operand.imm, Operand.reg, Operand.none]), Opcode.kCheckStack: const Format( Encoding.kA, const [Operand.imm, Operand.none, Operand.none]), - Opcode.kDebugCheck: const Format( + Opcode.kNop: const Format( Encoding.k0, const [Operand.none, Operand.none, Operand.none]), Opcode.kAllocate: const Format( Encoding.kD, const [Operand.lit, Operand.none, Operand.none]), diff --git a/runtime/vm/constants_kbc.h b/runtime/vm/constants_kbc.h index 0edb7e31157..7ffe6a1e958 100644 --- a/runtime/vm/constants_kbc.h +++ b/runtime/vm/constants_kbc.h @@ -48,7 +48,7 @@ namespace dart { V(CheckFunctionTypeArgs, A_E, ORDN, num, reg, ___) \ V(CheckFunctionTypeArgs_Wide, A_E, WIDE, num, reg, ___) \ V(CheckStack, A, ORDN, num, ___, ___) \ - V(DebugCheck, 0, ORDN, ___, ___, ___) \ + V(Nop, 0, ORDN, ___, ___, ___) \ V(JumpIfUnchecked, T, ORDN, tgt, ___, ___) \ V(JumpIfUnchecked_Wide, T, WIDE, tgt, ___, ___) \ V(Allocate, D, ORDN, lit, ___, ___) \ diff --git a/runtime/vm/interpreter.cc b/runtime/vm/interpreter.cc index 05faf931761..a543d15bd81 100644 --- a/runtime/vm/interpreter.cc +++ b/runtime/vm/interpreter.cc @@ -1992,7 +1992,7 @@ SwitchDispatchNoSingleStep: } { - BYTECODE(DebugCheck, 0); + BYTECODE(Nop, 0); DISPATCH(); }