Fwd: mozilla::TemporaryRef is gone; please use already_AddRefed
With regard to what #2 is doing, I threw the following into a file with
nsRefPtr, and got clang to dump the AST:
struct Foo {void AddRef() {} void Release() {}};
nsRefPtrFoo bar;
void foo() {
Foo* x = true ? nullptr : bar;
}
`-FunctionDecl 0x103805750 line:943:1, line:945:1 line:943:6 foo 'void
(void)'
`-CompoundStmt 0x103807708 col:12, line:945:1
`-DeclStmt 0x1038076f0 line:944:3, col:32
`-VarDecl 0x103805800 col:3, col:29 col:8 x 'struct Foo *' cinit
`-ExprWithCleanups 0x1038076d8 col:12, col:29 'struct Foo *'
`-ImplicitCastExpr 0x1038076c0 col:12, col:29 'struct Foo *'
UserDefinedConversion
`-CXXMemberCallExpr 0x103807698 col:12, col:29 'struct Foo *'
`-MemberExpr 0x103807668 col:12, col:29 'bound member
function type' .operator Foo * 0x1038048a0
`-ImplicitCastExpr 0x103807650 col:12, col:29 'const
class nsRefPtrstruct Foo' NoOp
`-ConditionalOperator 0x103807618 col:12, col:29
'nsRefPtrstruct Foo':'class nsRefPtrstruct Foo'
|-CXXBoolLiteralExpr 0x103805858 col:12 '_Bool' true
|-CXXBindTemporaryExpr 0x1038074a8 col:19
'nsRefPtrstruct Foo':'class nsRefPtrstruct Foo' (CXXTemporary
0x1038074a0)
| `-CXXConstructExpr 0x103807468 col:19
'nsRefPtrstruct Foo':'class nsRefPtrstruct Foo' 'void (nsRefPtrstruct
Foo )' elidable
| `-MaterializeTemporaryExpr 0x103807450 col:19
'nsRefPtrstruct Foo':'class nsRefPtrstruct Foo' xvalue
| `-CXXBindTemporaryExpr 0x103807358 col:19
'nsRefPtrstruct Foo':'class nsRefPtrstruct Foo' (CXXTemporary
0x103807350)
| `-CXXConstructExpr 0x103807318 col:19
'nsRefPtrstruct Foo':'class nsRefPtrstruct Foo' 'void (nsRefPtrstruct
Foo )' elidable
| `-MaterializeTemporaryExpr 0x103807300
col:19 'nsRefPtrstruct Foo':'class nsRefPtrstruct Foo' xvalue
| `-CXXBindTemporaryExpr 0x103806f38 col:19
'nsRefPtrstruct Foo':'class nsRefPtrstruct Foo' (CXXTemporary
0x103806f30)
| `-ImplicitCastExpr 0x103806f10 col:19
'nsRefPtrstruct Foo':'class nsRefPtrstruct Foo' ConstructorConversion
| `-CXXConstructExpr 0x103806ed8 col:19
'nsRefPtrstruct Foo':'class nsRefPtrstruct Foo' 'void (struct Foo *)'
| `-ImplicitCastExpr 0x103806ec0
col:19 'struct Foo *' NullToPointer
| `-CXXNullPtrLiteralExpr 0x103805870
col:19 'nullptr_t'
`-CXXBindTemporaryExpr 0x1038075f8 col:29
'nsRefPtrstruct Foo':'class nsRefPtrstruct Foo' (CXXTemporary
0x1038075f0)
`-CXXConstructExpr 0x1038075b8 col:29
'nsRefPtrstruct Foo':'class nsRefPtrstruct Foo' 'void (const
nsRefPtrstruct Foo )'
`-ImplicitCastExpr 0x1038075a0 col:29 'const
nsRefPtrstruct Foo':'const class nsRefPtrstruct Foo' lvalue NoOp
`-DeclRefExpr 0x103805888 col:29
'nsRefPtrstruct Foo':'class nsRefPtrstruct Foo' lvalue Var 0x103084da0
'bar' 'nsRefPtrstruct Foo':'class nsRefPtrstruct Foo'
In summary, the nsRefPtr is copied into a temporary in it's side of the
conditional. the nullptr is cast to a struct Foo*, which is constructed
into a nsRefPtr, which is bound to a temporary, and then moved around a bit
between temporaries. The resulting temporary xvalue then has the raw
pointer extracted from it (which uses the rvalue-reference cast call - the
part which is causing the problem), which is stored locally after the
temporaries are destroyed. The temporaries which are created in the
conditional are then destroyed, which causes a Release(), but that is OK
because there was an AddRef due to the extra copy in the nsRefPtr branch.
So the ternary actually causes an unnecessary AddRef/Release pair, neat.
The problem here appears to be that when deciding the type of the ternary,
c++ chooses nsRefPtrFoo, rather than Foo*. Adding the get() makes C++
choose the correct type for the ternary, and avoids the cast of the rvalue
reference.
On Wed, Jul 1, 2015 at 3:48 PM, Nathan Froyd nfr...@mozilla.com wrote:
On Wed, Jul 1, 2015 at 7:39 AM, Aryeh Gregor a...@aryeh.name wrote:
On Wed, Jul 1, 2015 at 6:24 AM, Joshua Cranmer [image: ]
pidgeo...@gmail.com
wrote:
You'd get the same benefit, I think, by making operator T*() =
delete;, syntax which is accepted on gcc 4.8.1+, clang, and MSVC 2015
IIRC.
I once tried this and found it had problematic side effects that made
deploying it difficult in practice. I think one involved the ternary
operator. Unfortunately, I can't remember exactly what they were. :(
Anyone who's interested can try for themselves by just adding that
line to nsCOMPtr.h, recompiling, and seeing what breaks.
I tried this, fixed a few compilation errors, then decided this wasn't
worth it just yet and threw my work into a bug. Comments
Re: mozilla::TemporaryRef is gone; please use already_AddRefed
On Fri, Jul 3, 2015 at 8:56 AM, Neil n...@parkwaycc.co.uk wrote: Sure, but that won't stop someone from writing ArgFoo foo = ReturnFoo2(); will it? Something like that is fairly trivial for a static analysis system like our clang plugin to catch, if we want to create a type like that. Restricting certain types to only exist in temporaries and parameter declarations would not be too hard. ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: mozilla::TemporaryRef is gone; please use already_AddRefed
Nathan Froyd wrote: I guess we could fix this in current m-c to be smarter about the parameter passing, perhaps by declaring that ParameterStoragensCOMPtrT is StoreCopyPassByConstLRefnsCOMPtrT? (Similarly for nsRefPtrT.) So what's StorensRefPtrPassByPtr for? -- Warning: May contain traces of nuts. ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: mozilla::TemporaryRef is gone; please use already_AddRefed
Aryeh Gregor wrote: we still want a new type for function parameters that accepts implicit conversions from nsRefPtr/nsCOMPtr, to use instead of raw pointers. Sure, but that won't stop someone from writing ArgFoo foo = ReturnFoo2(); will it? -- Warning: May contain traces of nuts. ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: xulrunner 31, swt 4.5, ZOOM
昂鞥哦啊嗯嗯o鞥偶鞥/t身材一
2015年7月3日 下午3:30於 mihai.slavcov...@gmail.com寫道：
Hi,
I have an SWT application that uses a browser to display HTML pages.
Latest SWT 4.5 now support XulRunner v31, but something has changed: zoom
is not working anymore. It was working before with SWT 4.4 and XulRunner
v10.
I have no experience with XulRunner or XUL. Did something changed between
10 and 31 regarding zoom?
Here is what I used for zooming:
var winWatcher = Components.classes[@
mozilla.org/embedcomp/window-watcher;1
].getService(Components.interfaces.nsIWindowWatcher);
var enumerator = winWatcher.getWindowEnumerator();
var win = null;
while (enumerator.hasMoreElements()) {
var checkWin = enumerator.getNext();
if (checkWin.document.location == '@{currentLocation}') {
win = checkWin;
break;
}
}
if (win != null) {
var interfaceRequestor =
win.QueryInterface(Components.interfaces.nsIInterfaceRequestor);
var webNavigation =
interfaceRequestor.getInterface(Components.interfaces.nsIWebNavigation);
var docShell =
webNavigation.QueryInterface(Components.interfaces.nsIDocShell);
var docViewer =
docShell.contentViewer.QueryInterface(Components.interfaces.nsIMarkupDocumentViewer);
docViewer.fullZoom = @{zoomLevel};
}
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform
Re: xulrunner 31, swt 4.5, ZOOM
On 07/03/2015 10:27 AM, mihai.slavcov...@gmail.com wrote:
Hi,
I have an SWT application that uses a browser to display HTML pages. Latest SWT
4.5 now support XulRunner v31, but something has changed: zoom is not working
anymore. It was working before with SWT 4.4 and XulRunner v10.
I have no experience with XulRunner or XUL. Did something changed between 10
and 31 regarding zoom?
Here is what I used for zooming:
var winWatcher =
Components.classes[@mozilla.org/embedcomp/window-watcher;1].getService(Components.interfaces.nsIWindowWatcher);
var enumerator = winWatcher.getWindowEnumerator();
var win = null;
while (enumerator.hasMoreElements()) {
var checkWin = enumerator.getNext();
if (checkWin.document.location == '@{currentLocation}') {
win = checkWin;
break;
}
}
if (win != null) {
var interfaceRequestor =
win.QueryInterface(Components.interfaces.nsIInterfaceRequestor);
var webNavigation =
interfaceRequestor.getInterface(Components.interfaces.nsIWebNavigation);
var docShell =
webNavigation.QueryInterface(Components.interfaces.nsIDocShell);
var docViewer =
docShell.contentViewer.QueryInterface(Components.interfaces.nsIMarkupDocumentViewer);
docViewer.fullZoom = @{zoomLevel};
}
Are you sure you're on v31?
https://bugzilla.mozilla.org/show_bug.cgi?id=1036694 merged
nsIMarkupDocumentViewer to nsIContentViewer.
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform
xulrunner 31, swt 4.5, ZOOM
Hi,
I have an SWT application that uses a browser to display HTML pages. Latest SWT
4.5 now support XulRunner v31, but something has changed: zoom is not working
anymore. It was working before with SWT 4.4 and XulRunner v10.
I have no experience with XulRunner or XUL. Did something changed between 10
and 31 regarding zoom?
Here is what I used for zooming:
var winWatcher =
Components.classes[@mozilla.org/embedcomp/window-watcher;1].getService(Components.interfaces.nsIWindowWatcher);
var enumerator = winWatcher.getWindowEnumerator();
var win = null;
while (enumerator.hasMoreElements()) {
var checkWin = enumerator.getNext();
if (checkWin.document.location == '@{currentLocation}') {
win = checkWin;
break;
}
}
if (win != null) {
var interfaceRequestor =
win.QueryInterface(Components.interfaces.nsIInterfaceRequestor);
var webNavigation =
interfaceRequestor.getInterface(Components.interfaces.nsIWebNavigation);
var docShell =
webNavigation.QueryInterface(Components.interfaces.nsIDocShell);
var docViewer =
docShell.contentViewer.QueryInterface(Components.interfaces.nsIMarkupDocumentViewer);
docViewer.fullZoom = @{zoomLevel};
}
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform
Re: mozilla::TemporaryRef is gone; please use already_AddRefed
On Thu, Jul 2, 2015 at 3:50 PM, Neil n...@parkwaycc.co.uk wrote:
Would you mind reminding me what the failure case this avoids is?
already_AddRefedFoo
ReturnFoo1()
{
nsRefPtrFoo foo = new Foo();
return foo.forget();
}
nsRefPtrFoo
ReturnFoo2()
{
return new Foo();
}
// This doesn't compile
Foo* foo = ReturnFoo1();
// This happily compiles and causes use-after-free
Foo* foo = ReturnFoo2();
Naturally, the hard-to-catch case is when the function returns
something that usually has a refcount above one, so it works fine, but
sometimes will return something with a refcount of exactly one, which
causes a sec-critical arbitrary code execution on Firefox stable.
It's worth pointing out that if we only remove the implicit conversion
to T* for rvalues, you still won't be able to directly pass the
returned value to a function that wants T*, even though this is
perfectly safe (because the destructor for the temporary won't be
called until the function call returns, IIUC). So we still want a new
type for function parameters that accepts implicit conversions from
nsRefPtr/nsCOMPtr, to use instead of raw pointers. But you can't pass
an already_AddRefed directly to such a function right now anyway, so
this isn't actually a disadvantage relative to the status quo.
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform
