Skip to content

[Driver][DragonFly] Fixes for linker path and command-line option handling #69095

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
Oct 19, 2023

Conversation

brad0
Copy link
Contributor

@brad0 brad0 commented Oct 15, 2023

  • Add in some other linker command line options that the other BSD's handle
  • Make use of AddFilePathLibArgs()
  • Handle OpenMP

@llvmbot llvmbot added clang Clang issues not falling into any other category clang:driver 'clang' and 'clang++' user-facing binaries. Not 'clang-cl' labels Oct 15, 2023
@llvmbot
Copy link
Member

llvmbot commented Oct 15, 2023

@llvm/pr-subscribers-clang-driver

@llvm/pr-subscribers-clang

Author: Brad Smith (brad0)

Changes

Add in some other linker command line options that the other BSD's handle and make use of AddFilePathLibArgs().


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

2 Files Affected:

  • (modified) clang/lib/Driver/ToolChains/DragonFly.cpp (+7-6)
  • (modified) clang/test/Driver/dragonfly.c (+14-2)
diff --git a/clang/lib/Driver/ToolChains/DragonFly.cpp b/clang/lib/Driver/ToolChains/DragonFly.cpp
index a58983aba5a12f2..d42196fa94a2c2b 100644
--- a/clang/lib/Driver/ToolChains/DragonFly.cpp
+++ b/clang/lib/Driver/ToolChains/DragonFly.cpp
@@ -56,7 +56,9 @@ void dragonfly::Linker::ConstructJob(Compilation &C, const JobAction &JA,
                                      const InputInfoList &Inputs,
                                      const ArgList &Args,
                                      const char *LinkingOutput) const {
-  const Driver &D = getToolChain().getDriver();
+  const toolchains::DragonFly &ToolChain =
+      static_cast<const toolchains::DragonFly &>(getToolChain());
+  const Driver &D = ToolChain.getDriver();
   ArgStringList CmdArgs;
 
   if (!D.SysRoot.empty())
@@ -115,16 +117,15 @@ void dragonfly::Linker::ConstructJob(Compilation &C, const JobAction &JA,
           Args.MakeArgString(getToolChain().GetFilePath("crtbegin.o")));
   }
 
