Skip to content
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

[Clang] Don't use crtbegin/crtend when building for musl. #85089

Closed
wants to merge 2 commits into from

Conversation

al45tair
Copy link
Contributor

musl doesn't supply crtbegin/crtend objects, so we don't want to try to link them there.

rdar://123436174

musl doesn't supply crtbegin/crtend objects, so we don't want to
try to link them there.

rdar://123436174
@llvmbot llvmbot added clang Clang issues not falling into any other category clang:driver 'clang' and 'clang++' user-facing binaries. Not 'clang-cl' labels Mar 13, 2024
@llvmbot
Copy link
Member

llvmbot commented Mar 13, 2024

@llvm/pr-subscribers-clang-driver

@llvm/pr-subscribers-clang

Author: Alastair Houghton (al45tair)

Changes

musl doesn't supply crtbegin/crtend objects, so we don't want to try to link them there.

rdar://123436174


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

1 Files Affected:

  • (modified) clang/lib/Driver/ToolChains/Gnu.cpp (+4-2)
diff --git a/clang/lib/Driver/ToolChains/Gnu.cpp b/clang/lib/Driver/ToolChains/Gnu.cpp
index a9c9d2475809d7..7b7d9194a773ec 100644
--- a/clang/lib/Driver/ToolChains/Gnu.cpp
+++ b/clang/lib/Driver/ToolChains/Gnu.cpp
@@ -371,13 +371,15 @@ void tools::gnutools::Linker::ConstructJob(Compilation &C, const JobAction &JA,
   const llvm::Triple::ArchType Arch = ToolChain.getArch();
   const bool isOHOSFamily = ToolChain.getTriple().isOHOSFamily();
   const bool isAndroid = ToolChain.getTriple().isAndroid();
+  const bool isMusl = ToolChain.getTriple().isMusl();
   const bool IsIAMCU = ToolChain.getTriple().isOSIAMCU();
   const bool IsVE = ToolChain.getTriple().isVE();
   const bool IsStaticPIE = getStaticPIE(Args, ToolChain);
   const bool IsStatic = getStatic(Args);
   const bool HasCRTBeginEndFiles =
-      ToolChain.getTriple().hasEnvironment() ||
-      (ToolChain.getTriple().getVendor() != llvm::Triple::MipsTechnologies);
+    !isMusl && (ToolChain.getTriple().hasEnvironment() ||
+                (ToolChain.getTriple().getVendor()
+                 != llvm::Triple::MipsTechnologies));
 
   ArgStringList CmdArgs;
 

Copy link

github-actions bot commented Mar 13, 2024

✅ With the latest revision this PR passed the C/C++ code formatter.

Tweak code formatting slightly to make clang-format happy.

rdar://123436174
Copy link
Member

@MaskRay MaskRay left a comment

Choose a reason for hiding this comment

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

Thanks for working on musl but I don't think this is correct.
The compiler provides crtbegin/crtend files, not musl.
A musl-cross-make build does use crtbegin/crtend, though technically the files can be empty.
However, there is no point to diverge, and even when crtbegin/crtend files are empty, users can customize them to do other stuff (e.g. specifying the section order).

@al45tair
Copy link
Contributor Author

The compiler provides crtbegin/crtend files, not musl.

Interesting. Looking at the LLVM repo, it seems compiler-rt might provide them, but I don't believe I have them in spite of building compiler-rt. I'll look at this again and see if maybe I need to tweak my compiler-rt build to get it to generate or install them in the right place.

@al45tair
Copy link
Contributor Author

(I had assumed that they came from the C library, since that's where crt1.o comes from, but if they don't then it sounds like I just need to build them.)

@al45tair
Copy link
Contributor Author

Regardless of who provides them, if someone is relying on this then this is the wrong change. Closing.

@al45tair al45tair closed this Mar 14, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
clang:driver 'clang' and 'clang++' user-facing binaries. Not 'clang-cl' clang Clang issues not falling into any other category
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants