-
Notifications
You must be signed in to change notification settings - Fork 3.4k
Make -gsource-map independent of name sections and JS minification #25238
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
081aad7
d45dbdf
031edff
5640b59
0a13954
3d5720e
7491915
04db06b
1479cb3
488a12e
963e3dd
beefb4d
ac9ee93
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -3145,21 +3145,23 @@ def test_dwarf_sourcemap_names(self): | |
(['-O1', '-g'], True, False, True), | ||
(['-O3', '-g'], True, False, True), | ||
(['-gsplit-dwarf'], True, False, True), | ||
# TODO: It seems odd that -gsource-map leaves behind a name section. Should it? | ||
(['-gsource-map'], False, True, True), | ||
(['-g1', '-Oz', '-gsource-map'], False, True, True), | ||
(['-gsource-map'], False, True, False), | ||
(['-g2', '-gsource-map'], False, True, True), | ||
(['-g1', '-Oz', '-gsource-map'], False, True, False), | ||
(['-gsource-map', '-g0'], False, False, False), | ||
# --emit-symbol-map should not affect the results | ||
(['--emit-symbol-map', '-gsource-map'], False, True, True), | ||
(['--emit-symbol-map', '-gsource-map'], False, True, False), | ||
(['--emit-symbol-map'], False, False, False), | ||
(['--emit-symbol-map', '-Oz'], False, False, False), | ||
(['-sASYNCIFY=1', '-g0'], False, False, False), | ||
(['-sASYNCIFY=1', '-gsource-map'], False, True, True), | ||
(['-sASYNCIFY=1', '-gsource-map'], False, True, False), | ||
(['-sASYNCIFY=1', '-gsource-map', '-g2'], False, True, True), | ||
(['-g', '-gsource-map'], True, True, True), | ||
(['-g2', '-gsource-map'], False, True, True), | ||
(['-gsplit-dwarf', '-gsource-map'], True, True, True), | ||
(['-gsource-map', '-sERROR_ON_WASM_CHANGES_AFTER_LINK'], False, True, True), | ||
(['-Oz', '-gsource-map'], False, True, True), | ||
(['-Oz', '-gsource-map'], False, True, False), | ||
(['-gsource-map', '-sERROR_ON_WASM_CHANGES_AFTER_LINK'], False, True, False), | ||
(['-gsource-map', '-Og', '-sERROR_ON_WASM_CHANGES_AFTER_LINK'], False, True, False), | ||
]: | ||
print(flags, expect_dwarf, expect_sourcemap, expect_names) | ||
self.emcc(test_file(source_file), flags, js_file) | ||
|
@@ -9250,19 +9252,22 @@ def test_binaryen_debug(self): | |
for args, expect_clean_js, expect_whitespace_js, expect_closured in [ | ||
(['-O0'], False, True, False), | ||
(['-O0', '-g1'], False, True, False), | ||
(['-O0', '-g2'], False, True, False), # in -g2+, we emit -g to asm2wasm so function names are saved | ||
(['-O0', '-g2'], False, True, False), | ||
(['-O0', '-g'], False, True, False), | ||
(['-O0', '--profiling-funcs'], False, True, False), | ||
(['-O0', '-gline-tables-only'], False, True, False), | ||
(['-O1'], False, True, False), | ||
(['-O3'], True, False, False), | ||
(['-Oz', '-gsource-map'], False, True, False), # TODO: fix this (#20462) | ||
(['-Oz', '-gsource-map'], True, False, False), | ||
(['-O2'], True, False, False), | ||
(['-O2', '-gz'], True, False, False), # -gz means debug compression, it should not enable debugging | ||
(['-O2', '-g1'], False, True, False), | ||
(['-O2', '-g'], False, True, False), | ||
(['-O2', '--closure=1'], True, False, True), | ||
(['-O2', '--closure=1', '-g1'], True, True, True), | ||
(['-O2', '--minify=0'], False, True, False), | ||
(['-O2', '--profiling-funcs'], True, False, False), | ||
(['-O2', '--profiling'], False, True, False), | ||
]: | ||
print(args, expect_clean_js, expect_whitespace_js, expect_closured) | ||
delete_file('a.out.wat') | ||
|
@@ -12383,7 +12388,7 @@ def test_lsan_leaks(self, ext, args): | |
def test_lsan_stack_trace(self, ext, regexes): | ||
self.do_runf( | ||
'other/test_lsan_leaks.' + ext, | ||
cflags=['-fsanitize=leak', '-gsource-map'], | ||
cflags=['-fsanitize=leak', '-gsource-map', '-g2'], | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Same question. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Same answer: the test expects both names and line numbers. |
||
regex=True, | ||
assert_all=True, | ||
assert_returncode=NON_ZERO, | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -368,7 +368,8 @@ def consume_arg_file(): | |
settings.EMIT_NAME_SECTION = 1 | ||
# if we don't need to preserve LLVM debug info, do not keep this flag | ||
# for clang | ||
if settings.DEBUG_LEVEL < 3: | ||
if (settings.DEBUG_LEVEL < 3 and not | ||
(settings.GENERATE_SOURCE_MAP or settings.SEPARATE_DWARF)): | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why does something about And has There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yeah, this logic is really tricky. Basically if you have -gsource-map and -g2 together at compile time (to get SM and names), then you set Similarly with separate-dwarf which has the side effect of enabling debug info (rather than purely specifying that dwarf should be separated). This change keeps that from happening, but I also think the fact that -g2 rounds to -g0 at compile time is a bug to fix in a future PR. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm not sure if I understand. And Also where do we process There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. the problem is when you have both flags you make 2 trips through this loop with gsource-map first. the first time it sets DEBUG_LEVEL to 3 and then the second time it sets it back to 2 and sets the 'g0' flag to clang, meaning no debug info at all. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ah thanks, I forgot this code was within a loop... |
||
newargs[i] = '-g0' | ||
else: | ||
# for 3+, report -g3 to clang as -g4 etc. are not accepted | ||
|
@@ -394,9 +395,9 @@ def consume_arg_file(): | |
else: | ||
settings.SEPARATE_DWARF = True | ||
settings.GENERATE_DWARF = 1 | ||
settings.DEBUG_LEVEL = 3 | ||
elif requested_level in ['source-map', 'source-map=inline']: | ||
settings.GENERATE_SOURCE_MAP = 1 if requested_level == 'source-map' else 2 | ||
settings.EMIT_NAME_SECTION = 1 | ||
newargs[i] = '-g' | ||
elif requested_level == 'z': | ||
# Ignore `-gz`. We don't support debug info compression. | ||
|
@@ -407,10 +408,7 @@ def consume_arg_file(): | |
# clang and make the emscripten code treat it like any other DWARF. | ||
settings.GENERATE_DWARF = 1 | ||
settings.EMIT_NAME_SECTION = 1 | ||
# In all cases set the emscripten debug level to 3 so that we do not | ||
# strip during link (during compile, this does not make a difference). | ||
# TODO: possibly decouple some of these flags from the final debug level (#20462) | ||
settings.DEBUG_LEVEL = 3 | ||
settings.DEBUG_LEVEL = 3 | ||
elif check_flag('-profiling') or check_flag('--profiling'): | ||
settings.DEBUG_LEVEL = max(settings.DEBUG_LEVEL, 2) | ||
settings.EMIT_NAME_SECTION = 1 | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this needed in the test now? If so maybe add a comment why? (I assume the source map is needed for logging, but not sure why
-g2
)There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The test expects both names and source lines in the output.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I see, thanks, so
-g2
is for the names section.