https://github.com/vabridgers updated 
https://github.com/llvm/llvm-project/pull/68276

>From 24660d933c3dbdbf9c3d9976a0e36c274635f668 Mon Sep 17 00:00:00 2001
From: Vince Bridgers <vince.a.bridg...@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        |  5 +++
 clang/lib/Sema/SemaChecking.cpp               | 15 +++++++-
 clang/test/SemaCXX/bitfield-width.c           | 37 +++++++++++++++++++
 5 files changed, 61 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 d2a435041a1542f..8452878aec6e1eb 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -165,6 +165,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 28f1db29e62fa91..d3f86aca8e73216 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -6171,6 +6171,11 @@ 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_bitfield_assign : Note<
+  "Bit-field %0 (%1 bits) is too narrow to store assigned value %2 (%3 bits)">;
 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 446e35218bff0ad..22117d52cbbc5b5 100644
--- a/clang/lib/Sema/SemaChecking.cpp
+++ b/clang/lib/Sema/SemaChecking.cpp
@@ -14299,6 +14299,20 @@ 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_bitfield_assign)
+            << Bitfield << FieldWidth << OriginalInit->getType()
+            << LikelySourceRange.Width << WidthExpr->getSourceRange();
+      }
     }
 
     return false;
@@ -15196,7 +15210,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..a97f1f94198c8e9
--- /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{{Bit-field 'bf' (9 bits) is too narrow to 
store assigned value 'int' (32 bits)}}
+     // expected-note@-1{{Bit-field 'bf' (9 bits) is too narrow to store 
assigned value 'short' (16 bits)}}
+     // expected-note@-2{{Bit-field 'bf' (9 bits) is too narrow to store 
assigned value 'int' (32 bits)}}
+     // expected-note@-3{{Bit-field 'bf' (9 bits) is too narrow to store 
assigned value 'int' (10 bits)}}
+ } 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@-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;
+ }

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to