[WebAssembly] Add support for visibility flag

[sbc100]

Summary

Patch by Nicholas Wilson (https://reviews.llvm.org/D40555)

[sbc100]

I hope you don't mind I made a few mods to your patch so that the entry point was alwasy exports (regardless of visibility). This is because wasm clang will defaults to hidden for all symbols.

[ruiu]

Can you submit typo-fixing changes as a separate patch? You don't need any code review for such trivial change.

wasm/Writer.cpp

273–274

Can this condition happen? I thought that it is an error if -entry was given but the symbol was not found.

[sbc100]

wasm/Writer.cpp

273–274

We have an --allow-undefined flag, which basically skips the step when we check for undefined symbols in the symbol table. Using this flag you can end up with and undefined _start in your binary, I'm not sure we want to allow that, perhaps we should error out where if _start is not defined? That can be a separate change perhaps?

[sbc100]

• rebase

[ruiu]

wasm/Writer.cpp

262–265

I think that bool ExportEntry = !Config->Relocatable && EntrySym && EntrySym->isDefined() is more readable.

273–274

Ah, that's true. Even though the --allow-undefined flag is mostly for debugging, it is a valid use case. But if that's the case, don't you want to check if EntrySym is not nullptr?

279

Please use error to report a soft error. We use fatal only to report corrupted input files.

[ncw]

Thanks for adding those improvements, good with me.

ruiu's spotted a couple of minor improvements.

I think it is already an error if the entry-point symbol can't be found, but the actual check for that is in Driver.cpp, the check in Writer is just a belt-and-braces thing and doesn't matter too much.

[ncw]

Hmm, I can't update this Phabricator diff to apply ruiu's requested fixes... Sorry Sam, you'll have to apply them yourself.

Here below are the changes ruiu requested. Also included below is a suggested change to get rid of the if (Sym == EntrySym) special-casing, and reuse the existing WrittenToSymtab tracking and save a couple of lines.

diff --git a/wasm/Writer.cpp b/wasm/Writer.cpp
index c9455b0aa..bae8e3a51 100644
--- a/wasm/Writer.cpp
+++ b/wasm/Writer.cpp
@@ -259,10 +259,10 @@ void Writer::createTableSection() {
 void Writer::createExportSection() {
   // Memory is and main function are exported for executables.
   bool ExportMemory = !Config->Relocatable && !Config->ImportMemory;
-  bool ExportEntry = !Config->Relocatable;
   bool ExportOther = true; // ??? TODO Config->Relocatable;
   bool ExportHidden = Config->Relocatable;
-  Symbol *EntrySym = ExportEntry ? Symtab->find(Config->Entry) : nullptr;
+  Symbol *EntrySym = !Config->Relocatable ? Symtab->find(Config->Entry) : nullptr;
+  bool ExportEntry = EntrySym && EntrySym->isDefined();
 
   uint32_t NumExports = 0;
 
@@ -270,13 +270,11 @@ void Writer::createExportSection() {
     ++NumExports;
 
   if (ExportEntry) {
-    if (!EntrySym->isDefined())
-      ExportEntry = false;
-    else
-      ++NumExports;
+    EntrySym->WrittenToSymtab = true;
+    ++NumExports;
 
     if (!EntrySym->isFunction())
-      fatal("entry point is not a function: " + EntrySym->getName());
+      error("entry point is not a function: " + EntrySym->getName());
   }
 
   if (ExportOther) {
@@ -285,8 +283,6 @@ void Writer::createExportSection() {
         if (!Sym->isFunction() || Sym->isLocal() || Sym->isUndefined() ||
             (Sym->isHidden() && !ExportHidden) || Sym->WrittenToSymtab)
           continue;
-        if (Sym == EntrySym)
-          continue;
         Sym->WrittenToSymtab = true;
         ++NumExports;
       }
@@ -310,6 +306,8 @@ void Writer::createExportSection() {
   }
 
   if (ExportEntry) {
+    EntrySym->WrittenToSymtab = false;
+    log("Export entrypoint: " + EntrySym->getName());
     WasmExport EntryExport;
     EntryExport.Name = Config->Entry;
     EntryExport.Kind = WASM_EXTERNAL_FUNCTION;
@@ -323,8 +321,6 @@ void Writer::createExportSection() {
         if (!Sym->isFunction() || Sym->isLocal() || Sym->isUndefined() ||
             (Sym->isHidden() && !ExportHidden) || !Sym->WrittenToSymtab)
           continue;
-        if (Sym == EntrySym)
-          continue;
         Sym->WrittenToSymtab = false;
         log("Export: " + Sym->getName());
         WasmExport Export;

[sbc100]

wasm/Writer.cpp

273–274

Entry is added to the symbol table as undefined in Driver so it should always be part of the symbol table, but that symbols table can have undefined symbols it in iff --allow-undefined is passed. So it wont be null in that case.

[ruiu]

wasm/Writer.cpp

273–274

Good point! I didn't notice that.

[ruiu]

Can you upload the final version again?

[sbc100]

• review feedback

[sbc100]

Sorry, I thought i had uploaded that already.

[ruiu]

LGTM

test/wasm/Inputs/call-indirect.ll

 target datalayout = "e-m:e-p:32:32-i64:64-n32:64-S128"
 target triple = "wasm32-unknown-unknown-wasm"
 
 @indirect_bar = hidden local_unnamed_addr global i32 ()* @bar, align 4
 
 ; Function Attrs: norecurse nounwind readnone
-define hidden i32 @bar() #0 {
+define i32 @bar() #0 {
 entry:
   ret i32 1
 }
 
 ; Function Attrs: nounwind
-define hidden void @call_bar_indirect() local_unnamed_addr #1 {
+define void @call_bar_indirect() local_unnamed_addr #1 {
 entry:
   %0 = load i32 ()*, i32 ()** @indirect_bar, align 4
   %call = tail call i32 %0() #2
   ret void
 }

test/wasm/Inputs/hidden.ll

 target datalayout = "e-m:e-p:32:32-i64:64-n32:64-S128"
 target triple = "wasm32-unknown-unknown-wasm"
 
 ; Function Attrs: norecurse nounwind readnone
-define hidden i32 @ret32(float %arg) #0 {
+define hidden i32 @archiveHidden() #0 {
+entry:
+    ret i32 0
+}
+
+; Function Attrs: norecurse nounwind readnone
+define i32 @archiveDefault() #1 {
 entry:
     ret i32 0
-     ; ptrtoint (i32 (float)* @ret32 to i32)
 }

test/wasm/Inputs/ret32.ll

 target datalayout = "e-m:e-p:32:32-i64:64-n32:64-S128"
 target triple = "wasm32-unknown-unknown-wasm"
 
 ; Function Attrs: norecurse nounwind readnone
-define hidden i32 @ret32(float %arg) #0 {
+define i32 @ret32(float %arg) #0 {
 entry:
     ret i32 0
      ; ptrtoint (i32 (float)* @ret32 to i32)
 }

test/wasm/Inputs/ret64.ll

 target datalayout = "e-m:e-p:32:32-i64:64-n32:64-S128"
 target triple = "wasm32-unknown-unknown-wasm"
 
-define hidden i64 @ret64(double %arg) local_unnamed_addr #0 {
+define i64 @ret64(double %arg) local_unnamed_addr #0 {
 entry:
     ret i64 1
 }

test/wasm/Inputs/weak-alias.ll

 define i32 @foo() #0 {
 entry:
   ret i32 0
 }
 
 @bar = weak alias i32 (), i32 ()* @foo
 
