Skip to content

[clang-linker-wrapper] Add error handling for missing linker path #113613

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 2 commits into from
Oct 25, 2024

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