Skip to content

Commit

Permalink
Fix relative paths in wasm backend source maps. Fixes #9837 (#9882)
Browse files Browse the repository at this point in the history
  • Loading branch information
@kripken
kripken authored Nov 27, 2019
1 parent e7ef5b2 commit 5486c4a
Show file tree
Hide file tree
Showing 4 changed files with 66 additions and 20 deletions.
12 changes: 9 additions & 3 deletions tests/test_core.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7181,6 +7181,12 @@ def encode_utf8(data):
else:
return data

def source_map_file_loc(name):
if shared.Settings.WASM_BACKEND:
return name
# in fastcomp, we have the absolute path, which is not good
return os.path.abspath(name)

data = json.load(open(map_filename))
if str is bytes:
# Python 2 compatibility
Expand All @@ -7190,7 +7196,7 @@ def encode_utf8(data):
# the output file.
self.assertPathsIdentical(map_referent, data['file'])
assert len(data['sources']) == 1, data['sources']
self.assertPathsIdentical(os.path.abspath('src.cpp'), data['sources'][0])
self.assertPathsIdentical(source_map_file_loc('src.cpp'), data['sources'][0])
if hasattr(data, 'sourcesContent'):
# the sourcesContent attribute is optional, but if it is present it
# needs to containt valid source text.
Expand All @@ -7203,7 +7209,7 @@ def encode_utf8(data):
mappings = encode_utf8(mappings)
seen_lines = set()
for m in mappings:
self.assertPathsIdentical(os.path.abspath('src.cpp'), m['source'])
self.assertPathsIdentical(source_map_file_loc('src.cpp'), m['source'])
seen_lines.add(m['originalLine'])
# ensure that all the 'meaningful' lines in the original code get mapped
# when optimizing, the binaryen optimizer may remove some of them (by inlining, etc.)
Expand Down Expand Up @@ -8243,7 +8249,7 @@ def test_ubsan_full_static_cast(self, args):
'g4': ('-g4', [
"src.cpp:3:12: runtime error: reference binding to null pointer of type 'int'",
'in main ',
'/src.cpp:3:8'
'src.cpp:3:8'
]),
})
@no_fastcomp('ubsan not supported on fastcomp')
Expand Down
44 changes: 34 additions & 10 deletions tests/test_other.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8894,7 +8894,8 @@ def test_wasm_sourcemap(self):
'--dwarfdump-output',
path_from_root('tests', 'other', 'wasm_sourcemap', 'foo.wasm.dump'),
'-o', 'a.out.wasm.map',
path_from_root('tests', 'other', 'wasm_sourcemap', 'foo.wasm')]
path_from_root('tests', 'other', 'wasm_sourcemap', 'foo.wasm'),
'--basepath=' + os.getcwd()]
run_process(wasm_map_cmd)
output = open('a.out.wasm.map').read()
# has "sources" entry with file (includes also `--prefix =wasm-src:///` replacement)
Expand All @@ -8909,12 +8910,35 @@ def test_wasm_sourcemap_dead(self):
'--dwarfdump-output',
path_from_root('tests', 'other', 'wasm_sourcemap_dead', 't.wasm.dump'),
'-o', 'a.out.wasm.map',
path_from_root('tests', 'other', 'wasm_sourcemap_dead', 't.wasm')]
path_from_root('tests', 'other', 'wasm_sourcemap_dead', 't.wasm'),
'--basepath=' + os.getcwd()]
run_process(wasm_map_cmd, stdout=PIPE, stderr=PIPE)
output = open('a.out.wasm.map').read()
# has only two entries
self.assertRegexpMatches(output, r'"mappings":\s*"[A-Za-z0-9+/]+,[A-Za-z0-9+/]+"')

@no_fastcomp()
def test_wasm_sourcemap_relative_paths(self):

This comment has been minimized.

Sign in to view

Copy link
@sbc100

sbc100 Nov 29, 2019

Collaborator

This test is failing with we try to roll into emscripten-release:
https://chromium-review.googlesource.com/c/emscripten-releases/+/1943614
https://logs.chromium.org/logs/emscripten-releases/buildbucket/cr-buildbucket.appspot.com/8895424225079553280/+/steps/Emscripten_testsuite__upstream__other_/0/stdout