-define hidden i32 @call_bar() #0 {
+define i32 @call_bar() #0 {
 entry:
   %call = call i32 @bar()
   ret i32 %call
 }

test/wasm/call-indirect.ll

 ; RUN: llc -filetype=obj %p/Inputs/call-indirect.ll -o %t2.o
 ; RUN: llc -filetype=obj %s -o %t.o
 ; RUN: lld -flavor wasm -o %t.wasm %t2.o %t.o
 ; RUN: obj2yaml %t.wasm | FileCheck %s
 
 ; bitcode generated from the following C code:
 ; int foo(void) { return 1; }
 ; int (*indirect_func)(void) = &foo;
 ; void _start(void) { indirect_func(); }
 
 target datalayout = "e-m:e-p:32:32-i64:64-n32:64-S128"
 target triple = "wasm32-unknown-unknown-wasm"
 
-@indirect_func = hidden local_unnamed_addr global i32 ()* @foo, align 4
+@indirect_func = local_unnamed_addr global i32 ()* @foo, align 4
 
 ; Function Attrs: norecurse nounwind readnone
-define hidden i32 @foo() #0 {
+define i32 @foo() #0 {
 entry:
   ret i32 1
 }
 
 ; Function Attrs: nounwind
-define hidden void @_start() local_unnamed_addr #1 {
+define void @_start() local_unnamed_addr #1 {
 entry:
   %0 = load i32 ()*, i32 ()** @indirect_func, align 4
   %call = tail call i32 %0() #2
   ret void
 }
 
 ; CHECK:      !WASM
 ; CHECK-NEXT: FileHeader:      

 ; CHECK-NEXT:         InitExpr:        
 ; CHECK-NEXT:           Opcode:          I32_CONST
 ; CHECK-NEXT:           Value:           66576
 ; CHECK-NEXT:   - Type:            EXPORT
 ; CHECK-NEXT:     Exports:         
 ; CHECK-NEXT:       - Name:            memory
 ; CHECK-NEXT:         Kind:            MEMORY
 ; CHECK-NEXT:         Index:           0
