[lld][WebAssembly] Always search *.so for -Bdynamic #84288

yamt · 2024-03-07T08:41:55Z

Search *.so libraries regardless of -pie to make it a bit more straightforward to build non-pie dynamic-linked executables.

Flip the default to -Bstatic (unless -pie or -shared is specified) as I think it's what most users expect for the default as of today.
The assumption here is that, because dynamic-linking is not widely used for WebAssembly, the most users do not specify -Bdynamic or -Bstatic, expecting static link.
Although the recent wasi-sdk ships *.so files, there are not many wasm runtimes providing the support of dynamic-linking. (only emscripten and toywasm as far as i know.)

llvmbot · 2024-03-07T08:42:26Z

@llvm/pr-subscribers-lld-wasm

@llvm/pr-subscribers-lld

Author: YAMAMOTO Takashi (yamt)

Changes

Search *.so libraries regardless of -pie to make it a bit more straightforward to build non-pie dynamic-linked executables.

Flip the default to -Bstatic as I think it's what most users expect for the default as of today.
The assumption here is that, because dynamic-linking is not widely used for WebAssembly, the most users do not specify -Bdynamic or -Bstatic, expecting static link.

Full diff: https://github.com/llvm/llvm-project/pull/84288.diff

3 Files Affected:

(modified) lld/wasm/Config.h (+1-1)
(modified) lld/wasm/Driver.cpp (+1-3)
(modified) lld/wasm/Options.td (+2-2)

