[lld][WebAssembly] Avoid importing/exporting hidden symbols in shared libraries

[sbc100]

Summary

We have some special handling for weakly defined symbols where we both
import and export them, but this is not needed for hidden symbols which
should never be imported or exported.

See https://github.com/emscripten-core/emscripten/pull/16972

This should also help with:
https://github.com/emscripten-core/emscripten/issues/15487

[dschuff]

what happens when you use attribute(export-name) in clang, along with -fvisibility=hidden? (or is there some other way force an export?)

lld/test/wasm/shared-weak-symbols.s

73

should we put a CHECK-NEXT for the next section (after export) here to ensure there are only 3 exports?

[sbc100]

In D126491#3540946, @dschuff wrote:

what happens when you use attribute(export-name) in clang, along with -fvisibility=hidden? (or is there some other way force an export?)

attribute(export-name) will be honored whenever a symbol is exported.

hidden symbols can indeed be exported explicitly via --export on the command line. Not sure if that is good or a bad thing.

[sbc100]

• feedback

[dschuff]

Yeah, letting the user override with the manual mechanisms seems like a good thing, probably?

lld/test/wasm/shared-weak-symbols.s

 # RUN: llvm-mc -filetype=obj -triple=wasm32-unknown-unknown -o %t.o %s
 # RUN: wasm-ld --experimental-pic -shared -o %t.wasm %t.o
 # RUN: obj2yaml %t.wasm | FileCheck %s
 # RUN: llvm-objdump -d %t.wasm | FileCheck %s -check-prefix=ASM
 
-# Verify the weakly defined fuctions (weak_func) are both
-# imported and exported, and that internal usage (direct call)
-# always uses the imported version.
-
+# Verify the weakly defined fuctions (weak_func) are both imported and exported,
+# and that internal usage (direct call) always uses the imported version.
+# Hidden functions, even if weak, should not be imported or exported.
 
 .globl weak_func
 .weak weak_func
 weak_func:
   .functype weak_func () -> (i32)
   i32.const 0
   end_function
 
+.globl hidden_weak_func
+.hidden hidden_weak_func
+.weak hidden_weak_func
+hidden_weak_func:
+  .functype hidden_weak_func () -> (i32)
+  i32.const 42
+  end_function
+
 .globl call_weak
 call_weak:
 # ASM: <call_weak>:
   .functype call_weak () -> (i32)
   call weak_func
 # ASM:           10 80 80 80 80 00      call  0
+  drop
+  call hidden_weak_func
+# ASM:           10 84 80 80 80 00      call  4
   end_function
 # ASM-NEXT:      0b                     end
 
 # CHECK:       - Type:            IMPORT
 # CHECK-NEXT:    Imports:
 # CHECK-NEXT:      - Module:          env
 # CHECK-NEXT:        Field:           memory
 # CHECK-NEXT:        Kind:            MEMORY
 # CHECK-NEXT:        Memory:
 # CHECK-NEXT:          Minimum:         0x0
 # CHECK-NEXT:      - Module:          env
 # CHECK-NEXT:        Field:           __memory_base
 # CHECK-NEXT:        Kind:            GLOBAL
 # CHECK-NEXT:        GlobalType:      I32
 # CHECK-NEXT:        GlobalMutable:   false
 # CHECK-NEXT:      - Module:          env
 # CHECK-NEXT:        Field:           __table_base
 # CHECK-NEXT:        Kind:            GLOBAL
 # CHECK-NEXT:        GlobalType:      I32
 # CHECK-NEXT:        GlobalMutable:   false
 # CHECK-NEXT:      - Module:          env
 # CHECK-NEXT:        Field:           weak_func
 # CHECK-NEXT:        Kind:            FUNCTION
 # CHECK-NEXT:        SigIndex:        0
+# CHECK-NEXT:  - Type:            FUNCTION
+
+# CHECK:        - Type:            EXPORT
+# CHECK-NEXT:     Exports:
+# CHECK-NEXT:       - Name:            __wasm_call_ctors
+# CHECK-NEXT:         Kind:            FUNCTION
+# CHECK-NEXT:         Index:           1
+# CHECK-NEXT:       - Name:            weak_func
+# CHECK-NEXT:         Kind:            FUNCTION
+# CHECK-NEXT:         Index:           3
+# CHECK-NEXT:       - Name:            call_weak
+# CHECK-NEXT:         Kind:            FUNCTION
+# CHECK-NEXT:         Index:           5
+# CHECK-NEXT:   - Type:            START
 
 #      CHECK:   - Type:            CUSTOM
 # CHECK-NEXT:     Name:            name
 # CHECK-NEXT:     FunctionNames:
 # CHECK-NEXT:       - Index:           0
 # CHECK-NEXT:         Name:            weak_func
 # CHECK-NEXT:       - Index:           1
 # CHECK-NEXT:         Name:            __wasm_call_ctors
 # CHECK-NEXT:       - Index:           2
 # CHECK-NEXT:         Name:            __wasm_apply_data_relocs
 # CHECK-NEXT:       - Index:           3
 # CHECK-NEXT:         Name:            weak_func
+# CHECK-NEXT:       - Index:           4
+# CHECK-NEXT:         Name:            hidden_weak_func
+# CHECK-NEXT:       - Index:           5
+# CHECK-NEXT:         Name:            call_weak

lld/wasm/Symbols.cpp

void Symbol::setHidden(bool isHidden) {
   flags &= ~WASM_SYMBOL_VISIBILITY_MASK;
   if (isHidden)
     flags |= WASM_SYMBOL_VISIBILITY_HIDDEN;
   else
     flags |= WASM_SYMBOL_VISIBILITY_DEFAULT;
 }
 
 bool Symbol::isExported() const {
+  if (!isDefined() || isLocal())
+    return false;
+
   // Shared libraries must export all weakly defined symbols
   // in case they contain the version that will be chosen by
   // the dynamic linker.
-  if (config->shared && isLive() && isDefined() && isWeak())
+  if (config->shared && isLive() && isWeak() && !isHidden())
     return true;
 
-  if (!isDefined() || isLocal())
-    return false;
-
   if (config->exportAll || (config->exportDynamic && !isHidden()))
     return true;
 
   return isExportedExplicit();
 }
 
 bool Symbol::isExportedExplicit() const {
   return forceExport || flags & WASM_SYMBOL_EXPORTED;

lld/wasm/Writer.cpp

static bool shouldImport(Symbol *sym) {
   if (!sym->isLive())
     return false;
   if (!sym->isUsedInRegularObj)
     return false;
 
   // When a symbol is weakly defined in a shared library we need to allow
   // it to be overridden by another module so need to both import
   // and export the symbol.
-  if (config->shared && sym->isDefined() && sym->isWeak())
+  if (config->shared && sym->isWeak() && !sym->isUndefined() &&
+      !sym->isHidden())
     return true;
   if (!sym->isUndefined())
     return false;
   if (sym->isWeak() && !config->relocatable && !config->isPic)
     return false;
 
   // In PIC mode we only need to import functions when they are called directly.
   // Indirect usage all goes via GOT imports.