ClockMan created this revision.
Herald added subscribers: carlosgalvezp, xazax.hun.
Herald added a reviewer: njames93.
Herald added a project: All.
ClockMan published this revision for review.
ClockMan added a comment.
Herald added a project: clang-tools-extra.
Herald added a subscriber: cfe-commits.
Ready for review.
Implemented support for bit-field members as a loop variable
or upper limit. Supporting also non bit-field integer members.
Fixes issues: https://github.com/llvm/llvm-project/issues/58614
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D142587
Files:
clang-tools-extra/clang-tidy/bugprone/TooSmallLoopVariableCheck.cpp
clang-tools-extra/docs/ReleaseNotes.rst
clang-tools-extra/test/clang-tidy/checkers/bugprone/too-small-loop-variable.cpp
Index: clang-tools-extra/test/clang-tidy/checkers/bugprone/too-small-loop-variable.cpp
===================================================================
--- clang-tools-extra/test/clang-tidy/checkers/bugprone/too-small-loop-variable.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/bugprone/too-small-loop-variable.cpp
@@ -46,6 +46,16 @@
}
}
+void voidBadForLoop7() {
+ struct Int {
+ int value;
+ } i;
+
+ for (i.value = 0; i.value < size(); ++i.value) {
+ // CHECK-MESSAGES: :[[@LINE-1]]:21: warning: loop variable has narrower type 'int' than iteration's upper bound 'long' [bugprone-too-small-loop-variable]
+ }
+}
+
void voidForLoopUnsignedBound() {
unsigned size = 3147483647;
for (int i = 0; i < size; ++i) {
@@ -253,3 +263,113 @@
for (short i = 0; i < size; ++i) { // no warning
}
}
+
+// Should detect proper size of upper bound bitfield
+void voidForLoopWithBitfieldOnUpperBound() {
+ struct StructWithBitField {
+ unsigned bitfield : 5;
+ } value = {};
+
+ for(unsigned char i = 0U; i < value.bitfield; ++i) { // no warning
+ }
+}
+
+// Should detect proper size of loop variable bitfield
+void voidForLoopWithBitfieldOnLoopVar() {
+ struct StructWithBitField {
+ unsigned bitfield : 9;
+ } value = {};
+
+ unsigned char upperLimit = 100U;
+
+ for(value.bitfield = 0U; value.bitfield < upperLimit; ++value.bitfield) {
+ }
+}
+
+// Should detect proper size of loop variable and upper bound
+void voidForLoopWithBitfieldOnLoopVarAndUpperBound() {
+ struct StructWithBitField {
+ unsigned var : 5, limit : 4;
+ } value = {};
+
+ for(value.var = 0U; value.var < value.limit; ++value.var) {
+ }
+}
+
+// Should detect proper size of loop variable and upper bound on integers
+void voidForLoopWithBitfieldOnLoopVarAndUpperBoundOnInt() {
+ struct StructWithBitField {
+ unsigned var : 5;
+ int limit : 6;
+ } value = {};
+
+ for(value.var = 0U; value.var < value.limit; ++value.var) {
+ }
+}
+
+void badForLoopWithBitfieldOnUpperBound() {
+ struct StructWithBitField {
+ unsigned bitfield : 9;
+ } value = {};
+
+ for(unsigned char i = 0U; i < value.bitfield; ++i) {
+ // CHECK-MESSAGES: :[[@LINE-1]]:29: warning: loop variable has narrower type 'unsigned char' than iteration's upper bound 'unsigned int:9' [bugprone-too-small-loop-variable]
+ }
+}
+
+void badForLoopWithBitfieldOnLoopVar() {
+ struct StructWithBitField {
+ unsigned bitfield : 7;
+ } value = {};
+
+ unsigned char upperLimit = 100U;
+
+ for(value.bitfield = 0U; value.bitfield < upperLimit; ++value.bitfield) {
+ // CHECK-MESSAGES: :[[@LINE-1]]:28: warning: loop variable has narrower type 'unsigned int:7' than iteration's upper bound 'unsigned char' [bugprone-too-small-loop-variable]
+ }
+}
+
+void badForLoopWithBitfieldOnLoopVarAndUpperBound() {
+ struct StructWithBitField {
+ unsigned var : 5, limit : 6;
+ } value = {};
+
+ for(value.var = 0U; value.var < value.limit; ++value.var) {
+ // CHECK-MESSAGES: :[[@LINE-1]]:23: warning: loop variable has narrower type 'unsigned int:5' than iteration's upper bound 'unsigned int:6' [bugprone-too-small-loop-variable]
+ }
+}
+
+void badForLoopWithBitfieldOnLoopVarOnIntAndUpperBound() {
+ struct StructWithBitField {
+ int var : 5;
+ unsigned limit : 5;
+ } value = {};
+
+ for(value.var = 0U; value.var < value.limit; ++value.var) {
+ // CHECK-MESSAGES: :[[@LINE-1]]:23: warning: loop variable has narrower type 'int:5' than iteration's upper bound 'unsigned int:5' [bugprone-too-small-loop-variable]
+ }
+}
+
+void badForLoopWithBitfieldOnLoopVarAndUpperBoundOnInt() {
+ struct StructWithBitField {
+ unsigned var : 5;
+ int limit : 7;
+ } value = {};
+
+ for(value.var = 0U; value.var < value.limit; ++value.var) {
+ // CHECK-MESSAGES: :[[@LINE-1]]:23: warning: loop variable has narrower type 'unsigned int:5' than iteration's upper bound 'int:7' [bugprone-too-small-loop-variable]
+ }
+}
+
+void badForLoopWithBitfieldOnLoopVarAndUpperBoundOnPtr() {
+ struct StructWithBitField {
+ unsigned var : 5, limit : 6;
+ } value = {};
+
+ StructWithBitField* ptr = &value;
+
+ for(ptr->var = 0U; ptr->var < ptr->limit; ++ptr->var) {
+ // CHECK-MESSAGES: :[[@LINE-1]]:22: warning: loop variable has narrower type 'unsigned int:5' than iteration's upper bound 'unsigned int:6' [bugprone-too-small-loop-variable]
+ }
+}
+
Index: clang-tools-extra/docs/ReleaseNotes.rst
===================================================================
--- clang-tools-extra/docs/ReleaseNotes.rst
+++ clang-tools-extra/docs/ReleaseNotes.rst
@@ -103,6 +103,10 @@
Changes in existing checks
^^^^^^^^^^^^^^^^^^^^^^^^^^
+- Improved :doc:`bugprone-too-small-loop-variable
+ <clang-tidy/checks/bugprone/too-small-loop-variable>` check. Basic support
+ for bit-field and integer members as a loop variable or upper limit were added.
+
Removed checks
^^^^^^^^^^^^^^
Index: clang-tools-extra/clang-tidy/bugprone/TooSmallLoopVariableCheck.cpp
===================================================================
--- clang-tools-extra/clang-tidy/bugprone/TooSmallLoopVariableCheck.cpp
+++ clang-tools-extra/clang-tidy/bugprone/TooSmallLoopVariableCheck.cpp
@@ -9,6 +9,7 @@
#include "TooSmallLoopVariableCheck.h"
#include "clang/AST/ASTContext.h"
#include "clang/ASTMatchers/ASTMatchFinder.h"
+#include <utility>
using namespace clang::ast_matchers;
@@ -50,8 +51,9 @@
///
void TooSmallLoopVariableCheck::registerMatchers(MatchFinder *Finder) {
StatementMatcher LoopVarMatcher =
- expr(
- ignoringParenImpCasts(declRefExpr(to(varDecl(hasType(isInteger()))))))
+ expr(ignoringParenImpCasts(
+ anyOf(declRefExpr(to(varDecl(hasType(isInteger())))),
+ memberExpr(member(fieldDecl(hasType(isInteger())))))))
.bind(LoopVarName);
// We need to catch only those comparisons which contain any integer cast.
@@ -93,20 +95,29 @@
}
/// Returns the magnitude bits of an integer type.
-static unsigned calcMagnitudeBits(const ASTContext &Context,
- const QualType &IntExprType) {
+static std::pair<unsigned, unsigned>
+calcMagnitudeBits(const ASTContext &Context, const QualType &IntExprType,
+ const Expr *IntExpr) {
assert(IntExprType->isIntegerType());
- return IntExprType->isUnsignedIntegerType()
- ? Context.getIntWidth(IntExprType)
- : Context.getIntWidth(IntExprType) - 1;
+ unsigned IntWidth = 0U;
+ unsigned BitFieldWidth = 0U;
+
+ if (const auto *BitField = IntExpr->getSourceBitField())
+ BitFieldWidth = BitField->getBitWidthValue(Context);
+ else
+ IntWidth = Context.getIntWidth(IntExprType);
+
+ return {std::max(BitFieldWidth, IntWidth) -
+ (IntExprType->isUnsignedIntegerType() ? 0U : 1U),
+ BitFieldWidth};
}
/// Calculate the upper bound expression's magnitude bits, but ignore
/// constant like values to reduce false positives.
-static unsigned calcUpperBoundMagnitudeBits(const ASTContext &Context,
- const Expr *UpperBound,
- const QualType &UpperBoundType) {
+static std::pair<unsigned, unsigned>
+calcUpperBoundMagnitudeBits(const ASTContext &Context, const Expr *UpperBound,
+ const QualType &UpperBoundType) {
// Ignore casting caused by constant values inside a binary operator.
// We are interested in variable values' magnitude bits.
if (const auto *BinOperator = dyn_cast<BinaryOperator>(UpperBound)) {
@@ -117,7 +128,7 @@
QualType LHSEType = LHSE->getType();
if (!RHSEType->isIntegerType() || !LHSEType->isIntegerType())
- return 0;
+ return {};
bool RHSEIsConstantValue = RHSEType->isEnumeralType() ||
RHSEType.isConstQualified() ||
@@ -128,17 +139,17 @@
// Avoid false positives produced by two constant values.
if (RHSEIsConstantValue && LHSEIsConstantValue)
- return 0;
+ return {};
if (RHSEIsConstantValue)
- return calcMagnitudeBits(Context, LHSEType);
+ return calcMagnitudeBits(Context, LHSEType, LHSE);
if (LHSEIsConstantValue)
- return calcMagnitudeBits(Context, RHSEType);
+ return calcMagnitudeBits(Context, RHSEType, RHSE);
- return std::max(calcMagnitudeBits(Context, LHSEType),
- calcMagnitudeBits(Context, RHSEType));
+ return std::max(calcMagnitudeBits(Context, LHSEType, LHSE),
+ calcMagnitudeBits(Context, RHSEType, RHSE));
}
- return calcMagnitudeBits(Context, UpperBoundType);
+ return calcMagnitudeBits(Context, UpperBoundType, UpperBound);
}
void TooSmallLoopVariableCheck::check(const MatchFinder::MatchResult &Result) {
@@ -157,20 +168,38 @@
ASTContext &Context = *Result.Context;
- unsigned LoopVarMagnitudeBits = calcMagnitudeBits(Context, LoopVarType);
- unsigned UpperBoundMagnitudeBits =
+ std::pair<unsigned, unsigned> LoopVarMagnitudeBits =
+ calcMagnitudeBits(Context, LoopVarType, LoopVar);
+ std::pair<unsigned, unsigned> UpperBoundMagnitudeBits =
calcUpperBoundMagnitudeBits(Context, UpperBound, UpperBoundType);
- if (UpperBoundMagnitudeBits == 0)
+ if (UpperBoundMagnitudeBits.first == 0)
+ return;
+
+ if (LoopVarMagnitudeBits.first > MagnitudeBitsUpperLimit)
return;
- if (LoopVarMagnitudeBits > MagnitudeBitsUpperLimit)
+ if (LoopVarMagnitudeBits.first >= UpperBoundMagnitudeBits.first)
return;
- if (LoopVarMagnitudeBits < UpperBoundMagnitudeBits)
- diag(LoopVar->getBeginLoc(), "loop variable has narrower type %0 than "
- "iteration's upper bound %1")
- << LoopVarType << UpperBoundType;
+ auto Msg =
+ diag(LoopVar->getBeginLoc(), "loop variable has narrower type '%0%1%2' "
+ "than iteration's upper bound '%3%4%5'");
+ Msg << LoopVarType.getAsString();
+ if (LoopVarMagnitudeBits.second) {
+ Msg << ":" << LoopVarMagnitudeBits.second;
+ } else {
+ Msg << ""
+ << "";
+ }
+
+ Msg << UpperBoundType.getAsString();
+ if (UpperBoundMagnitudeBits.second) {
+ Msg << ":" << UpperBoundMagnitudeBits.second;
+ } else {
+ Msg << ""
+ << "";
+ }
}
} // namespace clang::tidy::bugprone
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
