Resolve synthetic compilation dir `/emsdk/emscripten` to obtain actual path for better source map #20779

kon72 · 2023-11-25T09:01:57Z

We are using a synthetic path /emsdk/emscripten as a compilation directory for building prebuilt system libraries in order to achieve a deterministic build, not being affected by the directory structure of buildbot.
However, those dummy paths still remain in the final source map, making it unable to locate system source files while debugging.

This change replaces /emsdk/emscripten with the actual path to be able to locate system source files.

sbc100 · 2023-11-27T20:27:07Z

Seems like a good idea. I wonder if we can write a test for this, or modify an existing source map test to ensure this is working.

sbc100 · 2023-11-27T20:27:56Z

tools/wasm-sourcemap.py

@@ -230,6 +230,9 @@ def read_dwarf_entries(wasm, options):
    for file in re.finditer(r"file_names\[\s*(\d+)\]:\s+name: \"([^\"]*)\"\s+dir_index: (\d+)", line_chunk):
      dir = include_directories[file.group(3)]
      file_path = (dir + '/' if file.group(2)[0] != '/' else '') + file.group(2)
+      if file_path.startswith('/emsdk/emscripten/'):
+        sub_dir = os.path.relpath(file_path, '/emsdk/emscripten')


I wonder if we can factor out this constant which is duplicated here and also shared with system_libs.py?

kon72 · 2023-12-03T12:05:34Z

Will address reviews and reopen after #20776 merged.

…l path for better source map

kon72 · 2023-12-16T12:36:08Z

Hi @sbc100,

I added a test for this change. Could you please take another look? Thanks!

sbc100 · 2023-12-18T02:32:16Z

test/test_other.py

+    self.run_process(wasm_map_cmd, stdout=PIPE, stderr=PIPE)
+    output = read_file('a.out.wasm.map')
+    # has an actual path to the source file of system library, not a dummy path (/emsdk/emscripten/**)
+    self.assertIn('"system/lib/compiler-rt/stack_ops.S"', output)


Can you use assertContains here instead?

sbc100 · 2023-12-18T02:32:45Z

test/test_other.py

+                    '-o', 'a.out.wasm.map',
+                    test_file('other/wasm_sourcemap_resolve_dummy_comp_dir/foo.wasm'),
+                    '--basepath=' + path_from_root()]
+    self.run_process(wasm_map_cmd, stdout=PIPE, stderr=PIPE)


Maybe don't capture stdout and stderr here since if this command ever fails it could be good to show the output.

sbc100 · 2023-12-18T02:34:57Z

test/test_other.py

+                    '--dwarfdump-output',
+                    test_file('other/wasm_sourcemap_resolve_dummy_comp_dir/foo.wasm.dump'),
+                    '-o', 'a.out.wasm.map',
+                    test_file('other/wasm_sourcemap_resolve_dummy_comp_dir/foo.wasm'),


Can you use use just compile test/hello_world.c here rather then checking in the foo.wasm binary?

Also it looks like you accidentally added the .dump file to git too.

dschuff · 2023-12-18T22:20:43Z

I noticed a couple of things here: One is that the paths used in DWARF (which is used both to generate the source maps, and also with the Chrome C++ debugger) don't match the paths in the source maps. The other is that while the DWARF stays deterministic, the source map does not. Is this what we want? Since the source map is just as much a part of the build output as the wasm file; so with this patch it's not possible to get a deterministic source map even if a user wanted to.

I also took a look at the current DWARF-generation behavior of emcc and noticed that when we build with embuilder we use the deterministic output, but when emcc builds system libs on demand, it doesn't. This doesn't have anything to do with your proposed change here of course, but it seems odd since if a user uses a library version that's not prebuilt with emsdk, then I don't know if there's a way to even get deterministic output in that case.

And then I noticed that when I used the deterministic embuilder build, all of the fields in the source map actually seem to be using relative paths (e.g. 'system/lib/compiler-rt') and I didn't see any appearance of the dummy path in the source map at all. So I'm wondering whether I'm compiling somehow differently than you are, and I'd like to understand your use case better before we make a fix.

