Skip to content

Commit c6c0e5b

Browse files
creliercommit-bot@chromium.org
authored andcommitted
[vm/debugger] Make use of variable descriptors in bytecode debugger.
Change-Id: Ibf4c8f638065c8173c5c21c8fa9c40ee5cecb587 Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/106427 Commit-Queue: Régis Crelier <regis@google.com> Reviewed-by: Alexander Markov <alexmarkov@google.com>
1 parent d2caa7f commit c6c0e5b

3 files changed

Lines changed: 125 additions & 40 deletions

File tree

runtime/vm/compiler/frontend/bytecode_reader.cc

Lines changed: 9 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -266,8 +266,16 @@ void BytecodeReaderHelper::ReadCode(const Function& function,
266266
if (FLAG_dump_kernel_bytecode) {
267267
KernelBytecodeDisassembler::Disassemble(closure);
268268
}
269+
270+
#if !defined(PRODUCT)
271+
thread_->isolate()->debugger()->NotifyBytecodeLoaded(closure);
272+
#endif
269273
}
270274
}
275+
276+
#if !defined(PRODUCT)
277+
thread_->isolate()->debugger()->NotifyBytecodeLoaded(function);
278+
#endif
271279
}
272280

273281
static intptr_t IndexFor(Zone* zone,
@@ -2343,13 +2351,6 @@ RawError* BytecodeReader::ReadFunctionBytecode(Thread* thread,
23432351
bytecode_reader.ReadCode(function, code_offset);
23442352
}
23452353
}
2346-
2347-
#if !defined(PRODUCT)
2348-
if (function.HasBytecode()) {
2349-
thread->isolate()->debugger()->NotifyBytecodeLoaded(function);
2350-
}
2351-
#endif
2352-
23532354
return Error::null();
23542355
} else {
23552356
return thread->StealStickyError();
@@ -2416,6 +2417,7 @@ RawLocalVarDescriptors* BytecodeReader::ComputeLocalVarDescriptors(
24162417
function.token_pos() <= var_info.end_pos) ||
24172418
(function.token_pos() <= var_info.begin_pos &&
24182419
var_info.begin_pos <= function.end_token_pos()))) {
2420+
var_info.scope_id++; // One level higher in the context chain.
24192421
vars.Add(
24202422
VarDesc{&String::Handle(zone, parent_vars.GetName(i)), var_info});
24212423
}

runtime/vm/debugger.cc

