Skip to content

Commit 5c23e6b

Browse files
Eric LeeseCommit Bot
Eric Leese
authored and
Commit Bot
committed
V8 Wasm locations should always be based on byte offsets
Currently there are two ways wasm locations are represented in the inspector. This remains unchanged for now. Also, currently there are multiple ways location is represented within V8, with the line number sometimes being a function index and sometimes being 0, and the column number being a byte offset which is sometimes function relative and sometimes module relative. With this change, the line number is never used within V8 (it is always 0), and the column number is always a byte offset from the beginning of the module. This simplifies translation logic and keeps it in one place, and will simplify future changes to wasm location representation in the inspector API. Bug: chromium:1013527 Change-Id: I8813d47c881988f9ab49d7529fb81fe10dbbccff Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1886915 Commit-Queue: Eric Leese <leese@chromium.org> Reviewed-by: Simon Zünd <szuend@chromium.org> Reviewed-by: Yang Guo <yangguo@chromium.org> Reviewed-by: Clemens Backes <clemensb@chromium.org> Cr-Commit-Position: refs/heads/master@{#64774}
1 parent 4fa5f2b commit 5c23e6b

31 files changed

Lines changed: 234 additions & 152 deletions

‎include/v8.h‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2013,6 +2013,12 @@ class V8_EXPORT Message {
20132013
*/
20142014
int GetEndPosition() const;
20152015

2016+
/**
2017+
* Returns the Wasm function index where the error occurred. Returns -1 if
2018+
* message is not from a Wasm script.
2019+
*/
2020+
int GetWasmFunctionIndex() const;
2021+
20162022
/**
20172023
* Returns the error level of the message.
20182024
*/
@@ -2045,6 +2051,7 @@ class V8_EXPORT Message {
20452051
static const int kNoLineNumberInfo = 0;
20462052
static const int kNoColumnInfo = 0;
20472053
static const int kNoScriptIdInfo = 0;
2054+
static const int kNoWasmFunctionIndexInfo = -1;
20482055
};
20492056

20502057

‎src/api/api.cc‎

Lines changed: 31 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -2920,6 +2920,26 @@ int Message::GetStartColumn() const {
29202920
return self->GetColumnNumber();
29212921
}
29222922

2923+
int Message::GetWasmFunctionIndex() const {
2924+
auto self = Utils::OpenHandle(this);
2925+
i::Isolate* isolate = self->GetIsolate();
2926+
ENTER_V8_NO_SCRIPT_NO_EXCEPTION(isolate);
2927+
EscapableHandleScope handle_scope(reinterpret_cast<Isolate*>(isolate));
2928+
i::JSMessageObject::EnsureSourcePositionsAvailable(isolate, self);
2929+
int start_position = self->GetColumnNumber();
2930+
if (start_position == -1) return Message::kNoWasmFunctionIndexInfo;
2931+
2932+
i::Handle<i::Script> script(self->script(), isolate);
2933+
2934+
if (script->type() != i::Script::TYPE_WASM) {
2935+
return Message::kNoWasmFunctionIndexInfo;
2936+
}
2937+
2938+
auto debug_script = ToApiHandle<debug::Script>(script);
2939+
return Local<debug::WasmScript>::Cast(debug_script)
2940+
->GetContainingFunction(start_position);
2941+
}
2942+
29232943
Maybe<int> Message::GetStartColumn(Local<Context> context) const {
29242944
return Just(GetStartColumn());
29252945
}
@@ -9446,12 +9466,6 @@ bool debug::Script::GetPossibleBreakpoints(
94469466
int debug::Script::GetSourceOffset(const debug::Location& location) const {
94479467
i::Handle<i::Script> script = Utils::OpenHandle(this);
94489468
if (script->type() == i::Script::TYPE_WASM) {
9449-
if (this->SourceMappingURL().IsEmpty()) {
9450-
i::wasm::NativeModule* native_module = script->wasm_native_module();
9451-
const i::wasm::WasmModule* module = native_module->module();
9452-
return i::wasm::GetWasmFunctionOffset(module, location.GetLineNumber()) +
9453-
location.GetColumnNumber();
9454-
}
94559469
DCHECK_EQ(0, location.GetLineNumber());
94569470
return location.GetColumnNumber();
94579471
}
@@ -9576,6 +9590,17 @@ std::pair<int, int> debug::WasmScript::GetFunctionRange(
95769590
static_cast<int>(func.code.end_offset()));
95779591
}
95789592

9593+
int debug::WasmScript::GetContainingFunction(int byte_offset) const {
9594+
i::DisallowHeapAllocation no_gc;
9595+
i::Handle<i::Script> script = Utils::OpenHandle(this);
9596+
DCHECK_EQ(i::Script::TYPE_WASM, script->type());
9597+
i::wasm::NativeModule* native_module = script->wasm_native_module();
9598+
const i::wasm::WasmModule* module = native_module->module();
9599+
DCHECK_LE(0, byte_offset);
9600+
9601+
return i::wasm::GetContainingWasmFunction(module, byte_offset);
9602+
}
9603+
95799604
uint32_t debug::WasmScript::GetFunctionHash(int function_index) {
95809605
i::DisallowHeapAllocation no_gc;
95819606
i::Handle<i::Script> script = Utils::OpenHandle(this);

‎src/builtins/base.tq‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1210,6 +1210,7 @@ extern class StackFrameInfo extends Struct {
12101210
column_number: Smi;
12111211
promise_all_index: Smi;
12121212
script_id: Smi;
1213+
wasm_function_index: Smi;
12131214
script_name: String|Null|Undefined;
12141215
script_name_or_source_url: String|Null|Undefined;
12151216
function_name: String|Null|Undefined;

‎src/d8/d8.cc‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1680,9 +1680,9 @@ void Shell::ReportException(Isolate* isolate, v8::TryCatch* try_catch) {
16801680
printf("%s\n", exception_string);
16811681
} else if (message->GetScriptOrigin().Options().IsWasm()) {
16821682
// Print wasm-function[(function index)]:(offset): (message).
1683-
int function_index = message->GetLineNumber(context).FromJust() - 1;
1683+
int function_index = message->GetWasmFunctionIndex();
16841684
int offset = message->GetStartColumn(context).FromJust();
1685-
printf("wasm-function[%d]:%d: %s\n", function_index, offset,
1685+
printf("wasm-function[%d]:0x%x: %s\n", function_index, offset,
16861686
exception_string);
16871687
} else {
16881688
// Print (filename):(line number): (message).

‎src/debug/debug-interface.h‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -162,6 +162,7 @@ class WasmScript : public Script {
162162
MemorySpan<const uint8_t> Bytecode() const;
163163

164164
std::pair<int, int> GetFunctionRange(int function_index) const;
165+
int GetContainingFunction(int byte_offset) const;
165166

166167
debug::WasmDisassembly DisassembleFunction(int function_index) const;
167168
uint32_t GetFunctionHash(int function_index);

‎src/execution/messages.cc‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -320,6 +320,8 @@ Handle<Object> StackFrameBase::GetWasmModuleName() {
320320
return isolate_->factory()->undefined_value();
321321
}
322322

323+
int StackFrameBase::GetWasmFunctionIndex() { return StackFrameBase::kNone; }
324+
323325
Handle<Object> StackFrameBase::GetWasmInstance() {
324326
return isolate_->factory()->undefined_value();
325327
}

‎src/execution/messages.h‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -83,6 +83,8 @@ class StackFrameBase {
8383
virtual int GetLineNumber() = 0;
8484
// Return 1-based column number, including column offset if first line.
8585
virtual int GetColumnNumber() = 0;
86+
// Return 0-based Wasm function index. Returns -1 for non-Wasm frames.
87+
virtual int GetWasmFunctionIndex();
8688

8789
// Returns index for Promise.all() async frames, or -1 for other frames.
8890
virtual int GetPromiseIndex() const = 0;
@@ -174,8 +176,9 @@ class WasmStackFrame : public StackFrameBase {
174176
Handle<Object> GetWasmInstance() override;
175177

176178
int GetPosition() const override;
177-
int GetLineNumber() override { return wasm_func_index_; }
179+
int GetLineNumber() override { return 0; }
178180
int GetColumnNumber() override;
181+
int GetWasmFunctionIndex() override { return wasm_func_index_; }
179182

180183
int GetPromiseIndex() const override { return GetPosition(); }
181184

‎src/heap/factory.cc‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3693,6 +3693,7 @@ Handle<StackFrameInfo> Factory::NewStackFrameInfo(
36933693

36943694
int line = frame->GetLineNumber();
36953695
int column = frame->GetColumnNumber();
3696+
int wasm_function_index = frame->GetWasmFunctionIndex();
36963697

36973698
const int script_id = frame->GetScriptId();
36983699

@@ -3744,6 +3745,7 @@ Handle<StackFrameInfo> Factory::NewStackFrameInfo(
37443745
info->set_is_user_java_script(is_user_java_script);
37453746
info->set_line_number(line);
37463747
info->set_column_number(column);
3748+
info->set_wasm_function_index(wasm_function_index);
37473749
info->set_script_id(script_id);
37483750

37493751
info->set_script_name(*script_name);

‎src/inspector/v8-debugger-script.cc‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -394,9 +394,9 @@ class WasmVirtualScript : public V8DebuggerScript {
394394

395395
v8::debug::Location translatedEnd = end;
396396
if (translatedEnd.IsEmpty()) {
397-
// Stop before the start of the next function.
398-
translatedEnd =
399-
v8::debug::Location(translatedStart.GetLineNumber() + 1, 0);
397+
// Stop at the end of the function.
398+
int end_offset = m_wasmTranslation->GetEndOffset(scriptId());
399+
translatedEnd = v8::debug::Location(0, end_offset);
400400
} else {
401401
TranslateProtocolLocationToV8Location(m_wasmTranslation, &translatedEnd,
402402
scriptId(), v8ScriptId);

‎src/inspector/wasm-translation.cc‎

Lines changed: 40 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -102,9 +102,17 @@ class WasmTranslation::TranslatorImpl {
102102
}
103103

104104
void Translate(TransLocation* loc) {
105-
const OffsetTable& offset_table = GetOffsetTable(loc);
105+
v8::Isolate* isolate = loc->translation->isolate_;
106+
int func_index = GetFunctionIndexFromLocation(loc);
107+
DCHECK_GE(func_index, 0);
108+
const OffsetTable& offset_table = GetOffsetTable(isolate, func_index);
106109
DCHECK(!offset_table.empty());
107-
uint32_t byte_offset = static_cast<uint32_t>(loc->column);
110+
std::pair<int, int> func_range =
111+
script_.Get(isolate)->GetFunctionRange(func_index);
112+
DCHECK_LE(func_range.first, loc->column);
113+
DCHECK_LT(loc->column, func_range.second);
114+
uint32_t byte_offset =
115+
static_cast<uint32_t>(loc->column - func_range.first);
108116

109117
// Binary search for the given offset.
110118
unsigned left = 0; // inclusive
@@ -143,25 +151,25 @@ class WasmTranslation::TranslatorImpl {
143151
// Binary search for the given line and column.
144152
auto element = std::lower_bound(reverse_table.begin(), reverse_table.end(),
145153
*loc, LessThan);
154+
std::pair<int, int> func_range =
155+
script_.Get(isolate)->GetFunctionRange(func_index);
156+
DCHECK_LE(func_range.first, func_range.second);
146157

147-
int found_byte_offset = 0;
158+
int found_byte_offset = func_range.first;
148159
// We want an entry on the same line if possible.
149160
if (element == reverse_table.end()) {
150161
// We did not find an element, so this points after the function.
151-
std::pair<int, int> func_range =
152-
script_.Get(isolate)->GetFunctionRange(func_index);
153-
DCHECK_LE(func_range.first, func_range.second);
154-
found_byte_offset = func_range.second - func_range.first;
162+
found_byte_offset = func_range.second;
155163
} else if (element->line == loc->line || element == reverse_table.begin()) {
156-
found_byte_offset = element->byte_offset;
164+
found_byte_offset = element->byte_offset + func_range.first;
157165
} else {
158166
auto prev = element - 1;
159167
DCHECK(prev->line == loc->line);
160-
found_byte_offset = prev->byte_offset;
168+
found_byte_offset = prev->byte_offset + func_range.first;
161169
}
162170

163171
loc->script_id = String16::fromInteger(script_.Get(isolate)->Id());
164-
loc->line = func_index;
172+
loc->line = 0;
165173
loc->column = found_byte_offset;
166174
}
167175

@@ -197,6 +205,7 @@ class WasmTranslation::TranslatorImpl {
197205
}
198206

199207
private:
208+
friend class WasmTranslation;
200209
String16 GetFakeScriptUrl(v8::Isolate* isolate, int func_index) {
201210
v8::Local<v8::debug::WasmScript> script = script_.Get(isolate);
202211
String16 script_name =
@@ -222,7 +231,7 @@ class WasmTranslation::TranslatorImpl {
222231
return String16::concat(script_id, '-', String16::fromInteger(func_index));
223232
}
224233
String16 GetFakeScriptId(const TransLocation* loc) {
225-
return GetFakeScriptId(loc->script_id, loc->line);
234+
return GetFakeScriptId(loc->script_id, GetFunctionIndexFromLocation(loc));
226235
}
227236

228237
void ReportFakeScript(v8::Isolate* isolate,
@@ -249,10 +258,15 @@ class WasmTranslation::TranslatorImpl {
249258
return func_index;
250259
}
251260

252-
const OffsetTable& GetOffsetTable(const TransLocation* loc) {
253-
int func_index = loc->line;
254-
return GetSourceInformation(loc->translation->isolate_, func_index)
255-
.offset_table;
261+
int GetFunctionIndexFromLocation(const TransLocation* loc) {
262+
DCHECK_EQ(0, loc->line);
263+
v8::Isolate* isolate = loc->translation->isolate_;
264+
v8::Local<v8::debug::WasmScript> script = script_.Get(isolate);
265+
return script->GetContainingFunction(loc->column);
266+
}
267+
268+
const OffsetTable& GetOffsetTable(v8::Isolate* isolate, int func_index) {
269+
return GetSourceInformation(isolate, func_index).offset_table;
256270
}
257271

258272
const OffsetTable& GetReverseTable(v8::Isolate* isolate, int func_index) {
@@ -384,6 +398,17 @@ bool WasmTranslation::TranslateProtocolLocationToWasmScriptLocation(
384398
return true;
385399
}
386400

401+
// Find the end byte offset for a fake script.
402+
int WasmTranslation::GetEndOffset(const String16& script_id) {
403+
auto it = fake_scripts_.find(script_id);
404+
DCHECK_NE(fake_scripts_.end(), it);
405+
TranslatorImpl* translator = it->second;
406+
int func_index = translator->GetFunctionIndexFromFakeScriptId(script_id);
407+
std::pair<int, int> func_range =
408+
translator->script_.Get(isolate_)->GetFunctionRange(func_index);
409+
return func_range.second;
410+
}
411+
387412
void WasmTranslation::AddFakeScript(const String16& scriptId,
388413
TranslatorImpl* translator) {
389414
DCHECK_EQ(0, fake_scripts_.count(scriptId));

0 commit comments

Comments
 (0)