Skip to content

Commit d3052d5

Browse files
sbc100sbc100
committedJan 18, 2018
[WebAssembly] Add missing function exports and SYM_INFO to --relocatable output
When writing relocatable files we were exporting for all globals (including file-local syms), but not for functions. Oops. To be consistent with non-relocatable output, all symbols (file-local and global) should be exported. Any symbol targetted by further relocations needs to be exported. The lack of local function exports was just an omission, I think. Second bug: Local symbol names can collide, causing an illegal Wasm file to be generated! Oops again. This only previously affected producing relocatable output from two files, where each had a global with the same name. We need to "budge" the symbol names for locals that are exported on relocatable output. Third bug: LLD's relocatable output wasn't writing out any symbol flags! Thus the local globals weren't being marked as local, and the hidden flag was also stripped... Added tests to exercise colliding local names with/without relocatable flag Patch by Nicholas Wilson! Differential Revision: https://reviews.llvm.org/D42105 llvm-svn: 322908
·
llvmorg-21-initllvmorg-7.0.0-rc1
1 parent a19b748 commit d3052d5

8 files changed

+794
-40
lines changed
 

‎lld/test/wasm/Inputs/locals-duplicate1.ll

+49
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,49 @@
1+
; Will collide: local (internal linkage) with global (external) linkage
2+
@colliding_global1 = internal default global i32 0, align 4
3+
; Will collide: global with local
4+
@colliding_global2 = default global i32 0, align 4
5+
; Will collide: local with local
6+
@colliding_global3 = internal default global i32 0, align 4
7+
8+
; Will collide: local with global
9+
define internal i32 @colliding_func1() {
10+
entry:
11+
ret i32 2
12+
}
13+
; Will collide: global with local
14+
define i32 @colliding_func2() {
15+
entry:
16+
ret i32 2
17+
}
18+
; Will collide: local with local
19+
define internal i32 @colliding_func3() {
20+
entry:
21+
ret i32 2
22+
}
23+
24+
25+
define i32* @get_global1A() {
26+
entry:
27+
ret i32* @colliding_global1
28+
}
29+
define i32* @get_global2A() {
30+
entry:
31+
ret i32* @colliding_global2
32+
}
33+
define i32* @get_global3A() {
34+
entry:
35+
ret i32* @colliding_global3
36+
}
37+
38+
define i32 ()* @get_func1A() {
39+
entry:
40+
ret i32 ()* @colliding_func1
41+
}
42+
define i32 ()* @get_func2A() {
43+
entry:
44+
ret i32 ()* @colliding_func2
45+
}
46+
define i32 ()* @get_func3A() {
47+
entry:
48+
ret i32 ()* @colliding_func3
49+
}

‎lld/test/wasm/Inputs/locals-duplicate2.ll

+49
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,49 @@
1+
; Will collide: local (internal linkage) with global (external) linkage
2+
@colliding_global1 = default global i32 0, align 4
3+
; Will collide: global with local
4+
@colliding_global2 = internal default global i32 0, align 4
5+
; Will collide: local with local
6+
@colliding_global3 = internal default global i32 0, align 4
7+
8+
; Will collide: local with global
9+
define i32 @colliding_func1() {
10+
entry:
11+
ret i32 2
12+
}
13+
; Will collide: global with local
14+
define internal i32 @colliding_func2() {
15+
entry:
16+
ret i32 2
17+
}
18+
; Will collide: local with local
19+
define internal i32 @colliding_func3() {
20+
entry:
21+
ret i32 2
22+
}
23+
24+
25+
define i32* @get_global1B() {
26+
entry:
27+
ret i32* @colliding_global1
28+
}
29+
define i32* @get_global2B() {
30+
entry:
31+
ret i32* @colliding_global2
32+
}
33+
define i32* @get_global3B() {
34+
entry:
35+
ret i32* @colliding_global3
36+
}
37+
38+
define i32 ()* @get_func1B() {
39+
entry:
40+
ret i32 ()* @colliding_func1
41+
}
42+
define i32 ()* @get_func2B() {
43+
entry:
44+
ret i32 ()* @colliding_func2
45+
}
46+
define i32 ()* @get_func3B() {
47+
entry:
48+
ret i32 ()* @colliding_func3
49+
}

