-
Notifications
You must be signed in to change notification settings - Fork 13.7k
[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
Conversation
Signed-off-by: Arvind Sudarsanam <[email protected]>
@llvm/pr-subscribers-clang-driver @llvm/pr-subscribers-clang Author: Arvind Sudarsanam (asudarsa) ChangesIn clang-linker-wrapper, we do not explicitly check if --linker-path is provided. Thanks Full diff: https://github.com/llvm/llvm-project/pull/113613.diff 2 Files Affected:
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.
|
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"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
return createStringError("Host linker is not available"); | |
return createStringError("linker path missing, must pass 'linker-path'"); |
There was a problem hiding this comment.
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]>
…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]>
In clang-linker-wrapper, we do not explicitly check if --linker-path is provided.
This PR adds a check to capture this.
Thanks