======================================================================
FAIL: test_wasm_sourcemap_relative_paths (test_other.other)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "C:\b\s\w\ir\cipd_bin_packages\cpython\bin\Lib\unittest\case.py", line 329, in run
    testMethod()
  File "C:\b\s\w\ir\k\install\emscripten\tests\runner.py", line 116, in decorated
    func(self, *args, **kwargs)
  File "C:\b\s\w\ir\k\install\emscripten\tests\test_other.py", line 8948, in test_wasm_sourcemap_relative_paths
    test(os.path.join('inner', 'a.cpp'), 'inner')
  File "C:\b\s\w\ir\k\install\emscripten\tests\test_other.py", line 8943, in test
    self.assertIn('"%s"' % expected_source_map_path, str(f.read()))
  File "C:\b\s\w\ir\cipd_bin_packages\cpython\bin\Lib\unittest\case.py", line 803, in assertIn
    self.fail(self._formatMessage(msg, standardMsg))
  File "C:\b\s\w\ir\cipd_bin_packages\cpython\bin\Lib\unittest\case.py", line 410, in fail
    raise self.failureException(msg)
AssertionError: '"inner\\a.cpp"' not found in '{"version":3,"sources":["inner\\\\a.cpp"],"names":[],"mappings":"8WAOA,kCACE"}'
def test(infile, source_map_added_dir=''):
expected_source_map_path = os.path.join(source_map_added_dir, 'a.cpp')
print(infile, expected_source_map_path)
shutil.copyfile(path_from_root('tests', 'hello_123.c'), infile)
infiles = [
infile,
os.path.abspath(infile),
'./' + infile
]
for curr in infiles:
print(' ', curr)
run_process([PYTHON, EMCC, curr, '-g4'])
with open('a.out.wasm.map', 'r') as f:
self.assertIn('"%s"' % expected_source_map_path, str(f.read()))

test('a.cpp')

os.mkdir('inner')
test(os.path.join('inner', 'a.cpp'), 'inner')

def test_wasm_producers_section(self):
# no producers section by default
run_process([PYTHON, EMCC, path_from_root('tests', 'hello_world.c')])
Expand Down Expand Up @@ -9690,17 +9714,17 @@ def test_lsan_leaks(self, ext):
@parameterized({
'c': ['c', [
r'in malloc.*a\.out\.wasm\+0x',
r'(?im)in f (/|[a-z]:).*/test_lsan_leaks\.c:6:21$',
r'(?im)in main (/|[a-z]:).*/test_lsan_leaks\.c:10:16$',
r'(?im)in main (/|[a-z]:).*/test_lsan_leaks\.c:12:3$',
r'(?im)in main (/|[a-z]:).*/test_lsan_leaks\.c:13:3$',
r'(?im)in f (|[/a-z\.]:).*/test_lsan_leaks\.c:6:21$',

This comment has been minimized.

Sign in to view

Copy link
@sbc100

sbc100 Nov 29, 2019

Collaborator

I'm not sure that why this needed to change. The old code looks like it was matching either "/" or a driver letter with a colon. This new group (|[/a-z\.]:) looks like it match either the empty string or a drive letter or a . followed by a colon? Perhaps it meant to at a "." as an extra option for the first letter? In which case it would be more like (/|\.| [a-z]:) which means "forward slash or dot or a driver letter with a colon".
r'(?im)in main (|[/a-z\.]:).*/test_lsan_leaks\.c:10:16$',
r'(?im)in main (|[/a-z\.]:).*/test_lsan_leaks\.c:12:3$',
r'(?im)in main (|[/a-z\.]:).*/test_lsan_leaks\.c:13:3$',
]],
'cpp': ['cpp', [
r'in operator new\[\]\(unsigned long\).*a\.out\.wasm\+0x',
r'(?im)in f\(\) (/|[a-z]:).*/test_lsan_leaks\.cpp:4:21$',
r'(?im)in main (/|[a-z]:).*/test_lsan_leaks\.cpp:8:16$',
r'(?im)in main (/|[a-z]:).*/test_lsan_leaks\.cpp:10:3$',
r'(?im)in main (/|[a-z]:).*/test_lsan_leaks\.cpp:11:3$',
r'(?im)in f\(\) (|[/a-z\.]:).*/test_lsan_leaks\.cpp:4:21$',
r'(?im)in main (|[/a-z\.]:).*/test_lsan_leaks\.cpp:8:16$',
r'(?im)in main (|[/a-z\.]:).*/test_lsan_leaks\.cpp:10:3$',
r'(?im)in main (|[/a-z\.]:).*/test_lsan_leaks\.cpp:11:3$',
]],
})
@no_fastcomp('lsan not supported on fastcomp')
Expand Down
6 changes: 5 additions & 1 deletion tools/shared.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2871,10 +2871,14 @@ def path_to_system_js_libraries(library_name):

