[WebAssembly] Symbol changes #4d: export import flags, LLD only

[ncw]

Summary

This fixes an issue Sam identified in D42105.

D42105 was landed without the fix, and this is the followup patch to correct the omission.

Changes:

• Symbol flags should be written out for imports as well as exports. D'oh.

[sbc100]

wasm/Writer.cpp

389

I think this should follow the CamelCase convention for local variables (at least that is what others seems to do in lld).

[sbc100]

Neither of those two tests seems to pass now.

Perhaps because I'm trying to apply this to HEAD?

[ncw]

In D42306#982316, @sbc100 wrote:

Neither of those two tests seems to pass now.

Perhaps because I'm trying to apply this to HEAD?

Ah yes, I could fix that if you want to apply it before #2 and #3 - I probably should have done that anyway since #4a and #4b were already merged, sorry.

wasm/Writer.cpp

389

Oops, I renamed it following some code I saw elsewhere. I checked the LLVM style guide and it didn't say what to do for lambdas - are they lowercase as functions, or uppercase as variables?

I see about 50/50 looking at the first 20 grep results for \[&\], so I guess it doesn't matter.

[sbc100]

wasm/Writer.cpp

389

In ELF and COFF directories I only see the CamelCase style, and I think we should follow them.

test/wasm/init-fini.ll

 
 
 ; RUN: lld -flavor wasm -r %t.o %t.global-ctor-dtor.o -o %t.reloc.wasm
 ; RUN: obj2yaml %t.reloc.wasm | FileCheck -check-prefix=RELOC %s
 
 ; RELOC:          Name:            linking
 ; RELOC-NEXT:     DataSize:        0
 ; RELOC-NEXT:     SymbolInfo:      
+; RELOC-NEXT:       - Name:            __dso_handle
+; RELOC-NEXT:         Flags:           [ BINDING_WEAK, VISIBILITY_HIDDEN ]
 ; RELOC-NEXT:       - Name:            func1
 ; RELOC-NEXT:         Flags:           [ VISIBILITY_HIDDEN ]
 ; RELOC-NEXT:       - Name:            func2
 ; RELOC-NEXT:         Flags:           [ VISIBILITY_HIDDEN ]
 ; RELOC-NEXT:       - Name:            func3
 ; RELOC-NEXT:         Flags:           [ VISIBILITY_HIDDEN ]
 ; RELOC-NEXT:       - Name:            func4
 ; RELOC-NEXT:         Flags:           [ VISIBILITY_HIDDEN ]

test/wasm/relocatable.ll

 ; CHECK-NEXT:         Offset:
 ; CHECK-NEXT:           Opcode:          I32_CONST
 ; CHECK-NEXT:           Value:           28
 ; CHECK-NEXT:         Content:         '616263'
 ; CHECK-NEXT:   - Type:            CUSTOM
 ; CHECK-NEXT:     Name:            linking
 ; CHECK-NEXT:     DataSize:        31
 ; CHECK-NEXT:     SymbolInfo:
+; CHECK-NEXT:       - Name:            bar_import
+; CHECK-NEXT:         Flags:           [ BINDING_WEAK ]
 ; CHECK-NEXT:       - Name:            hello
 ; CHECK-NEXT:         Flags:           [ VISIBILITY_HIDDEN ]
 ; CHECK-NEXT:       - Name:            my_func
 ; CHECK-NEXT:         Flags:           [ VISIBILITY_HIDDEN ]
 ; CHECK-NEXT:       - Name:            func_comdat
 ; CHECK-NEXT:         Flags:           [ BINDING_WEAK ]
 ; CHECK-NEXT:       - Name:            data_comdat
 ; CHECK-NEXT:         Flags:           [ BINDING_WEAK ]

wasm/Writer.cpp

void Writer::createLinkingSection() {
   writeUleb128(DataSizeSubSection.getStream(), DataSize, "data size");
   DataSizeSubSection.finalizeContents();
   DataSizeSubSection.writeToStream(OS);
 
   if (!Config->Relocatable)
     return;
 
   std::vector<std::pair<StringRef, uint32_t>> SymbolInfo;
-  for (const WasmExportEntry &E : ExportedSymbols) {
+  auto addSymInfo = [&](const Symbol *Sym, StringRef ExternalName) {
     uint32_t Flags =
-        (E.Sym->isLocal() ? WASM_SYMBOL_BINDING_LOCAL :
-         E.Sym->isWeak() ? WASM_SYMBOL_BINDING_WEAK : 0) |
-        (E.Sym->isHidden() ? WASM_SYMBOL_VISIBILITY_HIDDEN : 0);
+        (Sym->isLocal() ? WASM_SYMBOL_BINDING_LOCAL :
+         Sym->isWeak() ? WASM_SYMBOL_BINDING_WEAK : 0) |
+        (Sym->isHidden() ? WASM_SYMBOL_VISIBILITY_HIDDEN : 0);
     if (Flags)
-      SymbolInfo.emplace_back(E.FieldName, Flags);
-  }
+      SymbolInfo.emplace_back(ExternalName, Flags);
+  };
+  // (Imports can't have internal linkage, their names don't need to be budged.)
+  for (const Symbol *Sym : ImportedFunctions)
+    addSymInfo(Sym, Sym->getName());
+  for (const Symbol *Sym : ImportedGlobals)
+    addSymInfo(Sym, Sym->getName());
+  for (const WasmExportEntry &E : ExportedSymbols)
+    addSymInfo(E.Sym, E.FieldName);
   if (!SymbolInfo.empty()) {
     SubSection SubSection(WASM_SYMBOL_INFO);
     writeUleb128(SubSection.getStream(), SymbolInfo.size(), "num sym info");
     for (auto Pair: SymbolInfo) {
       writeStr(SubSection.getStream(), Pair.first, "sym name");
       writeUleb128(SubSection.getStream(), Pair.second, "sym flags");
     }
     SubSection.finalizeContents();