‎lld/test/wasm/call-indirect.ll

+3-3
Original file line numberDiff line numberDiff line change
@@ -85,15 +85,15 @@ define void @call_ptr(i64 (i64)* %arg) {
8585
; CHECK-NEXT: - Name: _start
8686
; CHECK-NEXT: Kind: FUNCTION
8787
; CHECK-NEXT: Index: 3
88-
; CHECK-NEXT: - Name: foo
89-
; CHECK-NEXT: Kind: FUNCTION
90-
; CHECK-NEXT: Index: 2
9188
; CHECK-NEXT: - Name: bar
9289
; CHECK-NEXT: Kind: FUNCTION
9390
; CHECK-NEXT: Index: 0
9491
; CHECK-NEXT: - Name: call_bar_indirect
9592
; CHECK-NEXT: Kind: FUNCTION
9693
; CHECK-NEXT: Index: 1
94+
; CHECK-NEXT: - Name: foo
95+
; CHECK-NEXT: Kind: FUNCTION
96+
; CHECK-NEXT: Index: 2
9797
; CHECK-NEXT: - Name: call_ptr
9898
; CHECK-NEXT: Kind: FUNCTION
9999
; CHECK-NEXT: Index: 4

‎lld/test/wasm/init-fini.ll

+38-1
Original file line numberDiff line numberDiff line change
@@ -107,7 +107,44 @@ entry:
107107

108108
; RELOC: Name: linking
109109
; RELOC-NEXT: DataSize: 0
110-
; RELOC-NEXT: InitFunctions:
110+
; RELOC-NEXT: SymbolInfo:
111+
; RELOC-NEXT: - Name: func1
112+
; RELOC-NEXT: Flags: [ VISIBILITY_HIDDEN ]
113+
; RELOC-NEXT: - Name: func2
114+
; RELOC-NEXT: Flags: [ VISIBILITY_HIDDEN ]
115+
; RELOC-NEXT: - Name: func3
116+
; RELOC-NEXT: Flags: [ VISIBILITY_HIDDEN ]
117+
; RELOC-NEXT: - Name: func4
118+
; RELOC-NEXT: Flags: [ VISIBILITY_HIDDEN ]
119+
; RELOC-NEXT: - Name: _start
120+
; RELOC-NEXT: Flags: [ VISIBILITY_HIDDEN ]
121+
; RELOC-NEXT: - Name: .Lcall_dtors.101
122+
; RELOC-NEXT: Flags: [ BINDING_LOCAL ]
123+
; RELOC-NEXT: - Name: .Lregister_call_dtors.101
124+
; RELOC-NEXT: Flags: [ BINDING_LOCAL ]
125+
; RELOC-NEXT: - Name: .Lbitcast
126+
; RELOC-NEXT: Flags: [ BINDING_LOCAL ]
127+
; RELOC-NEXT: - Name: .Lcall_dtors.1001
128+
; RELOC-NEXT: Flags: [ BINDING_LOCAL ]
129+
; RELOC-NEXT: - Name: .Lregister_call_dtors.1001
130+
; RELOC-NEXT: Flags: [ BINDING_LOCAL ]
131+
; RELOC-NEXT: - Name: myctor
132+
; RELOC-NEXT: Flags: [ VISIBILITY_HIDDEN ]
133+
; RELOC-NEXT: - Name: mydtor
134+
; RELOC-NEXT: Flags: [ VISIBILITY_HIDDEN ]
135+
; RELOC-NEXT: - Name: .Lcall_dtors.101.1
136+
; RELOC-NEXT: Flags: [ BINDING_LOCAL ]
137+
; RELOC-NEXT: - Name: .Lregister_call_dtors.101.1
138+
; RELOC-NEXT: Flags: [ BINDING_LOCAL ]
139+
; RELOC-NEXT: - Name: .Lcall_dtors.202
140+
; RELOC-NEXT: Flags: [ BINDING_LOCAL ]
141+
; RELOC-NEXT: - Name: .Lregister_call_dtors.202
142+
; RELOC-NEXT: Flags: [ BINDING_LOCAL ]
143+
; RELOC-NEXT: - Name: .Lcall_dtors.2002
144+
; RELOC-NEXT: Flags: [ BINDING_LOCAL ]
145+
; RELOC-NEXT: - Name: .Lregister_call_dtors.2002
146+
; RELOC-NEXT: Flags: [ BINDING_LOCAL ]
147+
; RELOC-NEXT: InitFunctions:
111148
; RELOC-NEXT: - Priority: 101
112149
; RELOC-NEXT: FunctionIndex: 0
113150
; RELOC-NEXT: - Priority: 101

‎lld/test/wasm/locals-duplicate.test

+543
Large diffs are not rendered by default.

‎lld/test/wasm/relocatable.ll

+15
Original file line numberDiff line numberDiff line change
@@ -200,6 +200,21 @@ entry:
200200
; CHECK-NEXT: - Type: CUSTOM
201201
; CHECK-NEXT: Name: linking
202202
; CHECK-NEXT: DataSize: 23
203+
; CHECK-NEXT: SymbolInfo:
204+
; CHECK-NEXT: - Name: hello
205+
; CHECK-NEXT: Flags: [ VISIBILITY_HIDDEN ]
206+
; CHECK-NEXT: - Name: my_func
207+
; CHECK-NEXT: Flags: [ VISIBILITY_HIDDEN ]
208+
; CHECK-NEXT: - Name: func_comdat
209+
; CHECK-NEXT: Flags: [ BINDING_WEAK ]
210+
; CHECK-NEXT: - Name: data_comdat
211+
; CHECK-NEXT: Flags: [ BINDING_WEAK ]
212+
; CHECK-NEXT: - Name: func_addr1
213+
; CHECK-NEXT: Flags: [ VISIBILITY_HIDDEN ]
214+
; CHECK-NEXT: - Name: func_addr2
215+
; CHECK-NEXT: Flags: [ VISIBILITY_HIDDEN ]
216+
; CHECK-NEXT: - Name: data_addr1
217+
; CHECK-NEXT: Flags: [ VISIBILITY_HIDDEN ]
203218
; CHECK-NEXT: SegmentInfo:
204219
; CHECK-NEXT: - Index: 0
205220
; CHECK-NEXT: Name: .rodata.hello_str

‎lld/test/wasm/visibility-hidden.ll

+3-3
Original file line numberDiff line numberDiff line change
@@ -37,12 +37,12 @@ entry:
3737
; CHECK-NEXT: - Name: _start
3838
; CHECK-NEXT: Kind: FUNCTION
3939
; CHECK-NEXT: Index: 2
40-
; CHECK-NEXT: - Name: archiveDefault
41-
; CHECK-NEXT: Kind: FUNCTION
42-
; CHECK-NEXT: Index: 4
4340
; CHECK-NEXT: - Name: objectDefault
4441
; CHECK-NEXT: Kind: FUNCTION
4542
; CHECK-NEXT: Index: 1
43+
; CHECK-NEXT: - Name: archiveDefault
44+
; CHECK-NEXT: Kind: FUNCTION
45+
; CHECK-NEXT: Index: 4
4646
; CHECK-NEXT: - Name: __heap_base
4747
; CHECK-NEXT: Kind: GLOBAL
4848
; CHECK-NEXT: Index: 1

‎lld/wasm/Writer.cpp

+94-33
Original file line numberDiff line numberDiff line change
@@ -62,6 +62,12 @@ struct WasmSignatureDenseMapInfo {
6262
}
6363
};
6464

