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