Skip to content

Commit 9503969

Browse files
alexmarkovcommit-bot@chromium.org
alexmarkov
authored and
[email protected]
committed
[vm/bytecode] Add DebugCheck bytecode instruction
DebugCheck bytecode instruction is generated after parameter variables are declared and copied into their locations in the prologue. It helps debugger to stop in the beginning of a function at the point where parameters can be inspected. It is generated only if '--bytecode-options=debugger-stops' is specified. Change-Id: I0f3b1ea8dc45d762a5dcee75b5d3a4ffc0b2a1b1 Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/108371 Reviewed-by: Ryan Macnak <[email protected]> Reviewed-by: Régis Crelier <[email protected]> Commit-Queue: Alexander Markov <[email protected]>
1 parent fc542be commit 9503969

File tree

12 files changed

+62
-26
lines changed

12 files changed

+62
-26
lines changed

‎pkg/vm/bin/kernel_service.dart‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -146,8 +146,8 @@ abstract class Compiler {
146146

147147
if (options.bytecode && errors.isEmpty) {
148148
await runWithFrontEndCompilerContext(script, options, component, () {
149-
// TODO(alexmarkov): disable source positions, local variables info
150-
// and source files in VM PRODUCT mode.
149+
// TODO(alexmarkov): disable source positions, local variables info,
150+
// debugger stops and source files in VM PRODUCT mode.
151151
// TODO(alexmarkov): disable asserts if they are not enabled in VM.
152152
// TODO(rmacnak): disable annotations if mirrors are not enabled.
153153
generateBytecode(component,
@@ -156,6 +156,7 @@ abstract class Compiler {
156156
environmentDefines: options.environmentDefines,
157157
emitSourcePositions: true,
158158
emitLocalVarInfo: true,
159+
emitDebuggerStops: true,
159160
emitSourceFiles: true,
160161
emitAnnotations: true));
161162
});

‎pkg/vm/lib/bytecode/assembler.dart‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -513,6 +513,11 @@ class BytecodeAssembler {
513513
_emitInstructionA(Opcode.kCheckStack, ra);
514514
}
515515

516+
void emitDebugCheck() {
517+
emitSourcePosition();
518+
_emitInstruction0(Opcode.kDebugCheck);
519+
}
520+
516521
void emitCheckFunctionTypeArgs(int ra, int re) {
517522
emitSourcePosition();
518523
_emitInstructionAE(Opcode.kCheckFunctionTypeArgs, ra, re);

‎pkg/vm/lib/bytecode/dbc.dart‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@ library vm.bytecode.dbc;
1010
/// Before bumping current bytecode version format, make sure that
1111
/// all users have switched to a VM which is able to consume new
1212
/// version of bytecode.
13-
const int currentBytecodeFormatVersion = 12;
13+
const int currentBytecodeFormatVersion = 13;
1414

1515
enum Opcode {
1616
kUnusedOpcode000,
@@ -99,8 +99,6 @@ enum Opcode {
9999
kUnusedOpcode083,
100100
kUnusedOpcode084,
101101

102-
// Bytecode instructions since bytecode format v7:
103-
104102
kTrap,
105103

106104
// Prologue and stack management.
@@ -117,7 +115,7 @@ enum Opcode {
117115
kCheckFunctionTypeArgs,
118116
kCheckFunctionTypeArgs_Wide,
119117
kCheckStack,
120-
kUnused01,
118+
kDebugCheck,
121119
kUnused02, // Reserved for CheckParameterTypes
122120
kUnused03, // Reserved for CheckParameterTypes_Wide
123121

@@ -370,6 +368,8 @@ const Map<Opcode, Format> BytecodeFormats = const {
370368
Encoding.kAE, const [Operand.imm, Operand.reg, Operand.none]),
371369
Opcode.kCheckStack: const Format(
372370
Encoding.kA, const [Operand.imm, Operand.none, Operand.none]),
371+
Opcode.kDebugCheck: const Format(
372+
Encoding.k0, const [Operand.none, Operand.none, Operand.none]),
373373
Opcode.kAllocate: const Format(
374374
Encoding.kD, const [Operand.lit, Operand.none, Operand.none]),
375375
Opcode.kAllocateT: const Format(

‎pkg/vm/lib/bytecode/gen_bytecode.dart‎

Lines changed: 17 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1534,12 +1534,6 @@ class BytecodeGenerator extends RecursiveVisitor<Null> {
15341534
} else {
15351535
asm.emitEntry(locals.frameSize);
15361536
}
1537-
// TODO(alexmarkov): Introduce a new bytecode triggering a debug check in
1538-
// the interpreter. Its token position should correspond to the declaration
1539-
// position of the last parameter, which the debugger can inspect at the
1540-
// point of the debug check.
1541-
// TODO(regis): Support the new bytecode in the interpreter and dissociate
1542-
// the debug check from the CheckStack bytecode.
15431537
asm.emitCheckStack(0);
15441538

15451539
if (isClosure) {
@@ -1666,6 +1660,23 @@ class BytecodeGenerator extends RecursiveVisitor<Null> {
16661660
function.positionalParameters.forEach(_copyParamIfCaptured);
16671661
locals.sortedNamedParameters.forEach(_copyParamIfCaptured);
16681662
}
1663+
1664+
if (options.emitDebuggerStops) {
1665+
// DebugCheck instruction should be emitted after parameter variables
1666+
// are declared and copied into context.
1667+
// The debugger expects the source position to correspond to the
1668+
// declaration position of the last parameter, if any, or of the function.
1669+
if (options.emitSourcePositions && function != null) {
1670+
var pos = function.fileOffset;
1671+
if (function.namedParameters.isNotEmpty) {
1672+
pos = function.namedParameters.last.fileOffset;
1673+
} else if (function.positionalParameters.isNotEmpty) {
1674+
pos = function.positionalParameters.last.fileOffset;
1675+
}
1676+
_recordSourcePosition(pos);
1677+
}
1678+
asm.emitDebugCheck();
1679+
}
16691680
}
16701681

16711682
void _copyParamIfCaptured(VariableDeclaration variable) {

‎pkg/vm/lib/bytecode/options.dart‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ class BytecodeOptions {
99
static Map<String, String> commandLineFlags = {
1010
'annotations': 'Emit Dart annotations',
1111
'local-var-info': 'Emit debug information about local variables',
12+
'debugger-stops': 'Emit bytecode instructions for stopping in the debugger',
1213
'show-bytecode-size-stat': 'Show bytecode size breakdown',
1314
'source-positions': 'Emit source positions',
1415
};
@@ -18,6 +19,7 @@ class BytecodeOptions {
1819
bool emitSourcePositions;
1920
bool emitSourceFiles;
2021
bool emitLocalVarInfo;
22+
bool emitDebuggerStops;
2123
bool emitAnnotations;
2224
bool omitAssertSourcePositions;
2325
bool showBytecodeSizeStatistics;
@@ -29,6 +31,7 @@ class BytecodeOptions {
2931
this.emitSourcePositions = false,
3032
this.emitSourceFiles = false,
3133
this.emitLocalVarInfo = false,
34+
this.emitDebuggerStops = false,
3235
this.emitAnnotations = false,
3336
this.omitAssertSourcePositions = false,
3437
this.showBytecodeSizeStatistics = false,
@@ -49,6 +52,9 @@ class BytecodeOptions {
4952
case 'local-var-info':
5053
emitLocalVarInfo = true;
5154
break;
55+
case 'debugger-stops':
56+
emitDebuggerStops = true;
57+
break;
5258
case 'annotations':
5359
emitAnnotations = true;
5460
break;

‎runtime/vm/compiler/frontend/base_flow_graph_builder.cc‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -873,6 +873,15 @@ Fragment BaseFlowGraphBuilder::BuildFfiAsFunctionInternalCall(
873873
return code;
874874
}
875875

876+
Fragment BaseFlowGraphBuilder::DebugStepCheck(TokenPosition position) {
877+
#ifdef PRODUCT
878+
return Fragment();
879+
#else
880+
return Fragment(new (Z) DebugStepCheckInstr(
881+
position, RawPcDescriptors::kRuntimeCall, GetNextDeoptId()));
882+
#endif
883+
}
884+
876885
} // namespace kernel
877886
} // namespace dart
878887

‎runtime/vm/compiler/frontend/base_flow_graph_builder.h‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -301,6 +301,8 @@ class BaseFlowGraphBuilder {
301301
const Class& klass,
302302
intptr_t argument_count);
303303

304+
Fragment DebugStepCheck(TokenPosition position);
305+
304306
protected:
305307
intptr_t AllocateBlockId() { return ++last_used_block_id_; }
306308

‎runtime/vm/compiler/frontend/bytecode_flow_graph_builder.cc‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -730,6 +730,13 @@ void BytecodeFlowGraphBuilder::BuildCheckStack() {
730730
}
731731
}
732732

733+
void BytecodeFlowGraphBuilder::BuildDebugCheck() {
734+
if (is_generating_interpreter()) {
735+
UNIMPLEMENTED(); // TODO(alexmarkov): interpreter
736+
}
737+
code_ += B->DebugStepCheck(position_);
738+
}
739+
733740
void BytecodeFlowGraphBuilder::BuildPushConstant() {
734741
PushConstant(ConstantAt(DecodeOperandD()));
735742
}

‎runtime/vm/compiler/frontend/kernel_to_il.cc‎

Lines changed: 0 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1220,15 +1220,6 @@ bool FlowGraphBuilder::NeedsDebugStepCheck(Value* value,
12201220
return definition->IsLoadLocal();
12211221
}
12221222

1223-
Fragment FlowGraphBuilder::DebugStepCheck(TokenPosition position) {
1224-
#ifdef PRODUCT
1225-
return Fragment();
1226-
#else
1227-
return Fragment(new (Z) DebugStepCheckInstr(
1228-
position, RawPcDescriptors::kRuntimeCall, GetNextDeoptId()));
1229-
#endif
1230-
}
1231-
12321223
Fragment FlowGraphBuilder::EvaluateAssertion() {
12331224
const Class& klass =
12341225
Class::ZoneHandle(Z, Library::LookupCoreClass(Symbols::AssertionError()));

‎runtime/vm/compiler/frontend/kernel_to_il.h‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -212,7 +212,6 @@ class FlowGraphBuilder : public BaseFlowGraphBuilder {
212212

213213
bool NeedsDebugStepCheck(const Function& function, TokenPosition position);
214214
bool NeedsDebugStepCheck(Value* value, TokenPosition position);
215-
Fragment DebugStepCheck(TokenPosition position);
216215

217216
// Truncates (instead of deoptimizing) if the origin does not fit into the
218217
// target representation.

0 commit comments

Comments
 (0)