Skip to content

[lld][WebAssembly] Don't mark --start-lib/--end-lib files as live #137714

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Apr 28, 2025

Conversation

sbc100
Copy link
Collaborator

@sbc100 sbc100 commented Apr 28, 2025

Without this change files in --start-lib/--end-lib groups were being marked as live, which means there static constructors were being included in the link.

Without this change files in `--start-lib`/`--end-lib` groups were
being marked as live, which means there static constructors were being
included in the link.
@llvmbot
Copy link
Member

llvmbot commented Apr 28, 2025

@llvm/pr-subscribers-lld-wasm

@llvm/pr-subscribers-lld

Author: Sam Clegg (sbc100)

Changes

Without this change files in --start-lib/--end-lib groups were being marked as live, which means there static constructors were being included in the link.


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

3 Files Affected:

  • (modified) lld/test/wasm/Inputs/start-lib1.s (+7)
  • (modified) lld/wasm/Driver.cpp (-4)
  • (modified) lld/wasm/InputFiles.cpp (+4-2)
diff --git a/lld/test/wasm/Inputs/start-lib1.s b/lld/test/wasm/Inputs/start-lib1.s
index 229f67a4bd897..9ebfdbc2f61e1 100644
--- a/lld/test/wasm/Inputs/start-lib1.s
+++ b/lld/test/wasm/Inputs/start-lib1.s
@@ -5,3 +5,10 @@ foo:
   .functype foo () -> ()
   call bar
   end_function
+
+# Static constructor inserted here to ensure the object file is not
+# being processed as "live".  Live object files have their static constructors
+# preserved even if no symbol within is used.
+.section .init_array,"",@
+  .p2align 2
+  .int32 foo
diff --git a/lld/wasm/Driver.cpp b/lld/wasm/Driver.cpp
index de976947474e1..86c4ce397e357 100644
--- a/lld/wasm/Driver.cpp
+++ b/lld/wasm/Driver.cpp
@@ -317,10 +317,6 @@ void LinkerDriver::addFile(StringRef path) {
     if (inWholeArchive) {
       for (const auto &[m, offset] : members) {
         auto *object = createObjectFile(m, path, offset);
-        // Mark object as live; object members are normally not
-        // live by default but -whole-archive is designed to treat
-        // them as such.
-        object->markLive();
         files.push_back(object);
       }
 
diff --git a/lld/wasm/InputFiles.cpp b/lld/wasm/InputFiles.cpp
index 614cddddd1b19..1d1b82c9879b9 100644
--- a/lld/wasm/InputFiles.cpp
+++ b/lld/wasm/InputFiles.cpp
@@ -423,8 +423,10 @@ ObjFile::ObjFile(MemoryBufferRef m, StringRef archiveName, bool lazy)
   // https://github.com/llvm/llvm-project/issues/98778
   checkArch(wasmObj->getArch());
 
-  // If this isn't part of an archive, it's eagerly linked, so mark it live.
-  if (archiveName.empty())
+  // Unless we are processing this as a lazy object file (e.g. part of an
+  // archive file or within `--start-lib`/`--end-lib`, it's eagerly linked, so
+  // mark it live.
+  if (!lazy)
     markLive();
 }
 

// Mark object as live; object members are normally not
// live by default but -whole-archive is designed to treat
// them as such.
object->markLive();
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was unnecessary because the forth argument to createObjectFile (lazy) defaults to false which then calls markLive.

@dschuff
Copy link
Member

dschuff commented Apr 28, 2025

So is --start-lib foo.o bar.o --end-lib just supposed to be the equivalent of linking an archive file containing foo.o and bar.o?

@sbc100
Copy link
Collaborator Author

sbc100 commented Apr 28, 2025

So is --start-lib foo.o bar.o --end-lib just supposed to be the equivalent of linking an archive file containing foo.o and bar.o?

Exactly yes.

@sbc100 sbc100 merged commit afdc4b1 into llvm:main Apr 28, 2025
14 checks passed
@sbc100 sbc100 deleted the start_lib branch April 28, 2025 22:30
IanWood1 pushed a commit to IanWood1/llvm-project that referenced this pull request May 6, 2025
…llvm#137714)

Without this change files in `--start-lib`/`--end-lib` groups were being
marked as live, which means there static constructors were being
included in the link.
IanWood1 pushed a commit to IanWood1/llvm-project that referenced this pull request May 6, 2025
…llvm#137714)

Without this change files in `--start-lib`/`--end-lib` groups were being
marked as live, which means there static constructors were being
included in the link.
IanWood1 pushed a commit to IanWood1/llvm-project that referenced this pull request May 6, 2025
…llvm#137714)

Without this change files in `--start-lib`/`--end-lib` groups were being
marked as live, which means there static constructors were being
included in the link.
GeorgeARM pushed a commit to GeorgeARM/llvm-project that referenced this pull request May 7, 2025
…llvm#137714)

Without this change files in `--start-lib`/`--end-lib` groups were being
marked as live, which means there static constructors were being
included in the link.
Ankur-0429 pushed a commit to Ankur-0429/llvm-project that referenced this pull request May 9, 2025
…llvm#137714)

Without this change files in `--start-lib`/`--end-lib` groups were being
marked as live, which means there static constructors were being
included in the link.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants