Skip to content

Commit e11dd44

Browse files
vabridgerseinvbri
authored andcommitted
[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: llvm/llvm-project#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.
1 parent 7060422 commit e11dd44

File tree

5 files changed

+62
-1
lines changed

5 files changed

+62
-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())) {
Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,42 @@
1+
// RUN: %clang_cc1 -Wconversion -fsyntax-only -verify %s
2+
// RUN: %clang_cc1 -Wbitfield-conversion -fsyntax-only -verify %s
3+
// RUN: %clang_cc1 -triple armebv7-unknown-linux -Wbitfield-conversion \
4+
// RUN: -fsyntax-only -verify %s
5+
// RUN: %clang_cc1 -triple arm64-unknown-linux -Wbitfield-conversion \
6+
// RUN: -fsyntax-only -verify %s
7+
// RUN: %clang_cc1 -triple arm-unknown-linux -Wbitfield-conversion \
8+
// RUN: -fsyntax-only -verify %s
9+
// RUN: %clang_cc1 -triple aarch64-unknown-linux -Wbitfield-conversion \
10+
// RUN: -fsyntax-only -verify %s
11+
// RUN: %clang_cc1 -triple mipsel-unknown-linux -Wbitfield-conversion \
12+
// RUN: -fsyntax-only -verify %s
13+
// RUN: %clang_cc1 -triple mips64el-unknown-linux -Wbitfield-conversion \
14+
// RUN: -fsyntax-only -verify %s
15+
16+
typedef struct _xx {
17+
int bf:9; // expected-note 3{{declared here}}
18+
} xx, *pxx;
19+
20+
xx vxx;
21+
22+
void foo1(int x) {
23+
vxx.bf = x; // expected-warning{{conversion from 'int' (32 bits) to bit-field 'bf' (9 bits) may change value}}
24+
}
25+
void foo2(short x) {
26+
vxx.bf = x; // expected-warning{{conversion from 'short' (16 bits) to bit-field 'bf' (9 bits) may change value}}
27+
}
28+
void foo3(char x) {
29+
vxx.bf = x; // no warning expected
30+
}
31+
void foo4(short x) {
32+
vxx.bf = 0xff & x; // no warning expected
33+
}
34+
void foo5(short x) {
35+
vxx.bf = 0x1ff & x; // no warning expected
36+
}
37+
void foo6(short x) {
38+
vxx.bf = 0x3ff & x; // expected-warning{{conversion from 'int' (10 bits) to bit-field 'bf' (9 bits) may change value}}
39+
}
40+
int fee(void) {
41+
return 0;
42+
}

0 commit comments

Comments
 (0)