+; CHECK-NEXT:       - Name:            _start
+; CHECK-NEXT:         Kind:            FUNCTION
+; CHECK-NEXT:         Index:           3
 ; CHECK-NEXT:       - Name:            bar
 ; CHECK-NEXT:         Kind:            FUNCTION
 ; CHECK-NEXT:         Index:           0
 ; CHECK-NEXT:       - Name:            call_bar_indirect
 ; CHECK-NEXT:         Kind:            FUNCTION
 ; CHECK-NEXT:         Index:           1
 ; CHECK-NEXT:       - Name:            foo
 ; CHECK-NEXT:         Kind:            FUNCTION
 ; CHECK-NEXT:         Index:           2
-; CHECK-NEXT:       - Name:            _start
-; CHECK-NEXT:         Kind:            FUNCTION
-; CHECK-NEXT:         Index:           3
 ; CHECK:        - Type:            ELEM
 ; CHECK-NEXT:     Segments:        
 ; CHECK-NEXT:       - Offset:          
 ; CHECK-NEXT:           Opcode:          I32_CONST
 ; CHECK-NEXT:           Value:           1
 ; CHECK-NEXT:         Functions:       [ 0, 2 ]
 ; CHECK-NEXT:   - Type:            CODE
 ; CHECK-NEXT:     Functions:       

test/wasm/entry.ll

 ; RUN: llc -filetype=obj %s -o %t.o
 ; RUN: lld -flavor wasm -e entry -o %t.wasm %t.o
 ; RUN: obj2yaml %t.wasm | FileCheck %s
 ; RUN: lld -flavor wasm --entry=entry -o %t.wasm %t.o
 ; RUN: obj2yaml %t.wasm | FileCheck %s
 
 target datalayout = "e-m:e-p:32:32-i64:64-n32:64-S128"
 target triple = "wasm32-unknown-unknown-wasm"
 
-define hidden void @entry() local_unnamed_addr #0 {
+define void @entry() local_unnamed_addr #0 {
 entry:
   ret void
 }
 
 ; CHECK:   - Type:            EXPORT
 ; CHECK:     Exports:         
 ; CHECK:       - Name:            memory
 ; CHECK:         Kind:            MEMORY
 ; CHECK:         Index:           0
 ; CHECK:       - Name:            entry
 ; CHECK:         Kind:            FUNCTION
 ; CHECK:         Index:           0

test/wasm/visibility-hidden.ll

