https://github.com/earnol updated https://github.com/llvm/llvm-project/pull/197554
>From d44aba5af53c76cd18b79670e0b8e44ab5806090 Mon Sep 17 00:00:00 2001 From: Vladislav Aranov <[email protected]> Date: Wed, 13 May 2026 22:13:04 +0200 Subject: [PATCH] [clang-tidy] Fix bugprone-misplaced-widening-cast false positive on bitfield assignments When CheckImplicitCasts=true, the checker used the declared type of a bitfield insread of the actual bitfield width to determine if widening occurs. This caused false positives when assigning to a bitfield whose declared type is wider than the source, but whose bitfield width actually matches the source type. This behavior is fixed. --- .../bugprone/MisplacedWideningCastCheck.cpp | 28 ++- ...d-widening-cast-bitfield-explicit-only.cpp | 60 +++++ ...idening-cast-bitfield-implicit-enabled.cpp | 235 ++++++++++++++++++ 3 files changed, 320 insertions(+), 3 deletions(-) create mode 100644 clang-tools-extra/test/clang-tidy/checkers/bugprone/misplaced-widening-cast-bitfield-explicit-only.cpp create mode 100644 clang-tools-extra/test/clang-tidy/checkers/bugprone/misplaced-widening-cast-bitfield-implicit-enabled.cpp diff --git a/clang-tools-extra/clang-tidy/bugprone/MisplacedWideningCastCheck.cpp b/clang-tools-extra/clang-tidy/bugprone/MisplacedWideningCastCheck.cpp index f040235322a4f..0bba5b6252067 100644 --- a/clang-tools-extra/clang-tidy/bugprone/MisplacedWideningCastCheck.cpp +++ b/clang-tools-extra/clang-tidy/bugprone/MisplacedWideningCastCheck.cpp @@ -42,7 +42,10 @@ void MisplacedWideningCastCheck::registerMatchers(MatchFinder *Finder) { Finder->addMatcher(varDecl(hasInitializer(Cast)), this); Finder->addMatcher(returnStmt(hasReturnValue(Cast)), this); Finder->addMatcher(callExpr(hasAnyArgument(Cast)), this); - Finder->addMatcher(binaryOperator(hasOperatorName("="), hasRHS(Cast)), this); + Finder->addMatcher(binaryOperator(hasOperatorName("="), + hasLHS(expr().bind("AssignLHS")), + hasRHS(Cast)), + this); Finder->addMatcher( binaryOperator(isComparisonOperator(), hasEitherOperand(Cast)), this); } @@ -195,14 +198,33 @@ void MisplacedWideningCastCheck::check(const MatchFinder::MatchResult &Result) { const QualType CastType = Cast->getType(); const QualType CalcType = Calc->getType(); + // If assigning to a bitfield, use the bitfield width as the effective target + // width. The declared type may be wider than the actual bitfield storage. + unsigned TargetWidth = Context.getIntWidth(CastType); + bool IsBitfieldAssign = false; + if (const auto *AssignLHS = Result.Nodes.getNodeAs<Expr>("AssignLHS")) { + if (const auto *ME = + dyn_cast<MemberExpr>(AssignLHS->IgnoreParenImpCasts())) { + if (const auto *FD = dyn_cast<FieldDecl>(ME->getMemberDecl())) { + if (FD->isBitField()) { + TargetWidth = FD->getBitWidthValue(); + IsBitfieldAssign = true; + } + } + } + } + // Explicit truncation using cast. - if (Context.getIntWidth(CastType) < Context.getIntWidth(CalcType)) + if (TargetWidth < Context.getIntWidth(CalcType)) return; // If CalcType and CastType have same size then there is no real danger, but // there can be a portability problem. - if (Context.getIntWidth(CastType) == Context.getIntWidth(CalcType)) { + if (TargetWidth == Context.getIntWidth(CalcType)) { + // Bitfield width is fixed across platforms — no portability concern. + if (IsBitfieldAssign) + return; const auto *CastBuiltinType = dyn_cast<BuiltinType>(CastType->getUnqualifiedDesugaredType()); const auto *CalcBuiltinType = diff --git a/clang-tools-extra/test/clang-tidy/checkers/bugprone/misplaced-widening-cast-bitfield-explicit-only.cpp b/clang-tools-extra/test/clang-tidy/checkers/bugprone/misplaced-widening-cast-bitfield-explicit-only.cpp new file mode 100644 index 0000000000000..16d05393ded0d --- /dev/null +++ b/clang-tools-extra/test/clang-tidy/checkers/bugprone/misplaced-widening-cast-bitfield-explicit-only.cpp @@ -0,0 +1,60 @@ +// RUN: %check_clang_tidy %s bugprone-misplaced-widening-cast %t -- \ +// RUN: -config="{CheckOptions: {bugprone-misplaced-widening-cast.CheckImplicitCasts: false}}" \ +// RUN: -- -target x86_64-unknown-unknown +// RUN: %check_clang_tidy %s bugprone-misplaced-widening-cast %t -- \ +// RUN: -config="{CheckOptions: {bugprone-misplaced-widening-cast.CheckImplicitCasts: false}}" \ +// RUN: -- -target i386-unknown-unknown + +// Tests rely on specific type sizes: +// unsigned int = 32, unsigned short = 16, unsigned char = 8, +// unsigned long = 64, unsigned long long = 64 bits. + +struct BitfieldHeader { + unsigned long long field40 : 40; + unsigned long field16 : 16; +}; + +// Source (unsigned short, 16-bit) == bitfield width (16-bit). No warnings. +void explicit_cast_same_to_declared(unsigned short size) { + struct BitfieldHeader h = {}; + h.field16 = (unsigned long)(size << 1); +} + +void explicit_cast_same_to_bitfield(unsigned short size) { + struct BitfieldHeader h = {}; + h.field16 = (unsigned short)(size << 1); +} + +void explicit_cast_same_to_narrower(unsigned short size) { + struct BitfieldHeader h = {}; + h.field16 = (unsigned char)(size << 1); +} + +// Source (unsigned int, 32-bit) > bitfield width (16-bit). Truncation, no warnings. +void explicit_cast_wider_to_declared(unsigned int size) { + struct BitfieldHeader h = {}; + h.field16 = (unsigned long)(size << 1U); +} + +void explicit_cast_wider_to_bitfield(unsigned int size) { + struct BitfieldHeader h = {}; + h.field16 = (unsigned short)(size << 1U); +} + +void explicit_cast_wider_to_narrower(unsigned int size) { + struct BitfieldHeader h = {}; + h.field16 = (unsigned char)(size << 1U); +} + +// Source (unsigned int, 32-bit) < bitfield width (40-bit). Widening — should warn. +void explicit_cast_widen_shift(unsigned int size) { + struct BitfieldHeader h = {}; + h.field40 = (unsigned long long)(size << 1U); + // CHECK-MESSAGES: :[[@LINE-1]]:15: warning: either cast from 'unsigned int' to 'unsigned long long' +} + +void explicit_cast_widen_multiply(unsigned int size) { + struct BitfieldHeader h = {}; + h.field40 = (unsigned long long)(size * 2U); + // CHECK-MESSAGES: :[[@LINE-1]]:15: warning: either cast from 'unsigned int' to 'unsigned long long' +} diff --git a/clang-tools-extra/test/clang-tidy/checkers/bugprone/misplaced-widening-cast-bitfield-implicit-enabled.cpp b/clang-tools-extra/test/clang-tidy/checkers/bugprone/misplaced-widening-cast-bitfield-implicit-enabled.cpp new file mode 100644 index 0000000000000..7c0eaa3096f6a --- /dev/null +++ b/clang-tools-extra/test/clang-tidy/checkers/bugprone/misplaced-widening-cast-bitfield-implicit-enabled.cpp @@ -0,0 +1,235 @@ +// RUN: %check_clang_tidy %s bugprone-misplaced-widening-cast %t -- \ +// RUN: -config="{CheckOptions: {bugprone-misplaced-widening-cast.CheckImplicitCasts: true}}" \ +// RUN: -- -target x86_64-unknown-unknown +// RUN: %check_clang_tidy %s bugprone-misplaced-widening-cast %t -- \ +// RUN: -config="{CheckOptions: {bugprone-misplaced-widening-cast.CheckImplicitCasts: true}}" \ +// RUN: -- -target i386-unknown-unknown + +// Tests rely on specific type sizes: +// unsigned int = 32, unsigned short = 16, unsigned char = 8, +// unsigned long = 64, unsigned long long = 64 bits. + +struct BitfieldHeader { + unsigned long long field32 : 32; + unsigned long field16 : 16; + unsigned int field8 : 8; + unsigned long long field40 : 40; + unsigned long long field24 : 24; + long long sfield32 : 32; + long sfield16 : 16; +}; + +// 32-bit bitfield from unsigned int (32-bit) — no widening. +void bitfield32_shift(unsigned int size) { + struct BitfieldHeader h = {}; + h.field32 = size << 1U; +} + +void bitfield32_multiply(unsigned int size) { + struct BitfieldHeader h = {}; + h.field32 = size * 2U; +} + +void bitfield32_add(unsigned int size) { + struct BitfieldHeader h = {}; + h.field32 = size + 1U; +} + +void bitfield32_subtract(unsigned int size) { + struct BitfieldHeader h = {}; + h.field32 = size - 1U; +} + +void bitfield32_not(unsigned int size) { + struct BitfieldHeader h = {}; + h.field32 = ~size; +} + +// 16-bit bitfield from unsigned short (16-bit) — no widening. +// Note: integer promotion makes CalcType 'int' (32-bit), but bitfield is 16-bit, +// so this is truncation, not widening. No warning expected. +void bitfield16_shift(unsigned short size) { + struct BitfieldHeader h = {}; + h.field16 = size << 1; +} + +void bitfield16_multiply(unsigned short size) { + struct BitfieldHeader h = {}; + h.field16 = size * 2; +} + +void bitfield16_add(unsigned short size) { + struct BitfieldHeader h = {}; + h.field16 = size + 1; +} + +void bitfield16_subtract(unsigned short size) { + struct BitfieldHeader h = {}; + h.field16 = size - 1; +} + +void bitfield16_not(unsigned short size) { + struct BitfieldHeader h = {}; + h.field16 = ~size; +} + +// 8-bit bitfield from unsigned char (8-bit) — no widening. +// Same: integer promotion makes CalcType 'int' (32-bit), bitfield is 8-bit = truncation. +void bitfield8_shift(unsigned char size) { + struct BitfieldHeader h = {}; + h.field8 = size << 1; +} + +void bitfield8_multiply(unsigned char size) { + struct BitfieldHeader h = {}; + h.field8 = size * 2; +} + +void bitfield8_add(unsigned char size) { + struct BitfieldHeader h = {}; + h.field8 = size + 1; +} + +void bitfield8_subtract(unsigned char size) { + struct BitfieldHeader h = {}; + h.field8 = size - 1; +} + +void bitfield8_not(unsigned char size) { + struct BitfieldHeader h = {}; + h.field8 = ~size; +} + +// 40-bit bitfield from unsigned int (32-bit) — widening DOES occur. Should warn. +void bitfield40_shift(unsigned int size) { + struct BitfieldHeader h = {}; + h.field40 = size << 1U; + // CHECK-MESSAGES: :[[@LINE-1]]:15: warning: either cast from 'unsigned int' to 'unsigned long long' +} + +void bitfield40_multiply(unsigned int size) { + struct BitfieldHeader h = {}; + h.field40 = size * 2U; + // CHECK-MESSAGES: :[[@LINE-1]]:15: warning: either cast from 'unsigned int' to 'unsigned long long' +} + +void bitfield40_add(unsigned int size) { + struct BitfieldHeader h = {}; + h.field40 = size + 1U; + // CHECK-MESSAGES: :[[@LINE-1]]:15: warning: either cast from 'unsigned int' to 'unsigned long long' +} + +void bitfield40_subtract(unsigned int size) { + struct BitfieldHeader h = {}; + h.field40 = size - 1U; + // FIXME: checker doesn't detect potential widening for subtraction. + // E.g. if size==0, result is 0xFFFFFFFF (32-bit), but in 40-bit space + // it should be 0xFFFFFFFFFF. Limitation of getMaxCalculationWidth. +} + +void bitfield40_not(unsigned int size) { + struct BitfieldHeader h = {}; + h.field40 = ~size; + // CHECK-MESSAGES: :[[@LINE-1]]:15: warning: either cast from 'unsigned int' to 'unsigned long long' +} + +// 24-bit bitfield from unsigned short (16-bit) — after integer promotion, +// CalcType is 'int' (32-bit) which is wider than the 24-bit bitfield. +// This is truncation, not widening. No warning expected. +void bitfield24_shift(unsigned short size) { + struct BitfieldHeader h = {}; + h.field24 = size << 1; +} + +void bitfield24_multiply(unsigned short size) { + struct BitfieldHeader h = {}; + h.field24 = size * 2; +} + +void bitfield24_add(unsigned short size) { + struct BitfieldHeader h = {}; + h.field24 = size + 1; +} + +void bitfield24_subtract(unsigned short size) { + struct BitfieldHeader h = {}; + h.field24 = size - 1; +} + +void bitfield24_not(unsigned short size) { + struct BitfieldHeader h = {}; + h.field24 = ~size; +} + +// Bitfield assigned to a normal (non-bitfield) variable. +// h.field8 has declared type 'unsigned int' (32-bit), so h.field8 << 1 is 'unsigned int'. +// Assigning to 'long' (64-bit) is widening. +void bitfield_to_normal_widen(struct BitfieldHeader h) { + long l; + l = h.field8 << 1; + // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: either cast from 'int' to 'long' +} + +void bitfield_to_normal_no_warn(struct BitfieldHeader h) { + unsigned int i; + i = h.field8 << 1; +} + +// Bitfields of different sizes assigned to each other. +void bitfield_small_to_large(struct BitfieldHeader h) { + struct BitfieldHeader h2 = {}; + h2.field40 = h.field8 << 1; + // CHECK-MESSAGES: :[[@LINE-1]]:16: warning: either cast from 'int' to 'unsigned long long' +} + +void bitfield_same_size(struct BitfieldHeader h) { + struct BitfieldHeader h2 = {}; + h2.field32 = h.field32 << 1; +} + +void bitfield_large_to_small(struct BitfieldHeader h) { + struct BitfieldHeader h2 = {}; + h2.field8 = h.field32 << 1; +} + +// Signed bitfield tests: no false positive when bitfield width == source type width. +// int is 32 bits on x86, sfield32 is 32-bit signed — no widening. +void sbitfield32_shift(int size) { + struct BitfieldHeader h = {}; + h.sfield32 = size << 1; +} + +void sbitfield32_multiply(int size) { + struct BitfieldHeader h = {}; + h.sfield32 = size * 2; +} + +void sbitfield32_add(int size) { + struct BitfieldHeader h = {}; + h.sfield32 = size + 1; +} + +// short promotes to int (32-bit), sfield16 is 16-bit — truncation, no warning. +void sbitfield16_shift(short size) { + struct BitfieldHeader h = {}; + h.sfield16 = size << 1; +} + +void sbitfield16_multiply(short size) { + struct BitfieldHeader h = {}; + h.sfield16 = size * 2; +} + +void sbitfield16_add(short size) { + struct BitfieldHeader h = {}; + h.sfield16 = size + 1; +} + +// FIXME: Subtraction with short: short promotes to int (32-bit), assigning to long long +// (64-bit) is widening. Checker doesn't warn for '-' (limitation of getMaxCalculationWidth). +// E.g. if size==0, (short)0 - 1 = -1 (0xFFFFFFFF as int), but in 64-bit +// it should be 0xFFFFFFFFFFFFFFFF. +void subtract_short_widen(short size) { + long long l; + l = size - 1; +} _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
