[clang][UBSan] Make sure that the implicit-conversion group is compatible with minimal runtime #114865
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-driver @llvm/pr-subscribers-clang Author: Axel Lundberg (Zonotora) ChangesWe are currently getting:
when running
because Full diff: https://github.com/llvm/llvm-project/pull/114865.diff 1 Files Affected:
diff --git a/clang/lib/Driver/SanitizerArgs.cpp b/clang/lib/Driver/SanitizerArgs.cpp
index 89f1215afd0c81..6a00d99ba03428 100644
--- a/clang/lib/Driver/SanitizerArgs.cpp
+++ b/clang/lib/Driver/SanitizerArgs.cpp
@@ -68,9 +68,9 @@ static const SanitizerMask AlwaysRecoverable = SanitizerKind::KernelAddress |
static const SanitizerMask NeedsLTO = SanitizerKind::CFI;
static const SanitizerMask TrappingSupported =
(SanitizerKind::Undefined & ~SanitizerKind::Vptr) | SanitizerKind::Integer |
- SanitizerKind::Nullability | SanitizerKind::LocalBounds |
- SanitizerKind::CFI | SanitizerKind::FloatDivideByZero |
- SanitizerKind::ObjCCast;
+ SanitizerKind::ImplicitConversion | SanitizerKind::Nullability |
+ SanitizerKind::LocalBounds | SanitizerKind::CFI |
+ SanitizerKind::FloatDivideByZero | SanitizerKind::ObjCCast;
static const SanitizerMask TrappingDefault = SanitizerKind::CFI;
static const SanitizerMask CFIClasses =
SanitizerKind::CFIVCall | SanitizerKind::CFINVCall |
|
|
Hi @zygoloid and @efriedma-quic, would you mind taking a look? Thanks! |
|
@vitalybuka This is connected to #75481 that was merged a couple of months ago.. |
zygoloid
reviewed
Nov 19, 2024
There was a problem hiding this comment.
Please can you also add a test that we allow combining -fsanitize=implicit-conversion and -fsanitize-minimal-runtime so that this doesn't regress in a similar way in future?
(It'd be great if the test covered all the sanitizer groups that are currently compatible with the minimal runtime, so we get an early warning if we add any non-compatible sanitizers to those groups, but at minimum I'd like to see some test coverage for this change.)
|
Will do |
…ible with minimal runtime
Zonotora
force-pushed
the
implicit-conversion-minimal-runtime
branch
from
9cb9672 to
5630203
Compare
zygoloid
approved these changes
Nov 20, 2024
|
@zygoloid I don't have write access, please merge on my behalf! |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/195/builds/1335 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.
We are currently getting:
clang: error: invalid argument '-fsanitize-minimal-runtime' not allowed with '-fsanitize=implicit-conversion'when running
-fsanitize=implicit-conversion -fsanitize-minimal-runtimebecause
implicit-conversionnow includesimplicit-bitfield-conversionwhich is not included in theintegercheck. Theintegercheck includes theimplicit-integer-conversionchecks and is supported by the trapping option and because of that compatible with the minimal runtime. It is thus reasonable to makeimplicit-bitfield-conversioncompatible with the minimal runtime.