Commit e3f21e6d authored by Benedikt Meurer's avatar Benedikt Meurer Committed by V8 LUCI CQ

[wasm] Consider only function names from the name section.

As per WebAssembly Web API[1], the engine should only consider names
from the name section to synthesize function names in the context of
call stacks. We previously also added support to harvest the exports
table here in an attempt to improve the DevTools debugging experience,
but that needs a separate fix specifically for the inspector (which
should also take into account the imports to harvest names).

[1]: https://webassembly.github.io/spec/web-api/index.html#conventions

Fixed: chromium:1164305
Change-Id: I4bde5c8398a5164f1d8ac9060ad3743ed494c41e
Bug: chromium:1159307, chromium:1164241, chromium:1071432
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2874464
Commit-Queue: Benedikt Meurer <bmeurer@chromium.org>
Commit-Queue: Clemens Backes <clemensb@chromium.org>
Auto-Submit: Benedikt Meurer <bmeurer@chromium.org>
Reviewed-by: 's avatarClemens Backes <clemensb@chromium.org>
Cr-Commit-Position: refs/heads/master@{#74382}
parent 912118c0
...@@ -158,8 +158,7 @@ RUNTIME_FUNCTION(Runtime_WasmTraceEnter) { ...@@ -158,8 +158,7 @@ RUNTIME_FUNCTION(Runtime_WasmTraceEnter) {
wasm::ModuleWireBytes wire_bytes = wasm::ModuleWireBytes wire_bytes =
wasm::ModuleWireBytes(frame->native_module()->wire_bytes()); wasm::ModuleWireBytes(frame->native_module()->wire_bytes());
wasm::WireBytesRef name_ref = wasm::WireBytesRef name_ref =
module->lazily_generated_names.LookupFunctionName( module->lazily_generated_names.LookupFunctionName(wire_bytes, func_index);
wire_bytes, func_index, VectorOf(module->export_table));
wasm::WasmName name = wire_bytes.GetNameOrNull(name_ref); wasm::WasmName name = wire_bytes.GetNameOrNull(name_ref);
wasm::WasmCode* code = frame->wasm_code(); wasm::WasmCode* code = frame->wasm_code();
......
...@@ -2388,8 +2388,7 @@ bool FindNameSection(Decoder* decoder) { ...@@ -2388,8 +2388,7 @@ bool FindNameSection(Decoder* decoder) {
} // namespace } // namespace
void DecodeFunctionNames(const byte* module_start, const byte* module_end, void DecodeFunctionNames(const byte* module_start, const byte* module_end,
std::unordered_map<uint32_t, WireBytesRef>* names, std::unordered_map<uint32_t, WireBytesRef>* names) {
const Vector<const WasmExport> export_table) {
DCHECK_NOT_NULL(names); DCHECK_NOT_NULL(names);
DCHECK(names->empty()); DCHECK(names->empty());
...@@ -2421,13 +2420,6 @@ void DecodeFunctionNames(const byte* module_start, const byte* module_end, ...@@ -2421,13 +2420,6 @@ void DecodeFunctionNames(const byte* module_start, const byte* module_end,
} }
} }
} }
// Extract from export table.
for (const WasmExport& exp : export_table) {
if (exp.kind == kExternalFunction && names->count(exp.index) == 0) {
names->insert(std::make_pair(exp.index, exp.name));
}
}
} }
NameMap DecodeNameMap(Vector<const uint8_t> module_bytes, NameMap DecodeNameMap(Vector<const uint8_t> module_bytes,
......
...@@ -187,13 +187,11 @@ V8_EXPORT_PRIVATE std::vector<CustomSectionOffset> DecodeCustomSections( ...@@ -187,13 +187,11 @@ V8_EXPORT_PRIVATE std::vector<CustomSectionOffset> DecodeCustomSections(
// function. // function.
AsmJsOffsetsResult DecodeAsmJsOffsets(Vector<const uint8_t> encoded_offsets); AsmJsOffsetsResult DecodeAsmJsOffsets(Vector<const uint8_t> encoded_offsets);
// Decode the function names from the name section and also look at export // Decode the function names from the name section. Returns the result as an
// table. Returns the result as an unordered map. Only names with valid utf8 // unordered map. Only names with valid utf8 encoding are stored and conflicts
// encoding are stored and conflicts are resolved by choosing the last name // are resolved by choosing the last name read.
// read.
void DecodeFunctionNames(const byte* module_start, const byte* module_end, void DecodeFunctionNames(const byte* module_start, const byte* module_end,
std::unordered_map<uint32_t, WireBytesRef>* names, std::unordered_map<uint32_t, WireBytesRef>* names);
const Vector<const WasmExport> export_table);
// Decode the requested subsection of the name section. // Decode the requested subsection of the name section.
// The result will be empty if no name section is present. On encountering an // The result will be empty if no name section is present. On encountering an
......
...@@ -225,8 +225,8 @@ void WasmCode::LogCode(Isolate* isolate, const char* source_url, ...@@ -225,8 +225,8 @@ void WasmCode::LogCode(Isolate* isolate, const char* source_url,
ModuleWireBytes wire_bytes(native_module_->wire_bytes()); ModuleWireBytes wire_bytes(native_module_->wire_bytes());
const WasmModule* module = native_module_->module(); const WasmModule* module = native_module_->module();
WireBytesRef name_ref = module->lazily_generated_names.LookupFunctionName( WireBytesRef name_ref =
wire_bytes, index(), VectorOf(module->export_table)); module->lazily_generated_names.LookupFunctionName(wire_bytes, index());
WasmName name = wire_bytes.GetNameOrNull(name_ref); WasmName name = wire_bytes.GetNameOrNull(name_ref);
const WasmDebugSymbols& debug_symbols = module->debug_symbols; const WasmDebugSymbols& debug_symbols = module->debug_symbols;
......
...@@ -32,13 +32,12 @@ namespace internal { ...@@ -32,13 +32,12 @@ namespace internal {
namespace wasm { namespace wasm {
WireBytesRef LazilyGeneratedNames::LookupFunctionName( WireBytesRef LazilyGeneratedNames::LookupFunctionName(
const ModuleWireBytes& wire_bytes, uint32_t function_index, const ModuleWireBytes& wire_bytes, uint32_t function_index) const {
Vector<const WasmExport> export_table) const {
base::MutexGuard lock(&mutex_); base::MutexGuard lock(&mutex_);
if (!function_names_) { if (!function_names_) {
function_names_.reset(new std::unordered_map<uint32_t, WireBytesRef>()); function_names_.reset(new std::unordered_map<uint32_t, WireBytesRef>());
DecodeFunctionNames(wire_bytes.start(), wire_bytes.end(), DecodeFunctionNames(wire_bytes.start(), wire_bytes.end(),
function_names_.get(), export_table); function_names_.get());
} }
auto it = function_names_->find(function_index); auto it = function_names_->find(function_index);
if (it == function_names_->end()) return WireBytesRef(); if (it == function_names_->end()) return WireBytesRef();
...@@ -179,7 +178,7 @@ WasmName ModuleWireBytes::GetNameOrNull(WireBytesRef ref) const { ...@@ -179,7 +178,7 @@ WasmName ModuleWireBytes::GetNameOrNull(WireBytesRef ref) const {
WasmName ModuleWireBytes::GetNameOrNull(const WasmFunction* function, WasmName ModuleWireBytes::GetNameOrNull(const WasmFunction* function,
const WasmModule* module) const { const WasmModule* module) const {
return GetNameOrNull(module->lazily_generated_names.LookupFunctionName( return GetNameOrNull(module->lazily_generated_names.LookupFunctionName(
*this, function->func_index, VectorOf(module->export_table))); *this, function->func_index));
} }
std::ostream& operator<<(std::ostream& os, const WasmFunctionName& name) { std::ostream& operator<<(std::ostream& os, const WasmFunctionName& name) {
......
...@@ -192,8 +192,7 @@ struct ModuleWireBytes; ...@@ -192,8 +192,7 @@ struct ModuleWireBytes;
class V8_EXPORT_PRIVATE LazilyGeneratedNames { class V8_EXPORT_PRIVATE LazilyGeneratedNames {
public: public:
WireBytesRef LookupFunctionName(const ModuleWireBytes& wire_bytes, WireBytesRef LookupFunctionName(const ModuleWireBytes& wire_bytes,
uint32_t function_index, uint32_t function_index) const;
Vector<const WasmExport> export_table) const;
void AddForTesting(int function_index, WireBytesRef name); void AddForTesting(int function_index, WireBytesRef name);
......
...@@ -226,7 +226,7 @@ MaybeHandle<String> WasmModuleObject::GetFunctionNameOrNull( ...@@ -226,7 +226,7 @@ MaybeHandle<String> WasmModuleObject::GetFunctionNameOrNull(
wasm::WireBytesRef name = wasm::WireBytesRef name =
module_object->module()->lazily_generated_names.LookupFunctionName( module_object->module()->lazily_generated_names.LookupFunctionName(
wasm::ModuleWireBytes(module_object->native_module()->wire_bytes()), wasm::ModuleWireBytes(module_object->native_module()->wire_bytes()),
func_index, VectorOf(module_object->module()->export_table)); func_index);
if (!name.is_set()) return {}; if (!name.is_set()) return {};
return ExtractUtf8StringFromModuleBytes(isolate, module_object, name, return ExtractUtf8StringFromModuleBytes(isolate, module_object, name,
kNoInternalize); kNoInternalize);
...@@ -253,8 +253,8 @@ Vector<const uint8_t> WasmModuleObject::GetRawFunctionName(int func_index) { ...@@ -253,8 +253,8 @@ Vector<const uint8_t> WasmModuleObject::GetRawFunctionName(int func_index) {
DCHECK_GT(module()->functions.size(), func_index); DCHECK_GT(module()->functions.size(), func_index);
wasm::ModuleWireBytes wire_bytes(native_module()->wire_bytes()); wasm::ModuleWireBytes wire_bytes(native_module()->wire_bytes());
wasm::WireBytesRef name_ref = wasm::WireBytesRef name_ref =
module()->lazily_generated_names.LookupFunctionName( module()->lazily_generated_names.LookupFunctionName(wire_bytes,
wire_bytes, func_index, VectorOf(module()->export_table)); func_index);
wasm::WasmName name = wire_bytes.GetNameOrNull(name_ref); wasm::WasmName name = wire_bytes.GetNameOrNull(name_ref);
return Vector<const uint8_t>::cast(name); return Vector<const uint8_t>::cast(name);
} }
......
...@@ -8,7 +8,7 @@ Calling main() ...@@ -8,7 +8,7 @@ Calling main()
Paused: Paused:
Script wasm://wasm/22e4830a byte offset 107: Wasm opcode 0x21 (kExprLocalSet) Script wasm://wasm/22e4830a byte offset 107: Wasm opcode 0x21 (kExprLocalSet)
Scope: Scope:
at main (0:107): at func0 (0:107):
- scope (wasm-expression-stack): - scope (wasm-expression-stack):
0: Array ((ref $ArrC)) 0: Array ((ref $ArrC))
object details: object details:
......
...@@ -6,6 +6,6 @@ Paused on 'debugger;' ...@@ -6,6 +6,6 @@ Paused on 'debugger;'
Number of frames: 5 Number of frames: 5
- [0] call_debugger - [0] call_debugger
- [1] func1 - [1] func1
- [2] main - [2] func2
- [3] testFunction - [3] testFunction
- [4] - [4]
...@@ -8,7 +8,7 @@ load("test/mjsunit/wasm/wasm-module-builder.js"); ...@@ -8,7 +8,7 @@ load("test/mjsunit/wasm/wasm-module-builder.js");
var builder = new WasmModuleBuilder(); var builder = new WasmModuleBuilder();
builder.setName('test-module'); builder.setName('test-module');
builder.addFunction(undefined, kSig_i_v) builder.addFunction('main', kSig_i_v)
.addBody([kExprUnreachable]) .addBody([kExprUnreachable])
.exportAs('main'); .exportAs('main');
builder.instantiate().exports.main(); builder.instantiate().exports.main();
wasm-function[0]:0x22: RuntimeError: unreachable wasm-function[0]:0x22: RuntimeError: unreachable
RuntimeError: unreachable RuntimeError: unreachable
at main (<anonymous>:wasm-function[0]:0x22) at <anonymous>:wasm-function[0]:0x22
at *%(basename)s:{NUMBER}:31 at *%(basename)s:{NUMBER}:31
...@@ -9,7 +9,7 @@ load('test/mjsunit/wasm/wasm-module-builder.js'); ...@@ -9,7 +9,7 @@ load('test/mjsunit/wasm/wasm-module-builder.js');
var builder = new WasmModuleBuilder(); var builder = new WasmModuleBuilder();
builder.setName('test-module'); builder.setName('test-module');
builder.addFunction(undefined, kSig_i_v) builder.addFunction('main', kSig_i_v)
.addBody([kExprUnreachable]) .addBody([kExprUnreachable])
.exportAs('main'); .exportAs('main');
let buffer = builder.toBuffer(); let buffer = builder.toBuffer();
......
RuntimeError: unreachable RuntimeError: unreachable
at main (<anonymous>:wasm-function[0]:0x22) at <anonymous>:wasm-function[0]:0x22
at *%(basename)s:{NUMBER}:27 at *%(basename)s:{NUMBER}:27
at test/mjsunit/mjsunit.js:* at test/mjsunit/mjsunit.js:*
RuntimeError: unreachable RuntimeError: unreachable
at main (<anonymous>:wasm-function[0]:0x22) at <anonymous>:wasm-function[0]:0x22
at test/message/wasm-no-name-async.js:{NUMBER}:27 at test/message/wasm-no-name-async.js:{NUMBER}:27
at test/mjsunit/mjsunit.js:* at test/mjsunit/mjsunit.js:*
...@@ -11,7 +11,7 @@ load('test/mjsunit/wasm/wasm-module-builder.js'); ...@@ -11,7 +11,7 @@ load('test/mjsunit/wasm/wasm-module-builder.js');
builder.addMemory(16, 32, false); builder.addMemory(16, 32, false);
builder.addType(makeSig([kWasmI32, kWasmI32, kWasmI32], [kWasmI32])); builder.addType(makeSig([kWasmI32, kWasmI32, kWasmI32], [kWasmI32]));
// Generate function 1 (out of 1). // Generate function 1 (out of 1).
builder.addFunction(undefined, 0 /* sig */) builder.addFunction('main', 0 /* sig */)
.addBodyWithEnd([ .addBodyWithEnd([
// signature: i_iii // signature: i_iii
// body: // body:
......
...@@ -56,8 +56,7 @@ builder.addFunction("exec_unreachable", kSig_v_v) ...@@ -56,8 +56,7 @@ builder.addFunction("exec_unreachable", kSig_v_v)
// Make this function unnamed, just to test also this case. // Make this function unnamed, just to test also this case.
var mem_oob_func = builder.addFunction(undefined, kSig_i_v) var mem_oob_func = builder.addFunction(undefined, kSig_i_v)
// Access the memory at offset -1, to provoke a trap. // Access the memory at offset -1, to provoke a trap.
.addBody([kExprI32Const, 0x7f, kExprI32LoadMem8S, 0, 0]) .addBody([kExprI32Const, 0x7f, kExprI32LoadMem8S, 0, 0]);
.exportAs("mem_out_of_bounds");
// Call the mem_out_of_bounds function, in order to have two wasm stack frames. // Call the mem_out_of_bounds function, in order to have two wasm stack frames.
builder.addFunction("call_mem_out_of_bounds", kSig_i_v) builder.addFunction("call_mem_out_of_bounds", kSig_i_v)
...@@ -70,9 +69,9 @@ var module = builder.instantiate({mod: {func: STACK}}); ...@@ -70,9 +69,9 @@ var module = builder.instantiate({mod: {func: STACK}});
var expected_string = 'Error\n' + var expected_string = 'Error\n' +
// The line numbers below will change as this test gains / loses lines.. // The line numbers below will change as this test gains / loses lines..
' at STACK (stack.js:38:11)\n' + // -- ' at STACK (stack.js:38:11)\n' + // --
' at main (<anonymous>:wasm-function[1]:0x86)\n' + // -- ' at main (<anonymous>:wasm-function[1]:0x72)\n' + // --
' at testSimpleStack (stack.js:77:18)\n' + // -- ' at testSimpleStack (stack.js:76:18)\n' + // --
' at stack.js:79:3'; // -- ' at stack.js:78:3'; // --
module.exports.main(); module.exports.main();
assertEquals(expected_string, stripPath(stack)); assertEquals(expected_string, stripPath(stack));
...@@ -90,9 +89,9 @@ Error.prepareStackTrace = function(error, frames) { ...@@ -90,9 +89,9 @@ Error.prepareStackTrace = function(error, frames) {
verifyStack(stack, [ verifyStack(stack, [
// isWasm function line pos file offset funcIndex // isWasm function line pos file offset funcIndex
[ false, "STACK", 38, 0, "stack.js"], [ false, "STACK", 38, 0, "stack.js"],
[ true, "main", 1, 0x86, "wasm://wasm/7168ab72", '0x86', 1], [ true, "main", 1, 0x72, "wasm://wasm/862e1cf6", '0x72', 1],
[ false, "testStackFrames", 88, 0, "stack.js"], [ false, "testStackFrames", 87, 0, "stack.js"],
[ false, null, 97, 0, "stack.js"] [ false, null, 96, 0, "stack.js"]
]); ]);
})(); })();
...@@ -104,9 +103,9 @@ Error.prepareStackTrace = function(error, frames) { ...@@ -104,9 +103,9 @@ Error.prepareStackTrace = function(error, frames) {
assertContains("unreachable", e.message); assertContains("unreachable", e.message);
verifyStack(e.stack, [ verifyStack(e.stack, [
// isWasm function line pos file offset funcIndex // isWasm function line pos file offset funcIndex
[ true, "exec_unreachable", 1, 0x8b, "wasm://wasm/7168ab72", '0x8b', 2], [ true, "exec_unreachable", 1, 0x77, "wasm://wasm/862e1cf6", '0x77', 2],
[ false, "testWasmUnreachable", 101, 0, "stack.js"], [ false, "testWasmUnreachable", 100, 0, "stack.js"],
[ false, null, 112, 0, "stack.js"] [ false, null, 111, 0, "stack.js"]
]); ]);
} }
})(); })();
...@@ -119,10 +118,10 @@ Error.prepareStackTrace = function(error, frames) { ...@@ -119,10 +118,10 @@ Error.prepareStackTrace = function(error, frames) {
assertContains("out of bounds", e.message); assertContains("out of bounds", e.message);
verifyStack(e.stack, [ verifyStack(e.stack, [
// isWasm function line pos file offset funcIndex // isWasm function line pos file offset funcIndex
[ true, "mem_out_of_bounds", 1, 0x91, "wasm://wasm/7168ab72", '0x91', 3], [ true, null, 1, 0x7d, "wasm://wasm/862e1cf6", '0x7d', 3],
[ true, "call_mem_out_of_bounds", 1, 0x97, "wasm://wasm/7168ab72", '0x97', 4], [ true, "call_mem_out_of_bounds", 1, 0x83, "wasm://wasm/862e1cf6", '0x83', 4],
[ false, "testWasmMemOutOfBounds", 116, 0, "stack.js"], [ false, "testWasmMemOutOfBounds", 115, 0, "stack.js"],
[ false, null, 128, 0, "stack.js"] [ false, null, 127, 0, "stack.js"]
]); ]);
} }
})(); })();
...@@ -177,8 +176,8 @@ Error.prepareStackTrace = function(error, frames) { ...@@ -177,8 +176,8 @@ Error.prepareStackTrace = function(error, frames) {
verifyStack(e.stack, [ verifyStack(e.stack, [
// isWasm function line pos file offset funcIndex // isWasm function line pos file offset funcIndex
[ true, 'main', 1, unreachable_pos + 0x25, 'wasm://wasm/000600e6', hexOffset, 0], [ true, 'main', 1, unreachable_pos + 0x25, 'wasm://wasm/000600e6', hexOffset, 0],
[ false, 'testBigOffset', 172, 0, 'stack.js'], [ false, 'testBigOffset', 171, 0, 'stack.js'],
[ false, null, 184, 0, 'stack.js'] [ false, null, 183, 0, 'stack.js']
]); ]);
} }
})(); })();
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