+; RUN: llc -mtriple wasm32-unknown-unknown-wasm -filetype=obj -o %t.o %s
+; RUN: llc -mtriple=wasm32-unknown-unknown-wasm -filetype=obj %S/Inputs/hidden.ll -o %t2.o
+; RUN: llvm-ar rcs %t2.a %t2.o
+; RUN: lld -flavor wasm %t.o %t2.a -o %t.wasm
+; RUN: obj2yaml %t.wasm | FileCheck %s
+
+; Test that hidden symbols are not exported, whether pulled in from an archive
+; or directly.
+
+define hidden i32 @objectHidden() {
+entry:
+    ret i32 0
+}
+
+define i32 @objectDefault() {
+entry:
+    ret i32 0
+}
+
+declare i32 @archiveHidden()
+declare i32 @archiveDefault()
+
+define i32 @_start() {
+entry:
+  %call1 = call i32 @objectHidden()
+  %call2 = call i32 @objectDefault()
+  %call3 = call i32 @archiveHidden()
+  %call4 = call i32 @archiveDefault()
+  ret i32 0
+}
+
+; CHECK:        - Type:            EXPORT
+; CHECK-NEXT:     Exports:         
+; CHECK-NEXT:       - Name:            memory
+; CHECK-NEXT:         Kind:            MEMORY
+; CHECK-NEXT:         Index:           0
+; CHECK-NEXT:       - Name:            _start
+; CHECK-NEXT:         Kind:            FUNCTION
+; CHECK-NEXT:         Index:           2
+; CHECK-NEXT:       - Name:            archiveDefault
+; CHECK-NEXT:         Kind:            FUNCTION
+; CHECK-NEXT:         Index:           4
+; CHECK-NEXT:       - Name:            objectDefault
+; CHECK-NEXT:         Kind:            FUNCTION
+; CHECK-NEXT:         Index:           1
+; CHECK-NEXT:   - Type:

test/wasm/weak-alias-overide.ll

 ; CHECK-NEXT:         InitExpr:        
 ; CHECK-NEXT:           Opcode:          I32_CONST
 ; CHECK-NEXT:           Value:           66560
 ; CHECK-NEXT:   - Type:            EXPORT
 ; CHECK-NEXT:     Exports:         
 ; CHECK-NEXT:       - Name:            memory
 ; CHECK-NEXT:         Kind:            MEMORY
 ; CHECK-NEXT:         Index:           0
-; CHECK-NEXT:       - Name:            bar
-; CHECK-NEXT:         Kind:            FUNCTION
-; CHECK-NEXT:         Index:           0
 ; CHECK-NEXT:       - Name:            _start
 ; CHECK-NEXT:         Kind:            FUNCTION
 ; CHECK-NEXT:         Index:           1
+; CHECK-NEXT:       - Name:            bar
+; CHECK-NEXT:         Kind:            FUNCTION
+; CHECK-NEXT:         Index:           0
 ; CHECK-NEXT:       - Name:            foo
 ; CHECK-NEXT:         Kind:            FUNCTION
 ; CHECK-NEXT:         Index:           2
 ; CHECK-NEXT:       - Name:            call_bar
 ; CHECK-NEXT:         Kind:            FUNCTION
 ; CHECK-NEXT:         Index:           3
 ; CHECK-NEXT:   - Type:            CODE
 ; CHECK-NEXT:     Functions:       

test/wasm/weak-alias.ll

 ; CHECK-NEXT:         InitExpr:        
 ; CHECK-NEXT:           Opcode:          I32_CONST
 ; CHECK-NEXT:           Value:           66560
 ; CHECK-NEXT:   - Type:            EXPORT
 ; CHECK-NEXT:     Exports:         
 ; CHECK-NEXT:       - Name:            memory
 ; CHECK-NEXT:         Kind:            MEMORY
 ; CHECK-NEXT:         Index:           0
-; CHECK-NEXT:       - Name:            bar
-; CHECK-NEXT:         Kind:            FUNCTION
-; CHECK-NEXT:         Index:           1
 ; CHECK-NEXT:       - Name:            _start
 ; CHECK-NEXT:         Kind:            FUNCTION
 ; CHECK-NEXT:         Index:           0
+; CHECK-NEXT:       - Name:            bar
+; CHECK-NEXT:         Kind:            FUNCTION
+; CHECK-NEXT:         Index:           1
 ; CHECK-NEXT:       - Name:            foo
 ; CHECK-NEXT:         Kind:            FUNCTION
 ; CHECK-NEXT:         Index:           1
 ; CHECK-NEXT:       - Name:            call_bar
 ; CHECK-NEXT:         Kind:            FUNCTION
 ; CHECK-NEXT:         Index:           2
 ; CHECK-NEXT:   - Type:            CODE
 ; CHECK-NEXT:     Functions:       

