Add --enable-source-map support for WebAssembly module

[sbc100]

What is the problem this feature will solve?

I will allow developers to get better backtrace information from wasm module that are built with source maps.

What is the feature you are proposing to solve the problem?

Reading of the sourceMappingURL section from any WebAssembly modules that get loaded.

What alternatives have you considered?

No response

[sbc100]

Initial support for JS source maps was added in #29564, but as far as I can tell the sourceMappingURL section of wasm files is not yet being honored.

To build a wasm file with source map support in emscripten you can do emcc -gsource-map hello.c.

Here is an example that I used to test:

#include <stdio.h>

int main() {
  printf("hello, world!\n");
  __builtin_trap();
  return 0;
}
$ ./emcc test/hello_world.c -gsource-map
$ $ node --enable-source-maps ./a.out.js 
hello, world!
/usr/local/google/home/sbc/dev/wasm/emscripten/a.out.js:131
      throw ex;
      ^

RuntimeError: unreachable
    at __original_main (wasm://wasm/073677ae:wasm-function[3]:0x2af)
    at main (wasm://wasm/073677ae:wasm-function[4]:0x2b5)
    at /usr/local/google/home/sbc/dev/wasm/emscripten/a.out.js:929:22
    at callMain (/usr/local/google/home/sbc/dev/wasm/emscripten/a.out.js:1706:15)
    at doRun (/usr/local/google/home/sbc/dev/wasm/emscripten/a.out.js:1756:23)
    at run (/usr/local/google/home/sbc/dev/wasm/emscripten/a.out.js:1771:5)
    at runCaller (/usr/local/google/home/sbc/dev/wasm/emscripten/a.out.js:1691:19)
    at removeRunDependency (/usr/local/google/home/sbc/dev/wasm/emscripten/a.out.js:838:7)
    at receiveInstance (/usr/local/google/home/sbc/dev/wasm/emscripten/a.out.js:1072:5)
    at receiveInstantiationResult (/usr/local/google/home/sbc/dev/wasm/emscripten/a.out.js:1091:5)

Node.js v19.0.0
$  wasm-objdump -s -j sourceMappingURL a.out.wasm  

a.out.wasm:	file format wasm 0x1

Contents of section Custom:
00032ff: 1073 6f75 7263 654d 6170 7069 6e67 5552  .sourceMappingUR
000330f: 4c0e 612e 6f75 742e 7761 736d 2e6d 6170  L.a.out.wasm.map

[sbc100]

@bcoe who originally added the --enable-source-maps option

[bnoordhuis]

I believe @bcoe isn't actively involved anymore.

Pull request welcome. Node needs to call isolate->SetWasmLoadSourceMapCallback(callback) and the callback should return the source map as a Local<String>.

[debadree25]

If I understand correctly we are supposed to make a call to isolate->SetWasmLoadSourceMapCallback(callback) function somewhere around here? @bnoordhuis

node/src/node_contextify.cc

Lines 1270 to 1279 in f94ef7c

	if (result->Set(parsing_context, env->cache_key_string(), cache_key)
	.IsNothing())
	return;
	if (result
	->Set(parsing_context,
	env->source_map_url_string(),
	fn->GetScriptOrigin().SourceMapUrl())
	.IsNothing())
	return;

[bnoordhuis]

That's for the vm module. For general use, I'd add it in SetIsolateMiscHandlers() in src/api/environment.cc.

[debadree25]

I tried something like this

diff --git a/src/api/environment.cc b/src/api/environment.cc
index f56ee8d12b..c73a47a35a 100644
--- a/src/api/environment.cc
+++ b/src/api/environment.cc
@@ -260,6 +260,11 @@ void SetIsolateErrorHandlers(v8::Isolate* isolate, const IsolateSettings& s) {
   }
 }
 
+Local<String> WASMLoadSourceMapCb(Isolate* isolate, const char* path) {
+  std::cout << "WASMLoadSourceMapCb" << std::endl;
+  return String::Empty(isolate);
+}
+
 void SetIsolateMiscHandlers(v8::Isolate* isolate, const IsolateSettings& s) {
   isolate->SetMicrotasksPolicy(s.policy);
 
@@ -267,6 +272,10 @@ void SetIsolateMiscHandlers(v8::Isolate* isolate, const IsolateSettings& s) {
     s.allow_wasm_code_generation_callback : AllowWasmCodeGenerationCallback;
   isolate->SetAllowWasmCodeGenerationCallback(allow_wasm_codegen_cb);
 
+  auto* wasm_load_source_map_callback = s.wasm_load_source_map_callback ? 
+    s.wasm_load_source_map_callback : WASMLoadSourceMapCb;
+  isolate->SetWasmLoadSourceMapCallback(wasm_load_source_map_callback);
+
   auto* modify_code_generation_from_strings_callback =
       ModifyCodeGenerationFromStrings;
   if (s.modify_code_generation_from_strings_callback != nullptr) {
diff --git a/src/node.h b/src/node.h
index fc2531f867..3e8003b820 100644
--- a/src/node.h
+++ b/src/node.h
@@ -483,6 +483,7 @@ struct IsolateSettings {
       allow_wasm_code_generation_callback = nullptr;
   v8::ModifyCodeGenerationFromStringsCallback2
       modify_code_generation_from_strings_callback = nullptr;
+  v8::WasmLoadSourceMapCallback wasm_load_source_map_callback = nullptr;
 };
 
 // Represents a startup snapshot blob, e.g. created by passing

just to test, it seems the callback is never called, I tried searching for any docs on SetWasmLoadSourceMapCallback but nothing useful found, hence asking here 😅😅

[bnoordhuis]

On closer inspection it looks like V8 currently supports source maps only for profiling, not for exception stack traces.

Your change would improve the usability of e.g. the --perf-basic-prof flag.

[debadree25]

I tried experimenting with loading source maps for profiling here debadree25@60ead86 it seems the callbacks are correctly called and loaded but I don't seem to find much of a difference between flamegraphs created when profiling because I am able to see the C function in both of them so quite confusing 😅

[bnoordhuis]

WASM files can contain DWARF debug data. V8 uses it if there's no sourcemap available.

[debadree25]

Understood, thank you so much for explaining it seems emscripten automatically adds the DWARF info when building with -gsource-map so won't make much of a difference to enable this feature for profiling will try to pick this up when v8 enables having this for error stack traces

[sbc100]

WASM files can contain DWARF debug data. V8 uses it if there's no sourcemap available.

Does it? I thought all the DWARF parsing was in the chrome dev tools extension. I am not aware of any DWARF parser in v8 itself. Can you point to it?

[bnoordhuis]

Yeah, I take that back. V8 recognizes the .debug_info section but it doesn't do much with it. I was thinking of the unwind info (DWARF subset) that's written to /tmp/perf-<pid>.map. Unwind info is probably why @debadree25 didn't see a difference.

[sbc100]

I think v8 only reads the name section, which just gives you function names in the backtraces. I don't think it knows about .debug_info (DWARF info).

Adding basic source map support would give us filenames in line numbers in node backtraces which would be really useful in my daily work.

[debadree25]

FWIW heres what I tried and the output

Here's the C code i used:

#include <stdio.h>

int callme(int a, int b) {
  // run a random loop upto 1 million
  for (int i = 0; i < 1000000; i++) {
    a = a + b;
  }
  return a;
}

int main() {
  printf("hello, world!\n");
  int i = callme(3, 2);
  printf("i = %d\n", i);
  return 0;
}

Building from main branch of node:

./node-main --prof a.out.js
./node-main --prof-process --preprocess -j isolate-0x7f79eb816000-60805-v8.log > flames1.json

Output from flamebearer

Building from debadree25@60ead86

./node-prof --prof a.out.js
./node-prof --prof-process --preprocess -j isolate-0x7f8513016000-60708-v8.log > flames2.json

And the flamebearer output

Note that callme function is found in both places and thats what I was referring to here #46873 (comment)

[sbc100]

It looks like you have function name in formation there in both cases, but filename and line numbers are not there (like they are for the JS code). Source map would would provide those IIUC.

[debadree25]

Ahh I see, then maybe SetWasmLoadSourceMapCallback probably doesnt support that yet? or maybe I could be doing the code wrong at debadree25@60ead86 😅😅

[sbc100]

Is your WASMLoadSourceMapCb getting called? i.e. is v8 able to read the source map location from the sourceMappingURL custom section in your wasm file?

Can you confirm that your wasm file has that section? (Using wasm-objdump -h for example).

[debadree25]

Yes the callback is indeed called, I added some logs and checked

diff --git a/src/api/environment.cc b/src/api/environment.cc
index d9b102d9ec..793ce2975d 100644
--- a/src/api/environment.cc
+++ b/src/api/environment.cc
@@ -262,9 +262,11 @@ void SetIsolateErrorHandlers(v8::Isolate* isolate, const IsolateSettings& s) {
 }
 
 Local<String> WASMLoadSourceMapCb(Isolate* isolate, const char* path) {
+  std::cout << "callback is called with path: " << path << std::endl;
   std::string contents;
   int r = ReadFileSync(&contents, path);
   if (r == 0) {
+    std::cout << "file was read successfully: " << contents.length() << std::endl;
     return String::NewFromUtf8(isolate, contents.c_str()).ToLocalChecked();
   }
   return String::Empty(isolate);

callback is called with path: a.out.wasm.map
file was read successfully: 8799
hello, world!
i = 2000003

[debadree25]

I wondering should we consider SetWasmLoadSourceMapCallback not loading the line numbers could be considered a bug? Wasnt able to find any more relevant docs on this

[debadree25]

Was reading through the v8 code surrounding this found the following comment

node/deps/v8/src/wasm/wasm-module-sourcemap.cc

Lines 144 to 145 in cce5a8c

	// Column number in source file is always 0 in source map generated by
	// Emscripten. We just decode this value without further usage of it.

does the comment still hold true? and could this be the reason why are not seeing the code positions, apprently this is just about columns

[github-actions]

There has been no activity on this feature request for 5 months and it is unlikely to be implemented. It will be closed 6 months after the last non-automated comment.

For more information on how the project manages feature requests, please consult the feature request management document.

[sbc100]

Was reading through the v8 code surrounding this found the following comment

node/deps/v8/src/wasm/wasm-module-sourcemap.cc

Lines 144 to 145 in cce5a8c

	// Column number in source file is always 0 in source map generated by
	// Emscripten. We just decode this value without further usage of it.

does the comment still hold true? and could this be the reason why are not seeing the code positions, apprently this is just about columns

Even if the column number is always zero I would still expect to see filename and line number.

[github-actions]

There has been no activity on this feature request for 5 months and it is unlikely to be implemented. It will be closed 6 months after the last non-automated comment.

For more information on how the project manages feature requests, please consult the feature request management document.

[github-actions]

There has been no activity on this feature request and it is being closed. If you feel closing this issue is not the right thing to do, please leave a comment.

For more information on how the project manages feature requests, please consult the feature request management document.

[HerrCai0907]

Has this feature been implemented?