[WebAssembly] Reorder reloc sections to come after symtab

[ncw]

Summary

This matches LLVM change D43940, and must land before or with that change.

No tests require updating :)

[sbc100]

Interesting that no test changes needed. I guess its because obj2yaml doesn't display the reloc sections themselves... but injects them into the section they apply to.

wasm/Writer.cpp

635

Why not just swap these two lines? I'm not sure we should care about where these sections come relative the name section.

[ncw]

wasm/Writer.cpp

635

It's easier to have the name section come before the reloc section - it just makes the yaml2wasm logic a bit simpler. That's not really a good reason is it :( But it would be an extra few lines of code to have "name" come after "reloc".

You're right we do have free choice on that ordering; all the other orderings are driven by decent reasons.

[sbc100]

wasm/Writer.cpp

635

Can you remind me why it makes yaml2wasm simpler?b

[ncw]

wasm/Writer.cpp

635

Sure, it's silly really, but yaml2wasm currently just iterates over all the sections and writes them out to Wasm - then does a second loop to catch the relocations and writes those out (since they're stored on the section to which the relocations apply).

So writing out relocs in between the "linking" and "name" section would just make the diff there fatter, so I took the line of least resistance. If there's some aesthetic or other reason to put "name" at the end I can do that though.

[sbc100]

it sounds like we shouldn't care about the ordering of name vs reloc sections. It up to the generator. yaml2obj can always put the relocs last if it likes. that doesn't mean lld needs t to do the same.

[ncw]

In D43946#1024049, @sbc100 wrote:

it sounds like we shouldn't care about the ordering of name vs reloc sections. It up to the generator. yaml2obj can always put the relocs last if it likes. that doesn't mean lld needs t to do the same.

I personally think things should be consistent - and spec'ed in as constrained a way as possible/reasonable.

This came up in the discussion about Wasm's new "exception" section, where there was some discussion about how strict the ordering for it should be, relative to the existing sections. Andreas Rossberg pointed out that the "Wasmy way" so far was to specify the precise ordering of all sections (built-in sections and also the custom "name" section), and that was generally preferable to allowing sections to be laid out in different orders by different tools. So the new "exception" section is having a precise ordering given to it.

I know that our custom linking/reloc sections aren't quite the same, but they are nonetheless semi-official "spec'ed" sections, just spec'ed in a different document. The Linking.md spec would ideally say exactly what their ordering is relative to the official sections - and it's simpler for everyone just to give one official order than list several possibilities.

I've decided overnight it's a big "ugly" to put the name section in between "linking" and "reloc.", so regardless of what we agree about how/whether to validate the order, I think I'll make it so that LLD and yaml2wasm are consistent and put the "name" section at the very end. I don't like different tools to have unnecessary differences in their output! Plus, things should roundtrip eg LLD -> obj2yaml -> yaml2obj shouldn't reorder sections.

[ncw]

Updated as requested by Sam - just reordering the two lines.

Regardless of what think about how the section order will be validated - I think this change is good to go! Doesn't need anything in LLVM to land first.

wasm/Writer.cpp

void Writer::createSections() {
   createGlobalSection();
   createExportSection();
   createElemSection();
   createCodeSection();
   createDataSection();
 
   // Custom sections
   if (Config->Relocatable) {
-    createRelocSections();
     createLinkingSection();
+    createRelocSections();
   }
   if (!Config->StripDebug && !Config->StripAll)
     createNameSection();
 
   for (OutputSection *S : OutputSections) {
     S->setOffset(FileSize);
     S->finalizeContents();
     FileSize += S->getSize();