test/wasm/weak-external.ll

 ; RUN: llc -mtriple wasm32-unknown-unknown-wasm -filetype=obj -o %t.o %s
 ; RUN: lld -flavor wasm -strip-debug %t.o -o %t.wasm
 ; RUN: obj2yaml %t.wasm | FileCheck %s
 
 ; Test that undefined weak externals (global_var) and (foo) don't cause
 ; link failures and resolve to zero.
 
 @global_var = extern_weak global i32, align 4
 
 declare extern_weak i32 @foo()
 
-define hidden i8* @get_address_of_foo() #0 {
+define i8* @get_address_of_foo() #0 {
 entry:
   ret i8* bitcast (i32 ()* @foo to i8*)
 }
 
-define hidden i32* @get_address_of_global_var() #0 {
+define i32* @get_address_of_global_var() #0 {
   ret i32* @global_var
 }
 
-define hidden i32 @_start() #0 {
+define i32 @_start() #0 {
 entry:
     %0 = load i32, i32* @global_var, align 4
     ret i32 %0
 }
 
 ; CHECK:      --- !WASM
 ; CHECK-NEXT: FileHeader:
 ; CHECK-NEXT:   Version:         0x00000001

 ; CHECK-NEXT:         InitExpr:
 ; CHECK-NEXT:           Opcode:          I32_CONST
 ; CHECK-NEXT:           Value:           66560
 ; CHECK-NEXT:   - Type:            EXPORT
 ; CHECK-NEXT:     Exports:
 ; CHECK-NEXT:       - Name:            memory
 ; CHECK-NEXT:         Kind:            MEMORY
 ; CHECK-NEXT:         Index:           0
+; CHECK-NEXT:       - Name:            _start
+; CHECK-NEXT:         Kind:            FUNCTION
+; CHECK-NEXT:         Index:           2
 ; CHECK-NEXT:       - Name:            get_address_of_foo
 ; CHECK-NEXT:         Kind:            FUNCTION
 ; CHECK-NEXT:         Index:           0
 ; CHECK-NEXT:       - Name:            get_address_of_global_var
 ; CHECK-NEXT:         Kind:            FUNCTION
 ; CHECK-NEXT:         Index:           1
-; CHECK-NEXT:       - Name:            _start
-; CHECK-NEXT:         Kind:            FUNCTION
-; CHECK-NEXT:         Index:           2
 ; CHECK-NEXT:   - Type:            ELEM
 ; CHECK-NEXT:     Segments:        
 ; CHECK-NEXT:       - Offset:          
 ; CHECK-NEXT:           Opcode:          I32_CONST
 ; CHECK-NEXT:           Value:           1
 ; CHECK-NEXT:         Functions:       [ 0 ]
 ; CHECK-NEXT:   - Type:            CODE
 ; CHECK-NEXT:     Functions:

wasm/Driver.cpp

static Optional<std::string> findFile(StringRef Path1, const Twine &Path2) {
   path::append(S, Path1, Path2);
   if (fs::exists(S))
     return S.str().str();
   return None;
 }
 
 // Inject a new wasm global into the output binary with the given value.
 // Wasm global are used in relocatable object files to model symbol imports
