Skip to content

Only strip the producers' section in optimized builds #11996

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

Merged
merged 5 commits into from
Aug 23, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions ChangeLog.md
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,13 @@ See docs/process.md for how version tagging works.

Current Trunk
-------------
- Only strip the LLVM producer's section in release builds. In `-O0` builds, we
try to leave the wasm from LLVM unmodified as much as possible, so if it
emitted the producers section, it will be there. Normally that only matters
in release builds, which is not changing here. If you want to not have a
producer's section in debug builds, you can remove it a tool like
`wasm-opt --strip-producers` (which is what Emscripten still does in release
builds, as always) or use `llvm-objcopy`.
- Do not remove `__original_main` using `--inline-main`. We used to do this
so that it didn't show up in stack traces (which could be confusing because
it is added by the linker - it's not in the source code). But this has had
Expand Down
5 changes: 3 additions & 2 deletions emcc.py
Original file line number Diff line number Diff line change
Expand Up @@ -603,8 +603,9 @@ def backend_binaryen_passes():
passes += ['--low-memory-unused']
if shared.Settings.DEBUG_LEVEL < 3:
passes += ['--strip-debug']
if not shared.Settings.EMIT_PRODUCERS_SECTION:
passes += ['--strip-producers']
if shared.Settings.OPT_LEVEL > 0:
if not shared.Settings.EMIT_PRODUCERS_SECTION:
passes += ['--strip-producers']
if shared.Settings.AUTODEBUG:
# adding '--flatten' here may make these even more effective
passes += ['--instrument-locals']
Expand Down
15 changes: 9 additions & 6 deletions src/settings.js
Original file line number Diff line number Diff line change
Expand Up @@ -1148,13 +1148,16 @@ var WASM_ASYNC_COMPILATION = 1;
var WASM_BIGINT = 0;

// WebAssembly defines a "producers section" which compilers and tools can
// annotate themselves in. Emscripten does not emit this by default, as it
// increases code size, and some users may not want information about their tools
// to be included in their builds for privacy or security reasons, see
// annotate themselves in, and LLVM emits this by default. In release builds,
// Emscripten will strip that out so that it is *not* emitted because it
// increases code size, and also some users may not want information
// about their tools to be included in their builds for privacy or security
// reasons, see
// https://github.com/WebAssembly/tool-conventions/issues/93.
// TODO: currently this flag just controls whether we run the binaryen pass
// to strip it out from the wasm (where the LLVM wasm backend may have
// created it)
// (In debug builds (-O0) we leave the wasm file as it is from LLVM, in which
// case it may contain this section, if you didn't tell LLVM to not emit it. You
// can also run wasm-opt --strip-producers manually, which is what Emscripten
// does in release builds for you automatically.)
var EMIT_PRODUCERS_SECTION = 0;

// If set then generated WASM files will contain a custom
Expand Down
39 changes: 33 additions & 6 deletions tests/test_other.py
Original file line number Diff line number Diff line change
Expand Up @@ -2015,12 +2015,29 @@ def compile_to_debug_executable(compile_args):
self.assertLess(line_size, full_size)

def compile_to_release_executable(compile_args):
return compile_to_executable(compile_args, [])
return compile_to_executable(compile_args, ['-O1'])

no_size, line_size, full_size = test(compile_to_release_executable)
self.assertEqual(no_size, line_size)
self.assertEqual(line_size, full_size)

# "-O0 executable" means compiling without optimizations but *also* without
# -g (so, not a true debug build). the results here may change over time,
# since we are telling emcc both to try to do as little as possible during
# link (-O0), but also that debug info is not needed (no -g). if we end up
# doing post-link changes then we will strip the debug info, but if not then
# we don't.
def compile_to_O0_executable(compile_args):
return compile_to_executable(compile_args, [])

no_size, line_size, full_size = test(compile_to_O0_executable)
# the difference between these two is due to the producer's section which
# LLVM emits, and which we do not strip as this is not a release build.
# the specific difference is that LLVM emits language info (C_plus_plus_14)
# when emitting debug info, but not otherwise.
self.assertLess(no_size, line_size)
self.assertEqual(line_size, full_size)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we need this change, given that we already test this specifically below?

I find this test already to be a little to long/complex for a unittest.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This test is important in general for checking what happens with debug info when compiling to object files first and then linking them. That is not checked by the main test for the producer's section that you mentioned lower down (it compiles straight to an executable, and it doesn't look at debug info). I think it's worth adding this as I found a curious interaction between the two (mentioned in the comment here) which confused me when working on this. The test addition here both explains that, and will prevent accidentally breaking anything with the producers' section / debugging interaction as we make changes on both sides.


def test_dwarf(self):
def compile_with_dwarf(args, output):
# Test that -g enables dwarf info in object files and linked wasm
Expand Down Expand Up @@ -7862,13 +7879,23 @@ def test_separate_dwarf_with_filename_and_path(self):
with open('a.out.wasm', 'rb') as f:
self.assertIn(b'somewhere.com/hosted.wasm', f.read())

def test_wasm_producers_section(self):
# no producers section by default
self.run_process([EMCC, path_from_root('tests', 'hello_world.c')])
@parameterized({
'O0': (True, ['-O0']), # unoptimized builds try not to modify the LLVM wasm.
'O1': (False, ['-O1']), # optimized builds strip the producer's section
'O2': (False, ['-O2']), # by default.
})
def test_wasm_producers_section(self, expect_producers_by_default, args):
self.run_process([EMCC, path_from_root('tests', 'hello_world.c')] + args)
with open('a.out.wasm', 'rb') as f:
self.assertNotIn('clang', str(f.read()))
data = f.read()
if expect_producers_by_default:
self.assertIn('clang', str(data))
return
# if there is no producers section expected by default, verify that, and
# see that the flag works to add it.
self.assertNotIn('clang', str(data))
size = os.path.getsize('a.out.wasm')
self.run_process([EMCC, path_from_root('tests', 'hello_world.c'), '-s', 'EMIT_PRODUCERS_SECTION=1'])
self.run_process([EMCC, path_from_root('tests', 'hello_world.c'), '-s', 'EMIT_PRODUCERS_SECTION=1'] + args)
with open('a.out.wasm', 'rb') as f:
self.assertIn('clang', str(f.read()))
size_with_section = os.path.getsize('a.out.wasm')
Expand Down