mir/libreoffice
2
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

loplugin:refcounting check for one more case

Browse Source 
where we might be holding something newly created by pointer
instead of by *::Reference

Change-Id: Ife6f7acae4252bf56dcdeb95d72e43c523444f97
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/112138
Tested-by: Jenkins
Reviewed-by: Noel Grandin <noel.grandin@collabora.co.uk>
This commit is contained in:
Noel 2021-03-05 15:51:07 +02:00 committed by Noel Grandin
parent 72841008bf
commit 04e7a34a19
8 changed files with 104 additions and 15 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

4
accessibility/source/standard/vclxaccessibletoolbox.cxx
View File

@ -593,8 +593,8 @@ void VCLXAccessibleToolBox::ProcessWindowEvent( const VclWindowEvent& rVclWindow
if ( pWin && pWin->GetParent() &&
pWin->GetParent()->GetType() == WindowType::TOOLBOX )
{
VCLXAccessibleToolBox* pParent = static_cast< VCLXAccessibleToolBox* >(
pWin->GetParent()->GetAccessible()->getAccessibleContext().get() );
auto pParentAccContext = pWin->GetParent()->GetAccessible()->getAccessibleContext();
VCLXAccessibleToolBox* pParent = static_cast< VCLXAccessibleToolBox* >( pParentAccContext.get() );
if ( pParent )
pParent->ReleaseSubToolBox(static_cast<ToolBox *>(pWin.get()));
}

3
canvas/source/opengl/ogl_canvashelper.cxx
View File

@ -679,7 +679,8 @@ namespace oglcanvas
ScopedVclPtrInstance< VirtualDevice > pVDev;
pVDev->EnableOutput(false);
CanvasFont* pFont=dynamic_cast<CanvasFont*>(xLayoutetText->getFont().get());
auto pLayoutFont = xLayoutetText->getFont();
CanvasFont* pFont=dynamic_cast<CanvasFont*>(pLayoutFont.get());
const rendering::StringContext& rTxt=xLayoutetText->getText();
if( pFont && rTxt.Length )
{

6
chart2/source/controller/main/ChartController_Window.cxx
View File

@ -863,7 +863,8 @@ void ChartController::execute_MouseButtonUp( const MouseEvent& rMEvt )
m_xUndoManager );
bool bChanged = false;
ChartModel* pModel = dynamic_cast<ChartModel*>(getModel().get());
css::uno::Reference< css::frame::XModel > xModel = getModel();
ChartModel* pModel = dynamic_cast<ChartModel*>(xModel.get());
assert(pModel);
if ( eObjectType == OBJECTTYPE_LEGEND )
bChanged = DiagramHelper::switchDiagramPositioningToExcludingPositioning( *pModel, false , true );
@ -2090,7 +2091,8 @@ void ChartController::sendPopupRequest(OUString const & rCID, tools::Rectangle a
OUString sPivotTableName = xPivotTableDataProvider->getPivotTableName();
PopupRequest* pPopupRequest = dynamic_cast<PopupRequest*>(pChartModel->getPopupRequest().get());
css::uno::Reference<css::awt::XRequestCallback> xPopupRequest = pChartModel->getPopupRequest();
PopupRequest* pPopupRequest = dynamic_cast<PopupRequest*>(xPopupRequest.get());
if (!pPopupRequest)
return;

72
compilerplugins/clang/refcounting.cxx
View File

@ -73,6 +73,7 @@ private:
const RecordDecl* parent, const std::string& rDeclName);
bool visitTemporaryObjectExpr(Expr const * expr);
bool isCastingReference(const Expr* expr);
};
bool containsXInterfaceSubclass(const clang::Type* pType0);
@ -693,6 +694,56 @@ bool RefCounting::VisitVarDecl(const VarDecl * varDecl) {
<< pointeeType
<< varDecl->getSourceRange();
}
if (isCastingReference(varDecl->getInit()))
{
// TODO false+ code
StringRef fileName = getFilenameOfLocation(compiler.getSourceManager().getSpellingLoc(compat::getBeginLoc(varDecl)));
if (loplugin::isSamePathname(fileName, SRCDIR "/sw/source/core/unocore/unotbl.cxx"))
return true;
auto pointeeType = varDecl->getType()->getPointeeType();
if (containsOWeakObjectSubclass(pointeeType))
report(
DiagnosticsEngine::Warning,
"cppu::OWeakObject subclass %0 being managed via raw pointer, should be managed via rtl::Reference",
varDecl->getLocation())
<< pointeeType
<< varDecl->getSourceRange();
}
}
return true;
}
/**
Look for code like
static_cast<FooChild*>(makeFoo().get());
where makeFoo() returns a Reference<Foo>
*/
bool RefCounting::isCastingReference(const Expr* expr)
{
expr = compat::IgnoreImplicit(expr);
auto castExpr = dyn_cast<CastExpr>(expr);
if (!castExpr)
return false;
auto memberCallExpr = dyn_cast<CXXMemberCallExpr>(castExpr->getSubExpr());
if (!memberCallExpr)
return false;
if (!memberCallExpr->getMethodDecl()->getIdentifier() || memberCallExpr->getMethodDecl()->getName() != "get")
return false;
QualType objectType = memberCallExpr->getImplicitObjectArgument()->getType();
if (!loplugin::TypeCheck(objectType).Class("Reference"))
return false;
// ignore "x.get()" where x is a var
auto obj = compat::IgnoreImplicit(memberCallExpr->getImplicitObjectArgument());
if (isa<DeclRefExpr>(obj) || isa<MemberExpr>(obj))
return false;
// if the foo in foo().get() returns "rtl::Reference<T>&" then the variable
// we are assigning to does not __have__ to be Reference, since the method called
// must already be holding a reference.
if (auto callExpr = dyn_cast<CallExpr>(obj))
{
if (auto callMethod = callExpr->getDirectCallee())
if (callMethod->getReturnType()->isReferenceType())
return false;
}
return true;
}
@ -706,14 +757,14 @@ bool RefCounting::VisitBinaryOperator(const BinaryOperator * binaryOperator)
if (!binaryOperator->getLHS()->getType()->isPointerType())
return true;
// deliberately does not want to keep track at the allocation site
StringRef fileName = getFilenameOfLocation(compiler.getSourceManager().getSpellingLoc(compat::getBeginLoc(binaryOperator)));
if (loplugin::isSamePathname(fileName, SRCDIR "/vcl/unx/generic/dtrans/X11_selection.cxx"))
return true;
auto newExpr = dyn_cast<CXXNewExpr>(compat::IgnoreImplicit(binaryOperator->getRHS()));
if (newExpr)
{
// deliberately does not want to keep track at the allocation site
StringRef fileName = getFilenameOfLocation(compiler.getSourceManager().getSpellingLoc(compat::getBeginLoc(binaryOperator)));
if (loplugin::isSamePathname(fileName, SRCDIR "/vcl/unx/generic/dtrans/X11_selection.cxx"))
return true;
auto pointeeType = binaryOperator->getLHS()->getType()->getPointeeType();
if (containsOWeakObjectSubclass(pointeeType))
{
@ -725,6 +776,17 @@ bool RefCounting::VisitBinaryOperator(const BinaryOperator * binaryOperator)
<< binaryOperator->getSourceRange();
}
}
if (isCastingReference(binaryOperator->getRHS()))
{
auto pointeeType = binaryOperator->getLHS()->getType()->getPointeeType();
if (containsOWeakObjectSubclass(pointeeType))
report(
DiagnosticsEngine::Warning,
"cppu::OWeakObject subclass %0 being managed via raw pointer, should be managed via rtl::Reference",
compat::getBeginLoc(binaryOperator))
<< pointeeType
<< binaryOperator->getSourceRange();
}
return true;
}