@staticmethod
def emit_wasm_source_map(wasm_file, map_file):
# source file paths must be relative to the location of the map (which is
# emitted alongside the wasm)
base_path = os.path.dirname(os.path.abspath(Settings.WASM_BINARY_FILE))
sourcemap_cmd = [PYTHON, path_from_root('tools', 'wasm-sourcemap.py'),
wasm_file,
'--dwarfdump=' + LLVM_DWARFDUMP,
'-o', map_file]
'-o', map_file,
'--basepath=' + base_path]
check_call(sourcemap_cmd)

@staticmethod
Expand Down
24 changes: 18 additions & 6 deletions tools/wasm-sourcemap.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@
"""

import argparse
from collections import OrderedDict, namedtuple
from collections import OrderedDict
import json
import logging
from math import floor, log
Expand Down Expand Up @@ -39,6 +39,7 @@ def parse_args():
parser.add_argument('-u', '--source-map-url', nargs='?', help='specifies sourceMappingURL section contest')
parser.add_argument('--dwarfdump', help="path to llvm-dwarfdump executable")
parser.add_argument('--dwarfdump-output', nargs='?', help=argparse.SUPPRESS)
parser.add_argument('--basepath', help='base path for source files, which will be relative to this')
return parser.parse_args()


Expand Down Expand Up @@ -73,7 +74,13 @@ def resolve(self, name):
# SourceMapPrefixes contains resolver for file names that are:
# - "sources" is for names that output to source maps JSON
# - "load" is for paths that used to load source text
SourceMapPrefixes = namedtuple('SourceMapPrefixes', 'sources, load')
class SourceMapPrefixes:
def __init__(self, sources, load):
self.sources = sources
self.load = load

def provided(self):
return bool(self.sources.prefixes or self.load.prefixes)


def encode_vlq(n):
Expand Down Expand Up @@ -243,11 +250,10 @@ def read_dwarf_entries(wasm, options):
return sorted(entries, key=lambda entry: entry['address'])


def build_sourcemap(entries, code_section_offset, prefixes, collect_sources):
def build_sourcemap(entries, code_section_offset, prefixes, collect_sources, base_path):
sources = []
sources_content = [] if collect_sources else None
mappings = []

sources_map = {}
last_address = 0
last_source_id = 0
Expand All @@ -264,7 +270,13 @@ def build_sourcemap(entries, code_section_offset, prefixes, collect_sources):
column = 1
address = entry['address'] + code_section_offset
file_name = entry['file']
source_name = prefixes.sources.resolve(file_name)
# if prefixes were provided, we use that; otherwise, we emit a relative
# path
if prefixes.provided():
source_name = prefixes.sources.resolve(file_name)
else:
file_name = os.path.relpath(os.path.abspath(file_name), base_path)
source_name = file_name
if source_name not in sources_map:
source_id = len(sources)
sources_map[source_name] = source_id
Expand Down Expand Up @@ -311,7 +323,7 @@ def main():
prefixes = SourceMapPrefixes(sources=Prefixes(options.prefix), load=Prefixes(options.load_prefix))

logger.debug('Saving to %s' % options.output)
map = build_sourcemap(entries, code_section_offset, prefixes, options.sources)
map = build_sourcemap(entries, code_section_offset, prefixes, options.sources, options.basepath)
with open(options.output, 'w') as outfile:
json.dump(map, outfile, separators=(',', ':'))

Expand Down

0 comments on commit 5486c4a

Please sign in to comment.