[clang] [Sema] Add check for bitfield assignments to integral types (PR #69049)
via cfe-commits
cfe-commits at lists.llvm.org
Sat Oct 14 02:25:17 PDT 2023
https://github.com/vabridgers created https://github.com/llvm/llvm-project/pull/69049
This change introduces the bitfield conversion check after fixes to the test case. The original submission unfortunately did not comprehend differences in architecture for the diagnostic messages, leading to unanticipated failures in the arm build bots.
Original PR: https://github.com/llvm/llvm-project/pull/68276
Clang does not check for bitfield assignment widths, while gcc checks this.
gcc produced a warning like so for it's -Wconversion flag:
```
$ gcc -Wconversion -c test.c
test.c: In function 'foo':
test.c:10:15: warning: conversion from 'int' to 'signed char:7' may
change value [-Wconversion]
10 | vxx.bf = x; // no warning
| ^
```
This change simply adds this check for integral types under the -Wbitfield-conversion compiler option.
>From e11dd44ab0c89888b9531bbf4dc0cf6ee407be10 Mon Sep 17 00:00:00 2001
From: Vince Bridgers <vince.a.bridgers at gmail.com>
Date: Sat, 14 Oct 2023 10:14:52 +0200
Subject: [PATCH] [Sema] Add check for bitfield assignments to integral types
This change introduces the bitfield conversion check after fixes to the
test case. The original submission unfortunately did not comprehend
differences in architecture for the diagnostic messages, leading to
unanticipated failures in the arm build bots.
Original PR: https://github.com/llvm/llvm-project/pull/68276
Clang does not check for bitfield assignment widths, while gcc checks this.
gcc produced a warning like so for it's -Wconversion flag:
```
$ gcc -Wconversion -c test.c
test.c: In function 'foo':
test.c:10:15: warning: conversion from 'int' to 'signed char:7' may
change value [-Wconversion]
10 | vxx.bf = x; // no warning
| ^
```
This change simply adds this check for integral types under the
-Wbitfield-conversion compiler option.
---
clang/docs/ReleaseNotes.rst | 3 ++
clang/include/clang/Basic/DiagnosticGroups.td | 2 +
.../clang/Basic/DiagnosticSemaKinds.td | 3 ++
clang/lib/Sema/SemaChecking.cpp | 13 +++++-
clang/test/SemaCXX/bitfield-width.c | 42 +++++++++++++++++++
5 files changed, 62 insertions(+), 1 deletion(-)
create mode 100644 clang/test/SemaCXX/bitfield-width.c
diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 2d918967e7f0b02..31969201a1cac8c 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -185,6 +185,9 @@ New Compiler Flags
the preprocessed text to the output. This can greatly reduce the size of the
preprocessed output, which can be helpful when trying to reduce a test case.
+* ``-Wbitfield-conversion`` was added to detect assignments of integral
+ types to a bitfield that may change the value.
+
Deprecated Compiler Flags
-------------------------
diff --git a/clang/include/clang/Basic/DiagnosticGroups.td b/clang/include/clang/Basic/DiagnosticGroups.td
index 0b09c002191848a..674eb9f4ef2e73f 100644
--- a/clang/include/clang/Basic/DiagnosticGroups.td
+++ b/clang/include/clang/Basic/DiagnosticGroups.td
@@ -53,6 +53,7 @@ def SingleBitBitFieldConstantConversion :
def BitFieldConstantConversion : DiagGroup<"bitfield-constant-conversion",
[SingleBitBitFieldConstantConversion]>;
def BitFieldEnumConversion : DiagGroup<"bitfield-enum-conversion">;
+def BitFieldConversion : DiagGroup<"bitfield-conversion">;
def BitFieldWidth : DiagGroup<"bitfield-width">;
def CompoundTokenSplitByMacro : DiagGroup<"compound-token-split-by-macro">;
def CompoundTokenSplitBySpace : DiagGroup<"compound-token-split-by-space">;
@@ -933,6 +934,7 @@ def Conversion : DiagGroup<"conversion",
ConstantConversion,
EnumConversion,
BitFieldEnumConversion,
+ BitFieldConversion,
FloatConversion,
Shorten64To32,
IntConversion,
diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index c1a6e3831127e56..ab7fe881976aad2 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -6171,6 +6171,9 @@ def warn_signed_bitfield_enum_conversion : Warning<
"signed bit-field %0 needs an extra bit to represent the largest positive "
"enumerators of %1">,
InGroup<BitFieldEnumConversion>, DefaultIgnore;
+def warn_bitfield_too_small_for_integral_type : Warning<
+ "conversion from %2 (%3 bits) to bit-field %0 (%1 bits) may change value">,
+ InGroup<BitFieldConversion>, DefaultIgnore;
def note_change_bitfield_sign : Note<
"consider making the bitfield type %select{unsigned|signed}0">;
diff --git a/clang/lib/Sema/SemaChecking.cpp b/clang/lib/Sema/SemaChecking.cpp
index 35b36db2049db09..cd61459cfbb13d6 100644
--- a/clang/lib/Sema/SemaChecking.cpp
+++ b/clang/lib/Sema/SemaChecking.cpp
@@ -14298,6 +14298,18 @@ static bool AnalyzeBitFieldAssignment(Sema &S, FieldDecl *Bitfield, Expr *Init,
S.Diag(WidthExpr->getExprLoc(), diag::note_widen_bitfield)
<< BitsNeeded << ED << WidthExpr->getSourceRange();
}
+ } else if (OriginalInit->getType()->isIntegralType(S.Context)) {
+ IntRange LikelySourceRange =
+ GetExprRange(S.Context, Init, S.isConstantEvaluatedContext(),
+ /*Approximate=*/true);
+
+ if (LikelySourceRange.Width > FieldWidth) {
+ Expr *WidthExpr = Bitfield->getBitWidth();
+ S.Diag(InitLoc, diag::warn_bitfield_too_small_for_integral_type)
+ << Bitfield << FieldWidth << OriginalInit->getType()
+ << LikelySourceRange.Width;
+ S.Diag(WidthExpr->getExprLoc(), diag::note_declared_at);
+ }
}
return false;
@@ -15195,7 +15207,6 @@ static void CheckImplicitConversion(Sema &S, Expr *E, QualType T,
if (LikelySourceRange.Width > TargetRange.Width) {
// If the source is a constant, use a default-on diagnostic.
- // TODO: this should happen for bitfield stores, too.
Expr::EvalResult Result;
if (E->EvaluateAsInt(Result, S.Context, Expr::SE_AllowSideEffects,
S.isConstantEvaluatedContext())) {
diff --git a/clang/test/SemaCXX/bitfield-width.c b/clang/test/SemaCXX/bitfield-width.c
new file mode 100644
index 000000000000000..7b4e4444c245b0e
--- /dev/null
+++ b/clang/test/SemaCXX/bitfield-width.c
@@ -0,0 +1,42 @@
+// RUN: %clang_cc1 -Wconversion -fsyntax-only -verify %s
+// RUN: %clang_cc1 -Wbitfield-conversion -fsyntax-only -verify %s
+// RUN: %clang_cc1 -triple armebv7-unknown-linux -Wbitfield-conversion \
+// RUN: -fsyntax-only -verify %s
+// RUN: %clang_cc1 -triple arm64-unknown-linux -Wbitfield-conversion \
+// RUN: -fsyntax-only -verify %s
+// RUN: %clang_cc1 -triple arm-unknown-linux -Wbitfield-conversion \
+// RUN: -fsyntax-only -verify %s
+// RUN: %clang_cc1 -triple aarch64-unknown-linux -Wbitfield-conversion \
+// RUN: -fsyntax-only -verify %s
+// RUN: %clang_cc1 -triple mipsel-unknown-linux -Wbitfield-conversion \
+// RUN: -fsyntax-only -verify %s
+// RUN: %clang_cc1 -triple mips64el-unknown-linux -Wbitfield-conversion \
+// RUN: -fsyntax-only -verify %s
+
+typedef struct _xx {
+ int bf:9; // expected-note 3{{declared here}}
+ } xx, *pxx;
+
+ xx vxx;
+
+ void foo1(int x) {
+ vxx.bf = x; // expected-warning{{conversion from 'int' (32 bits) to bit-field 'bf' (9 bits) may change value}}
+ }
+ void foo2(short x) {
+ vxx.bf = x; // expected-warning{{conversion from 'short' (16 bits) to bit-field 'bf' (9 bits) may change value}}
+ }
+ void foo3(char x) {
+ vxx.bf = x; // no warning expected
+ }
+ void foo4(short x) {
+ vxx.bf = 0xff & x; // no warning expected
+ }
+ void foo5(short x) {
+ vxx.bf = 0x1ff & x; // no warning expected
+ }
+ void foo6(short x) {
+ vxx.bf = 0x3ff & x; // expected-warning{{conversion from 'int' (10 bits) to bit-field 'bf' (9 bits) may change value}}
+ }
+ int fee(void) {
+ return 0;
+ }
More information about the cfe-commits
mailing list