Skip to content

Commit c0ee8f9

Browse files
bmeurerCommit Bot
bmeurer
authored and
Commit Bot
committed
[wasm][inspector] Don't use Script::source_url to store URL.
The `Script::source_url` field holds the value of the magic `//# sourceURL` comment if found, and the `Script::name` field is supposed to hold the actual name of the resource (as provided by the embedder ideally), in case of Chromium that's supposed to be the URL (in case of Node.js it's often the local path). Using `source_url` worked by chance so far, but for loading DWARF symbol files correctly we need the initiator (which we pick from the embedderName of the Script as reported to DevTools). More importantly, the partial handling of `//# sourceURL` in V8 is a layering violation and causes trouble in DevTools, i.e. when users put relative paths here. So as part of refactoring and correctifying the handling of `//# sourceURL`, we need to make sure that the embedder provided name (the URL in case of Chromium) is always stored in the `Script::name` field. Bug: chromium:1183990, chromium:974543, chromium:1174507 Change-Id: I32e11def2b9b52be11bd2e0e64a2ab6bdcf5e52d Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2773584 Commit-Queue: Benedikt Meurer <bmeurer@chromium.org> Reviewed-by: Yang Guo <yangguo@chromium.org> Reviewed-by: Clemens Backes <clemensb@chromium.org> Cr-Commit-Position: refs/heads/master@{#73536}
1 parent ce85e66 commit c0ee8f9

8 files changed

Lines changed: 46 additions & 55 deletions

File tree

‎src/api/api.cc‎

Lines changed: 5 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -7400,13 +7400,12 @@ Local<ArrayBuffer> v8::WasmMemoryObject::Buffer() {
74007400

74017401
CompiledWasmModule WasmModuleObject::GetCompiledModule() {
74027402
#if V8_ENABLE_WEBASSEMBLY
7403-
i::Handle<i::WasmModuleObject> obj =
7404-
i::Handle<i::WasmModuleObject>::cast(Utils::OpenHandle(this));
7405-
auto source_url = i::String::cast(obj->script().source_url());
7403+
auto obj = i::Handle<i::WasmModuleObject>::cast(Utils::OpenHandle(this));
7404+
auto url =
7405+
i::handle(i::String::cast(obj->script().name()), obj->GetIsolate());
74067406
int length;
7407-
std::unique_ptr<char[]> cstring = source_url.ToCString(
7408-
i::DISALLOW_NULLS, i::FAST_STRING_TRAVERSAL, &length);
7409-
i::Handle<i::String> url(source_url, obj->GetIsolate());
7407+
std::unique_ptr<char[]> cstring =
7408+
url->ToCString(i::DISALLOW_NULLS, i::FAST_STRING_TRAVERSAL, &length);
74107409
return CompiledWasmModule(std::move(obj->shared_native_module()),
74117410
cstring.get(), length);
74127411
#else

‎src/profiler/profiler-listener.cc‎

Lines changed: 3 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -232,13 +232,9 @@ void ProfilerListener::CodeCreateEvent(LogEventsAndTags tag,
232232
CodeEventsContainer evt_rec(CodeEventRecord::CODE_CREATION);
233233
CodeCreateEventRecord* rec = &evt_rec.CodeCreateEventRecord_;
234234
rec->instruction_start = code->instruction_start();
235-
// Wasm modules always have a source URL. Asm.js modules never have one.
236-
DCHECK_EQ(code->native_module()->module()->origin == wasm::kWasmOrigin,
237-
source_url != nullptr);
238-
rec->entry = new CodeEntry(
239-
tag, GetName(name),
240-
source_url ? GetName(source_url) : CodeEntry::kEmptyResourceName, 1,
241-
code_offset + 1, nullptr, true, CodeEntry::CodeType::WASM);
235+
rec->entry =
236+
new CodeEntry(tag, GetName(name), GetName(source_url), 1, code_offset + 1,
237+
nullptr, true, CodeEntry::CodeType::WASM);
242238
rec->entry->set_script_id(script_id);
243239
rec->entry->set_position(code_offset);
244240
rec->instruction_size = code->instructions().length();

‎src/wasm/module-compiler.cc‎

Lines changed: 5 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1152,12 +1152,11 @@ bool CompileLazy(Isolate* isolate, Handle<WasmModuleObject> module_object,
11521152

11531153
if (WasmCode::ShouldBeLogged(isolate)) {
11541154
DisallowGarbageCollection no_gc;
1155-
Object source_url_obj = module_object->script().source_url();
1156-
DCHECK(source_url_obj.IsString() || source_url_obj.IsUndefined());
1157-
std::unique_ptr<char[]> source_url =
1158-
source_url_obj.IsString() ? String::cast(source_url_obj).ToCString()
1159-
: nullptr;
1160-
code->LogCode(isolate, source_url.get(), module_object->script().id());
1155+
Object url_obj = module_object->script().name();
1156+
DCHECK(url_obj.IsString() || url_obj.IsUndefined());
1157+
std::unique_ptr<char[]> url =
1158+
url_obj.IsString() ? String::cast(url_obj).ToCString() : nullptr;
1159+
code->LogCode(isolate, url.get(), module_object->script().id());
11611160
}
11621161

11631162
counters->wasm_lazily_compiled_functions()->Increment();

‎src/wasm/wasm-code-manager.cc‎

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -872,11 +872,10 @@ void NativeModule::LogWasmCodes(Isolate* isolate, Script script) {
872872
TRACE_EVENT1("v8.wasm", "wasm.LogWasmCodes", "functions",
873873
module_->num_declared_functions);
874874

875-
Object source_url_obj = script.source_url();
876-
DCHECK(source_url_obj.IsString() || source_url_obj.IsUndefined());
875+
Object url_obj = script.name();
876+
DCHECK(url_obj.IsString() || url_obj.IsUndefined());
877877
std::unique_ptr<char[]> source_url =
878-
source_url_obj.IsString() ? String::cast(source_url_obj).ToCString()
879-
: nullptr;
878+
url_obj.IsString() ? String::cast(url_obj).ToCString() : nullptr;
880879

881880
// Log all owned code, not just the current entries in the code table. This
882881
// will also include import wrappers.

‎src/wasm/wasm-engine.cc‎

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -130,11 +130,10 @@ class WasmGCForegroundTask : public CancelableTask {
130130
class WeakScriptHandle {
131131
public:
132132
explicit WeakScriptHandle(Handle<Script> script) : script_id_(script->id()) {
133-
DCHECK(script->source_url().IsString() ||
134-
script->source_url().IsUndefined());
135-
if (script->source_url().IsString()) {
133+
DCHECK(script->name().IsString() || script->name().IsUndefined());
134+
if (script->name().IsString()) {
136135
std::unique_ptr<char[]> source_url =
137-
String::cast(script->source_url()).ToCString();
136+
String::cast(script->name()).ToCString();
138137
// Convert from {unique_ptr} to {shared_ptr}.
139138
source_url_ = {source_url.release(), source_url.get_deleter()};
140139
}
@@ -802,7 +801,7 @@ Handle<Script> CreateWasmScript(Isolate* isolate,
802801
.ToHandleChecked();
803802
}
804803
}
805-
script->set_source_url(*url_str);
804+
script->set_name(*url_str);
806805

807806
const WasmDebugSymbols& debug_symbols = module->debug_symbols;
808807
if (debug_symbols.type == WasmDebugSymbols::Type::SourceMap &&

‎test/cctest/wasm/test-wasm-serialization.cc‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -205,8 +205,8 @@ TEST(DeserializeWithSourceUrl) {
205205
const std::string url = "http://example.com/example.wasm";
206206
Handle<WasmModuleObject> module_object;
207207
CHECK(test.Deserialize(VectorOf(url)).ToHandle(&module_object));
208-
String source_url = String::cast(module_object->script().source_url());
209-
CHECK_EQ(url, source_url.ToCString().get());
208+
String url_str = String::cast(module_object->script().name());
209+
CHECK_EQ(url, url_str.ToCString().get());
210210
}
211211
test.CollectGarbage();
212212
}

‎test/cctest/wasm/wasm-run-utils.cc‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -564,8 +564,7 @@ void WasmFunctionCompiler::Build(const byte* start, const byte* end) {
564564
DCHECK_NOT_NULL(code);
565565
DisallowGarbageCollection no_gc;
566566
Script script = builder_->instance_object()->module_object().script();
567-
std::unique_ptr<char[]> source_url =
568-
String::cast(script.source_url()).ToCString();
567+
std::unique_ptr<char[]> source_url = String::cast(script.name()).ToCString();
569568
if (WasmCode::ShouldBeLogged(isolate())) {
570569
code->LogCode(isolate(), source_url.get(), script.id());
571570
}

‎test/mjsunit/wasm/stack.js‎

Lines changed: 23 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -88,11 +88,11 @@ Error.prepareStackTrace = function(error, frames) {
8888
module.exports.main();
8989

9090
verifyStack(stack, [
91-
// isWasm function line pos file offset funcIndex
92-
[ false, "STACK", 38, 0, "stack.js"],
93-
[ true, "main", 1, 0x86, null, '0x86', 1],
94-
[ false, "testStackFrames", 88, 0, "stack.js"],
95-
[ false, null, 97, 0, "stack.js"]
91+
// isWasm function line pos file offset funcIndex
92+
[ false, "STACK", 38, 0, "stack.js"],
93+
[ true, "main", 1, 0x86, "wasm://wasm/7168ab72", '0x86', 1],
94+
[ false, "testStackFrames", 88, 0, "stack.js"],
95+
[ false, null, 97, 0, "stack.js"]
9696
]);
9797
})();
9898

@@ -103,10 +103,10 @@ Error.prepareStackTrace = function(error, frames) {
103103
} catch (e) {
104104
assertContains("unreachable", e.message);
105105
verifyStack(e.stack, [
106-
// isWasm function line pos file offset funcIndex
107-
[ true, "exec_unreachable", 1, 0x8b, null, '0x8b', 2],
108-
[ false, "testWasmUnreachable", 101, 0, "stack.js"],
109-
[ false, null, 112, 0, "stack.js"]
106+
// isWasm function line pos file offset funcIndex
107+
[ true, "exec_unreachable", 1, 0x8b, "wasm://wasm/7168ab72", '0x8b', 2],
108+
[ false, "testWasmUnreachable", 101, 0, "stack.js"],
109+
[ false, null, 112, 0, "stack.js"]
110110
]);
111111
}
112112
})();
@@ -118,11 +118,11 @@ Error.prepareStackTrace = function(error, frames) {
118118
} catch (e) {
119119
assertContains("out of bounds", e.message);
120120
verifyStack(e.stack, [
121-
// isWasm function line pos file offset funcIndex
122-
[ true, "mem_out_of_bounds", 1, 0x91, null, '0x91', 3],
123-
[ true, "call_mem_out_of_bounds", 1, 0x97, null, '0x97', 4],
124-
[ false, "testWasmMemOutOfBounds", 116, 0, "stack.js"],
125-
[ false, null, 128, 0, "stack.js"]
121+
// isWasm function line pos file offset funcIndex
122+
[ true, "mem_out_of_bounds", 1, 0x91, "wasm://wasm/7168ab72", '0x91', 3],
123+
[ true, "call_mem_out_of_bounds", 1, 0x97, "wasm://wasm/7168ab72", '0x97', 4],
124+
[ false, "testWasmMemOutOfBounds", 116, 0, "stack.js"],
125+
[ false, null, 128, 0, "stack.js"]
126126
]);
127127
}
128128
})();
@@ -147,11 +147,11 @@ Error.prepareStackTrace = function(error, frames) {
147147
assertEquals("Maximum call stack size exceeded", e.message, "trap reason");
148148
assertTrue(e.stack.length >= 4, "expected at least 4 stack entries");
149149
verifyStack(e.stack.splice(0, 4), [
150-
// isWasm function line pos file offset funcIndex
151-
[ true, "recursion", 1, 0x34, null, '0x34', 0],
152-
[ true, "recursion", 1, 0x37, null, '0x37', 0],
153-
[ true, "recursion", 1, 0x37, null, '0x37', 0],
154-
[ true, "recursion", 1, 0x37, null, '0x37', 0]
150+
// isWasm function line pos file offset funcIndex
151+
[ true, "recursion", 1, 0x34, "wasm://wasm/80a35e5a", '0x34', 0],
152+
[ true, "recursion", 1, 0x37, "wasm://wasm/80a35e5a", '0x37', 0],
153+
[ true, "recursion", 1, 0x37, "wasm://wasm/80a35e5a", '0x37', 0],
154+
[ true, "recursion", 1, 0x37, "wasm://wasm/80a35e5a", '0x37', 0]
155155
]);
156156
}
157157
})();
@@ -175,10 +175,10 @@ Error.prepareStackTrace = function(error, frames) {
175175
assertEquals('unreachable', e.message, 'trap reason');
176176
let hexOffset = '0x' + (unreachable_pos + 0x25).toString(16);
177177
verifyStack(e.stack, [
178-
// isWasm function line pos file offset funcIndex
179-
[ true, 'main', 1, unreachable_pos + 0x25, null, hexOffset, 0],
180-
[ false, 'testBigOffset', 172, 0, 'stack.js'],
181-
[ false, null, 184, 0, 'stack.js']
178+
// isWasm function line pos file offset funcIndex
179+
[ true, 'main', 1, unreachable_pos + 0x25, 'wasm://wasm/000600e6', hexOffset, 0],
180+
[ false, 'testBigOffset', 172, 0, 'stack.js'],
181+
[ false, null, 184, 0, 'stack.js']
182182
]);
183183
}
184184
})();

0 commit comments

Comments
 (0)