This is an archive of the discontinued LLVM Phabricator instance.

[WebAssembly] Don't export __data_end and __heap_start by default.
ClosedPublic

Authored by sbc100 on May 31 2019, 10:38 AM.

Event Timeline

sbc100 created this revision.May 31 2019, 10:38 AM
sbc100 updated this revision to Diff 202460.May 31 2019, 10:46 AM
  • cleanup
sbc100 updated this revision to Diff 202461.May 31 2019, 10:47 AM
  • cleanup
sbc100 edited the summary of this revision. (Show Details)May 31 2019, 10:54 AM
sbc100 added reviewers: sunfish, ruiu.
sbc100 updated this revision to Diff 202463.May 31 2019, 10:55 AM
  • cleanup
sunfish accepted this revision.May 31 2019, 2:29 PM

Cool, this is nicer than my version :-).

This revision is now accepted and ready to land.May 31 2019, 2:29 PM
This revision was automatically updated to reflect the committed changes.
ruiu added inline comments.Jun 2 2019, 9:16 PM
lld/trunk/wasm/Config.h
62 ↗(On Diff #202501)

IIRC, the iteration order of StringSet is different between 32-bit and 64-bit hosts. That's bad for build reproducibility because even if all input files, command line options, and the linker versions are the same, two lld executables create different binaries if one is a 32-bit executable and the other is 64-bit one.

I'd use llvm/ADT/SetVector, which guarantees the iteration order.

lld/trunk/wasm/SymbolTable.cpp
202 ↗(On Diff #202501)

This function seems a little tricky. Can you expand the comment? I believe the intended behavior is this:

Add a symbol with a given name if there's an undefined symbol with the same name. If we are exporting all symbols, a symbol is added unconditionally.
sbc100 marked 2 inline comments as done.Jun 3 2019, 2:19 PM
sbc100 added inline comments.
lld/trunk/wasm/Config.h
62 ↗(On Diff #202501)

We don't actually iterate over this set, we only test for presence symbol names.

lld/trunk/wasm/SymbolTable.cpp
202 ↗(On Diff #202501)

Yes, you are correct. I will add a comment.