This is an archive of the discontinued LLVM Phabricator instance.

[wasm-ld] Add __global_base symbol to mark the value of --global-base
ClosedPublic

Authored by quantum on Jun 26 2019, 11:36 AM.

Details

Summary

This is needed for address sanitizer on Emscripten. As everything in
memory starts at the value passed to --global-base, everything before
that can be used as shadow memory.

This symbol is added so that the library for the ASan runtime can know
where the shadow memory ends and real memory begins.

This is split from D63742.

Event Timeline

quantum created this revision.Jun 26 2019, 11:36 AM
Herald added a project: Restricted Project. · View Herald TranscriptJun 26 2019, 11:36 AM

As I said in D63742, I don't really like that fact that __global_base and __data_end are use for the start and end of the static data but use completely different naming conventions, but since we already expose the --global-base command line arg I guess its hard to make the align without changing the command line arg and/or change the name of __data_end. So I think I'm ok with this for now, but I'd like to find a way to make these consistent in the future.

lld/test/wasm/global-base.ll
3 ↗(On Diff #206717)

You could avoid creating a .ll file here maybe and simply make this a .test file (pure text) that compiles that simple wasm/Inputs/start.ll file.. then you could also drop the --no-entry

5 ↗(On Diff #206717)

Do you need --no-gc-sections here?

lld/wasm/Writer.cpp
223

Looks like we need to figure what to do with GlobalBase in the case of --stack-first. But that is somewhat independent of this change.

quantum updated this revision to Diff 206726.Jun 26 2019, 12:37 PM

Fix test as suggested in review.

quantum marked 6 inline comments as done.Jun 26 2019, 12:38 PM
quantum added inline comments.
lld/test/wasm/global-base.ll
3 ↗(On Diff #206717)

Done.

5 ↗(On Diff #206717)

Removed.

lld/wasm/Writer.cpp
223

I agree.

sbc100 accepted this revision.Jun 26 2019, 12:53 PM
sbc100 added inline comments.
lld/test/wasm/global-base.test
2

Can can remove all the leaving ; not too.

Still lgtm

This revision is now accepted and ready to land.Jun 26 2019, 12:53 PM
quantum updated this revision to Diff 206729.Jun 26 2019, 1:08 PM
quantum marked 3 inline comments as done.

Remove leading ;

This revision was automatically updated to reflect the committed changes.