Skip to content

Commit 47e3626

Browse files
vabridgerseinvbri
authored andcommitted
[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.
1 parent cff5007 commit 47e3626

File tree

5 files changed

+54
-1
lines changed

5 files changed

+54
-1
lines changed

clang/docs/ReleaseNotes.rst

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -185,6 +185,9 @@ New Compiler Flags
185185
the preprocessed text to the output. This can greatly reduce the size of the
186186
preprocessed output, which can be helpful when trying to reduce a test case.
187187

188+
* ``-Wbitfield-conversion`` was added to detect assignments of integral
189+
types to a bitfield that may change the value.
190+
188191
Deprecated Compiler Flags
189192
-------------------------
190193

clang/include/clang/Basic/DiagnosticGroups.td

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -53,6 +53,7 @@ def SingleBitBitFieldConstantConversion :
5353
def BitFieldConstantConversion : DiagGroup<"bitfield-constant-conversion",
5454
[SingleBitBitFieldConstantConversion]>;
5555
def BitFieldEnumConversion : DiagGroup<"bitfield-enum-conversion">;
56+
def BitFieldConversion : DiagGroup<"bitfield-conversion">;
5657
def BitFieldWidth : DiagGroup<"bitfield-width">;
5758
def CompoundTokenSplitByMacro : DiagGroup<"compound-token-split-by-macro">;
5859
def CompoundTokenSplitBySpace : DiagGroup<"compound-token-split-by-space">;
@@ -933,6 +934,7 @@ def Conversion : DiagGroup<"conversion",
933934
ConstantConversion,
934935
EnumConversion,
935936
BitFieldEnumConversion,
937+
BitFieldConversion,
936938
FloatConversion,
937939
Shorten64To32,
938940
IntConversion,

clang/include/clang/Basic/DiagnosticSemaKinds.td

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6171,6 +6171,9 @@ def warn_signed_bitfield_enum_conversion : Warning<
61716171
"signed bit-field %0 needs an extra bit to represent the largest positive "
61726172
"enumerators of %1">,
61736173
InGroup<BitFieldEnumConversion>, DefaultIgnore;
6174+
def warn_bitfield_too_small_for_integral_type : Warning<
6175+
"conversion from %2 (%3 bits) to bit-field %0 (%1 bits) may change value">,
6176+
InGroup<BitFieldConversion>, DefaultIgnore;
61746177
def note_change_bitfield_sign : Note<
61756178
"consider making the bitfield type %select{unsigned|signed}0">;
61766179

clang/lib/Sema/SemaChecking.cpp

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14298,6 +14298,18 @@ static bool AnalyzeBitFieldAssignment(Sema &S, FieldDecl *Bitfield, Expr *Init,
1429814298
S.Diag(WidthExpr->getExprLoc(), diag::note_widen_bitfield)
1429914299
<< BitsNeeded << ED << WidthExpr->getSourceRange();
1430014300
}
14301+
} else if (OriginalInit->getType()->isIntegralType(S.Context)) {
14302+
IntRange LikelySourceRange =
14303+
GetExprRange(S.Context, Init, S.isConstantEvaluatedContext(),
14304+
/*Approximate=*/true);
14305+
14306+
if (LikelySourceRange.Width > FieldWidth) {
14307+
Expr *WidthExpr = Bitfield->getBitWidth();
14308+
S.Diag(InitLoc, diag::warn_bitfield_too_small_for_integral_type)
14309+
<< Bitfield << FieldWidth << OriginalInit->getType()
14310+
<< LikelySourceRange.Width;
14311+
S.Diag(WidthExpr->getExprLoc(), diag::note_declared_at);
14312+
}
1430114313
}
1430214314

1430314315
return false;
@@ -15195,7 +15207,6 @@ static void CheckImplicitConversion(Sema &S, Expr *E, QualType T,
1519515207

1519615208
if (LikelySourceRange.Width > TargetRange.Width) {
1519715209
// If the source is a constant, use a default-on diagnostic.
15198-
// TODO: this should happen for bitfield stores, too.
1519915210
Expr::EvalResult Result;
1520015211
if (E->EvaluateAsInt(Result, S.Context, Expr::SE_AllowSideEffects,
1520115212
S.isConstantEvaluatedContext())) {

clang/test/SemaCXX/bitfield-width.c

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,34 @@
1+
// RUN: %clang_cc1 -Wconversion -fsyntax-only -verify %s
2+
// RUN: %clang_cc1 -Wbitfield-conversion -fsyntax-only -verify %s
3+
4+
typedef struct _xx {
5+
int bf:9; // expected-note 4{{declared here}}
6+
} xx, *pxx;
7+
8+
xx vxx;
9+
10+
void foo1(int x) {
11+
vxx.bf = x; // expected-warning{{conversion from 'int' (32 bits) to bit-field 'bf' (9 bits) may change value}}
12+
}
13+
void foo2(short x) {
14+
vxx.bf = x; // expected-warning{{conversion from 'short' (16 bits) to bit-field 'bf' (9 bits) may change value}}
15+
}
16+
void foo3(char x) {
17+
vxx.bf = x; // no warning expected
18+
}
19+
void foo5(void * x) {
20+
vxx.bf = (int)x; // expected-warning{{cast to smaller integer type 'int' from 'void *'}}
21+
// expected-warning@-1{{conversion from 'int' (32 bits) to bit-field 'bf' (9 bits) may change value}}
22+
}
23+
void foo6(short x) {
24+
vxx.bf = 0xff & x; // no warning expected
25+
}
26+
void foo7(short x) {
27+
vxx.bf = 0x1ff & x; // no warning expected
28+
}
29+
void foo8(short x) {
30+
vxx.bf = 0x3ff & x; // expected-warning{{conversion from 'int' (10 bits) to bit-field 'bf' (9 bits) may change value}}
31+
}
32+
int fee(void) {
33+
return 0;
34+
}

0 commit comments

Comments
 (0)