22
compilerplugins/clang/test/refcounting.cxx
View File

@ -27,6 +27,9 @@ public:
struct UnoObject : public cppu::OWeakObject
{
};
struct UnoSubObject : public UnoObject
{
};
//
// Note, getting duplicate warnings for some reason I cannot fathom
@ -94,5 +97,24 @@ rtl::Reference<UnoObject> foo6()
// no warning expected
return new UnoObject;
}
const rtl::Reference<UnoObject>& getConstRef();
void foo7()
{
// expected-error@+1 {{cppu::OWeakObject subclass 'UnoSubObject' being managed via raw pointer, should be managed via rtl::Reference [loplugin:refcounting]}}
UnoSubObject* p1 = static_cast<UnoSubObject*>(foo6().get());
(void)p1;
// expected-error@+1 {{cppu::OWeakObject subclass 'UnoSubObject' being managed via raw pointer, should be managed via rtl::Reference [loplugin:refcounting]}}
p1 = static_cast<UnoSubObject*>(foo6().get());
rtl::Reference<UnoObject> u2;
// no warning expected
UnoSubObject* p2 = static_cast<UnoSubObject*>(u2.get());
(void)p2;
p2 = static_cast<UnoSubObject*>(u2.get());
// no warning expected
UnoSubObject* p3 = static_cast<UnoSubObject*>(getConstRef().get());
(void)p3;
p3 = static_cast<UnoSubObject*>(getConstRef().get());
}
/* vim:set shiftwidth=4 softtabstop=4 expandtab cinoptions=b1,g0,N-s cinkeys+=0=break: */

