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

[Sam Clegg via llvm-commits]

Author: Sam Clegg
Date: 2022-05-26T13:43:25-07:00
New Revision: 198815e18dad31a9ff0f6c1b30f356db2fefe300

URL: https://github.com/llvm/llvm-project/commit/198815e18dad31a9ff0f6c1b30f356db2fefe300
DIFF: https://github.com/llvm/llvm-project/commit/198815e18dad31a9ff0f6c1b30f356db2fefe300.diff

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

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

Differential Revision: https://reviews.llvm.org/D126491

Added: 
    

Modified: 
    lld/test/wasm/shared-weak-symbols.s
    lld/wasm/Symbols.cpp
    lld/wasm/Writer.cpp

Removed: 
    


################################################################################
diff  --git a/lld/test/wasm/shared-weak-symbols.s b/lld/test/wasm/shared-weak-symbols.s
index 00b5d3a9212b..f22aaa543153 100644
--- a/lld/test/wasm/shared-weak-symbols.s
+++ b/lld/test/wasm/shared-weak-symbols.s
@@ -3,10 +3,9 @@
 # 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
@@ -15,12 +14,23 @@ weak_func:
   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
 
@@ -45,6 +55,20 @@ call_weak:
 # 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
@@ -57,3 +81,7 @@ call_weak:
 # 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

diff  --git a/lld/wasm/Symbols.cpp b/lld/wasm/Symbols.cpp
index 2ad21c8947f3..e0670cea6425 100644
--- a/lld/wasm/Symbols.cpp
+++ b/lld/wasm/Symbols.cpp
@@ -217,15 +217,15 @@ void Symbol::setHidden(bool isHidden) {
 }
 
 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;
 

diff  --git a/lld/wasm/Writer.cpp b/lld/wasm/Writer.cpp
index 9e3e32c183f1..4e254717e3b9 100644
--- a/lld/wasm/Writer.cpp
+++ b/lld/wasm/Writer.cpp
@@ -592,7 +592,8 @@ static bool shouldImport(Symbol *sym) {
   // 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;