summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
Diffstat
-rw-r--r--compilerplugins/clang/loopvartoosmall.cxx239
-rw-r--r--compilerplugins/clang/test/loopvartoosmall.cxx24
-rw-r--r--solenv/CompilerTest_compilerplugins_clang.mk1
-rw-r--r--svtools/source/brwbox/brwbox2.cxx4
-rw-r--r--svx/source/fmcomp/gridctrl.cxx2
-rw-r--r--sw/source/core/frmedt/fetab.cxx2
-rw-r--r--sw/source/core/text/itrpaint.cxx2
-rw-r--r--sw/source/filter/ww8/ww8par6.cxx2
-rw-r--r--sw/source/filter/ww8/ww8scan.cxx2
9 files changed, 207 insertions, 71 deletions
diff --git a/compilerplugins/clang/loopvartoosmall.cxx b/compilerplugins/clang/loopvartoosmall.cxx
index aaa664827298..eb4cb96d592d 100644
--- a/compilerplugins/clang/loopvartoosmall.cxx
+++ b/compilerplugins/clang/loopvartoosmall.cxx
@@ -7,11 +7,13 @@
* file, You can obtain one at http://mozilla.org/MPL/2.0/.
*/
-#include <string>
-#include <iostream>
+#include <algorithm>
+#include <cassert>
+#include <list>
+#include <map>
#include "plugin.hxx"
-#include "clang/AST/CXXInheritance.h"
+//#include "clang/AST/CXXInheritance.h"
// Idea from bubli. Check that the index variable in a for loop is able to cover the range
// revealed by the terminating condition.
@@ -30,86 +32,195 @@ public:
TraverseDecl(compiler.getASTContext().getTranslationUnitDecl());
}
- bool VisitForStmt( const ForStmt* stmt );
+ bool VisitForStmt( const ForStmt* stmt ) {
+ checkExpr(stmt->getCond());
+ return true;
+ }
+
+ bool VisitWhileStmt(WhileStmt const * stmt) {
+ checkExpr(stmt->getCond());
+ return true;
+ }
+
+ bool VisitDoStmt(DoStmt const * stmt) {
+ checkExpr(stmt->getCond());
+ return true;
+ }
private:
- StringRef getFilename(SourceLocation loc);
+ unsigned getIntValueWidth(QualType type) const;
+
+ void checkSubExpr(Expr const * expr, bool positive);
+
+ void checkExpr(Expr const * expr);
+
+ struct Comparison {
+ BinaryOperator const * op;
+ unsigned rhsWidth;
+ };
+ struct Comparisons {
+ std::list<Comparison> comparisons;
+ unsigned lhsWidth;
+ };
+
+ std::map<Decl const *, Comparisons> comparisons_;
};
-StringRef LoopVarTooSmall::getFilename(SourceLocation loc)
-{
- SourceLocation spellingLocation = compiler.getSourceManager().getSpellingLoc(loc);
- StringRef name { compiler.getSourceManager().getFilename(spellingLocation) };
- return name;
+unsigned LoopVarTooSmall::getIntValueWidth(QualType type) const {
+ if (auto const et = type->getAs<EnumType>()) {
+ auto const ed = et->getDecl();
+ if (!ed->isFixed()) {
+ unsigned pos = ed->getNumPositiveBits();
+ unsigned neg = ed->getNumNegativeBits();
+ return neg == 0 ? std::max(pos, 1U) : std::max(pos + 1, neg);
+ }
+ }
+ return compiler.getASTContext().getIntWidth(type);
}
-bool LoopVarTooSmall::VisitForStmt( const ForStmt* stmt )
-{
- if (ignoreLocation( stmt ))
- return true;
- // ignore sal/ module for now
- StringRef aFileName = getFilename(stmt->getLocStart());
- if (aFileName.startswith(SRCDIR "/sal/")) {
- return true;
+void LoopVarTooSmall::checkSubExpr(Expr const * expr, bool positive) {
+ auto const e = expr->IgnoreParenImpCasts();
+ if (auto const uo = dyn_cast<UnaryOperator>(e)) {
+ if (uo->getOpcode() == UO_LNot) {
+ checkSubExpr(uo->getSubExpr(), !positive);
+ }
+ return;
}
-
- const Stmt* initStmt = stmt->getInit();
- if (!initStmt || !isa<DeclStmt>(initStmt))
- return true;
- const DeclStmt* declStmt = dyn_cast<DeclStmt>(initStmt);
- if (!declStmt->getDeclGroup().isSingleDecl())
- return true;
- const Decl* decl = declStmt->getSingleDecl();
- if (!decl || !isa<VarDecl>(decl))
- return true;
- const VarDecl* varDecl = dyn_cast<VarDecl>(decl);
- QualType qt = varDecl->getType();
- if (!qt->isIntegralType(compiler.getASTContext()))
- return true;
- uint64_t qt1BitWidth = compiler.getASTContext().getTypeSize(qt);
-
- if (!stmt->getCond() || !isa<BinaryOperator>(stmt->getCond()))
- return true;
- const BinaryOperator* binOp = dyn_cast<BinaryOperator>(stmt->getCond());
- if (binOp->getOpcode() != BO_LT && binOp->getOpcode() != BO_LE)
- return true;
- const Expr* binOpRHS = binOp->getRHS();
- // ignore complex expressions for now, promotion rules on conditions like "i < (size()+1)"
- // make it hard to guess at a correct type.
- if (isa<BinaryOperator>(binOpRHS) || isa<ParenExpr>(binOpRHS))
- return true;
- if (isa<ImplicitCastExpr>(binOpRHS)) {
- const ImplicitCastExpr* castExpr = dyn_cast<ImplicitCastExpr>(binOpRHS);
- binOpRHS = castExpr->getSubExpr();
+ const BinaryOperator* binOp = dyn_cast<BinaryOperator>(e);
+ if (!binOp)
+ return;
+ bool less;
+ if (positive) {
+ switch (binOp->getOpcode()) {
+ case BO_LAnd:
+ checkSubExpr(binOp->getLHS(), true);
+ checkSubExpr(binOp->getRHS(), true);
+ return;
+ case BO_LT:
+ case BO_NE:
+ less = true;
+ break;
+ case BO_LE:
+ less = false;
+ break;
+ default:
+ return;
+ }
+ } else {
+ switch (binOp->getOpcode()) {
+ case BO_LOr:
+ checkSubExpr(binOp->getLHS(), false);
+ checkSubExpr(binOp->getRHS(), false);
+ return;
+ case BO_GE:
+ case BO_EQ:
+ less = true;
+ break;
+ case BO_GT:
+ less = false;
+ break;
+ default:
+ return;
+ }
+ }
+ auto lhs = dyn_cast<DeclRefExpr>(binOp->getLHS()->IgnoreParenImpCasts());
+ if (!lhs)
+ return;
+ QualType qt = lhs->getType();
+ if (!qt->isIntegralOrEnumerationType())
+ return;
+ unsigned qt1BitWidth = getIntValueWidth(qt);
+ auto lhsDecl = lhs->getDecl();
+ if (!isa<VarDecl>(lhsDecl)) {
+ if (auto fd = dyn_cast<FieldDecl>(lhsDecl)) {
+ if (fd->isBitField()) {
+ qt1BitWidth = std::max(
+ qt1BitWidth,
+ fd->getBitWidthValue(compiler.getASTContext()));
+ }
+ } else {
+ return;
+ }
}
+ const Expr* binOpRHS = binOp->getRHS()->IgnoreParenImpCasts();
QualType qt2 = binOpRHS->getType();
if (!qt2->isIntegralType(compiler.getASTContext()))
- return true;
- // check for comparison with constants where the compiler just tends to give me the type as "int"
+ return;
+ unsigned qt2BitWidth;
llvm::APSInt aIntResult;
- uint64_t qt2BitWidth = compiler.getASTContext().getTypeSize(qt2);
if (binOpRHS->EvaluateAsInt(aIntResult, compiler.getASTContext())) {
- if (aIntResult.getSExtValue() > 0 && aIntResult.getSExtValue() < 1<<7) {
- qt2BitWidth = 8;
- } else if (aIntResult.getSExtValue() > 0 && aIntResult.getSExtValue() < 1<<15) {
- qt2BitWidth = 16;
- } else if (aIntResult.getSExtValue() > 0 && aIntResult.getSExtValue() < 1L<<31) {
- qt2BitWidth = 32;
+ if (less && aIntResult.isStrictlyPositive()) {
+ --aIntResult;
+ }
+ qt2BitWidth = aIntResult.isUnsigned() || !aIntResult.isNegative()
+ ? std::max(aIntResult.getActiveBits(), 1U)
+ : aIntResult.getBitWidth() - aIntResult.countLeadingOnes() + 1;
+ } else {
+ // Ignore complex expressions for now, promotion rules on conditions
+ // like "i < (size()+1)" make it hard to guess at a correct type:
+ if (isa<BinaryOperator>(binOpRHS) || isa<ConditionalOperator>(binOpRHS))
+ {
+ return;
+ }
+ qt2BitWidth = getIntValueWidth(qt2);
+ if (auto dre = dyn_cast<DeclRefExpr>(binOpRHS)) {
+ if (auto fd = dyn_cast<FieldDecl>(dre->getDecl())) {
+ if (fd->isBitField()) {
+ qt2BitWidth = std::max(
+ qt2BitWidth,
+ fd->getBitWidthValue(compiler.getASTContext()));
+ }
+ }
}
}
- if (qt1BitWidth < qt2BitWidth) {
- report(
- DiagnosticsEngine::Warning,
- "loop index type %0 is narrower than length type %1",
- stmt->getInit()->getLocStart())
- << qt << qt2 << stmt->getInit()->getSourceRange();
- //stmt->getCond()->dump();
+ auto i = comparisons_.find(lhsDecl);
+ if (i == comparisons_.end()) {
+ i = (comparisons_.insert(
+ decltype(comparisons_)::value_type(lhsDecl, {{}, qt1BitWidth}))
+ .first);
+ } else {
+ assert(i->second.lhsWidth == qt1BitWidth);
+ }
+ bool ins = true;
+ for (auto j = i->second.comparisons.begin();
+ j != i->second.comparisons.end();)
+ {
+ if (qt2BitWidth > j->rhsWidth) {
+ ins = false;
+ break;
+ } else if (qt2BitWidth < j->rhsWidth) {
+ j = i->second.comparisons.erase(j);
+ } else {
+ ++j;
+ }
+ }
+ if (ins) {
+ i->second.comparisons.push_back({binOp, qt2BitWidth});
}
- return true;
}
+void LoopVarTooSmall::checkExpr(Expr const * expr) {
+ if (expr != nullptr && !ignoreLocation(expr)) {
+ assert(comparisons_.empty());
+ checkSubExpr(expr, true);
+ for (auto const & i: comparisons_) {
+ for (auto const & j: i.second.comparisons) {
+ if (i.second.lhsWidth < j.rhsWidth) {
+ report(
+ DiagnosticsEngine::Warning,
+ "loop index type %0 is narrower than length type %1",
+ j.op->getExprLoc())
+ << j.op->getLHS()->IgnoreImpCasts()->getType()
+ << j.op->getRHS()->IgnoreImpCasts()->getType()
+ << j.op->getSourceRange();
+ }
+ }
+ }
+ comparisons_.clear();
+ }
+}
loplugin::Plugin::Registration< LoopVarTooSmall > X("loopvartoosmall");
diff --git a/compilerplugins/clang/test/loopvartoosmall.cxx b/compilerplugins/clang/test/loopvartoosmall.cxx
new file mode 100644
index 000000000000..5cb5a23c289c
--- /dev/null
+++ b/compilerplugins/clang/test/loopvartoosmall.cxx
@@ -0,0 +1,24 @@
+/* -*- Mode: C++; tab-width: 4; indent-tabs-mode: nil; c-basic-offset: 4; fill-column: 100 -*- */
+/*
+ * This file is part of the LibreOffice project.
+ *
+ * This Source Code Form is subject to the terms of the Mozilla Public
+ * License, v. 2.0. If a copy of the MPL was not distributed with this
+ * file, You can obtain one at http://mozilla.org/MPL/2.0/.
+ */
+
+#include <cstdint>
+
+std::int32_t size() { return 1; }
+
+int main() {
+ for (std::int16_t i = 0; i < size(); ++i) {} // expected-error {{[loplugin:loopvartoosmall]}}
+ for (std::int16_t i = 0; i <= size(); ++i) {} // expected-error {{[loplugin:loopvartoosmall]}}
+ for (std::int16_t i = 0; i != size(); ++i) {} // expected-error {{[loplugin:loopvartoosmall]}}
+ std::int16_t j;
+ for (j = 0; j < size(); ++j) {} // expected-error {{[loplugin:loopvartoosmall]}}
+ for (j = 0; j <= size(); ++j) {} // expected-error {{[loplugin:loopvartoosmall]}}
+ for (j = 0; j != size(); ++j) {} // expected-error {{[loplugin:loopvartoosmall]}}
+}
+
+/* vim:set shiftwidth=4 softtabstop=4 expandtab cinoptions=b1,g0,N-s cinkeys+=0=break: */
diff --git a/solenv/CompilerTest_compilerplugins_clang.mk b/solenv/CompilerTest_compilerplugins_clang.mk
index 80508e8ce6a5..724a9ac07046 100644
--- a/solenv/CompilerTest_compilerplugins_clang.mk
+++ b/solenv/CompilerTest_compilerplugins_clang.mk
@@ -13,6 +13,7 @@ $(eval $(call gb_CompilerTest_add_exception_objects,compilerplugins_clang, \
compilerplugins/clang/test/datamembershadow \
compilerplugins/clang/test/externvar \
compilerplugins/clang/test/finalprotected \
+ compilerplugins/clang/test/loopvartoosmall \
compilerplugins/clang/test/passstuffbyref \
compilerplugins/clang/test/oslendian-1 \
compilerplugins/clang/test/oslendian-2 \
diff --git a/svtools/source/brwbox/brwbox2.cxx b/svtools/source/brwbox/brwbox2.cxx
index 76201c97fa37..5902077176c4 100644
--- a/svtools/source/brwbox/brwbox2.cxx
+++ b/svtools/source/brwbox/brwbox2.cxx
@@ -1271,12 +1271,12 @@ void BrowseBox::ColumnInserted( sal_uInt16 nPos )
sal_uInt16 BrowseBox::FrozenColCount() const
{
- sal_uInt16 nCol;
+ BrowserColumns::size_type nCol;
for ( nCol = 0;
nCol < pCols.size() && pCols[ nCol ]->IsFrozen();
++nCol )
/* empty loop */;
- return nCol;
+ return nCol; //TODO: BrowserColumns::size_type -> sal_uInt16!
}
diff --git a/svx/source/fmcomp/gridctrl.cxx b/svx/source/fmcomp/gridctrl.cxx
index db2ef8918926..e04a5499c314 100644
--- a/svx/source/fmcomp/gridctrl.cxx
+++ b/svx/source/fmcomp/gridctrl.cxx
@@ -1684,7 +1684,7 @@ sal_uInt16 DbGridControl::AppendColumn(const OUString& rName, sal_uInt16 nWidth,
}
// calculate the new id
- for (nId=1; (GetModelColumnPos(nId) != GRID_COLUMN_NOT_FOUND) && (nId <= m_aColumns.size()); ++nId)
+ for (nId=1; (GetModelColumnPos(nId) != GRID_COLUMN_NOT_FOUND) && sal::static_int_cast<DbGridColumns::size_type>(nId) <= m_aColumns.size(); ++nId)
;
DBG_ASSERT(GetViewColumnPos(nId) == GRID_COLUMN_NOT_FOUND, "DbGridControl::AppendColumn : inconsistent internal state !");
// my column's models say "there is no column with id nId", but the view (the base class) says "there is a column ..."
diff --git a/sw/source/core/frmedt/fetab.cxx b/sw/source/core/frmedt/fetab.cxx
index e9d927b9e84d..0cb6f864076a 100644
--- a/sw/source/core/frmedt/fetab.cxx
+++ b/sw/source/core/frmedt/fetab.cxx
@@ -1044,7 +1044,7 @@ static sal_uInt16 lcl_GetRowNumber( const SwPosition& rPos )
const SwTableLine* pTabLine = static_cast<const SwRowFrame*>(pRow)->GetTabLine();
sal_uInt16 nRet = USHRT_MAX;
sal_uInt16 nI = 0;
- while ( nI < pTabFrame->GetTable()->GetTabLines().size() )
+ while ( sal::static_int_cast<SwTableLines::size_type>(nI) < pTabFrame->GetTable()->GetTabLines().size() )
{
if ( pTabFrame->GetTable()->GetTabLines()[ nI ] == pTabLine )
{
diff --git a/sw/source/core/text/itrpaint.cxx b/sw/source/core/text/itrpaint.cxx
index 462ee043c8c1..117317b8047c 100644
--- a/sw/source/core/text/itrpaint.cxx
+++ b/sw/source/core/text/itrpaint.cxx
@@ -579,7 +579,7 @@ void SwTextPainter::CheckSpecialUnderline( const SwLinePortion* pPor,
sal_uInt16 nMaxBaseLineOfst = 0;
int nNumberOfPortions = 0;
- while( nTmpIdx <= nUnderEnd && pPor )
+ while( sal::static_int_cast<long>(nTmpIdx) <= nUnderEnd && pPor )
{
if ( pPor->IsFlyPortion() || pPor->IsFlyCntPortion() ||
pPor->IsBreakPortion() || pPor->IsMarginPortion() ||
diff --git a/sw/source/filter/ww8/ww8par6.cxx b/sw/source/filter/ww8/ww8par6.cxx
index 209ca6771078..f12707b2142b 100644
--- a/sw/source/filter/ww8/ww8par6.cxx
+++ b/sw/source/filter/ww8/ww8par6.cxx
@@ -1655,7 +1655,7 @@ void WW8FlyPara::ReadFull(sal_uInt8 nOrigSp29, SwWW8ImplReader* pIo)
WW8FlyPara *pNowStyleApo=nullptr;
sal_uInt16 nColl = pPap->GetIstd();
ww::sti eSti = eVer < ww::eWW6 ? ww::GetCanonicalStiFromStc( static_cast< sal_uInt8 >(nColl) ) : static_cast<ww::sti>(nColl);
- while (eSti != ww::stiNil && nColl < pIo->m_vColl.size() && nullptr == (pNowStyleApo = pIo->m_vColl[nColl].m_xWWFly.get()))
+ while (eSti != ww::stiNil && sal::static_int_cast<size_t>(nColl) < pIo->m_vColl.size() && nullptr == (pNowStyleApo = pIo->m_vColl[nColl].m_xWWFly.get()))
{
nColl = pIo->m_vColl[nColl].m_nBase;
eSti = eVer < ww::eWW6 ? ww::GetCanonicalStiFromStc( static_cast< sal_uInt8 >(nColl) ) : static_cast<ww::sti>(nColl);
diff --git a/sw/source/filter/ww8/ww8scan.cxx b/sw/source/filter/ww8/ww8scan.cxx
index ae3e44223ab8..3364acdf3f58 100644
--- a/sw/source/filter/ww8/ww8scan.cxx
+++ b/sw/source/filter/ww8/ww8scan.cxx
@@ -4123,7 +4123,7 @@ OUString WW8PLCFx_Book::GetBookmark(long nStart,long nEnd, sal_uInt16 &nIndex)
if (pBook[0] && pBook[1])
{
WW8_CP nStartAkt, nEndAkt;
- while (i < aBookNames.size())
+ while (sal::static_int_cast<decltype(aBookNames)::size_type>(i) < aBookNames.size())
{
void* p;
sal_uInt16 nEndIdx;
generated by cgit v1.2.3 (git 2.39.1) at 2024-05-22 00:36:50 +0000