[sanitizer] Fix empty string in unsupported argument error for -fsanitize-trap #136549
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
llvmbot
added
clang
|
@llvm/pr-subscribers-clang @llvm/pr-subscribers-clang-driver Author: Sinkevich Artem (ArtSin) ChangesWhen using Full diff: https://github.com/llvm/llvm-project/pull/136549.diff 2 Files Affected:
diff --git a/clang/lib/Driver/SanitizerArgs.cpp b/clang/lib/Driver/SanitizerArgs.cpp
index f27cb813012f2..05ca2656c6522 100644
--- a/clang/lib/Driver/SanitizerArgs.cpp
+++ b/clang/lib/Driver/SanitizerArgs.cpp
@@ -286,7 +286,7 @@ parseSanitizeArgs(const Driver &D, const llvm::opt::ArgList &Args,
Add & AlwaysOut & ~DiagnosedAlwaysOutViolations) {
if (DiagnoseErrors) {
SanitizerSet SetToDiagnose;
- SetToDiagnose.Mask |= KindsToDiagnose;
+ SetToDiagnose.Mask |= expandSanitizerGroups(KindsToDiagnose);
D.Diag(diag::err_drv_unsupported_option_argument)
<< Arg->getSpelling() << toString(SetToDiagnose);
DiagnosedAlwaysOutViolations |= KindsToDiagnose;
@@ -302,7 +302,7 @@ parseSanitizeArgs(const Driver &D, const llvm::opt::ArgList &Args,
Remove & AlwaysIn & ~DiagnosedAlwaysInViolations) {
if (DiagnoseErrors) {
SanitizerSet SetToDiagnose;
- SetToDiagnose.Mask |= KindsToDiagnose;
+ SetToDiagnose.Mask |= expandSanitizerGroups(KindsToDiagnose);
D.Diag(diag::err_drv_unsupported_option_argument)
<< Arg->getSpelling() << toString(SetToDiagnose);
DiagnosedAlwaysInViolations |= KindsToDiagnose;
diff --git a/clang/test/Driver/fsanitize.c b/clang/test/Driver/fsanitize.c
index c154e339941f2..434e6fd8c4a15 100644
--- a/clang/test/Driver/fsanitize.c
+++ b/clang/test/Driver/fsanitize.c
@@ -259,6 +259,9 @@
// RUN: not %clang --target=aarch64-linux -fsanitize=memtag -I +mte %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-SANMT-NOMT-1
// CHECK-SANMT-NOMT-1: '-fsanitize=memtag-stack' requires hardware support (+memtag)
+// RUN: not %clang --target=aarch64-linux-android31 -fsanitize-trap=memtag -march=armv8-a+memtag -c %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-SANMT-TRAP
+// CHECK-SANMT-TRAP: error: unsupported argument 'memtag-stack,memtag-heap,memtag-globals' to option '-fsanitize-trap='
+
// RUN: %clang --target=x86_64-linux-gnu -fsanitize=address -fsanitize-address-use-after-scope %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-USE-AFTER-SCOPE
// RUN: %clang_cl --target=x86_64-windows -fsanitize=address -fsanitize-address-use-after-scope -### -- %s 2>&1 | FileCheck %s --check-prefix=CHECK-USE-AFTER-SCOPE
// CHECK-USE-AFTER-SCOPE: -cc1{{.*}}-fsanitize-address-use-after-scope
|
|
PTAL @vitalybuka |
ArtSin
force-pushed
the
fix-fsanitize-trap-group-error
branch
from
459d375 to
f5c8b4e
Compare
|
PTAL @thurstond |
|
Ping @vitalybuka |
|
Sorry, I didn't notice being tagged earlier. It didn't show up in my review queue since the "Request Review" wasn't used. Thanks for the patch! I agree this is an improvement over the empty output, but it's arguably confusing to tell the user (clang/lib/Driver/SanitizerArgs.cpp) so that it works for SANITIZER_GROUP as well instead of only SANITIZER. |
thurstond
reviewed
May 14, 2025
clang/test/Driver/fsanitize.c
Outdated
| // CHECK-SANMT-NOMT-1: '-fsanitize=memtag-stack' requires hardware support (+memtag) | ||
|
|
||
| // RUN: not %clang --target=aarch64-linux-android31 -fsanitize-trap=memtag -march=armv8-a+memtag -c %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-SANMT-TRAP | ||
| // CHECK-SANMT-TRAP: error: unsupported argument 'memtag-stack,memtag-heap,memtag-globals' to option '-fsanitize-trap=' |
There was a problem hiding this comment.
Optional: precommit this test to show that it currently prints
unsupported argument '' to option '-fsanitize-trap='
…tize-trap When using `-fsanitize-trap` with a sanitizer group that doesn't support trapping, an empty argument is passed to `err_drv_unsupported_option_argument`. Use new `toStringWithGroups` for the diagnostic.
ArtSin
force-pushed
the
fix-fsanitize-trap-group-error
branch
from
f5c8b4e to
626d7e1
Compare
I agree, but adding groups to |
Hmm, that's a good point. I like your |
thurstond
approved these changes
May 15, 2025
|
@thurstond can you please merge this? I don't have write access. |
Sure! Thanks for the fix :-) |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/174/builds/17824 Here is the relevant piece of the build log for the reference |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
When using
-fsanitize-trapwith a sanitizer group that doesn't support trapping, an empty argument is passed toerr_drv_unsupported_option_argument. Use newtoStringWithGroupsfor the diagnostic.