Skip to content

Conversation

asudarsa
Copy link
Contributor

In clang-linker-wrapper, we do not explicitly check if --linker-path is provided.
This PR adds a check to capture this.

Thanks

@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 24, 2024
@llvmbot
Copy link
Member

llvmbot commented Oct 24, 2024

@llvm/pr-subscribers-clang-driver

@llvm/pr-subscribers-clang

Author: Arvind Sudarsanam (asudarsa)

Changes

In clang-linker-wrapper, we do not explicitly check if --linker-path is provided.
This PR adds a check to capture this.

Thanks


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

2 Files Affected:

  • (modified) clang/test/Driver/linker-wrapper.c (+4)
  • (modified) clang/tools/clang-linker-wrapper/ClangLinkerWrapper.cpp (+2)
diff --git a/clang/test/Driver/linker-wrapper.c b/clang/test/Driver/linker-wrapper.c
index 068ea2d7d3c663..4ab4051f37553e 100644
--- a/clang/test/Driver/linker-wrapper.c
+++ b/clang/test/Driver/linker-wrapper.c
@@ -250,3 +250,7 @@ __attribute__((visibility("protected"), used)) int x;
 //       MLLVM-SAME: -Xlinker -mllvm=-pass-remarks=foo,bar
 //  OFFLOAD-OPT-NOT: -Xlinker -mllvm=-pass-remarks=foo,bar
 // OFFLOAD-OPT-SAME: {{$}}
+
+// Error handling when --linker-path is not provided for clang-linker-wrapper
+// RUN: not clang-linker-wrapper 2>&1 | FileCheck --check-prefix=LINKER-PATH-NOT-PROVIDED %s
+// LINKER-PATH-NOT-PROVIDED: Host linker is not available
diff --git a/clang/tools/clang-linker-wrapper/ClangLinkerWrapper.cpp b/clang/tools/clang-linker-wrapper/ClangLinkerWrapper.cpp
index 9fea1fdcd5fb46..8000d0e1f48dad 100644
--- a/clang/tools/clang-linker-wrapper/ClangLinkerWrapper.cpp
+++ b/clang/tools/clang-linker-wrapper/ClangLinkerWrapper.cpp
@@ -370,6 +370,8 @@ Error runLinker(ArrayRef<StringRef> Files, const ArgList &Args) {
   // Render the linker arguments and add the newly created image. We add it
   // after the output file to ensure it is linked with the correct libraries.
   StringRef LinkerPath = Args.getLastArgValue(OPT_linker_path_EQ);
+  if (LinkerPath.empty())
+    return createStringError("Host linker is not available");
   ArgStringList NewLinkerArgs;
   for (const opt::Arg *Arg : Args) {
     // Do not forward arguments only intended for the linker wrapper.

@asudarsa
Copy link
Contributor Author

asudarsa commented Oct 24, 2024

@jhuber6

Can you please take a look when convenient and comment if this change looks ok?

Thanks

@@ -370,6 +370,8 @@ Error runLinker(ArrayRef<StringRef> Files, const ArgList &Args) {
// Render the linker arguments and add the newly created image. We add it
// after the output file to ensure it is linked with the correct libraries.
StringRef LinkerPath = Args.getLastArgValue(OPT_linker_path_EQ);
if (LinkerPath.empty())
return createStringError("Host linker is not available");
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
return createStringError("Host linker is not available");
return createStringError("linker path missing, must pass 'linker-path'");

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks. Addressed in next commit.

Signed-off-by: Arvind Sudarsanam <[email protected]>
@jhuber6 jhuber6 merged commit 6e73750 into llvm:main Oct 25, 2024
8 checks passed
NoumanAmir657 pushed a commit to NoumanAmir657/llvm-project that referenced this pull request Nov 4, 2024
…vm#113613)

In clang-linker-wrapper, we do not explicitly check if --linker-path is
provided.
This PR adds a check to capture this.

Thanks

---------

Signed-off-by: Arvind Sudarsanam <[email protected]>
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