Commit 93d35279 authored by Clemens Backes's avatar Clemens Backes Committed by Commit Bot

[wasm] Refactor debug side table

This refactors the debug side table such that we can easily add
register information later.
In particular
- vectors for types and stack offsets are combined into one;
- constants are stored in the same vector;
- locals and operand stack values are stored in the same vector.

A follow-up CL will extend the DebugSideTable to also encode locals
or operand stack values held in registers.

R=thibaudm@chromium.org

Bug: v8:10147, v8:10222
Change-Id: I97adb56b31afdb22896530c7ba2e8a24b5d31da9
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2062405
Commit-Queue: Clemens Backes <clemensb@chromium.org>
Reviewed-by: 's avatarThibaud Michaud <thibaudm@chromium.org>
Cr-Commit-Position: refs/heads/master@{#66374}
parent 7ccfcbb2
......@@ -157,19 +157,12 @@ class DebugSideTableBuilder {
public:
class EntryBuilder {
public:
explicit EntryBuilder(
int pc_offset, std::vector<ValueType> stack_types,
std::vector<int> stack_offsets,
std::vector<DebugSideTable::Entry::Constant> constants)
: pc_offset_(pc_offset),
stack_types_(std::move(stack_types)),
stack_offsets_(std::move(stack_offsets)),
constants_(std::move(constants)) {}
explicit EntryBuilder(int pc_offset,
std::vector<DebugSideTable::Entry::Value> values)
: pc_offset_(pc_offset), values_(std::move(values)) {}
DebugSideTable::Entry ToTableEntry() const {
return DebugSideTable::Entry{pc_offset_, std::move(stack_types_),
std::move(stack_offsets_),
std::move(constants_)};
DebugSideTable::Entry ToTableEntry() {
return DebugSideTable::Entry{pc_offset_, std::move(values_)};
}
int pc_offset() const { return pc_offset_; }
......@@ -177,9 +170,7 @@ class DebugSideTableBuilder {
private:
int pc_offset_;
std::vector<ValueType> stack_types_;
std::vector<int> stack_offsets_;
std::vector<DebugSideTable::Entry::Constant> constants_;
std::vector<DebugSideTable::Entry::Value> values_;
};
// Adds a new entry, and returns a pointer to a builder for modifying that
......@@ -188,48 +179,43 @@ class DebugSideTableBuilder {
LiftoffAssembler::VarState* stack_state) {
DCHECK_LE(num_locals, stack_height);
// Record stack types.
int stack_height_without_locals = stack_height - num_locals;
std::vector<ValueType> stack_types(stack_height_without_locals);
for (int i = 0; i < stack_height_without_locals; ++i) {
stack_types[i] = stack_state[num_locals + i].type();
}
// Record stack offsets.
std::vector<int> stack_offsets(stack_height_without_locals);
for (int i = 0; i < stack_height_without_locals; ++i) {
stack_offsets[i] = stack_state[num_locals + i].offset();
}
// Record all constants on the locals and stack.
std::vector<DebugSideTable::Entry::Constant> constants;
for (int idx = 0; idx < stack_height; ++idx) {
auto& slot = stack_state[idx];
if (slot.is_const()) constants.push_back({idx, slot.i32_const()});
}
entries_.emplace_back(pc_offset, std::move(stack_types),
std::move(stack_offsets), std::move(constants));
std::vector<DebugSideTable::Entry::Value> values(stack_height);
for (int i = 0; i < stack_height; ++i) {
const auto& slot = stack_state[i];
values[i].type = slot.type();
values[i].stack_offset = slot.offset();
if (slot.is_const()) {
values[i].kind = DebugSideTable::Entry::kConstant;
values[i].i32_const = slot.i32_const();
} else {
values[i].kind = DebugSideTable::Entry::kStack;
values[i].stack_offset = slot.offset();
}
}
entries_.emplace_back(pc_offset, std::move(values));
return &entries_.back();
}
void AddLocalType(ValueType type, int stack_offset) {
local_types_.push_back(type);
local_stack_offsets_.push_back(stack_offset);
void SetNumLocals(int num_locals) {
DCHECK_EQ(-1, num_locals_);
DCHECK_LE(0, num_locals);
num_locals_ = num_locals;
}
std::unique_ptr<DebugSideTable> GenerateDebugSideTable() {
std::vector<DebugSideTable::Entry> table_entries;
table_entries.reserve(entries_.size());
for (auto& entry : entries_) table_entries.push_back(entry.ToTableEntry());
std::sort(table_entries.begin(), table_entries.end(),
DCHECK_LE(0, num_locals_);
std::vector<DebugSideTable::Entry> entries;
entries.reserve(entries_.size());
for (auto& entry : entries_) entries.push_back(entry.ToTableEntry());
std::sort(entries.begin(), entries.end(),
[](DebugSideTable::Entry& a, DebugSideTable::Entry& b) {
return a.pc_offset() < b.pc_offset();
});
return std::make_unique<DebugSideTable>(std::move(local_types_),
std::move(local_stack_offsets_),
std::move(table_entries));
return std::make_unique<DebugSideTable>(num_locals_, std::move(entries));
}
private:
std::vector<ValueType> local_types_;
std::vector<int> local_stack_offsets_;
int num_locals_ = -1;
std::list<EntryBuilder> entries_;
};
......@@ -566,13 +552,10 @@ class LiftoffCompiler {
DCHECK_EQ(__ num_locals(), __ cache_state()->stack_height());
// Register local types and stack offsets for the debug side table.
if (V8_UNLIKELY(debug_sidetable_builder_)) {
for (uint32_t i = 0; i < __ num_locals(); ++i) {
debug_sidetable_builder_->AddLocalType(
__ local_type(i), __ cache_state()->stack_state[i].offset());
}
debug_sidetable_builder_->SetNumLocals(__ num_locals());
}
// The function-prologue stack check is associated with position 0, which
// is never a position of any instruction in the function.
StackCheck(0);
......
......@@ -579,9 +579,7 @@ class DebugInfoImpl {
const char* label = i < num_params ? "arg#%d" : "local#%d";
name = PrintFToOneByteString<true>(isolate, label, i);
}
WasmValue value =
GetValue(debug_side_table_entry, debug_side_table->local_type(i), i,
fp - debug_side_table->local_stack_offset(i));
WasmValue value = GetValue(debug_side_table_entry, i, fp);
Handle<Object> value_obj = WasmValueToValueObject(isolate, value);
// {name} can be a string representation of an element index.
LookupIterator::Key lookup_key{isolate, name};
......@@ -596,7 +594,6 @@ class DebugInfoImpl {
}
// Fill stack values.
int stack_count = debug_side_table_entry->stack_height();
// Use an object without prototype instead of an Array, for nicer displaying
// in DevTools. For Arrays, the length field and prototype is displayed,
// which does not make too much sense here.
......@@ -605,13 +602,12 @@ class DebugInfoImpl {
isolate->factory()->InternalizeString(StaticCharVector("stack"));
JSObject::AddProperty(isolate, local_scope_object, stack_name, stack_obj,
NONE);
for (int i = 0; i < stack_count; ++i) {
ValueType type = debug_side_table_entry->stack_type(i);
WasmValue value = GetValue(debug_side_table_entry, type, num_locals + i,
fp - debug_side_table_entry->stack_offset(i));
int value_count = debug_side_table_entry->num_values();
for (int i = num_locals; i < value_count; ++i) {
WasmValue value = GetValue(debug_side_table_entry, i, fp);
Handle<Object> value_obj = WasmValueToValueObject(isolate, value);
JSObject::AddDataElement(stack_obj, static_cast<uint32_t>(i), value_obj,
NONE);
JSObject::AddDataElement(stack_obj, static_cast<uint32_t>(i - num_locals),
value_obj, NONE);
}
return local_scope_object;
}
......@@ -701,16 +697,19 @@ class DebugInfoImpl {
// Get the value of a local (including parameters) or stack value. Stack
// values follow the locals in the same index space.
WasmValue GetValue(const DebugSideTable::Entry* debug_side_table_entry,
ValueType type, int index, Address stack_address) const {
if (debug_side_table_entry->IsConstant(index)) {
int index, Address stack_frame_base) const {
ValueType type = debug_side_table_entry->value_type(index);
if (debug_side_table_entry->is_constant(index)) {
DCHECK(type == kWasmI32 || type == kWasmI64);
return type == kWasmI32
? WasmValue(debug_side_table_entry->GetConstant(index))
? WasmValue(debug_side_table_entry->i32_constant(index))
: WasmValue(
int64_t{debug_side_table_entry->GetConstant(index)});
int64_t{debug_side_table_entry->i32_constant(index)});
}
// Otherwise load the value from the stack.
Address stack_address =
stack_frame_base - debug_side_table_entry->stack_offset(index);
switch (type) {
case kWasmI32:
return WasmValue(ReadUnalignedValue<int32_t>(stack_address));
......
......@@ -40,71 +40,57 @@ class DebugSideTable {
public:
class Entry {
public:
struct Constant {
int index;
int32_t i32_const;
// TODO(clemensb): Add |kRegister|.
enum ValueKind : int8_t { kConstant, kStack };
struct Value {
ValueType type;
ValueKind kind;
union {
int32_t i32_const; // if kind == kConstant
int stack_offset; // if kind == kStack
};
};
Entry(int pc_offset, std::vector<ValueType> stack_types,
std::vector<int> stack_offsets, std::vector<Constant> constants)
: pc_offset_(pc_offset),
stack_types_(std::move(stack_types)),
stack_offsets_(std::move(stack_offsets)),
constants_(std::move(constants)) {
DCHECK(std::is_sorted(constants_.begin(), constants_.end(),
ConstantIndexLess{}));
DCHECK_EQ(stack_types_.size(), stack_offsets_.size());
}
Entry(int pc_offset, std::vector<Value> values)
: pc_offset_(pc_offset), values_(std::move(values)) {}
// Constructor for map lookups (only initializes the {pc_offset_}).
explicit Entry(int pc_offset) : pc_offset_(pc_offset) {}
int pc_offset() const { return pc_offset_; }
int stack_height() const { return static_cast<int>(stack_types_.size()); }
ValueType stack_type(int stack_index) const {
return stack_types_[stack_index];
int num_values() const { return static_cast<int>(values_.size()); }
ValueType value_type(int index) const { return values_[index].type; }
auto values() const {
return base::make_iterator_range(values_.begin(), values_.end());
}
int stack_offset(int stack_index) const {
return stack_offsets_[stack_index];
int stack_offset(int index) const {
DCHECK_EQ(kStack, values_[index].kind);
return values_[index].stack_offset;
}
// {index} can point to a local or operand stack value.
bool IsConstant(int index) const {
return std::binary_search(constants_.begin(), constants_.end(),
Constant{index, 0}, ConstantIndexLess{});
bool is_constant(int index) const {
return values_[index].kind == kConstant;
}
int32_t GetConstant(int index) const {
DCHECK(IsConstant(index));
auto it = std::lower_bound(constants_.begin(), constants_.end(),
Constant{index, 0}, ConstantIndexLess{});
DCHECK_NE(it, constants_.end());
DCHECK_EQ(it->index, index);
return it->i32_const;
int32_t i32_constant(int index) const {
DCHECK_EQ(kConstant, values_[index].kind);
return values_[index].i32_const;
}
private:
struct ConstantIndexLess {
bool operator()(const Constant& a, const Constant& b) const {
return a.index < b.index;
}
};
int pc_offset_;
// TODO(clemensb): Merge these vectors into one.
std::vector<ValueType> stack_types_;
std::vector<int> stack_offsets_;
std::vector<Constant> constants_;
std::vector<Value> values_;
};
// Technically it would be fine to copy this class, but there should not be a
// reason to do so, hence mark it move only.
MOVE_ONLY_NO_DEFAULT_CONSTRUCTOR(DebugSideTable);
explicit DebugSideTable(std::vector<ValueType> local_types,
std::vector<int> local_stack_offsets,
std::vector<Entry> entries)
: local_types_(std::move(local_types)),
local_stack_offsets_(std::move(local_stack_offsets)),
entries_(std::move(entries)) {
explicit DebugSideTable(int num_locals, std::vector<Entry> entries)
: num_locals_(num_locals), entries_(std::move(entries)) {
DCHECK(
std::is_sorted(entries_.begin(), entries_.end(), EntryPositionLess{}));
}
......@@ -113,6 +99,7 @@ class DebugSideTable {
auto it = std::lower_bound(entries_.begin(), entries_.end(),
Entry{pc_offset}, EntryPositionLess{});
if (it == entries_.end() || it->pc_offset() != pc_offset) return nullptr;
DCHECK_LE(num_locals_, it->num_values());
return &*it;
}
......@@ -120,12 +107,7 @@ class DebugSideTable {
return base::make_iterator_range(entries_.begin(), entries_.end());
}
size_t num_entries() const { return entries_.size(); }
int num_locals() const { return static_cast<int>(local_types_.size()); }
ValueType local_type(int index) const { return local_types_[index]; }
int local_stack_offset(int index) const {
return local_stack_offsets_[index];
}
int num_locals() const { return num_locals_; }
private:
struct EntryPositionLess {
......@@ -134,8 +116,7 @@ class DebugSideTable {
}
};
std::vector<ValueType> local_types_;
std::vector<int32_t> local_stack_offsets_;
int num_locals_;
std::vector<Entry> entries_;
};
......
......@@ -122,30 +122,46 @@ class LiftoffCompileEnvironment {
};
struct DebugSideTableEntry {
std::vector<ValueType> stack_types;
std::vector<std::pair<int, int>> constants;
std::vector<DebugSideTable::Entry::Value> values;
// Construct via vector or implicitly via initializer list.
explicit DebugSideTableEntry(std::vector<DebugSideTable::Entry::Value> values)
: values(std::move(values)) {}
DebugSideTableEntry(
std::initializer_list<DebugSideTable::Entry::Value> values)
: values(values) {}
bool operator==(const DebugSideTableEntry& other) const {
return stack_types == other.stack_types && constants == other.constants;
if (values.size() != other.values.size()) return false;
for (size_t i = 0; i < values.size(); ++i) {
if (values[i].type != other.values[i].type) return false;
if (values[i].kind != other.values[i].kind) return false;
// Stack offsets and register codes are platform dependent, so only check
// constants here.
if (values[i].kind == DebugSideTable::Entry::kConstant &&
values[i].i32_const != other.values[i].i32_const) {
return false;
}
}
return true;
}
};
// Debug builds will print the vector of DebugSideTableEntry.
#ifdef DEBUG
std::ostream& operator<<(std::ostream& out, const DebugSideTableEntry& entry) {
out << "{stack types [";
out << "{";
const char* comma = "";
for (ValueType type : entry.stack_types) {
out << comma << ValueTypes::TypeName(type);
comma = ", ";
}
comma = "";
out << "], constants: [";
for (auto& c : entry.constants) {
out << comma << "<" << c.first << ", " << c.second << ">";
for (auto& v : entry.values) {
out << comma << ValueTypes::TypeName(v.type) << " ";
if (v.kind == DebugSideTable::Entry::kConstant) {
out << "const:" << v.i32_const;
} else {
out << "stack";
}
comma = ", ";
}
return out << "]}";
return out << "}";
}
std::ostream& operator<<(std::ostream& out,
......@@ -154,28 +170,30 @@ std::ostream& operator<<(std::ostream& out,
}
#endif // DEBUG
void CheckDebugSideTable(std::vector<ValueType> expected_local_types,
std::vector<DebugSideTableEntry> expected_entries,
// Named constructors to make the tests more readable.
DebugSideTable::Entry::Value Constant(ValueType type, int32_t constant) {
DebugSideTable::Entry::Value value;
value.type = type;
value.kind = DebugSideTable::Entry::kConstant;
value.i32_const = constant;
return value;
}
DebugSideTable::Entry::Value Stack(ValueType type) {
DebugSideTable::Entry::Value value;
value.type = type;
value.kind = DebugSideTable::Entry::kStack;
return value;
}
void CheckDebugSideTable(std::vector<DebugSideTableEntry> expected_entries,
const wasm::DebugSideTable* debug_side_table) {
std::vector<ValueType> local_types;
for (int i = 0; i < debug_side_table->num_locals(); ++i) {
local_types.push_back(debug_side_table->local_type(i));
}
std::vector<DebugSideTableEntry> entries;
for (auto& entry : debug_side_table->entries()) {
std::vector<ValueType> stack_types;
for (int i = 0; i < entry.stack_height(); ++i) {
stack_types.push_back(entry.stack_type(i));
}
std::vector<std::pair<int, int>> constants;
int locals_plus_stack =
debug_side_table->num_locals() + entry.stack_height();
for (int i = 0; i < locals_plus_stack; ++i) {
if (entry.IsConstant(i)) constants.emplace_back(i, entry.GetConstant(i));
}
entries.push_back({std::move(stack_types), std::move(constants)});
auto values = entry.values();
entries.push_back(
DebugSideTableEntry{std::vector<DebugSideTable::Entry::Value>{
values.begin(), values.end()}});
}
CHECK_EQ(expected_local_types, local_types);
CHECK_EQ(expected_entries, entries);
}
......@@ -223,12 +241,12 @@ TEST(Liftoff_debug_side_table_simple) {
auto debug_side_table = env.GenerateDebugSideTable(
{kWasmI32}, {kWasmI32, kWasmI32},
{WASM_I32_ADD(WASM_GET_LOCAL(0), WASM_GET_LOCAL(1))});
CheckDebugSideTable({kWasmI32, kWasmI32},
{
// OOL stack check, stack: {}
{{}, {}},
},
debug_side_table.get());
CheckDebugSideTable(
{
// OOL stack check, locals spilled, stack empty.
{Stack(kWasmI32), Stack(kWasmI32)},
},
debug_side_table.get());
}
TEST(Liftoff_debug_side_table_call) {
......@@ -237,14 +255,14 @@ TEST(Liftoff_debug_side_table_call) {
{kWasmI32}, {kWasmI32},
{WASM_I32_ADD(WASM_CALL_FUNCTION(0, WASM_GET_LOCAL(0)),
WASM_GET_LOCAL(0))});
CheckDebugSideTable({kWasmI32},
{
// call, stack: {}
{{}, {}},
// OOL stack check, stack: {}
{{}, {}},
},
debug_side_table.get());
CheckDebugSideTable(
{
// call, local spilled, stack empty.
{Stack(kWasmI32)},
// OOL stack check, local spilled, stack empty.
{Stack(kWasmI32)},
},
debug_side_table.get());
}
TEST(Liftoff_debug_side_table_call_const) {
......@@ -255,14 +273,14 @@ TEST(Liftoff_debug_side_table_call_const) {
{WASM_SET_LOCAL(0, WASM_I32V_1(kConst)),
WASM_I32_ADD(WASM_CALL_FUNCTION(0, WASM_GET_LOCAL(0)),
WASM_GET_LOCAL(0))});
CheckDebugSideTable({kWasmI32},
{
// call, stack: {}, local0 is kConst
{{}, {{0, kConst}}},
// OOL stack check, stack: {}
{{}, {}},
},
debug_side_table.get());
CheckDebugSideTable(
{
// call, local is kConst.
{Constant(kWasmI32, kConst)},
// OOL stack check, local spilled.
{Stack(kWasmI32)},
},
debug_side_table.get());
}
TEST(Liftoff_debug_side_table_indirect_call) {
......@@ -272,18 +290,18 @@ TEST(Liftoff_debug_side_table_indirect_call) {
{kWasmI32}, {kWasmI32},
{WASM_I32_ADD(WASM_CALL_INDIRECT(0, WASM_I32V_1(47), WASM_GET_LOCAL(0)),
WASM_GET_LOCAL(0))});
CheckDebugSideTable({kWasmI32},
{
// indirect call, stack: {}
{{}, {}},
// OOL stack check, stack: {}
{{}, {}},
// OOL trap (invalid index), stack: {kConst}
{{kWasmI32}, {{1, kConst}}},
// OOL trap (sig mismatch), stack: {kConst}
{{kWasmI32}, {{1, kConst}}},
},
debug_side_table.get());
CheckDebugSideTable(
{
// indirect call, local spilled, stack empty.
{Stack(kWasmI32)},
// OOL stack check, local spilled, stack empty.
{Stack(kWasmI32)},
// OOL trap (invalid index), local spilled, stack has {kConst}.
{Stack(kWasmI32), Constant(kWasmI32, kConst)},
// OOL trap (sig mismatch), local spilled, stack has {kConst}.
{Stack(kWasmI32), Constant(kWasmI32, kConst)},
},
debug_side_table.get());
}
TEST(Liftoff_debug_side_table_loop) {
......@@ -292,14 +310,14 @@ TEST(Liftoff_debug_side_table_loop) {
auto debug_side_table = env.GenerateDebugSideTable(
{kWasmI32}, {kWasmI32},
{WASM_I32V_1(kConst), WASM_LOOP(WASM_BR_IF(0, WASM_GET_LOCAL(0)))});
CheckDebugSideTable({kWasmI32},
{
// OOL stack check, stack: {}
{{}, {}},
// OOL loop stack check, stack: {kConst}
{{kWasmI32}, {{1, kConst}}},
},
debug_side_table.get());
CheckDebugSideTable(
{
// OOL stack check, local spilled, stack empty.
{Stack(kWasmI32)},
// OOL loop stack check, local spilled, stack has {kConst}.
{Stack(kWasmI32), Constant(kWasmI32, kConst)},
},
debug_side_table.get());
}
TEST(Liftoff_debug_side_table_trap) {
......@@ -307,16 +325,16 @@ TEST(Liftoff_debug_side_table_trap) {
auto debug_side_table = env.GenerateDebugSideTable(
{kWasmI32}, {kWasmI32, kWasmI32},
{WASM_I32_DIVS(WASM_GET_LOCAL(0), WASM_GET_LOCAL(1))});
CheckDebugSideTable({kWasmI32, kWasmI32},
{
// OOL stack check, stack: {}
{{}, {}},
// OOL trap (div by zero), stack: {}
{{}, {}},
// OOL trap (result unrepresentable), stack: {}
{{}, {}},
},
debug_side_table.get());
CheckDebugSideTable(
{
// OOL stack check, local spilled, stack empty.
{Stack(kWasmI32), Stack(kWasmI32)},
// OOL trap (div by zero), locals spilled, stack empty.
{Stack(kWasmI32), Stack(kWasmI32)},
// OOL trap (result unrepresentable), locals spilled, stack empty.
{Stack(kWasmI32), Stack(kWasmI32)},
},
debug_side_table.get());
}
} // namespace wasm
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment