Fix lld link issue for OHOS #118192

phuang · 2024-12-01T03:48:58Z

For ohos targets, libclang_rt.builtins.a, clang_rt.crtbegin.o and clang_rt.crtend.o are installed in
clang/20/lib/${arch}-unknown-linux-ohos. However OHOS toolchain search them in clang/20/lib/${arch}-linux-ohos folder. It causes link error. Fix the problem by seaching both folders.

github-actions · 2024-12-01T03:49:22Z

Thank you for submitting a Pull Request (PR) to the LLVM Project!

This PR will be automatically labeled and the relevant teams will be notified.

If you wish to, you can add reviewers by using the "Reviewers" section on this page.

If this is not working for you, it is probably because you do not have write permissions for the repository. In which case you can instead tag reviewers by name in a comment by using @ followed by their GitHub username.

If you have received no comments on your PR for a week, you can request a review by "ping"ing the PR by adding a comment “Ping”. The common courtesy "ping" rate is once a week. Please remember that you are asking for valuable time from other developers.

If you have further questions, they may be answered by the LLVM GitHub User Guide.

You can also ask questions in a comment on this PR, on the LLVM Discord or on the forums.

llvmbot · 2024-12-01T03:49:56Z

@llvm/pr-subscribers-clang-driver

Author: Peng Huang (phuang)

Changes

For ohos targets, libclang_rt.builtins.a, clang_rt.crtbegin.o and clang_rt.crtend.o are installed in
clang/20/lib/${arch}-unknown-linux-ohos. However OHOS toolchain search them in clang/20/lib/${arch}-linux-ohos folder. It causes link error. Fix the problem by seaching both folders.

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

1 Files Affected:

(modified) clang/lib/Driver/ToolChains/OHOS.cpp (+22-34)

diff --git a/clang/lib/Driver/ToolChains/OHOS.cpp b/clang/lib/Driver/ToolChains/OHOS.cpp
index 6e1a09ae908b2f..9a52fbf9b70959 100644
--- a/clang/lib/Driver/ToolChains/OHOS.cpp
+++ b/clang/lib/Driver/ToolChains/OHOS.cpp
@@ -19,8 +19,8 @@
 #include "llvm/ProfileData/InstrProf.h"
 #include "llvm/Support/FileSystem.h"
 #include "llvm/Support/Path.h"
-#include "llvm/Support/VirtualFileSystem.h"
 #include "llvm/Support/ScopedPrinter.h"
+#include "llvm/Support/VirtualFileSystem.h"
 
 using namespace clang::driver;
 using namespace clang::driver::toolchains;
@@ -58,11 +58,9 @@ static bool findOHOSMuslMultilibs(const Driver &D,
   return false;
 }
 
-static bool findOHOSMultilibs(const Driver &D,
-                                      const ToolChain &TC,
-                                      const llvm::Triple &TargetTriple,
-                                      StringRef Path, const ArgList &Args,
-                                      DetectedMultilibs &Result) {
+static bool findOHOSMultilibs(const Driver &D, const ToolChain &TC,
+                              const llvm::Triple &TargetTriple, StringRef Path,
+                              const ArgList &Args, DetectedMultilibs &Result) {
   Multilib::flags_list Flags;
   bool IsA7 = false;
   if (const Arg *A = Args.getLastArg(options::OPT_mcpu_EQ))
@@ -172,8 +170,7 @@ OHOS::OHOS(const Driver &D, const llvm::Triple &Triple, const ArgList &Args)
       Paths);
 }
 