-// and exports.  In the final exectuable the only use of wasm globals is the
+// and exports.  In the final executable the only use of wasm globals is
 // for the exlicit stack pointer (__stack_pointer).
 static void addSyntheticGlobal(StringRef Name, int32_t Value) {
   log("injecting global: " + Name);
   Symbol *S = Symtab->addDefinedGlobal(Name);
   S->setOutputIndex(Config->SyntheticGlobals.size());
 
   WasmGlobal Global;
   Global.Mutable = true;

wasm/SymbolTable.cpp

if (WasInserted) {
     S->update(Kind, F, Sym, Segment, NewSig);
   } else if (S->isLazy()) {
     // The existing symbol is lazy. Replace it without checking types since
     // lazy symbols don't have any type information.
     DEBUG(dbgs() << "replacing existing lazy symbol: " << Sym->Name << "\n");
     S->update(Kind, F, Sym, Segment, NewSig);
   } else if (!S->isDefined()) {
     // The existing symbol table entry is undefined. The new symbol replaces
-    // it, after checkign the type matches
+    // it, after checking the type matches
     DEBUG(dbgs() << "resolving existing undefined symbol: " << Sym->Name
                  << "\n");
     checkSymbolTypes(*S, *F, *Sym, NewSig);
     S->update(Kind, F, Sym, Segment, NewSig);
   } else if (Sym->isWeak()) {
     // the new symbol is weak we can ignore it
-    DEBUG(dbgs() << "existing symbol takes precensence\n");
+    DEBUG(dbgs() << "existing symbol takes precedence\n");
   } else if (S->isWeak()) {
     // the new symbol is not weak and the existing symbol is, so we replace
     // it
     DEBUG(dbgs() << "replacing existing weak symbol\n");
     checkSymbolTypes(*S, *F, *Sym, NewSig);
     S->update(Kind, F, Sym, Segment, NewSig);
   } else {
-    // niether symbol is week. They conflict.
+    // neither symbol is week. They conflict.
     reportDuplicate(S, F);
   }
   return S;
 }
 
 Symbol *SymbolTable::addUndefinedFunction(StringRef Name,
                                           const WasmSignature *Type) {
   Symbol *S;

wasm/Symbols.h

public:
   }
   bool isFunction() const {
     return SymbolKind == DefinedFunctionKind ||
            SymbolKind == UndefinedFunctionKind;
   }
   bool isGlobal() const { return !isFunction(); }
   bool isLocal() const { return IsLocal; }
   bool isWeak() const;
+  bool isHidden() const;
 
   // Returns the symbol name.
   StringRef getName() const { return Name; }
 
   // Returns the file from which this symbol was created.
   InputFile *getFile() const { return File; }
 
   uint32_t getGlobalIndex() const;

wasm/Symbols.cpp

void Symbol::update(Kind K, InputFile *F, const WasmSymbol *WasmSym,
   File = F;
   Sym = WasmSym;
   Segment = Seg;
   FunctionType = Sig;
 }
 
 bool Symbol::isWeak() const { return Sym && Sym->isWeak(); }
 
