Skip to content
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

Allow preserving name section without other debug info #14623

Closed
trybka opened this issue Jul 11, 2021 · 14 comments
Closed

Allow preserving name section without other debug info #14623

trybka opened this issue Jul 11, 2021 · 14 comments

Comments

@trybka
Copy link
Collaborator

trybka commented Jul 11, 2021

Usecase:
The name section is similar to the symbol table for ELF binaries. It can be used to symbolize functions at runtime (or after the fact as well).

This is exposed generally via emscripten_pc_get_function library call (and used by the sanitizer stacktraces).
It uses the wasmOffsetConverter which in turn uses the WebAssembly name section.

It is useful to be able to do symbolization without all the other debug info, and for ELF binaries, this is typically possible as long as the binary is not entirely stripped. This is typically the difference between --strip-debug and --strip-all options.

Unfortunately, binaryen and wasm-ld both treat the name section as debug info, and so it is stripped with the rest of the debug information.

I propose that the name section be treated like the symbol table in ELF, and preserved when only discarding debug info, but can be dropped if --strip-all/--strip-unneeded is passed.

@trybka
Copy link
Collaborator Author

trybka commented Jul 12, 2021

+cc @walkingeyerobot

@jeffRTC
Copy link

jeffRTC commented Jul 12, 2021

@trybka

but can be dropped if --strip-all/--strip-unneeded is passed

How about just having a new option like --prevent-striping-name-section instead just making it default?

@trybka
Copy link
Collaborator Author

trybka commented Jul 12, 2021

I don't have strong feelings. My preference was for the section to be treated like the symbol table.

Keep if --strip-debug
Discard if --strip-all

But a separate flag is fine, too, considering that my workaround is to use objcopy to drop just the .debug_* sections.

@kripken
Copy link
Member

kripken commented Jul 12, 2021

I think this should work today with --profiling-funcs.

@trybka
Copy link
Collaborator Author

trybka commented Jul 12, 2021

So --profiling-funcs does work. Sort of. Inlining seems to be affected which results in a more sparse stacktrace.
This may be because -g was inhibiting inlining in Binaryen.

@dschuff also has out https://reviews.llvm.org/D73820 which seems relevant.

In a case where I'm already setting -g, it still seems like it might be nice to have the name section treated like the symbol table and not discarded if I'm not also passing --profiling-funcs.

@dschuff
Copy link
Member

dschuff commented Jul 12, 2021

Is it the case that using -g at link time is stripping the name section now? I'm not sure that's expected.

@trybka
Copy link
Collaborator Author

trybka commented Jul 12, 2021

I think what I'm trying to say is that -Wl,--strip-debug at link time discards the name section.

@dschuff
Copy link
Member

dschuff commented Jul 12, 2021

Yeah, that is probably a holdover from before DWARF even existed. It probably makes sense to treat the name section like ELF symbol tables, and have --strip-debug keep them and -strip-all drop them.

@dschuff
Copy link
Member

dschuff commented Jul 13, 2021

Looking again at https://reviews.llvm.org/D73820 reminded me that wasm of course also does have a symbol table that plays the same role in linking as ELF's. The difference is that in ELF that linking symbol table is also used for symbolization, and in wasm it's the name section instead. So I think you could make the case for either behavior: 1) the name section plays no role in linking and it's really just for debugging (and profiling) purposes, therefore it makes sense to strip it with --strip-debug. or 2) the name section is used for some of the same purposes as the ELF symbol section (and there are use cases for having it without debug info) and it makes sense to keep when using --strip-debug.

Would changing the behavior of --strip-debug in the linker change any use cases that aren't already using -Wl flags? I think that for llvm-strip, making -strip-debug keep the name section would be ok. Most users of strip just use strip-all (the default), and without this behavior, strip-all is almost exactly the same as strip-debug for linked object files anyway (so making them be actually different seems useful).

@trybka
Copy link
Collaborator Author

trybka commented Jul 19, 2021

Would changing the behavior of --strip-debug in the linker change any use cases that aren't already using -Wl flags?

I'm not sure I understand the question. AFAICT it would be a behavior change, and could be noticed by folks. (i.e. now their binaries are bigger). At least llvm-strip was adding new functionality, rather than behavior changing.

Ultimately, I agree--I can see arguments for either (1) or (2).
One argument for (2) is to keep the linker and strip commands consistent. I think given the changes made to llvm-strip I would be OK if you closed this. Up to you.

@sbc100
Copy link
Collaborator

sbc100 commented Jul 23, 2021

I'm fine with (2) and I agree we should probably keep the meaning of --strip-debug the same between the linker and the strip tool.

I can't imagine there will be many user of wasm-ld --strip-debug who will be upset to learn that the name section is now preserved in this mode in that mode.

Also, the name section is now also used during objdump -d to provide dummy symbols so we are kind of using the way ELF does in this case. We should probably extend that so that llvm-nm also uses them.

@dschuff
Copy link
Member

dschuff commented Jul 23, 2021

Sounds good to me, let's go with 2).
So that means we should update llvm-objcopy/strip and also lld.

@dschuff
Copy link
Member

dschuff commented Jul 23, 2021

OK, so it turns out that as of llvm/llvm-project@7cb25f5 llvm-strip -strip-debug actually does preserve the name section, and --strip-all (the default behavior of llvm-strip) removes the name section. So I guess we just need to update wasm-ld

@dschuff
Copy link
Member

dschuff commented Jul 24, 2021

/cc @sunfishcode in case you have opinions, since this seems nice for non-emscripten use too.

arichardson pushed a commit to CTSRD-CHERI/llvm-project that referenced this issue Sep 29, 2021
Leave the name section in the output when using the --strip-debug
flag. This treats it more like ELF symbol tables, as the name
section has similar uses at runtime (e.g. wasm engines understand
it and it can be used for symbolization at runtime).

Fixes emscripten-core/emscripten#14623

Differential Revision: https://reviews.llvm.org/D106728
mem-frob pushed a commit to draperlaboratory/hope-llvm-project that referenced this issue Oct 7, 2022
Leave the name section in the output when using the --strip-debug
flag. This treats it more like ELF symbol tables, as the name
section has similar uses at runtime (e.g. wasm engines understand
it and it can be used for symbolization at runtime).

Fixes emscripten-core/emscripten#14623

Differential Revision: https://reviews.llvm.org/D106728
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants