Fix debug-compilation-dir for system libs by kon72 · Pull Request #20776 · emscripten-core/emscripten

kon72 · 2023-11-24T08:55:15Z

Fixes #20775

Before this change:

$ llvm-dwarfdump -debug-info -debug-line --recurse-depth=0  emscripten/cache/sysroot/lib/wasm32-emscripten/libcompiler_rt.a
...
              DW_AT_name    ("../../../system/lib/compiler-rt/__trap.c")
              DW_AT_comp_dir    ("/emsdk/emscripten")

After this change:

$ llvm-dwarfdump -debug-info -debug-line --recurse-depth=0  emscripten/cache/sysroot/lib/wasm32-emscripten/libcompiler_rt.a
...
              DW_AT_name    ("../../../system/lib/compiler-rt/__trap.c")
              DW_AT_comp_dir    ("/emsdk/emscripten/cache/build/libcompiler_rt-tmp")

RReverser · 2023-11-26T03:30:33Z

Good catch! It's worth adding a test for this in test_other.py too.

sbc100 · 2023-11-27T20:10:06Z

tools/system_libs.py

      source_dir = utils.path_from_root()
      cflags += [f'-ffile-prefix-map={source_dir}=/emsdk/emscripten',
-                 '-fdebug-compilation-dir=/emsdk/emscripten']
+                 f'-fdebug-compilation-dir={build_dir}']


This seems like maybe the wrong fix.

I think the mapping on the line about is the one that needs updating so it ends up being something like -ffile-prefix-map=../..=/emsdk/emscripten'. I think the best way would be to replace source_dir` with somekind of relative source dir?

You mean that the DWARF entries should be like this?

DW_AT_name ("/emsdk/emscripten/system/lib/compiler-rt/__trap.c") DW_AT_comp_dir ("/emsdk/emscripten")

I agree with your direction so that unnecessary build sub directories (like cache/build/libcompiler_rt-tmp) would be stripped out from dwarf info.

kripken · 2023-11-27T22:07:46Z

Regarding testing, maybe checking the contents of dwarfdump is simplest, like e.g. test_separate_dwarf does:

emscripten/test/test_other.py

Lines 9713 to 9720 in cfc68d8

    
           # Check that dwarfdump can dump the debug info 
        
           dwdump = self.run_process( 
        
               [LLVM_DWARFDUMP, 'subdir/output.wasm.debug.wasm', '-name', 'main'], 
        
               stdout=PIPE).stdout 
        
           # Basic check that the debug info is more than a skeleton. If so it will 
        
           # have a subprogram descriptor for main 
        
           self.assertIn('DW_TAG_subprogram', dwdump) 
        
           self.assertIn('DW_AT_name\t("main")', dwdump)

sbc100 · 2023-12-02T14:57:00Z

test/test_other.py

                      '-sSEPARATE_DWARF_URL=http://somewhere.com/hosted.wasm'])
    self.assertIn(b'somewhere.com/hosted.wasm', read_binary('a.out.wasm'))

+  def test_dwarf_system_lib(self):


Can you mark this new test a @crossplatform so it runs on mac and windows too?

It looks the test is failing on Windows due to path separators (could be related to WebAssembly/binaryen#2781)

From CI output:

DW_AT_name\t("system\\\\lib/emmalloc.c") DW_AT_comp_dir\t("/emsdk/emscripten")

@sbc100
Should we modify the test to make it pass on Windows? or drop @crossplatform?

I think it would be good to fix if possible. Perhaps if this is a pre-existing issue maybe we don't have to fix it now, but it would be good to fix if possible.

Any idea how system\\\\lib/emmalloc.c could end up like that?

After some digging on my local Windows machine, It seems backslashes in system\\\\lib/emmalloc.c are coming from the source file path we're passing to Clang, and the slash before basename is due to -ffile-reproducible.

Currently, embuilder is passing the source file path in the platform-specific format (with backslashes as separators on Windows):

clang.exe <redacted> -c ..\..\..\system\lib\emmalloc.c

and I get the following DWARF entry

DW_AT_name ("system\\lib/emmalloc.c")

Then when I replace backslashes in the source file path with slashes, the DWARF also have slashes in its file names.

clang.exe <redacted> -c ../../../system/lib/emmalloc.c
DW_AT_name ("system/lib/emmalloc.c")

Therefore, I think the best approach is to apply utils.normalize_path to source file names in the build command.
What do you think?

That looks right to me.

test/test_other.py

sbc100 · 2023-12-02T20:27:11Z

Looks like you need to add these lines to the new test:

    if config.FROZEN_CACHE:                                                                          
      self.skipTest("test doesn't work with frozen cache")

kripken · 2023-12-12T23:48:47Z

test/test_other.py

                      '-sSEPARATE_DWARF_URL=http://somewhere.com/hosted.wasm'])
    self.assertIn(b'somewhere.com/hosted.wasm', read_binary('a.out.wasm'))

+  def test_dwarf_system_lib(self):


That looks right to me.

sbc100 reviewed Nov 27, 2023

View reviewed changes

kripken mentioned this pull request Nov 27, 2023

Fix source map gen for relative include directories #20778

Merged

kon72 force-pushed the fix-debug-comp-dir branch from b5ea021 to 156691d Compare December 2, 2023 08:06

sbc100 reviewed Dec 2, 2023

View reviewed changes

RReverser reviewed Dec 2, 2023

View reviewed changes

test/test_other.py Outdated Show resolved Hide resolved

kon72 mentioned this pull request Dec 3, 2023

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

Open

kripken mentioned this pull request Dec 6, 2023

Fix parsing of dwarfdump output to be more resilient #20777

Merged

kon72 added 8 commits December 9, 2023 19:22

Fix debug-compilation-dir for system libs

9782fbd

Fix by using file-prefix-map instead

876e853

Add test

40e01fb

Run test on crossplatform

51b0f24

Skip test if FROZEN_CACHE is not set

ac473ca

clean

5180bf3

Use relative path for DW_AT_name

d927339

Normalize path separators of source files

ef45f57

kon72 force-pushed the fix-debug-comp-dir branch from 3ccba72 to ef45f57 Compare December 9, 2023 10:22

kripken approved these changes Dec 12, 2023

View reviewed changes

kripken merged commit 1852e20 into emscripten-core:main Dec 12, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix debug-compilation-dir for system libs#20776

Fix debug-compilation-dir for system libs#20776
kripken merged 8 commits intoemscripten-core:mainfrom
kon72:fix-debug-comp-dir

kon72 commented Nov 24, 2023

Uh oh!

RReverser commented Nov 26, 2023

Uh oh!

sbc100 Nov 27, 2023

Uh oh!

kon72 Dec 2, 2023

Uh oh!

kripken commented Nov 27, 2023

Uh oh!

sbc100 Dec 2, 2023

Uh oh!

kon72 Dec 3, 2023

Uh oh!

kon72 Dec 3, 2023 •

edited

Loading

Uh oh!

sbc100 Dec 4, 2023

Uh oh!

kon72 Dec 9, 2023

Uh oh!

kripken Dec 12, 2023

Uh oh!

Uh oh!

sbc100 commented Dec 2, 2023

Uh oh!

kripken Dec 12, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Conversation

kon72 commented Nov 24, 2023

Uh oh!

RReverser commented Nov 26, 2023

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

kripken commented Nov 27, 2023

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

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

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

sbc100 commented Dec 2, 2023

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

kon72 Dec 3, 2023 •

edited

Loading