From 198815e18dad31a9ff0f6c1b30f356db2fefe300 Mon Sep 17 00:00:00 2001
From: Sam Clegg <sbc@chromium.org>
Date: Thu, 26 May 2022 10:32:04 -0700
Subject: [PATCH] [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
---
 lld/test/wasm/shared-weak-symbols.s | 36 +++++++++++++++++++++++++----
 lld/wasm/Symbols.cpp                |  8 +++----
 lld/wasm/Writer.cpp                 |  3 ++-
 3 files changed, 38 insertions(+), 9 deletions(-)

diff --git a/lld/test/wasm/shared-weak-symbols.s b/lld/test/wasm/shared-weak-symbols.s
index 00b5d3a9212b3e..f22aaa5431539e 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 2ad21c8947f365..e0670cea6425e3 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 9e3e32c183f19e..4e254717e3b9c4 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;