3
sc/source/ui/Accessibility/AccessibleSpreadsheet.cxx
View File

@ -493,8 +493,9 @@ void ScAccessibleSpreadsheet::Notify( SfxBroadcaster& rBC, const SfxHint& rHint
if(aNewCell.Tab() != maActiveCell.Tab())
{
aEvent.EventId = AccessibleEventId::PAGE_CHANGED;
auto pAccParent = getAccessibleParent();
ScAccessibleDocument *pAccDoc =
static_cast<ScAccessibleDocument*>(getAccessibleParent().get());
static_cast<ScAccessibleDocument*>(pAccParent.get());
if(pAccDoc)
{
pAccDoc->CommitChange(aEvent);

3
sd/source/ui/dlg/sdtreelb.cxx
View File

@ -612,7 +612,8 @@ void SdPageObjsTLV::AddShapeToTransferable (
if ( ! (xFrameAccess->getByIndex(nIndex) >>= xFrame))
continue;
::sd::DrawController* pController = dynamic_cast<sd::DrawController*>(xFrame->getController().get());
auto xController = xFrame->getController();
::sd::DrawController* pController = dynamic_cast<sd::DrawController*>(xController.get());
if (pController == nullptr)
continue;
::sd::ViewShellBase* pBase = pController->GetViewShellBase();

6
writerfilter/source/ooxml/OOXMLFastHelper.hxx
View File

@ -28,7 +28,7 @@ template <class T>
class OOXMLFastHelper
{
public:
static OOXMLFastContextHandler* createAndSetParentAndDefine
static rtl::Reference<OOXMLFastContextHandler> createAndSetParentAndDefine
(OOXMLFastContextHandler * pHandler, sal_uInt32 nToken, Id nId, Id nDefine);
static void newProperty(OOXMLFastContextHandler * pHandler,
@ -36,9 +36,9 @@ public:
};
template <class T>
OOXMLFastContextHandler* OOXMLFastHelper<T>::createAndSetParentAndDefine (OOXMLFastContextHandler * pHandler, sal_uInt32 nToken, Id nId, Id nDefine)
rtl::Reference<OOXMLFastContextHandler> OOXMLFastHelper<T>::createAndSetParentAndDefine (OOXMLFastContextHandler * pHandler, sal_uInt32 nToken, Id nId, Id nDefine)
{
OOXMLFastContextHandler * pTmp = new T(pHandler);
rtl::Reference<OOXMLFastContextHandler> pTmp = new T(pHandler);
pTmp->setToken(nToken);
pTmp->setId(nId);