50d73574b6
69e9925ded
"sdext.pdfimport: resolves tdf#104597: RTL script text runs are reversed" andf6004e1c45
"tdf#151546: RTL text is reversed (Writer pdfimport)" had introduced two calls to comphelper::string::reverseString into sdext. That function reverts on the basis of individual UTF-16 code units, not on the basis of Unicode code points. And while at least some pre-existing callers of that function want the former semantics (see below), these two new callers in sdext apparently want the latter semantics. Therefore, introduce an additional function comphelper::string::reverseCodePoints with the latter semantics. I identified three other places that call comphelper::string::reverseString: * SbRtl_StrReverse in basic/source/runtime/methods1.cxx apparently implements some StrReverse Basic function, where a (presumably non-existing) Basic spec would need to decide which of the two semantics is called for. So leave it alone for now. * SvtFileDialog::IsolateFilterFromPath_Impl in fpicker/source/office/iodlg.cxx reverts a string, operates on it, then reverts (parts of) it back. Whether or not that is the most elegant code, using the latter semantics here would apparently be wrong, as double invocation of comphelper::string::reverseCodePoints is not idempotent when the input is a malformed sequence of UTF-16 code units containing a low surrogate followed by a high surrogate. * AccessibleCell::getCellName in svx/source/table/accessiblecell.cxx apparently always operates on a string consisting only of Latin uppercase letters A--Z, for which both semantics are equivalent. (So we can just as well stick with the simpler comphelper::string::reverseString here.) (Extending the tests in comphelper/qa/string/test_string.cxx ran into an issue where loplugin:stringliteralvar warns about deliberate uses of sal_Unicode arrays rather than UTF-16 string literals wrapped in OUStringLiteral, as those arrays deliberately contain malformed UTF-16 code unit sequences and thus converting them into UTF-16 string literals might be considered inappropriate, see the newly added comment at StringLiteralVar::isPotentiallyInitializedWithMalformedUtf16 in compilerplugins/clang/stringliteralvar.cxx for details. So that loplugin had to be improved here, too.) Change-Id: I641cc32c76b0c5f6339ae44d8aa85df0022ffb05 Reviewed-on: https://gerrit.libreoffice.org/c/core/+/142949 Tested-by: Jenkins Reviewed-by: Stephan Bergmann <sbergman@redhat.com>
320 lines
11 KiB
C++
320 lines
11 KiB
C++
/* -*- 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 constant character array variables that are either
|
|
// (a) passed into O[U]String constructors
|
|
// (b) assigned to O[U]String
|
|
// and should thus be turned into O[U]StringLiteral variables.
|
|
//
|
|
// Such a variable may have been used in multiple places, not all of which would be compatible with
|
|
// changing the variable's type to O[U]StringLiteral. However, this plugin is aggressive and
|
|
// ignores all but the first use of such a variable. In all cases of incompatible uses so far, it
|
|
// was possible to change to surrounding code (for the better) to make the changes work.
|
|
//
|
|
// The plugin also flags O[U]StringLiteral variables of automatic storage duration, and uses of such
|
|
// variables with sizeof---two likely errors that can occur once a variable has been changed from a
|
|
// character array to O[U]StringLiteral.
|
|
//
|
|
//TODO: In theory, we should not only look for variables, but also for non-static data members. In
|
|
// practice, those should be rare, though, as they should arguably have been static data members to
|
|
// begin with.
|
|
|
|
#include <cassert>
|
|
|
|
#include "check.hxx"
|
|
#include "compat.hxx"
|
|
#include "plugin.hxx"
|
|
|
|
namespace
|
|
{
|
|
bool isAutomaticVariable(VarDecl const* decl)
|
|
{
|
|
switch (cast<VarDecl>(decl)->getStorageDuration())
|
|
{
|
|
case SD_Automatic:
|
|
return true;
|
|
case SD_Thread:
|
|
case SD_Static:
|
|
return false;
|
|
case SD_FullExpression:
|
|
case SD_Dynamic:
|
|
assert(false);
|
|
default:
|
|
llvm_unreachable("unknown StorageDuration");
|
|
}
|
|
}
|
|
|
|
class StringLiteralVar final : public loplugin::FilteringPlugin<StringLiteralVar>
|
|
{
|
|
public:
|
|
explicit StringLiteralVar(loplugin::InstantiationData const& data)
|
|
: FilteringPlugin(data)
|
|
{
|
|
}
|
|
|
|
bool TraverseInitListExpr(InitListExpr* expr, DataRecursionQueue* queue = nullptr)
|
|
{
|
|
return WalkUpFromInitListExpr(expr)
|
|
&& TraverseSynOrSemInitListExpr(
|
|
expr->isSemanticForm() ? expr : expr->getSemanticForm(), queue);
|
|
}
|
|
|
|
bool VisitCXXConstructExpr(CXXConstructExpr const* expr)
|
|
{
|
|
if (ignoreLocation(expr))
|
|
{
|
|
return true;
|
|
}
|
|
loplugin::TypeCheck const tc(expr->getType());
|
|
if (!(tc.Class("OString").Namespace("rtl").GlobalNamespace()
|
|
|| tc.Class("OUString").Namespace("rtl").GlobalNamespace()))
|
|
{
|
|
return true;
|
|
}
|
|
auto const ctor = expr->getConstructor();
|
|
switch (ctor->getNumParams())
|
|
{
|
|
case 1:
|
|
{
|
|
auto const e = dyn_cast<DeclRefExpr>(expr->getArg(0)->IgnoreParenImpCasts());
|
|
if (e == nullptr)
|
|
{
|
|
return true;
|
|
}
|
|
auto const tc = loplugin::TypeCheck(e->getType());
|
|
if (!(tc.Class("OStringLiteral").Namespace("rtl").GlobalNamespace()
|
|
|| tc.Class("OUStringLiteral").Namespace("rtl").GlobalNamespace()))
|
|
{
|
|
return true;
|
|
}
|
|
auto const d = e->getDecl();
|
|
if (!isAutomaticVariable(cast<VarDecl>(d)))
|
|
{
|
|
return true;
|
|
}
|
|
if (!reportedAutomatic_.insert(d).second)
|
|
{
|
|
return true;
|
|
}
|
|
report(DiagnosticsEngine::Warning,
|
|
"variable %0 of type %1 with automatic storage duration most likely needs "
|
|
"to be static",
|
|
d->getLocation())
|
|
<< d << d->getType() << d->getSourceRange();
|
|
report(DiagnosticsEngine::Note, "first converted to %0 here", expr->getLocation())
|
|
<< expr->getType() << expr->getSourceRange();
|
|
}
|
|
break;
|
|
case 2:
|
|
{
|
|
auto const e1 = dyn_cast<DeclRefExpr>(expr->getArg(0)->IgnoreParenImpCasts());
|
|
if (e1 == nullptr)
|
|
{
|
|
return true;
|
|
}
|
|
auto const t = e1->getType();
|
|
if (!(t.isConstQualified() && t->isConstantArrayType()))
|
|
{
|
|
return true;
|
|
}
|
|
auto const e2 = expr->getArg(1);
|
|
if (!((isa<CXXDefaultArgExpr>(e2)
|
|
&& loplugin::TypeCheck(e2->getType())
|
|
.Struct("Dummy")
|
|
.Namespace("libreoffice_internal")
|
|
.Namespace("rtl")
|
|
.GlobalNamespace())
|
|
|| (loplugin::TypeCheck(ctor->getParamDecl(1)->getType())
|
|
.Typedef("sal_Int32")
|
|
.GlobalNamespace()
|
|
&& e2->isIntegerConstantExpr(compiler.getASTContext()))))
|
|
{
|
|
return true;
|
|
}
|
|
auto const d = e1->getDecl();
|
|
if (isPotentiallyInitializedWithMalformedUtf16(d))
|
|
{
|
|
return true;
|
|
}
|
|
if (!reportedArray_.insert(d).second)
|
|
{
|
|
return true;
|
|
}
|
|
report(DiagnosticsEngine::Warning,
|
|
"change type of variable %0 from constant character array (%1) to "
|
|
"%select{OStringLiteral|OUStringLiteral}2%select{|, and make it static}3",
|
|
d->getLocation())
|
|
<< d << d->getType()
|
|
<< (tc.Class("OString").Namespace("rtl").GlobalNamespace() ? 0 : 1)
|
|
<< isAutomaticVariable(cast<VarDecl>(d)) << d->getSourceRange();
|
|
report(DiagnosticsEngine::Note, "first passed into a %0 constructor here",
|
|
expr->getLocation())
|
|
<< expr->getType().getUnqualifiedType() << expr->getSourceRange();
|
|
}
|
|
break;
|
|
}
|
|
return true;
|
|
}
|
|
|
|
bool VisitCXXOperatorCallExpr(CXXOperatorCallExpr const* expr)
|
|
{
|
|
if (ignoreLocation(expr))
|
|
{
|
|
return true;
|
|
}
|
|
if (expr->getOperator() != OO_Equal)
|
|
{
|
|
return true;
|
|
}
|
|
loplugin::TypeCheck const tc(expr->getType());
|
|
if (!(tc.Class("OString").Namespace("rtl").GlobalNamespace()
|
|
|| tc.Class("OUString").Namespace("rtl").GlobalNamespace()))
|
|
{
|
|
return true;
|
|
}
|
|
if (expr->getNumArgs() != 2)
|
|
{
|
|
return true;
|
|
}
|
|
auto const e = dyn_cast<DeclRefExpr>(expr->getArg(1)->IgnoreParenImpCasts());
|
|
if (e == nullptr)
|
|
{
|
|
return true;
|
|
}
|
|
auto const t = e->getType();
|
|
if (!(t.isConstQualified() && t->isConstantArrayType()))
|
|
{
|
|
return true;
|
|
}
|
|
auto const d = e->getDecl();
|
|
if (isPotentiallyInitializedWithMalformedUtf16(d))
|
|
{
|
|
return true;
|
|
}
|
|
if (!reportedArray_.insert(d).second)
|
|
{
|
|
return true;
|
|
}
|
|
report(DiagnosticsEngine::Warning,
|
|
"change type of variable %0 from constant character array (%1) to "
|
|
"%select{OStringLiteral|OUStringLiteral}2%select{|, and make it static}3",
|
|
d->getLocation())
|
|
<< d << d->getType() << (tc.Class("OString").Namespace("rtl").GlobalNamespace() ? 0 : 1)
|
|
<< isAutomaticVariable(cast<VarDecl>(d)) << d->getSourceRange();
|
|
report(DiagnosticsEngine::Note, "first assigned here", expr->getBeginLoc())
|
|
<< expr->getSourceRange();
|
|
return true;
|
|
}
|
|
|
|
bool VisitUnaryExprOrTypeTraitExpr(UnaryExprOrTypeTraitExpr const* expr)
|
|
{
|
|
if (ignoreLocation(expr))
|
|
{
|
|
return true;
|
|
}
|
|
if (expr->getKind() != UETT_SizeOf)
|
|
{
|
|
return true;
|
|
}
|
|
if (expr->isArgumentType())
|
|
{
|
|
return true;
|
|
}
|
|
auto const e = dyn_cast<DeclRefExpr>(expr->getArgumentExpr()->IgnoreParenImpCasts());
|
|
if (e == nullptr)
|
|
{
|
|
return true;
|
|
}
|
|
auto const tc = loplugin::TypeCheck(e->getType());
|
|
if (!(tc.Class("OStringLiteral").Namespace("rtl").GlobalNamespace()
|
|
|| tc.Class("OUStringLiteral").Namespace("rtl").GlobalNamespace()))
|
|
{
|
|
return true;
|
|
}
|
|
auto const d = e->getDecl();
|
|
report(DiagnosticsEngine::Warning,
|
|
"variable %0 of type %1 suspiciously used in a sizeof expression", e->getLocation())
|
|
<< d << d->getType() << expr->getSourceRange();
|
|
return true;
|
|
}
|
|
|
|
bool preRun() override { return compiler.getLangOpts().CPlusPlus; }
|
|
|
|
private:
|
|
void run() override
|
|
{
|
|
if (preRun())
|
|
{
|
|
TraverseDecl(compiler.getASTContext().getTranslationUnitDecl());
|
|
}
|
|
}
|
|
|
|
// There is some confusion on the semantics of numeric-escape-sequences in string literals, see
|
|
// <https://www.open-std.org/jtc1/sc22/wg21/docs/papers/2020/p2029r4.html> "Proposed resolution
|
|
// for core issues 411, 1656, and 2333; numeric and universal character escapes in character and
|
|
// string literals", so suppress warnings about arrays that are deliberately not written as
|
|
// UTF-16 string literals because they contain lone surrogates:
|
|
bool isPotentiallyInitializedWithMalformedUtf16(ValueDecl const* decl) const
|
|
{
|
|
if (!decl->getType()->getArrayElementTypeNoTypeQual()->isChar16Type())
|
|
{
|
|
return false;
|
|
}
|
|
auto const init = cast<VarDecl>(decl)->getAnyInitializer();
|
|
if (init == nullptr)
|
|
{
|
|
return true;
|
|
}
|
|
auto const list = dyn_cast<InitListExpr>(init);
|
|
if (list == nullptr)
|
|
{
|
|
// Assuming that the initializer already is a string literal, assume that that string
|
|
// literal has no issues with malformed UTF-16:
|
|
if (isDebugMode())
|
|
{
|
|
assert(isa<clang::StringLiteral>(init));
|
|
}
|
|
return false;
|
|
}
|
|
auto highSurrogate = false;
|
|
for (auto const e : list->inits())
|
|
{
|
|
llvm::APSInt v;
|
|
if (!compat::EvaluateAsInt(e, v, compiler.getASTContext()))
|
|
{
|
|
return true;
|
|
}
|
|
if (highSurrogate)
|
|
{
|
|
if (v < 0xDC00 || v > 0xDFFF)
|
|
{
|
|
return true;
|
|
}
|
|
highSurrogate = false;
|
|
}
|
|
else if (v >= 0xD800 && v <= 0xDBFF)
|
|
{
|
|
highSurrogate = true;
|
|
}
|
|
else if (v >= 0xDC00 && v <= 0xDFFF)
|
|
{
|
|
return true;
|
|
}
|
|
}
|
|
return highSurrogate;
|
|
}
|
|
|
|
std::set<Decl const*> reportedAutomatic_;
|
|
std::set<Decl const*> reportedArray_;
|
|
};
|
|
|
|
static loplugin::Plugin::Registration<StringLiteralVar> reg("stringliteralvar");
|
|
}
|
|
|
|
/* vim:set shiftwidth=4 softtabstop=4 expandtab cinoptions=b1,g0,N-s cinkeys+=0=break: */
|