+bool Symbol::isHidden() const { return Sym && Sym->isHidden(); }
+
 std::string lld::toString(wasm::Symbol &Sym) {
   return wasm::displayName(Sym.getName());
 }
 
 std::string lld::toString(wasm::Symbol::Kind &Kind) {
   switch (Kind) {
   case wasm::Symbol::DefinedFunctionKind:
     return "DefinedFunction";

wasm/Writer.cpp

void Writer::createTableSection() {
   writeUleb128(OS, WASM_LIMITS_FLAG_HAS_MAX, "table flags");
   writeUleb128(OS, NumTableElems, "table initial size");
   writeUleb128(OS, NumTableElems, "table max size");
 }
 
 void Writer::createExportSection() {
   // Memory is and main function are exported for executables.
   bool ExportMemory = !Config->Relocatable && !Config->ImportMemory;
-  bool ExportMain = !Config->Relocatable;
-  bool ExportOther = true; // Config->Relocatable;
+  bool ExportEntry = !Config->Relocatable;
+  bool ExportOther = true; // ??? TODO Config->Relocatable;
+  bool ExportHidden = Config->Relocatable;
+  Symbol *EntrySym = ExportEntry ? Symtab->find(Config->Entry) : nullptr;
 
   uint32_t NumExports = 0;
 
   if (ExportMemory)
     ++NumExports;
 
-  if (ExportMain && !ExportOther)
+  if (ExportEntry) {
+    if (!EntrySym->isDefined())
+      ExportEntry = false;
+    else
       ++NumExports;
 
+    if (!EntrySym->isFunction())
+      fatal("entry point is not a function: " + EntrySym->getName());
+  }
+
   if (ExportOther) {
     for (ObjFile *File : Symtab->ObjectFiles) {
       for (Symbol *Sym : File->getSymbols()) {
         if (!Sym->isFunction() || Sym->isLocal() || Sym->isUndefined() ||
-            Sym->WrittenToSymtab)
+            (Sym->isHidden() && !ExportHidden) || Sym->WrittenToSymtab)
+          continue;
+        if (Sym == EntrySym)
           continue;
         Sym->WrittenToSymtab = true;
         ++NumExports;
       }
     }
   }
 
   if (!NumExports)
     return;
 
   SyntheticSection *Section = createSyntheticSection(WASM_SEC_EXPORT);
   raw_ostream &OS = Section->getStream();
 
   writeUleb128(OS, NumExports, "export count");
 
   if (ExportMemory) {
     WasmExport MemoryExport;
     MemoryExport.Name = "memory";
     MemoryExport.Kind = WASM_EXTERNAL_MEMORY;
     MemoryExport.Index = 0;
     writeExport(OS, MemoryExport);
   }
 
-  if (ExportMain) {
-    Symbol *Sym = Symtab->find(Config->Entry);
-    if (Sym->isDefined()) {
-      if (!Sym->isFunction())
-        fatal("entry point is not a function: " + Sym->getName());
-
-      if (!ExportOther) {
-        WasmExport MainExport;
-        MainExport.Name = Config->Entry;
-        MainExport.Kind = WASM_EXTERNAL_FUNCTION;
-        MainExport.Index = Sym->getOutputIndex();
-        writeExport(OS, MainExport);
-      }
-    }
+  if (ExportEntry) {
+    WasmExport EntryExport;
+    EntryExport.Name = Config->Entry;
+    EntryExport.Kind = WASM_EXTERNAL_FUNCTION;
+    EntryExport.Index = EntrySym->getOutputIndex();
+    writeExport(OS, EntryExport);
   }
 
   if (ExportOther) {
     for (ObjFile *File : Symtab->ObjectFiles) {
       for (Symbol *Sym : File->getSymbols()) {
-        if (!Sym->isFunction() || Sym->isLocal() | Sym->isUndefined() ||
-            !Sym->WrittenToSymtab)
+        if (!Sym->isFunction() || Sym->isLocal() || Sym->isUndefined() ||
+            (Sym->isHidden() && !ExportHidden) || !Sym->WrittenToSymtab)
+          continue;
+        if (Sym == EntrySym)
           continue;
         Sym->WrittenToSymtab = false;
         log("Export: " + Sym->getName());
         WasmExport Export;
         Export.Name = Sym->getName();
         Export.Index = Sym->getOutputIndex();
         if (Sym->isFunction())
           Export.Kind = WASM_EXTERNAL_FUNCTION;
         else
           Export.Kind = WASM_EXTERNAL_GLOBAL;
         writeExport(OS, Export);
       }
     }
-
-    // TODO(sbc): Export local symbols too, Even though they are not part
-    // of the symbol table?
   }
 }
 
 void Writer::createStartSection() {}
 
 void Writer::createElemSection() {
   if (!NumElements)
     return;
for (size_t i = 0; i < OrigSize; i++) {
     SyntheticSection *Section = createSyntheticSection(WASM_SEC_CUSTOM, name);
     raw_ostream &OS = Section->getStream();
     writeUleb128(OS, S->Type, "reloc section");
     writeUleb128(OS, Count, "reloc count");
     S->writeRelocations(OS);
   }
 }
 
-// Create the custome "linking" section containing linker metadata.
+// Create the custom "linking" section containing linker metadata.
 // This is only created when relocatable output is requested.
 void Writer::createLinkingSection() {
   SyntheticSection *Section =
       createSyntheticSection(WASM_SEC_CUSTOM, "linking");
   raw_ostream &OS = Section->getStream();
 
   SubSection DataSizeSubSection(WASM_DATA_SIZE);
   writeUleb128(DataSizeSubSection.getStream(), DataSize, "data size");

 }
 
 void Writer::writeSections() {
   uint8_t *Buf = Buffer->getBufferStart();
   parallelForEach(OutputSections, [Buf](OutputSection *S) { S->writeTo(Buf); });
 }
 
 // Fix the memory layout of the output binary.  This assigns memory offsets
-// to each of the intput data sections as well as the explicit stack region.
+// to each of the input data sections as well as the explicit stack region.
 void Writer::layoutMemory() {
   uint32_t MemoryPtr = 0;
   if (!Config->Relocatable) {
     MemoryPtr = Config->GlobalBase;
     debugPrint("mem: global base = %d\n", Config->GlobalBase);
   }
 
   createOutputSegments();