This is an archive of the discontinued LLVM Phabricator instance.

[WebAssembly] Implement @llvm.global_ctors and @llvm.global_dtors
ClosedPublic

Authored by sbc100 on Dec 13 2017, 4:46 PM.

Details

Summary
  • lowers @llvm.global_dtors by adding @llvm.global_ctors functions which register the destructors with __cxa_atexit.
  • impements @llvm.global_ctors with wasm start functions and linker metadata

This is simpler version of https://reviews.llvm.org/D41211 that doesn't use the start
section and builds on the already-landed change for supporting the extra meta data
in the linking section.

See here for more background.

Diff Detail

Repository
rL LLVM

Event Timeline

sbc100 created this revision.Dec 13 2017, 4:46 PM
sbc100 edited the summary of this revision. (Show Details)Dec 13 2017, 4:47 PM
sbc100 added reviewers: sunfish, ncw.
ncw accepted this revision.Dec 14 2017, 6:41 AM

Approach and implementation look good.

lib/MC/WasmObjectWriter.cpp
1315 ↗(On Diff #126873)

This is a whopping method already... at some point it should be split up, and we could at least avoid inlining another big independent bit of functionality.

1347 ↗(On Diff #126873)

We're checking it's all zeros - is this really necessary? And, I'm surprised that the contents of the data are "defined" to be zero, I might have expected that anything that has a fixup associated has essentially undefined contents (garbage). So really we're checking that the garbage is zero which doesn't seem very useful. If you did want to check this "properly" it would presumably involve checking that there are no bytes untouched by fixups, and that seems a bit overkill to calculate for an assertion.

This revision is now accepted and ready to land.Dec 14 2017, 6:41 AM
sunfish added inline comments.Dec 14 2017, 7:40 AM
lib/MC/WasmObjectWriter.cpp
1347 ↗(On Diff #126873)

My main thought when writing that was just to catch people accidentally putting unrelocated function indices in there, since that seems like something that could plausibly happen. I agree, for full checking we should check more things -- also, gaps and overlaps.

But now that you point it out, it occurs to me that in other places where we reference functions with relocations, we write the unrelocated index rather than 0, so that if you dump the output with a generic wasm tool, it'll look right. I think it makes sense to do that for .init_array section content too. I'll work on a patch for that.

This revision was automatically updated to reflect the committed changes.