1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
66
67
68
69
70
71
72
73
74
75
76
77
78
79
80
81
82
83
84
85
86
87
88
89
90
91
92
93
94
95
96
97
98
99
100
101
102
103
104
105
106
107
108
109
110
111
112
113
114
115
116
117
118
119
120
121
122
123
124
125
126
127
128
129
130
131
132
133
134
135
136
137
138
139
140
141
142
143
144
145
146
147
148
149
150
151
152
153
154
155
156
157
158
159
160
161
162
163
164
165
166
167
168
169
170
171
172
173
174
175
176
177
178
179
180
181
182
183
184
185
186
187
188
189
190
191
192
193
194
195
196
197
198
199
200
201
202
203
204
205
206
207
208
209
210
211
212
213
214
215
216
217
218
219
220
221
222
223
224
225
226
227
228
229
230
231
|
/* -*- 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/.
*/
// Find explicit casts from signed to unsigned integer in comparison against unsigned integer, where
// the cast is presumably used to avoid warnings about signed vs. unsigned comparisons, and could
// thus be replaced with o3tl::make_unsigned for clarity.
#include <cassert>
#include "config_clang.h"
#include "compat.hxx"
#include "plugin.hxx"
namespace
{
// clang::Type::isSignedIntegerType returns true for more types than what C++ defines as signed
// integer types:
bool isSignedIntegerType(QualType type)
{
if (auto const t = type->getAs<BuiltinType>())
{
// Assumes that the only extended signed integer type supported by Clang is Int128:
switch (t->getKind())
{
case BuiltinType::SChar:
case BuiltinType::Short:
case BuiltinType::Int:
case BuiltinType::Long:
case BuiltinType::LongLong:
case BuiltinType::Int128:
return true;
default:
break;
}
}
return false;
}
// clang::Type::isUnsignedIntegerType returns true for more types than what C++ defines as signed
// integer types:
bool isUnsignedIntegerType(QualType type)
{
if (auto const t = type->getAs<BuiltinType>())
{
// Assumes that the only extended unsigned integer type supported by Clang is UInt128:
switch (t->getKind())
{
case BuiltinType::UChar:
case BuiltinType::UShort:
case BuiltinType::UInt:
case BuiltinType::ULong:
case BuiltinType::ULongLong:
case BuiltinType::UInt128:
return true;
default:
break;
}
}
return false;
}
int getRank(QualType type)
{
auto const t = type->getAs<BuiltinType>();
assert(t != nullptr);
// Assumes that the only extended signed/unsigned integer types supported by Clang are Int128
// and UInt128:
switch (t->getKind())
{
case BuiltinType::SChar:
case BuiltinType::UChar:
return 0;
case BuiltinType::Short:
case BuiltinType::UShort:
return 1;
case BuiltinType::Int:
case BuiltinType::UInt:
return 2;
case BuiltinType::Long:
case BuiltinType::ULong:
return 3;
case BuiltinType::LongLong:
case BuiltinType::ULongLong:
return 4;
case BuiltinType::Int128:
case BuiltinType::UInt128:
return 5;
default:
llvm_unreachable("bad integer type");
}
}
int orderTypes(QualType type1, QualType type2)
{
auto const r1 = getRank(type1);
auto const r2 = getRank(type2);
return r1 < r2 ? -1 : r1 == r2 ? 0 : 1;
}
class UnsignedCompare : public loplugin::FilteringPlugin<UnsignedCompare>
{
public:
explicit UnsignedCompare(loplugin::InstantiationData const& data)
: FilteringPlugin(data)
{
}
bool VisitBinaryOperator(BinaryOperator const* expr)
{
if (ignoreLocation(expr))
{
return true;
}
// o3tl::make_unsigned requires its argument to be non-negative, but this plugin doesn't
// check that when it reports its finding, so will produce false positives when the cast is
// actually meant to e.g. clamp from a large signed type to a small unsigned type. The
// assumption is that this will only be likely the case for BO_EQ (==) and BO_NE (!=)
// comparisons, so filter these out here (not sure what case BO_Cmp (<=>) will turn out to
// be, so lets keep it here at least for now):
switch (expr->getOpcode())
{
#if CLANG_VERSION >= 60000
case BO_Cmp:
#endif
case BO_LT:
case BO_GT:
case BO_LE:
case BO_GE:
break;
default:
return true;
}
auto const castL = isCastToUnsigned(expr->getLHS());
auto const castR = isCastToUnsigned(expr->getRHS());
//TODO(?): Also report somewhat suspicious cases where both sides are cast to unsigned:
if ((castL == nullptr) == (castR == nullptr))
{
return true;
}
auto const cast = castL != nullptr ? castL : castR;
auto const other = castL != nullptr ? expr->getRHS() : expr->getLHS();
auto const otherT = other->IgnoreImpCasts()->getType();
if (!isUnsignedIntegerType(otherT))
{
return true;
}
auto const castFromT = cast->getSubExprAsWritten()->getType();
auto const castToT = cast->getTypeAsWritten();
report(DiagnosticsEngine::Warning,
"explicit cast from %0 to %1 (of %select{smaller|equal|larger}2 rank) in comparison "
"against %3: if the cast value is known to be non-negative, use o3tl::make_unsigned "
"instead of the cast",
cast->getExprLoc())
<< castFromT << castToT << (orderTypes(castToT, castFromT) + 1) << otherT
<< expr->getSourceRange();
return true;
}
private:
bool preRun() override
{
return compiler.getLangOpts().CPlusPlus
&& compiler.getPreprocessor()
.getIdentifierInfo("LIBO_INTERNAL_ONLY")
->hasMacroDefinition();
}
void run() override
{
if (preRun())
{
TraverseDecl(compiler.getASTContext().getTranslationUnitDecl());
}
}
ExplicitCastExpr const* isCastToUnsigned(Expr const* expr)
{
auto const e = dyn_cast<ExplicitCastExpr>(expr->IgnoreParenImpCasts());
if (e == nullptr)
{
return nullptr;
}
auto const t1 = e->getTypeAsWritten();
if (!isUnsignedIntegerType(t1))
{
return nullptr;
}
auto const e2 = e->getSubExprAsWritten();
auto const t2 = e2->getType();
if (!isSignedIntegerType(t2))
{
return nullptr;
}
// Filter out e.g. `size_t(-1)`:
APSInt val;
if (!e2->isValueDependent() && e2->isIntegerConstantExpr(val, compiler.getASTContext()))
{
if (val.isNegative())
{
return nullptr;
}
}
auto loc = compat::getBeginLoc(e);
while (compiler.getSourceManager().isMacroArgExpansion(loc))
{
loc = compiler.getSourceManager().getImmediateMacroCallerLoc(loc);
}
// This covers both "plain" code in such include files, as well as expansion of (object-like) macros like
//
// #define SAL_MAX_INT8 ((sal_Int8) 0x7F)
//
// defined in such include files:
if (isInUnoIncludeFile(compiler.getSourceManager().getSpellingLoc(loc)))
{ //TODO: '#ifdef LIBO_INTERNAL_ONLY' within UNO include files
return nullptr;
}
return e;
}
};
loplugin::Plugin::Registration<UnsignedCompare> unsignedcompare("unsignedcompare");
}
/* vim:set shiftwidth=4 softtabstop=4 expandtab cinoptions=b1,g0,N-s cinkeys+=0=break: */
|
