wine/gecko
0
0
Fork 0
mirror of https://gitlab.winehq.org/wine/wine-gecko.git synced 2024-09-13 09:24:08 -07:00
Code Issues Packages Projects Releases Wiki Activity

Bug 1156084 - Disallow AddRef() and Release() calls on the return value of methods returning XPCOM objects; r=jrmuizel

Browse Source 
When a method returns type D derived from RefCounted type B, there is an
ImplicitCastExpr (or an ExplicitCastExpr, if there is an explicit cast
to the base type in the code) in the AST between the CallExpr and
MemberExpr, which we didn't take into account before.  This caused the
analysis to not work on common patterns such as
nsCOMPtr<nsIXPCOMInterface>.
This commit is contained in:
Ehsan Akhgari 2015-04-19 13:22:35 -04:00
parent 8485cab82c
commit aee7f7274a
9 changed files with 68 additions and 14 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

8
build/clang-plugin/clang-plugin.cpp
View File

@ -690,11 +690,19 @@ DiagnosticsMatcher::DiagnosticsMatcher()
)).bind("node"),
&nanExprChecker);
// First, look for direct parents of the MemberExpr.
astMatcher.addMatcher(callExpr(callee(functionDecl(hasNoAddRefReleaseOnReturnAttr()).bind("func")),
hasParent(memberExpr(isAddRefOrRelease(),
hasParent(callExpr())).bind("member")
)).bind("node"),
&noAddRefReleaseOnReturnChecker);
// Then, look for MemberExpr that need to be casted to the right type using
// an intermediary CastExpr before we get to the CallExpr.
astMatcher.addMatcher(callExpr(callee(functionDecl(hasNoAddRefReleaseOnReturnAttr()).bind("func")),
hasParent(castExpr(hasParent(memberExpr(isAddRefOrRelease(),
hasParent(callExpr())).bind("member"))))
).bind("node"),
&noAddRefReleaseOnReturnChecker);
astMatcher.addMatcher(lambdaExpr(
hasDescendant(declRefExpr(hasType(pointerType(pointee(isRefCounted())))).bind("node"))

45
build/clang-plugin/tests/TestNoAddRefReleaseOnReturn.cpp
View File

@ -6,12 +6,20 @@ struct Test {
void foo();
};
struct TestD : Test {};
struct S {
Test* f() MOZ_NO_ADDREF_RELEASE_ON_RETURN;
Test& g() MOZ_NO_ADDREF_RELEASE_ON_RETURN;
Test h() MOZ_NO_ADDREF_RELEASE_ON_RETURN;
};
struct SD {
TestD* f() MOZ_NO_ADDREF_RELEASE_ON_RETURN;
TestD& g() MOZ_NO_ADDREF_RELEASE_ON_RETURN;
TestD h() MOZ_NO_ADDREF_RELEASE_ON_RETURN;
};
template<class T>
struct X {
T* f() MOZ_NO_ADDREF_RELEASE_ON_RETURN;
@ -28,6 +36,10 @@ Test* f() MOZ_NO_ADDREF_RELEASE_ON_RETURN;
Test& g() MOZ_NO_ADDREF_RELEASE_ON_RETURN;
Test h() MOZ_NO_ADDREF_RELEASE_ON_RETURN;
TestD* fd() MOZ_NO_ADDREF_RELEASE_ON_RETURN;
TestD& gd() MOZ_NO_ADDREF_RELEASE_ON_RETURN;
TestD hd() MOZ_NO_ADDREF_RELEASE_ON_RETURN;
void test() {
S s;
s.f()->AddRef(); // expected-error{{'AddRef' cannot be called on the return value of 'f'}}
@ -39,6 +51,16 @@ void test() {
s.h().AddRef(); // expected-error{{'AddRef' cannot be called on the return value of 'h'}}
s.h().Release(); // expected-error{{'Release' cannot be called on the return value of 'h'}}
s.h().foo();
SD sd;
sd.f()->AddRef(); // expected-error{{'AddRef' cannot be called on the return value of 'f'}}
sd.f()->Release(); // expected-error{{'Release' cannot be called on the return value of 'f'}}
sd.f()->foo();
sd.g().AddRef(); // expected-error{{'AddRef' cannot be called on the return value of 'g'}}
sd.g().Release(); // expected-error{{'Release' cannot be called on the return value of 'g'}}
sd.g().foo();
sd.h().AddRef(); // expected-error{{'AddRef' cannot be called on the return value of 'h'}}
sd.h().Release(); // expected-error{{'Release' cannot be called on the return value of 'h'}}
sd.h().foo();
X<Test> x;
x.f()->AddRef(); // expected-error{{'AddRef' cannot be called on the return value of 'f'}}
x.f()->Release(); // expected-error{{'Release' cannot be called on the return value of 'f'}}
@ -49,10 +71,24 @@ void test() {
x.h().AddRef(); // expected-error{{'AddRef' cannot be called on the return value of 'h'}}
x.h().Release(); // expected-error{{'Release' cannot be called on the return value of 'h'}}
x.h().foo();
X<TestD> xd;
xd.f()->AddRef(); // expected-error{{'AddRef' cannot be called on the return value of 'f'}}
xd.f()->Release(); // expected-error{{'Release' cannot be called on the return value of 'f'}}
xd.f()->foo();
xd.g().AddRef(); // expected-error{{'AddRef' cannot be called on the return value of 'g'}}
xd.g().Release(); // expected-error{{'Release' cannot be called on the return value of 'g'}}
xd.g().foo();
xd.h().AddRef(); // expected-error{{'AddRef' cannot be called on the return value of 'h'}}
xd.h().Release(); // expected-error{{'Release' cannot be called on the return value of 'h'}}
xd.h().foo();
SP<Test> sp;
sp->AddRef(); // expected-error{{'AddRef' cannot be called on the return value of 'operator->'}}
sp->Release(); // expected-error{{'Release' cannot be called on the return value of 'operator->'}}
sp->foo();
SP<TestD> spd;
spd->AddRef(); // expected-error{{'AddRef' cannot be called on the return value of 'operator->'}}
spd->Release(); // expected-error{{'Release' cannot be called on the return value of 'operator->'}}
spd->foo();
f()->AddRef(); // expected-error{{'AddRef' cannot be called on the return value of 'f'}}
f()->Release(); // expected-error{{'Release' cannot be called on the return value of 'f'}}
f()->foo();
@ -62,4 +98,13 @@ void test() {
h().AddRef(); // expected-error{{'AddRef' cannot be called on the return value of 'h'}}
h().Release(); // expected-error{{'Release' cannot be called on the return value of 'h'}}
h().foo();
fd()->AddRef(); // expected-error{{'AddRef' cannot be called on the return value of 'fd'}}
fd()->Release(); // expected-error{{'Release' cannot be called on the return value of 'fd'}}
fd()->foo();
gd().AddRef(); // expected-error{{'AddRef' cannot be called on the return value of 'gd'}}
gd().Release(); // expected-error{{'Release' cannot be called on the return value of 'gd'}}
gd().foo();
hd().AddRef(); // expected-error{{'AddRef' cannot be called on the return value of 'hd'}}
hd().Release(); // expected-error{{'Release' cannot be called on the return value of 'hd'}}
hd().foo();
}

7
dom/ipc/TabParent.cpp
View File

@ -3078,15 +3078,16 @@ public:
NS_IMETHOD SetOriginalURI(nsIURI*) NO_IMPL
NS_IMETHOD GetURI(nsIURI** aUri) override
{
NS_IF_ADDREF(mUri);
*aUri = mUri;
nsCOMPtr<nsIURI> copy = mUri;
copy.forget(aUri);
return NS_OK;
}
NS_IMETHOD GetOwner(nsISupports**) NO_IMPL
NS_IMETHOD SetOwner(nsISupports*) NO_IMPL
NS_IMETHOD GetLoadInfo(nsILoadInfo** aLoadInfo) override
{
NS_IF_ADDREF(*aLoadInfo = mLoadInfo);
nsCOMPtr<nsILoadInfo> copy = mLoadInfo;
copy.forget(aLoadInfo);
return NS_OK;
}
NS_IMETHOD SetLoadInfo(nsILoadInfo* aLoadInfo) override

4
dom/media/gmp/GMPService.cpp
View File

@ -275,8 +275,8 @@ GeckoMediaPluginService::GetThread(nsIThread** aThread)
InitializePlugins();
}
NS_ADDREF(mGMPThread);
*aThread = mGMPThread;
nsCOMPtr<nsIThread> copy = mGMPThread;
copy.forget(aThread);
return NS_OK;
}

2
gfx/2d/DrawTargetCG.cpp
View File

@ -251,7 +251,7 @@ GetRetainedImageFromSourceSurface(SourceSurface *aSurface)
if (!data) {
MOZ_CRASH("unsupported source surface");
}
data->AddRef();
data.get()->AddRef();
return CreateCGImage(releaseDataSurface, data.get(),
data->GetData(), data->GetSize(),
data->Stride(), data->GetFormat());

4
netwerk/protocol/ftp/FTPChannelParent.cpp
View File

@ -465,8 +465,8 @@ FTPChannelParent::GetInterface(const nsIID& uuid, void** result)
{
// Only support nsILoadContext if child channel's callbacks did too
if (uuid.Equals(NS_GET_IID(nsILoadContext)) && mLoadContext) {
NS_ADDREF(mLoadContext);
*result = static_cast<nsILoadContext*>(mLoadContext);
nsCOMPtr<nsILoadContext> copy = mLoadContext;
copy.forget(result);
return NS_OK;
}

4
netwerk/protocol/http/HttpChannelParent.cpp
View File

@ -223,8 +223,8 @@ HttpChannelParent::GetInterface(const nsIID& aIID, void **result)
// Only support nsILoadContext if child channel's callbacks did too
if (aIID.Equals(NS_GET_IID(nsILoadContext)) && mLoadContext) {
NS_ADDREF(mLoadContext);
*result = static_cast<nsILoadContext*>(mLoadContext);
nsCOMPtr<nsILoadContext> copy = mLoadContext;
copy.forget(result);
return NS_OK;
}

4
netwerk/protocol/websocket/WebSocketChannelParent.cpp
View File

@ -304,8 +304,8 @@ WebSocketChannelParent::GetInterface(const nsIID & iid, void **result)
// Only support nsILoadContext if child channel's callbacks did too
if (iid.Equals(NS_GET_IID(nsILoadContext)) && mLoadContext) {
NS_ADDREF(mLoadContext);
*result = static_cast<nsILoadContext*>(mLoadContext);
nsCOMPtr<nsILoadContext> copy = mLoadContext;
copy.forget(result);
return NS_OK;
}

4
netwerk/protocol/wyciwyg/WyciwygChannelParent.cpp
View File

@ -341,8 +341,8 @@ WyciwygChannelParent::GetInterface(const nsIID& uuid, void** result)
{
// Only support nsILoadContext if child channel's callbacks did too
if (uuid.Equals(NS_GET_IID(nsILoadContext)) && mLoadContext) {
NS_ADDREF(mLoadContext);
*result = static_cast<nsILoadContext*>(mLoadContext);
nsCOMPtr<nsILoadContext> copy = mLoadContext;
copy.forget(result);
return NS_OK;
}