65+
// A Wasm export to be written into the export section.
66+
struct WasmExportEntry {
67+
const Symbol *Symbol;
68+
StringRef FieldName; // may not match the Symbol name
69+
};
70+
6571
// The writer writes a SymbolTable result to a file.
6672
class Writer {
6773
public:
@@ -76,6 +82,7 @@ class Writer {
7682
void calculateInitFunctions();
7783
void assignIndexes();
7884
void calculateImports();
85+
void calculateExports();
7986
void calculateTypes();
8087
void createOutputSegments();
8188
void layoutMemory();
@@ -114,6 +121,7 @@ class Writer {
114121
DenseMap<WasmSignature, int32_t, WasmSignatureDenseMapInfo> TypeIndices;
115122
std::vector<const Symbol *> ImportedFunctions;
116123
std::vector<const Symbol *> ImportedGlobals;
124+
std::vector<WasmExportEntry> ExportedSymbols;
117125
std::vector<const Symbol *> DefinedGlobals;
118126
std::vector<InputFunction *> DefinedFunctions;
119127
std::vector<const Symbol *> IndirectFunctions;
@@ -259,35 +267,8 @@ void Writer::createTableSection() {
259267

260268
void Writer::createExportSection() {
261269
bool ExportMemory = !Config->Relocatable && !Config->ImportMemory;
262-
Symbol *EntrySym = Symtab->find(Config->Entry);
263-
bool ExportEntry = !Config->Relocatable && EntrySym && EntrySym->isDefined();
264-
bool ExportHidden = Config->EmitRelocs;
265-
266-
uint32_t NumExports = ExportMemory ? 1 : 0;
267-
268-
std::vector<const Symbol *> SymbolExports;
269-
if (ExportEntry)
270-
SymbolExports.emplace_back(EntrySym);
271-
272-
for (const Symbol *Sym : Symtab->getSymbols()) {
273-
if (Sym->isUndefined() || Sym->isGlobal())
274-
continue;
275-
if (Sym->isHidden() && !ExportHidden)
276-
continue;
277-
if (ExportEntry && Sym == EntrySym)
278-
continue;
279-
SymbolExports.emplace_back(Sym);
280-
}
281270

282-
for (const Symbol *Sym : DefinedGlobals) {
283-
// Can't export the SP right now because it mutable and mutable globals
284-
// connot be exported.
285-
if (Sym == Config->StackPointerSymbol)
286-
continue;
287-
SymbolExports.emplace_back(Sym);
288-
}
289-
290-
NumExports += SymbolExports.size();
271+
uint32_t NumExports = (ExportMemory ? 1 : 0) + ExportedSymbols.size();
291272
if (!NumExports)
292273
return;
293274

@@ -304,12 +285,12 @@ void Writer::createExportSection() {
304285
writeExport(OS, MemoryExport);
305286
}
306287

307-
for (const Symbol *Sym : SymbolExports) {
308-
DEBUG(dbgs() << "Export: " << Sym->getName() << "\n");
288+
for (const WasmExportEntry &E : ExportedSymbols) {
289+
DEBUG(dbgs() << "Export: " << E.Symbol->getName() << "\n");
309290
WasmExport Export;
310-
Export.Name = Sym->getName();
311-
Export.Index = Sym->getOutputIndex();
312-
if (Sym->isFunction())
291+
Export.Name = E.FieldName;
292+
Export.Index = E.Symbol->getOutputIndex();
293+
if (E.Symbol->isFunction())
313294
Export.Kind = WASM_EXTERNAL_FUNCTION;
314295
else
315296
Export.Kind = WASM_EXTERNAL_GLOBAL;
@@ -404,6 +385,26 @@ void Writer::createLinkingSection() {
404385
if (!Config->Relocatable)
405386
return;
406387

388+
std::vector<std::pair<StringRef, uint32_t>> SymbolInfo;
389+
for (const WasmExportEntry &E : ExportedSymbols) {
390+
uint32_t Flags =
391+
(E.Symbol->isLocal() ? WASM_SYMBOL_BINDING_LOCAL :
392+
E.Symbol->isWeak() ? WASM_SYMBOL_BINDING_WEAK : 0) |
393+
(E.Symbol->isHidden() ? WASM_SYMBOL_VISIBILITY_HIDDEN : 0);
394+
if (Flags)
395+
SymbolInfo.emplace_back(E.FieldName, Flags);
396+
}
397+
if (!SymbolInfo.empty()) {
398+
SubSection SubSection(WASM_SYMBOL_INFO);
399+
writeUleb128(SubSection.getStream(), SymbolInfo.size(), "num sym info");
400+
for (auto Pair: SymbolInfo) {
401+
writeStr(SubSection.getStream(), Pair.first, "sym name");
402+
writeUleb128(SubSection.getStream(), Pair.second, "sym flags");
403+
}
404+
SubSection.finalizeContents();
405+
SubSection.writeToStream(OS);
406+
}
407+
407408
if (Segments.size()) {
408409
SubSection SubSection(WASM_SEGMENT_INFO);
409410
writeUleb128(SubSection.getStream(), Segments.size(), "num data segments");
@@ -608,6 +609,64 @@ void Writer::calculateImports() {
608609
}
609610
}
610611

612+
void Writer::calculateExports() {
613+
Symbol *EntrySym = Symtab->find(Config->Entry);
614+
bool ExportEntry = !Config->Relocatable && EntrySym && EntrySym->isDefined();
615+
bool ExportHidden = Config->EmitRelocs;
616+
StringSet<> UsedNames;
617+
auto BudgeLocalName = [&](const Symbol *Sym) {
618+
StringRef SymName = Sym->getName();
619+
// We can't budge non-local names.
620+
if (!Sym->isLocal())
621+
return SymName;
622+
// We must budge local names that have a collision with a symbol that we
623+
// haven't yet processed.
624+
if (!Symtab->find(SymName) && UsedNames.insert(SymName).second)
625+
return SymName;
626+
for (unsigned I = 1; ; ++I) {
627+
std::string NameBuf = (SymName + "." + Twine(I)).str();
628+
if (!UsedNames.count(NameBuf)) {
629+
StringRef Name = Saver.save(NameBuf);
630+
UsedNames.insert(Name); // Insert must use safe StringRef from save()
631+
return Name;
632+
}
633+
}
634+
};
635+
636+
if (ExportEntry)
637+
ExportedSymbols.emplace_back(WasmExportEntry{EntrySym, EntrySym->getName()});
638+
639+
if (Config->CtorSymbol && ExportHidden &&
640+
!(ExportEntry && Config->CtorSymbol == EntrySym))
641+
ExportedSymbols.emplace_back(
642+
WasmExportEntry{Config->CtorSymbol, Config->CtorSymbol->getName()});
643+
644+
for (ObjFile *File : Symtab->ObjectFiles) {
645+
for (Symbol *Sym : File->getSymbols()) {
646+
if (!Sym->isDefined() || File != Sym->getFile())
647+
continue;
648+
if (Sym->isGlobal())
649+
continue;
650+
if (Sym->getFunction()->Discarded)
651+
continue;
652+
653+
if ((Sym->isHidden() || Sym->isLocal()) && !ExportHidden)
654+
continue;
655+
if (ExportEntry && Sym == EntrySym)
656+
continue;
657+
ExportedSymbols.emplace_back(WasmExportEntry{Sym, BudgeLocalName(Sym)});
658+
}
659+
}
660+
661+
for (const Symbol *Sym : DefinedGlobals) {
662+
// Can't export the SP right now because it's mutable and mutable globals
663+
// cannot be exported.
664+
if (Sym == Config->StackPointerSymbol)
665+
continue;
666+
ExportedSymbols.emplace_back(WasmExportEntry{Sym, BudgeLocalName(Sym)});
667+
}
668+
}
669+
611670
uint32_t Writer::lookupType(const WasmSignature &Sig) {
612671
auto It = TypeIndices.find(Sig);
613672
if (It == TypeIndices.end()) {
@@ -793,6 +852,8 @@ void Writer::run() {
793852
calculateImports();
794853
log("-- assignIndexes");
795854
assignIndexes();
855+
log("-- calculateExports");
856+
calculateExports();
796857
log("-- calculateInitFunctions");
797858
calculateInitFunctions();
798859
if (!Config->Relocatable)

0 commit comments

Comments
 (0)
Please sign in to comment.