-  Args.addAllArgs(CmdArgs, {options::OPT_L, options::OPT_T_Group});
+  Args.addAllArgs(CmdArgs,
+                  {options::OPT_L, options::OPT_T_Group, options::OPT_s,
+                   options::OPT_t, options::OPT_Z_Flag, options::OPT_r});
+  ToolChain.AddFilePathLibArgs(Args, CmdArgs);
 
   AddLinkerInputs(getToolChain(), Inputs, Args, CmdArgs, JA);
 
   if (!Args.hasArg(options::OPT_nostdlib, options::OPT_nodefaultlibs,
                    options::OPT_r)) {
-    SmallString<128> Dir(D.SysRoot);
-    llvm::sys::path::append(Dir, "/usr/lib/gcc80");
-    CmdArgs.push_back(Args.MakeArgString("-L" + Dir));
-
     if (!Args.hasArg(options::OPT_static)) {
       CmdArgs.push_back("-rpath");
       CmdArgs.push_back("/usr/lib/gcc80");
diff --git a/clang/test/Driver/dragonfly.c b/clang/test/Driver/dragonfly.c
index 8ba13c41d632c20..31583d3dec52bbb 100644
--- a/clang/test/Driver/dragonfly.c
+++ b/clang/test/Driver/dragonfly.c
@@ -2,7 +2,7 @@
 // RUN: FileCheck -input-file %t.log %s
 
 // CHECK: "-cc1" "-triple" "x86_64-pc-dragonfly"
-// CHECK: ld{{.*}}" "--eh-frame-hdr" "-dynamic-linker" "/usr/libexec/ld-elf.so.{{.*}}" "--hash-style=gnu" "--enable-new-dtags" "-o" "a.out" "{{.*}}crt1.o" "{{.*}}crti.o" "{{.*}}crtbegin.o" "{{.*}}.o" "-L{{.*}}gcc{{.*}}" "-rpath" "{{.*}}gcc{{.*}}" "-lc" "-lgcc" "{{.*}}crtend.o" "{{.*}}crtn.o"
+// CHECK: ld{{.*}}" "--eh-frame-hdr" "-dynamic-linker" "/usr/libexec/ld-elf.so.{{.*}}" "--hash-style=gnu" "--enable-new-dtags" "-o" "a.out" "{{.*}}crt1.o" "{{.*}}crti.o" "{{.*}}crtbegin.o" "{{.*}}" "-L/usr/lib" "-L/usr/lib/gcc80" "{{.*}}.o" "-rpath" "{{.*}}gcc80{{.*}}" "-lc" "-lgcc" "{{.*}}crtend.o" "{{.*}}crtn.o"
 
 // Check x86_64-unknown-dragonfly, X86_64
 // RUN: %clang -### %s 2>&1 --target=x86_64-unknown-dragonfly \
@@ -15,7 +15,8 @@
 // CHECK-LD-X86_64-SAME: "[[SYSROOT]]{{/|\\\\}}usr{{/|\\\\}}lib{{/|\\\\}}crt1.o"
 // CHECK-LD-X86_64-SAME: "[[SYSROOT]]{{/|\\\\}}usr{{/|\\\\}}lib{{/|\\\\}}crti.o"
 // CHECK-LD-X86_64-SAME: "[[SYSROOT]]{{/|\\\\}}usr{{/|\\\\}}lib{{/|\\\\}}gcc80{{/|\\\\}}crtbegin.o"
-// CHECK-LD-X86_64-SAME: "-L[[SYSROOT]]{{/|\\\\}}usr{{/|\\\\}}lib{{/|\\\\}}gcc80" "-rpath" "/usr/lib/gcc80" "-lc" "-lgcc" "--as-needed" "-lgcc_pic" "--no-as-needed"
+// CHECK-LD-X86_64-SAME: "-L[[SYSROOT]]{{/|\\\\}}usr{{/|\\\\}}lib" "-L[[SYSROOT]]{{/|\\\\}}usr{{/|\\\\}}lib{{/|\\\\}}gcc80"
+// CHECK-LD-X86_64-SAME: "-rpath" "/usr/lib/gcc80" "-lc" "-lgcc" "--as-needed" "-lgcc_pic" "--no-as-needed"
 // CHECK-LD-X86_64-SAME: "[[SYSROOT]]{{/|\\\\}}usr{{/|\\\\}}lib{{/|\\\\}}gcc80{{/|\\\\}}crtend.o"
 // CHECK-LD-X86_64-SAME: "[[SYSROOT]]{{/|\\\\}}usr{{/|\\\\}}lib{{/|\\\\}}crtn.o"
 
@@ -26,3 +27,14 @@
 // RELOCATABLE-NOT: "-dynamic-linker"
 // RELOCATABLE-NOT: "-l
 // RELOCATABLE-NOT: {{.*}}crt{{[^./]+}}.o
+
+// Check that the new linker flags are passed to DraonFly
+// RUN: %clang --target=x86_64-unknown-dragonfly -s -### %s 2>&1 \
+// RUN:   | FileCheck --check-prefix=CHECK-LD-S %s
+// RUN: %clang --target=x86_64-unknown-dragonfly -t -### %s 2>&1 \
+// RUN:   | FileCheck --check-prefix=CHECK-LD-T %s
+// RUN: %clang --target=x86_64-unknown-dragonfly -Z -### %s 2>&1 \
+// RUN:   | FileCheck --check-prefix=CHECK-LD-Z %s
+// CHECK-LD-S: ld{{.*}}" "{{.*}}" "-s"
+// CHECK-LD-T: ld{{.*}}" "{{.*}}" "-t"
+// CHECK-LD-Z: ld{{.*}}" "{{.*}}" "-Z"

@brad0 brad0 force-pushed the clang_driver_dragonfly_linkerflags branch from 59dcfdc to 39b8993 Compare October 15, 2023 08:08
@brad0 brad0 force-pushed the clang_driver_dragonfly_linkerflags branch 2 times, most recently from a8ff33a to 1840ed8 Compare October 16, 2023 02:28
@brad0
Copy link
Contributor Author

brad0 commented Oct 16, 2023

Fix a typo in a comment.

@brad0 brad0 requested a review from MaskRay October 17, 2023 03:59
…dling

- Add in some other linker command line options that the other BSD's handle
- Make use of AddFilePathLibArgs()
- Handle OpenMP
@brad0 brad0 force-pushed the clang_driver_dragonfly_linkerflags branch from 1840ed8 to 7009758 Compare October 18, 2023 03:22
@brad0 brad0 merged commit 0913a2d into llvm:main Oct 19, 2023
@brad0 brad0 deleted the clang_driver_dragonfly_linkerflags branch October 19, 2023 00:47
@AaronBallman
Copy link
Collaborator

This appears to have broken some bots:
https://lab.llvm.org/buildbot/#/builders/119/builds/15633
https://lab.llvm.org/buildbot/#/builders/60/builds/14449

Can you investigate and get those bots back to green?

@AaronBallman
Copy link
Collaborator

This appears to have broken some bots: https://lab.llvm.org/buildbot/#/builders/119/builds/15633 https://lab.llvm.org/buildbot/#/builders/60/builds/14449

Can you investigate and get those bots back to green?

Ping?

MaskRay added a commit that referenced this pull request Nov 2, 2023
If DEFAULT_SYSROOT is not empty, the /usr/lib/gcc80 path may have
leading path components
`getFilePaths().push_back(concat(getDriver().SysRoot, "/usr/lib/gcc80"))`
@MaskRay
Copy link
Member

MaskRay commented Nov 2, 2023

This appears to have broken some bots: lab.llvm.org/buildbot/#/builders/119/builds/15633 lab.llvm.org/buildbot/#/builders/60/builds/14449
Can you investigate and get those bots back to green?

Ping?

fixed by e65721c

I guess this Windows bot is related to the CMake DEFAULT_SYSROOT variable that I want to deprecate in https://reviews.llvm.org/D158218

@AaronBallman
Copy link
Collaborator

This appears to have broken some bots: lab.llvm.org/buildbot/#/builders/119/builds/15633 lab.llvm.org/buildbot/#/builders/60/builds/14449
Can you investigate and get those bots back to green?

Ping?

fixed by e65721c

I guess this Windows bot is related to the CMake DEFAULT_SYSROOT variable that I want to deprecate in https://reviews.llvm.org/D158218

Thank you for the fix! I'll keep my eyes on the bots to make sure they go back to green and report back if they don't. :-)

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.

4 participants