Lines changed: 115 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -652,8 +652,8 @@ intptr_t ActivationFrame::ColumnNumber() {
652652
void ActivationFrame::GetVarDescriptors() {
653653
if (var_descriptors_.IsNull()) {
654654
if (IsInterpreted()) {
655-
// TODO(regis): Kernel bytecode does not yet provide var descriptors.
656-
var_descriptors_ = Object::empty_var_descriptors().raw();
655+
var_descriptors_ = bytecode().GetLocalVarDescriptors();
656+
ASSERT(!var_descriptors_.IsNull());
657657
return;
658658
}
659659
Code& unoptimized_code = Code::Handle(function().unoptimized_code());
@@ -684,9 +684,14 @@ void ActivationFrame::PrintDescriptorsError(const char* message) {
684684
OS::PrintErr("deopt_id_ %" Px "\n", deopt_id_);
685685
OS::PrintErr("context_level_ %" Px "\n", context_level_);
686686
DisassembleToStdout formatter;
687-
if (IsInterpreted()) {
688-
bytecode().Disassemble(&formatter);
689-
PcDescriptors::Handle(bytecode().pc_descriptors()).Print();
687+
if (function().is_declared_in_bytecode()) {
688+
#if !defined(DART_PRECOMPILED_RUNTIME)
689+
ASSERT(function().HasBytecode());
690+
const Bytecode& bytecode = Bytecode::Handle(function().bytecode());
691+
bytecode.Disassemble(&formatter);
692+
#else
693+
UNREACHABLE();
694+
#endif // !defined(DART_PRECOMPILED_RUNTIME)
690695
} else {
691696
code().Disassemble(&formatter);
692697
PcDescriptors::Handle(code().pc_descriptors()).Print();
@@ -702,38 +707,97 @@ void ActivationFrame::PrintDescriptorsError(const char* message) {
702707
OS::Abort();
703708
}
704709

705-
// Calculate the context level at the current token index of the frame.
710+
// Calculate the context level at the current bytecode pc or code deopt id
711+
// of the frame.
706712
intptr_t ActivationFrame::ContextLevel() {
707-
// TODO(regis): get context level information using
708-
// BytecodeLocalVariablesIterator for interpreted frames and compiled frames
709-
// with a function coming from bytecode (function.is_declared_in_bytecode())
710713
const Context& ctx = GetSavedCurrentContext();
711714
if (context_level_ < 0 && !ctx.IsNull()) {
712-
ASSERT(!code_.is_optimized());
713-
714-
GetVarDescriptors();
715-
intptr_t deopt_id = DeoptId();
716-
if (deopt_id == DeoptId::kNone) {
717-
PrintDescriptorsError("Missing deopt id");
718-
}
719-
intptr_t var_desc_len = var_descriptors_.Length();
720-
bool found = false;
721-
for (intptr_t cur_idx = 0; cur_idx < var_desc_len; cur_idx++) {
722-
RawLocalVarDescriptors::VarInfo var_info;
723-
var_descriptors_.GetInfo(cur_idx, &var_info);
724-
const int8_t kind = var_info.kind();
725-
if ((kind == RawLocalVarDescriptors::kContextLevel) &&
726-
(deopt_id >= var_info.begin_pos.value()) &&
727-
(deopt_id <= var_info.end_pos.value())) {
728-
context_level_ = var_info.index();
729-
found = true;
730-
break;
715+
ASSERT(IsInterpreted() || !code_.is_optimized());
716+
if (function().is_declared_in_bytecode()) {
717+
#if !defined(DART_PRECOMPILED_RUNTIME)
718+
// Although this activation frame may not have bytecode, its code was
719+
// compiled from bytecode.
720+
if (!IsInterpreted()) {
721+
// TODO(regis): If this frame was compiled from bytecode, pc_ does not
722+
// reflect a bytecode pc. How do we map to one? We should generate new
723+
// LocalVarDescriptors for code compiled from bytecode so that they
724+
// provide deopt_id to context level mapping.
725+
UNIMPLEMENTED();
731726
}
727+
ASSERT(function().HasBytecode());
728+
Thread* thread = Thread::Current();
729+
Zone* zone = thread->zone();
730+
Bytecode& bytecode = Bytecode::Handle(zone, function().bytecode());
731+
if (!bytecode.HasLocalVariablesInfo()) {
732+
PrintDescriptorsError("Missing local variables info");
733+
}
734+
intptr_t pc_offset = pc_ - bytecode.PayloadStart();
735+
kernel::BytecodeLocalVariablesIterator local_vars(zone, bytecode);
736+
while (local_vars.MoveNext()) {
737+
if (local_vars.Kind() ==
738+
kernel::BytecodeLocalVariablesIterator::kScope) {
739+
if (local_vars.StartPC() <= pc_offset &&
740+
pc_offset < local_vars.EndPC()) {
741+
context_level_ = local_vars.ContextLevel();
742+
break;
743+
}
744+
}
745+
}
746+
if (context_level_ < 0 && function().IsClosureFunction()) {
747+
// Obtain the context level from the parent function.
748+
// TODO(alexmarkov): Define scope which includes the whole closure body.
749+
Function& parent = Function::Handle(zone, function().parent_function());
750+
intptr_t depth = 1;
751+
do {
752+
bytecode = parent.bytecode();
753+
kernel::BytecodeLocalVariablesIterator local_vars(zone, bytecode);
754+
while (local_vars.MoveNext()) {
755+
if (local_vars.Kind() ==
756+
kernel::BytecodeLocalVariablesIterator::kScope) {
757+
if (local_vars.StartTokenPos() <= TokenPos() &&
758+
TokenPos() <= local_vars.EndTokenPos()) {
759+
context_level_ = local_vars.ContextLevel() + depth;
760+
break;
761+
}
762+
}
763+
}
764+
if (context_level_ >= 0) break;
765+
parent = parent.parent_function();
766+
depth++;
767+
} while (!parent.IsNull());
768+
}
769+
if (context_level_ < 0) {
770+
PrintDescriptorsError("Missing context level in local variables info");
771+
}
772+
#else
773+
UNREACHABLE();
774+
#endif // !defined(DART_PRECOMPILED_RUNTIME)
775+
} else {
776+
ASSERT(!code_.is_optimized());
777+
GetVarDescriptors();
778+
intptr_t deopt_id = DeoptId();
779+
if (deopt_id == DeoptId::kNone) {
780+
PrintDescriptorsError("Missing deopt id");
781+
}
782+
intptr_t var_desc_len = var_descriptors_.Length();
783+
bool found = false;
784+
for (intptr_t cur_idx = 0; cur_idx < var_desc_len; cur_idx++) {
785+
RawLocalVarDescriptors::VarInfo var_info;
786+
var_descriptors_.GetInfo(cur_idx, &var_info);
787+
const int8_t kind = var_info.kind();
788+
if ((kind == RawLocalVarDescriptors::kContextLevel) &&
789+
(deopt_id >= var_info.begin_pos.value()) &&
790+
(deopt_id <= var_info.end_pos.value())) {
791+
context_level_ = var_info.index();
792+
found = true;
793+
break;
794+
}
795+
}
796+
if (!found) {
797+
PrintDescriptorsError("Missing context level");
798+
}
799+
ASSERT(context_level_ >= 0);
732800
}
733-
if (!found) {
734-
PrintDescriptorsError("Missing context level");
735-
}
736-
ASSERT(context_level_ >= 0);
737801
}
738802
return context_level_;
739803
}
@@ -1135,10 +1199,22 @@ DART_FORCE_INLINE static RawObject* GetVariableValue(uword addr) {
11351199
return *reinterpret_cast<RawObject**>(addr);
11361200
}
11371201

1202+
// Caution: GetParameter only works for fixed parameters.
11381203
RawObject* ActivationFrame::GetParameter(intptr_t index) {
11391204
intptr_t num_parameters = function().num_fixed_parameters();
11401205
ASSERT(0 <= index && index < num_parameters);
11411206

1207+
if (IsInterpreted()) {
1208+
if (function().NumOptionalParameters() > 0) {
1209+
// Note that we do not access optional but only fixed parameters, hence
1210+
// we do not need to replicate the logic of IndexFor() in bytecode reader.
1211+
return GetVariableValue(fp() + index * kWordSize);
1212+
} else {
1213+
return GetVariableValue(
1214+
fp() - (kKBCParamEndSlotFromFp + num_parameters - index) * kWordSize);
1215+
}
1216+
}
1217+
11421218
if (function().NumOptionalParameters() > 0) {
11431219
// If the function has optional parameters, the first positional parameter
11441220
// can be in a number of places in the caller's frame depending on how many
@@ -1159,6 +1235,13 @@ RawObject* ActivationFrame::GetClosure() {
11591235
}
11601236

11611237
RawObject* ActivationFrame::GetStackVar(VariableIndex variable_index) {
1238+
if (IsInterpreted()) {
1239+
intptr_t slot_index = -variable_index.value();
1240+
if (slot_index < 0) {
1241+
slot_index -= kKBCParamEndSlotFromFp; // Accessing a parameter.
1242+
}
1243+
return GetVariableValue(fp() + slot_index * kWordSize);
1244+
}
11621245
const intptr_t slot_index =
11631246
runtime_frame_layout.FrameSlotForVariableIndex(variable_index.value());
11641247
if (deopt_frame_.IsNull()) {

runtime/vm/scopes.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,7 @@ class LocalScope;
4242
// c) [LocalVariable]s referring to values on the expression stack. Those are
4343
// assigned by the flow graph builder. The indices of those variables are
4444
// assigned by the flow graph builder (it simulates the expression stack
45-
// height), they go from -NumVariabables - ExpressionHeight.
45+
// height), they go from -NumVariables - ExpressionHeight.
4646
//
4747
// -> These variables participate only partially in SSA renaming and can
4848
// therefore only be used with [LoadLocalInstr]s and with

0 commit comments

Comments
 (0)