-ToolChain::RuntimeLibType OHOS::GetRuntimeLibType(
-    const ArgList &Args) const {
+ToolChain::RuntimeLibType OHOS::GetRuntimeLibType(const ArgList &Args) const {
   if (Arg *A = Args.getLastArg(clang::driver::options::OPT_rtlib_EQ)) {
     StringRef Value = A->getValue();
     if (Value != "compiler-rt")
@@ -184,20 +181,19 @@ ToolChain::RuntimeLibType OHOS::GetRuntimeLibType(
   return ToolChain::RLT_CompilerRT;
 }
 
-ToolChain::CXXStdlibType
-OHOS::GetCXXStdlibType(const ArgList &Args) const {
+ToolChain::CXXStdlibType OHOS::GetCXXStdlibType(const ArgList &Args) const {
   if (Arg *A = Args.getLastArg(options::OPT_stdlib_EQ)) {
     StringRef Value = A->getValue();
     if (Value != "libc++")
       getDriver().Diag(diag::err_drv_invalid_stdlib_name)
-        << A->getAsString(Args);
+          << A->getAsString(Args);
   }
 
   return ToolChain::CST_Libcxx;
 }
 
 void OHOS::AddClangSystemIncludeArgs(const ArgList &DriverArgs,
-                                        ArgStringList &CC1Args) const {
+                                     ArgStringList &CC1Args) const {
   const Driver &D = getDriver();
   const llvm::Triple &Triple = getTriple();
   std::string SysRoot = computeSysRoot();
@@ -258,7 +254,7 @@ void OHOS::AddClangCXXStdlibIncludeArgs(const ArgList &DriverArgs,
 }
 
 void OHOS::AddCXXStdlibLibArgs(const ArgList &Args,
-                                  ArgStringList &CmdArgs) const {
+                               ArgStringList &CmdArgs) const {
   switch (GetCXXStdlibType(Args)) {
   case ToolChain::CST_Libcxx:
     CmdArgs.push_back("-lc++");
@@ -291,7 +287,8 @@ ToolChain::path_list OHOS::getRuntimePaths() const {
 
   // First try the triple passed to driver as --target=<triple>.
   P.assign(D.ResourceDir);
-  llvm::sys::path::append(P, "lib", D.getTargetTriple(), SelectedMultilib.gccSuffix());
+  llvm::sys::path::append(P, "lib", D.getTargetTriple(),
+                          SelectedMultilib.gccSuffix());
   Paths.push_back(P.c_str());
 
   // Second try the normalized triple.
@@ -340,26 +337,16 @@ std::string OHOS::getDynamicLinker(const ArgList &Args) const {
 
 std::string OHOS::getCompilerRT(const ArgList &Args, StringRef Component,
                                 FileType Type) const {
+  std::string CRTBasename =
+      buildCompilerRTBasename(Args, Component, Type, /*AddArch=*/false);
+
   SmallString<128> Path(getDriver().ResourceDir);
   llvm::sys::path::append(Path, "lib", getMultiarchTriple(getTriple()),
-                          SelectedMultilib.gccSuffix());
-  const char *Prefix =
-      Type == ToolChain::FT_Object ? "" : "lib";
-  const char *Suffix;
-  switch (Type) {
-  case ToolChain::FT_Object:
-    Suffix = ".o";
-    break;
-  case ToolChain::FT_Static:
-    Suffix = ".a";
-    break;
-  case ToolChain::FT_Shared:
-    Suffix = ".so";
-    break;
-  }
-  llvm::sys::path::append(
-      Path, Prefix + Twine("clang_rt.") + Component + Suffix);
-  return static_cast<std::string>(Path.str());
+                          SelectedMultilib.gccSuffix(), CRTBasename);
+  if (getVFS().exists(Path))
+      return std::string(Path);
+
+  return ToolChain::getCompilerRT(Args, Component, Type);
 }
 
 void OHOS::addExtraOpts(llvm::opt::ArgStringList &CmdArgs) const {
@@ -396,7 +383,7 @@ SanitizerMask OHOS::getSupportedSanitizers() const {
 
 // TODO: Make a base class for Linux and OHOS and move this there.
 void OHOS::addProfileRTLibs(const llvm::opt::ArgList &Args,
-                             llvm::opt::ArgStringList &CmdArgs) const {
+                            llvm::opt::ArgStringList &CmdArgs) const {
   // Add linker option -u__llvm_profile_runtime to cause runtime
   // initialization module to be linked in.
   if (needsProfileRT(Args))
@@ -413,7 +400,8 @@ ToolChain::path_list OHOS::getArchSpecificLibPaths() const {
   return Paths;
 }
 
-ToolChain::UnwindLibType OHOS::GetUnwindLibType(const llvm::opt::ArgList &Args) const {
+ToolChain::UnwindLibType
+OHOS::GetUnwindLibType(const llvm::opt::ArgList &Args) const {
   if (Args.getLastArg(options::OPT_unwindlib_EQ))
     return Generic_ELF::GetUnwindLibType(Args);
   return GetDefaultUnwindLibType();

llvmbot · 2024-12-01T03:49:56Z

@llvm/pr-subscribers-clang

Author: Peng Huang (phuang)

Changes

For ohos targets, libclang_rt.builtins.a, clang_rt.crtbegin.o and clang_rt.crtend.o are installed in
clang/20/lib/${arch}-unknown-linux-ohos. However OHOS toolchain search them in clang/20/lib/${arch}-linux-ohos folder. It causes link error. Fix the problem by seaching both folders.

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

1 Files Affected:

(modified) clang/lib/Driver/ToolChains/OHOS.cpp (+22-34)

diff --git a/clang/lib/Driver/ToolChains/OHOS.cpp b/clang/lib/Driver/ToolChains/OHOS.cpp
index 6e1a09ae908b2f..9a52fbf9b70959 100644
--- a/clang/lib/Driver/ToolChains/OHOS.cpp
+++ b/clang/lib/Driver/ToolChains/OHOS.cpp
@@ -19,8 +19,8 @@
 #include "llvm/ProfileData/InstrProf.h"
 #include "llvm/Support/FileSystem.h"
 #include "llvm/Support/Path.h"
-#include "llvm/Support/VirtualFileSystem.h"
 #include "llvm/Support/ScopedPrinter.h"
+#include "llvm/Support/VirtualFileSystem.h"
 
 using namespace clang::driver;
 using namespace clang::driver::toolchains;
@@ -58,11 +58,9 @@ static bool findOHOSMuslMultilibs(const Driver &D,
   return false;
 }
 
-static bool findOHOSMultilibs(const Driver &D,
-                                      const ToolChain &TC,
-                                      const llvm::Triple &TargetTriple,
-                                      StringRef Path, const ArgList &Args,
-                                      DetectedMultilibs &Result) {
+static bool findOHOSMultilibs(const Driver &D, const ToolChain &TC,
+                              const llvm::Triple &TargetTriple, StringRef Path,
+                              const ArgList &Args, DetectedMultilibs &Result) {
   Multilib::flags_list Flags;
   bool IsA7 = false;
   if (const Arg *A = Args.getLastArg(options::OPT_mcpu_EQ))
@@ -172,8 +170,7 @@ OHOS::OHOS(const Driver &D, const llvm::Triple &Triple, const ArgList &Args)
       Paths);
 }
 
-ToolChain::RuntimeLibType OHOS::GetRuntimeLibType(
-    const ArgList &Args) const {
+ToolChain::RuntimeLibType OHOS::GetRuntimeLibType(const ArgList &Args) const {
   if (Arg *A = Args.getLastArg(clang::driver::options::OPT_rtlib_EQ)) {
     StringRef Value = A->getValue();
     if (Value != "compiler-rt")
@@ -184,20 +181,19 @@ ToolChain::RuntimeLibType OHOS::GetRuntimeLibType(
   return ToolChain::RLT_CompilerRT;
 }
 
-ToolChain::CXXStdlibType
-OHOS::GetCXXStdlibType(const ArgList &Args) const {
+ToolChain::CXXStdlibType OHOS::GetCXXStdlibType(const ArgList &Args) const {
   if (Arg *A = Args.getLastArg(options::OPT_stdlib_EQ)) {
     StringRef Value = A->getValue();
     if (Value != "libc++")
       getDriver().Diag(diag::err_drv_invalid_stdlib_name)
-        << A->getAsString(Args);
+          << A->getAsString(Args);
   }
 
   return ToolChain::CST_Libcxx;
 }
 
 void OHOS::AddClangSystemIncludeArgs(const ArgList &DriverArgs,
-                                        ArgStringList &CC1Args) const {
+                                     ArgStringList &CC1Args) const {
   const Driver &D = getDriver();
   const llvm::Triple &Triple = getTriple();
   std::string SysRoot = computeSysRoot();
@@ -258,7 +254,7 @@ void OHOS::AddClangCXXStdlibIncludeArgs(const ArgList &DriverArgs,
 }
 
 void OHOS::AddCXXStdlibLibArgs(const ArgList &Args,
-                                  ArgStringList &CmdArgs) const {
+                               ArgStringList &CmdArgs) const {
   switch (GetCXXStdlibType(Args)) {
   case ToolChain::CST_Libcxx:
     CmdArgs.push_back("-lc++");
@@ -291,7 +287,8 @@ ToolChain::path_list OHOS::getRuntimePaths() const {
 
   // First try the triple passed to driver as --target=<triple>.
   P.assign(D.ResourceDir);
-  llvm::sys::path::append(P, "lib", D.getTargetTriple(), SelectedMultilib.gccSuffix());
+  llvm::sys::path::append(P, "lib", D.getTargetTriple(),
+                          SelectedMultilib.gccSuffix());
   Paths.push_back(P.c_str());
 
   // Second try the normalized triple.
@@ -340,26 +337,16 @@ std::string OHOS::getDynamicLinker(const ArgList &Args) const {
 
 std::string OHOS::getCompilerRT(const ArgList &Args, StringRef Component,
                                 FileType Type) const {
+  std::string CRTBasename =
+      buildCompilerRTBasename(Args, Component, Type, /*AddArch=*/false);
+
   SmallString<128> Path(getDriver().ResourceDir);
   llvm::sys::path::append(Path, "lib", getMultiarchTriple(getTriple()),
-                          SelectedMultilib.gccSuffix());
-  const char *Prefix =
-      Type == ToolChain::FT_Object ? "" : "lib";
-  const char *Suffix;
-  switch (Type) {
-  case ToolChain::FT_Object:
-    Suffix = ".o";
-    break;
-  case ToolChain::FT_Static:
-    Suffix = ".a";
-    break;
-  case ToolChain::FT_Shared:
-    Suffix = ".so";
-    break;
-  }
-  llvm::sys::path::append(
-      Path, Prefix + Twine("clang_rt.") + Component + Suffix);
-  return static_cast<std::string>(Path.str());
+                          SelectedMultilib.gccSuffix(), CRTBasename);
+  if (getVFS().exists(Path))
+      return std::string(Path);
+
+  return ToolChain::getCompilerRT(Args, Component, Type);
 }
 
 void OHOS::addExtraOpts(llvm::opt::ArgStringList &CmdArgs) const {
@@ -396,7 +383,7 @@ SanitizerMask OHOS::getSupportedSanitizers() const {
 
 // TODO: Make a base class for Linux and OHOS and move this there.
 void OHOS::addProfileRTLibs(const llvm::opt::ArgList &Args,
-                             llvm::opt::ArgStringList &CmdArgs) const {
+                            llvm::opt::ArgStringList &CmdArgs) const {
   // Add linker option -u__llvm_profile_runtime to cause runtime
   // initialization module to be linked in.
   if (needsProfileRT(Args))
@@ -413,7 +400,8 @@ ToolChain::path_list OHOS::getArchSpecificLibPaths() const {
   return Paths;
 }
 
-ToolChain::UnwindLibType OHOS::GetUnwindLibType(const llvm::opt::ArgList &Args) const {
+ToolChain::UnwindLibType
+OHOS::GetUnwindLibType(const llvm::opt::ArgList &Args) const {
   if (Args.getLastArg(options::OPT_unwindlib_EQ))
     return Generic_ELF::GetUnwindLibType(Args);
   return GetDefaultUnwindLibType();

For ohos targets, libclang_rt.builtins.a, clang_rt.crtbegin.o and clang_rt.crtend.o are installed in clang/20/lib/${arch}-unknown-linux-ohos. However OHOS toolchain search them in clang/20/lib/${arch}-linux-ohos folder. It causes link error. Fix the problem by seaching both folders.

phuang · 2024-12-06T14:42:20Z

friendly ping.

kpdev · 2024-12-07T09:58:30Z

@phuang Hi! Thank you for the patch. Will take a look in the nearest pair of days

github-actions · 2024-12-07T20:51:31Z

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

kpdev · 2024-12-08T19:45:02Z

LGTM

phuang · 2024-12-08T21:57:49Z

Thanks for reviewing it.

phuang · 2024-12-09T13:35:35Z

LGTM

Could you please help merge the PR? I don't see how to do it from my side. Thanks.

github-actions · 2024-12-12T14:53:12Z

@phuang Congratulations on having your first Pull Request (PR) merged into the LLVM Project!

Your changes will be combined with recent changes from other authors, then tested by our build bots. If there is a problem with a build, you may receive a report in an email or a comment on this PR.

Please check whether problems have been caused by your change specifically, as the builds can include changes from many authors. It is not uncommon for your change to be included in a build that fails due to someone else's changes, or infrastructure issues.

How to do this, and the rest of the post-merge process, is covered in detail here.

If your change does cause a problem, it may be reverted, or you can revert it yourself. This is a normal part of LLVM development. You can fix your changes and open a new PR to merge them again.

If you don't get any reports, no action is required from you. Your changes are working as expected, well done!

MaskRay · 2024-12-12T17:38:38Z

In general, functional changes need tests. When the initial https://reviews.llvm.org/D145227 landed, I did not have a chance to look, and the tests may have gap. When adding new functionality, ensure that the test coverage gets improved.

kpdev · 2024-12-12T17:53:11Z

Agree. Tests would be useful here. Thanks for mentioning it

nico · 2024-12-13T14:11:24Z

This broke check-clang on my linux box: http://45.33.8.238/linux/155432/step_6.txt

That bot uses the GN build – are there any prerequisite changes elsewhere that I need to port to it?

kpdev · 2024-12-13T17:22:40Z

Hm... No, it should work. @phuang May I kindly ask you to take a look at this issue? http://45.33.8.238/linux/155432/step_6.txt

phuang · 2024-12-13T19:49:22Z

I am out of town this week. I will take a look when I return.

kpdev · 2024-12-13T20:59:55Z

@nico Should I revert this patch for now?

nico · 2024-12-14T00:10:03Z

If all other bots are happy, then I'm doing something wrong probably. If you don't mind reverting, it'd be nice since then I can catch other, actual problems, with the bot, but it's not a requirement.

This reverts commit bc28be0. Some issues were discovered with GN buildbot http://45.33.8.238/linux/155432/step_6.txt Need to investigate it

kpdev · 2024-12-14T11:54:22Z

@phuang @nico Reverted. I will reland it after investigating the issue with GN build bot

phuang · 2024-12-16T14:27:37Z

are there any prerequisite changes elsewhere that I need to port to it?

HI @nico, how to repro the problem locally? Is there a doc for it? or could you please share steps? thanks

phuang · 2024-12-16T20:14:42Z

I found out how to run the test, but I cannot reproduce the problem with my change. I run the test with below steps:

1: cmake -S llvm -B build -G Ninja -DCMAKE_BUILD_TYPE=Release -DLLVM_ENABLE_PROJECTS="clang;lld"
2: ninja -C build
3: ./build/bin/llvm-lit ./clang/test/Driver/ohos.c -a -v

I checked the compile log at http://45.33.8.238/linux/155432/step_3.txt , it only compiles around 1000 files. But in my tree, it compiles 5329 files. @nico do you know why? what's your build steps?

This reverts commit 1911919.

phuang · 2024-12-16T23:15:14Z

I still cannot repro the problem locally, but base on th log in http://45.33.8.238/linux/155432/step_6.txt . Looks like clang tries to search runtime library at "/usr/local/google/home/thakis/src/llvm-project/out/gn/lib/clang/20/lib/linux/libclang_rt.builtins-x86_64.a" (It is the OldPath in https://github.com/llvm/llvm-project/blob/main/clang/lib/Driver/ToolChain.cpp#L771). Not sure why the file does exist on the gn bot. Maybe it is not a clean build. I workarounded the problem by adding "-ohos" suffix in the runtime library like android.

@kpdev @nico , could you please take a look the new PR #120159

Thanks.

nico · 2024-12-17T16:33:23Z

Aha! Yes, that bot does do incremental builds. (As do many devs.)

MaskRay

We need a test to clang/test/Driver/ohos.c similar to linux-ld.c clang_rt.crtbegin.o.

You can add UNSUPPORTED: system-windows to ohos.c so that we don't need backslashes.

The problem in original change is because OHOS::getCompilerRT() pickes a wrong builtin runtime `./lib/clang/20/lib/linux/libclang_rt.builtins-x86_64.a`, if `./lib/clang/20/lib/linux/libclang_rt.builtins-x86_64.a` exist on the test filesystem. It shouldn't happen with a clean build.

#120159)" This reverts commit bd154e8. Test fails with -DLLVM_ENABLE_PER_TARGET_RUNTIME_DIR=OFF, see #120159 (comment)

llvmbot added clang Clang issues not falling into any other category clang:driver 'clang' and 'clang++' user-facing binaries. Not 'clang-cl' labels Dec 1, 2024

phuang changed the title ~~Fix build error~~ Fix build error for OHOS Dec 1, 2024

phuang force-pushed the ohos_pull_request branch from 28314d9 to 206f8f8 Compare December 1, 2024 04:28

phuang changed the title ~~Fix build error for OHOS~~ Fix lld link issue for OHOS Dec 4, 2024

asl requested a review from kpdev December 4, 2024 23:49

Format OHOS.cpp

dd8788f

kpdev merged commit bc28be0 into llvm:main Dec 12, 2024
8 checks passed

kpdev added a commit that referenced this pull request Dec 14, 2024

Revert "[Driver][OHOS] Fix lld link issue for OHOS (#118192)"

1911919

This reverts commit bc28be0. Some issues were discovered with GN buildbot http://45.33.8.238/linux/155432/step_6.txt Need to investigate it

phuang added a commit to phuang/llvm-project that referenced this pull request Dec 16, 2024

Reapply "[Driver][OHOS] Fix lld link issue for OHOS (llvm#118192)"

db61f4b

This reverts commit 1911919.

MaskRay mentioned this pull request Dec 17, 2024

Reapply "[Driver][OHOS] Fix lld link issue for OHOS (#118192)" #120159

Merged

MaskRay reviewed Dec 25, 2024

View reviewed changes

nico added a commit that referenced this pull request Jan 2, 2025

Revert "Reapply "[Driver][OHOS] Fix lld link issue for OHOS (#118192)" (

a4deb80

#120159)" This reverts commit bd154e8. Test fails with -DLLVM_ENABLE_PER_TARGET_RUNTIME_DIR=OFF, see #120159 (comment)

kpdev mentioned this pull request Jan 7, 2025

Do you still need commit access? #122011

Closed

Fix lld link issue for OHOS #118192

Fix lld link issue for OHOS #118192

Uh oh!

Conversation

phuang commented Dec 1, 2024

Uh oh!

github-actions bot commented Dec 1, 2024

Uh oh!

llvmbot commented Dec 1, 2024

Uh oh!

llvmbot commented Dec 1, 2024

Uh oh!

phuang commented Dec 6, 2024

Uh oh!

kpdev commented Dec 7, 2024

Uh oh!

github-actions bot commented Dec 7, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

kpdev commented Dec 8, 2024

Uh oh!

phuang commented Dec 8, 2024

Uh oh!

phuang commented Dec 9, 2024

Uh oh!

Uh oh!

github-actions bot commented Dec 12, 2024

Uh oh!

MaskRay commented Dec 12, 2024

Uh oh!

kpdev commented Dec 12, 2024

Uh oh!

nico commented Dec 13, 2024

Uh oh!

kpdev commented Dec 13, 2024

Uh oh!

phuang commented Dec 13, 2024

Uh oh!

kpdev commented Dec 13, 2024

Uh oh!

nico commented Dec 14, 2024

Uh oh!

kpdev commented Dec 14, 2024

Uh oh!

phuang commented Dec 16, 2024

Uh oh!

phuang commented Dec 16, 2024

Uh oh!

phuang commented Dec 16, 2024

Uh oh!

nico commented Dec 17, 2024

Uh oh!

MaskRay left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

github-actions bot commented Dec 7, 2024 •

edited

Loading