diff --git a/lld/wasm/Config.h b/lld/wasm/Config.h
index 266348fef40316..c351e1cef10532 100644
--- a/lld/wasm/Config.h
+++ b/lld/wasm/Config.h
@@ -72,7 +72,7 @@ struct Configuration {
   bool stripAll;
   bool stripDebug;
   bool stackFirst;
-  bool isStatic = false;
+  bool isStatic = true;
   bool trace;
   uint64_t globalBase;
   uint64_t initialHeap;
diff --git a/lld/wasm/Driver.cpp b/lld/wasm/Driver.cpp
index df7d4d1cc3d679..2599eca9389348 100644
--- a/lld/wasm/Driver.cpp
+++ b/lld/wasm/Driver.cpp
@@ -331,9 +331,7 @@ static std::optional<std::string> findFromSearchPaths(StringRef path) {
 // search paths.
 static std::optional<std::string> searchLibraryBaseName(StringRef name) {
   for (StringRef dir : config->searchPaths) {
-    // Currently we don't enable dynamic linking at all unless -shared or -pie
-    // are used, so don't even look for .so files in that case..
-    if (ctx.isPic && !config->isStatic)
+    if (!config->isStatic)
       if (std::optional<std::string> s = findFile(dir, "lib" + name + ".so"))
         return s;
     if (std::optional<std::string> s = findFile(dir, "lib" + name + ".a"))
diff --git a/lld/wasm/Options.td b/lld/wasm/Options.td
index 70b5aadc26c2a0..7e954822ef6425 100644
--- a/lld/wasm/Options.td
+++ b/lld/wasm/Options.td
@@ -38,9 +38,9 @@ multiclass B<string name, string help1, string help2> {
 // The following flags are shared with the ELF linker
 def Bsymbolic: F<"Bsymbolic">, HelpText<"Bind defined symbols locally">;
 
-def Bdynamic: F<"Bdynamic">, HelpText<"Link against shared libraries (default)">;
+def Bdynamic: F<"Bdynamic">, HelpText<"Link against shared libraries">;
 
-def Bstatic: F<"Bstatic">, HelpText<"Do not link against shared libraries">;
+def Bstatic: F<"Bstatic">, HelpText<"Do not link against shared libraries (default)">;
 
 def build_id: F<"build-id">, HelpText<"Alias for --build-id=fast">;

sbc100

Seems like a good change.

How about we keep -Bdynamic as the default when building with -shared and/or -pie ? It seems like in those modes we want to consume shared libraries by default.

Another alternative is that we always prefer shared libraries unless -static is passed? then it would be up to the clang driver (or users) to specify -static in the presence of both .so and .a files.

Ideally we would have some flag

yamt · 2024-03-08T03:46:44Z

Seems like a good change.

How about we keep -Bdynamic as the default when building with -shared and/or -pie ? It seems like in those modes we want to consume shared libraries by default.

it sounds reasonable to me.

Another alternative is that we always prefer shared libraries unless -static is passed? then it would be up to the clang driver (or users) to specify -static in the presence of both .so and .a files.

as the latest wasi-sdk ships *.so files, i guess it surprises many users.

Ideally we would have some flag

what do you mean?

MaskRay · 2024-03-08T05:56:57Z

In ELF linkers, -Bdynamic is the default. Clang Driver -static passes -static to the linker.

sbc100 · 2024-03-08T16:22:25Z

In ELF linkers, -Bdynamic is the default. Clang Driver -static passes -static to the linker.

The problem is that dynamic linking under wasm is still experimental so we probably want to keep the default as static for now.

IIUC we have several options for how to achieve "opt-in" dynamic linking:

Make -Bstatic the default. The downside of this is that it will be annoying if we ever want to change it in the future.
Don't include any .so files in the normal wasi-sdk, but only in an experimental one. Users of the experimental sdk would have to know that they need to pass -static to get the current behaviour.
Have the clang driver enforce the "static-by-default" by having it inject -static for wasm/wasi targets. This has the same downside as (1) except direct callers of wasm-ld won't be effected by any change in defaults.

The simplest option for now seem like (1).

sbc100

I wonder if there is a test we can write for this?

I'm actually not sure what would happen if you did -Bdynamic without -shared or -pie .. I don't think that is something that is currently supported. Have you tried it?

sbc100 · 2024-03-15T16:12:18Z

lld/wasm/Config.h

@@ -72,7 +72,7 @@ struct Configuration {
  bool stripAll;
  bool stripDebug;
  bool stackFirst;
-  bool isStatic = false;
+  bool isStatic = true;


Perhaps a comment: "Because dyamanic linking under Wasm is still experimental we default to static linking"

yamt · 2024-03-16T02:47:28Z

I'm actually not sure what would happen if you did -Bdynamic without -shared or -pie .. I don't think that is something that is currently supported. Have you tried it?

yes. it has been working for toywasm at least.
https://github.com/yamt/garbage/blob/1ddf7388fc91c31beed54101685e5d90573b43ef/c/shlib/wasi.sh#L70-L91

Search *.so libraries regardless of -pie to make it a bit more straightforward to build non-pie dynamic-linked executables. Flip the default to -Bstatic as I think it's what most users expect for the default as of today. The assumption here is that, because dynamic-linking is not widely used for WebAssembly, the most users do not specify -Bdynamic or -Bstatic, expecting static link.

MaskRay · 2024-05-17T16:45:08Z

lld/wasm/Driver.cpp

+  // -Bdynamic by default if -pie or -shared is specified.
+  if (config->pie || config->shared) {
+    config->isStatic = false;
+  }


The convention removes braces in this single line statement case.

fixed. thank you.

yamt · 2024-06-10T09:15:43Z

can this land? any concerns?

dschuff · 2024-06-10T17:01:05Z

Yes, it looks ready. Do you just need someone to land it? Can you make sure the commit message is up-to-date in the top comment?

yamt · 2024-06-11T01:36:34Z

Yes, it looks ready. Do you just need someone to land it?

i think so, yes.

Can you make sure the commit message is up-to-date in the top comment?

i updated the description a bit.

llvmbot added lld lld:wasm labels Mar 7, 2024

sbc100 reviewed Mar 7, 2024

View reviewed changes

sbc100 reviewed Mar 15, 2024

View reviewed changes

yamt added 3 commits May 17, 2024 16:32

[lld][WebAssembly] -Bdynamic by default if -pie or -shared is specified

8eef814

[lld][WebAssembly] Add a comment on isStatic default

6b41b22

yamt force-pushed the wasm-ld-find-so branch from df411f7 to 6b41b22 Compare May 17, 2024 07:34

sbc100 approved these changes May 17, 2024

View reviewed changes

MaskRay approved these changes May 17, 2024

View reviewed changes

a style fix

dbd76ba

dschuff merged commit 2b6c6bb into llvm:main Jun 11, 2024
4 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

[lld][WebAssembly] Always search *.so for -Bdynamic #84288

[lld][WebAssembly] Always search *.so for -Bdynamic #84288

Uh oh!

yamt commented Mar 7, 2024 •

edited

Loading

Uh oh!

llvmbot commented Mar 7, 2024 •

edited

Loading

Uh oh!

sbc100 left a comment

Uh oh!

yamt commented Mar 8, 2024

Uh oh!

MaskRay commented Mar 8, 2024

Uh oh!

sbc100 commented Mar 8, 2024 •

edited

Loading

Uh oh!

sbc100 left a comment

Uh oh!

sbc100 Mar 15, 2024

Uh oh!

yamt May 17, 2024

Uh oh!

yamt commented Mar 16, 2024

Uh oh!

MaskRay May 17, 2024

Uh oh!

yamt May 20, 2024

Uh oh!

yamt commented Jun 10, 2024

Uh oh!

dschuff commented Jun 10, 2024

Uh oh!

yamt commented Jun 11, 2024

Uh oh!

Uh oh!

Uh oh!

[lld][WebAssembly] Always search *.so for -Bdynamic #84288

[lld][WebAssembly] Always search *.so for -Bdynamic #84288

Uh oh!

Conversation

yamt commented Mar 7, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

llvmbot commented Mar 7, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

sbc100 left a comment

Choose a reason for hiding this comment

Uh oh!

yamt commented Mar 8, 2024

Uh oh!

MaskRay commented Mar 8, 2024

Uh oh!

sbc100 commented Mar 8, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

sbc100 left a comment

Choose a reason for hiding this comment

Uh oh!

sbc100 Mar 15, 2024

Choose a reason for hiding this comment

Uh oh!

yamt May 17, 2024

Choose a reason for hiding this comment

Uh oh!

yamt commented Mar 16, 2024

Uh oh!

MaskRay May 17, 2024

Choose a reason for hiding this comment

Uh oh!

yamt May 20, 2024

Choose a reason for hiding this comment

Uh oh!

yamt commented Jun 10, 2024

Uh oh!

dschuff commented Jun 10, 2024

Uh oh!

yamt commented Jun 11, 2024

Uh oh!

Uh oh!

Uh oh!

yamt commented Mar 7, 2024 •

edited

Loading

llvmbot commented Mar 7, 2024 •

edited

Loading

sbc100 commented Mar 8, 2024 •

edited

Loading