[clang] [Sema] Add check for bitfield assignments to larger integral types (PR #68276)
via cfe-commits
cfe-commits at lists.llvm.org
Wed Oct 11 16:14:07 PDT 2023
https://github.com/vabridgers updated https://github.com/llvm/llvm-project/pull/68276
>From 3bb0c8483c61e04f5ff76de920dcad89793f1326 Mon Sep 17 00:00:00 2001
From: Vince Bridgers <vince.a.bridgers at gmail.com>
Date: Thu, 5 Oct 2023 02:39:12 +0200
Subject: [PATCH] [Sema] Add check for bitfield assignments to integral types
We noticed that clang does not check for bitfield assignment widths,
while gcc does check 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 for integral types under the -Wconversion
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 | 37 +++++++++++++++++++
5 files changed, 57 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..cd92012ef5f411c 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.
+* ``-conversion-bitfield-assign`` 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 2594a8f97f7d94e..1b2f8cf296d16b6 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..048451d718dbfb6
--- /dev/null
+++ b/clang/test/SemaCXX/bitfield-width.c
@@ -0,0 +1,37 @@
+// RUN: %clang_cc1 -Wconversion -fsyntax-only -verify %s
+// RUN: %clang_cc1 -Wbitfield-conversion -fsyntax-only -verify %s
+
+typedef struct _xx {
+ int bf:9; // expected-note{{declared here}}
+ // expected-note at -1{{declared here}}
+ // expected-note at -2{{declared here}}
+ // expected-note at -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 foo5(void * x) {
+ vxx.bf = (int)x; // expected-warning{{cast to smaller integer type 'int' from 'void *'}}
+ // expected-warning at -1{{conversion from 'int' (32 bits) to bit-field 'bf' (9 bits) may change value}}
+ }
+ void foo6(short x) {
+ vxx.bf = 0xff & x; // no warning expected
+ }
+ void foo7(short x) {
+ vxx.bf = 0x1ff & x; // no warning expected
+ }
+ void foo8(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