Provide source map settings to emcc (resolves #23686, resolves #22189) : - for embedding the sources content in the source map : `-gsource-map=inline`. - for applying path prefix substitution : `-sSOURCE_MAP_PREFIXES=["<old>=<new>"]`. Update documentation accordingly. Fix source file resolver : - Always fall back to the given filepath if prefix not provided or doesn't match. - Fix source content loading when no `--load-prefix` is given/needed. - Fix relative path in "sources" field when`--prefix` is given but doesn't match the given file. - Cache filepaths with no/unmatched prefix as well. - Resolve deterministic prefix when loading source content (related: #20779). - Don't emit relative paths for sources with a deterministic prefix. Improve existing test for wasm-sourcemap.py : - Parameterize `test_wasm_sourcemap()` and do proper checks according to the combinations of options given. - Fix regex for checking the "mappings" field (was checking only the first character). Add test for emcc covering the basic use cases where : - no option is given (users do path substitution as needed via their client or server configuration). - different prefixes are provided for source files and emscripten dependencies. - source content is embedded in the sourcemap.

Provide source map settings to emcc (resolves emscripten-core#23686, resolves emscripten-core#22189) : - for embedding the sources content in the source map : `-gsource-map=inline`. - for applying path prefix substitution : `-sSOURCE_MAP_PREFIXES=["<old>=<new>"]`. Update documentation accordingly. Fix source file resolver : - Always fall back to the given filepath if prefix not provided or doesn't match. - Fix source content loading when no `--load-prefix` is given/needed. - Fix relative path in "sources" field when`--prefix` is given but doesn't match the given file. - Cache filepaths with no/unmatched prefix as well. - Resolve deterministic prefix when loading source content (related: emscripten-core#20779). - Don't emit relative paths for sources with a deterministic prefix. Improve existing test for wasm-sourcemap.py : - Parameterize `test_wasm_sourcemap()` and do proper checks according to the combinations of options given. - Fix regex for checking the "mappings" field (was checking only the first character). Add test for emcc covering the basic use cases where : - no option is given (users do path substitution as needed via their client or server configuration). - different prefixes are provided for source files and emscripten dependencies. - source content is embedded in the sourcemap.

sbc100 reviewed Nov 27, 2023

View reviewed changes

kon72 marked this pull request as draft December 3, 2023 12:04

kon72 added 2 commits December 16, 2023 18:06

Resolve synthetic compilation dir /emsdk/emscripten to obtain actua…

ee488c9

…l path for better source map

Factor out /emsdk/emscripten

1087383

kon72 force-pushed the resolve-system-source-map branch from c26df4a to 0910f74 Compare December 16, 2023 12:33

kon72 marked this pull request as ready for review December 16, 2023 12:33

Add testing

1fac270

kon72 force-pushed the resolve-system-source-map branch from 0910f74 to 1fac270 Compare December 16, 2023 12:52

Use os.path.sep

a81c8e3

sbc100 reviewed Dec 18, 2023

View reviewed changes

sbc100 requested review from dschuff and kripken December 18, 2023 02:35

lvlte mentioned this pull request Feb 24, 2025

Sourcemap improvements #23741

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Resolve synthetic compilation dir `/emsdk/emscripten` to obtain actual path for better source map #20779

Resolve synthetic compilation dir `/emsdk/emscripten` to obtain actual path for better source map #20779

kon72 commented Nov 25, 2023

Uh oh!

sbc100 commented Nov 27, 2023

Uh oh!

sbc100 Nov 27, 2023

Uh oh!

kon72 Dec 16, 2023

Uh oh!

kon72 commented Dec 3, 2023

Uh oh!

kon72 commented Dec 16, 2023

Uh oh!

sbc100 Dec 18, 2023

Uh oh!

sbc100 Dec 18, 2023

Uh oh!

sbc100 Dec 18, 2023

Uh oh!

dschuff commented Dec 18, 2023 •

edited

Loading

Uh oh!

Uh oh!

Resolve synthetic compilation dir /emsdk/emscripten to obtain actual path for better source map #20779

Are you sure you want to change the base?

Resolve synthetic compilation dir /emsdk/emscripten to obtain actual path for better source map #20779

Conversation

kon72 commented Nov 25, 2023

Uh oh!

sbc100 commented Nov 27, 2023

Uh oh!

sbc100 Nov 27, 2023

Choose a reason for hiding this comment

Uh oh!

kon72 Dec 16, 2023

Choose a reason for hiding this comment

Uh oh!

kon72 commented Dec 3, 2023

Uh oh!

kon72 commented Dec 16, 2023

Uh oh!

sbc100 Dec 18, 2023

Choose a reason for hiding this comment

Uh oh!

sbc100 Dec 18, 2023

Choose a reason for hiding this comment

Uh oh!

sbc100 Dec 18, 2023

Choose a reason for hiding this comment

Uh oh!

dschuff commented Dec 18, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Resolve synthetic compilation dir `/emsdk/emscripten` to obtain actual path for better source map #20779

Resolve synthetic compilation dir `/emsdk/emscripten` to obtain actual path for better source map #20779

dschuff commented Dec 18, 2023 •

edited

Loading