1295197 - use non-null-checking operator new in xpcom/

Status: RESOLVED FIXED

(Core :: XPCOM, defect)

Description

[Nathan Froyd [:froydnj]]

The standard placement new function is declared to not throw, which
means that, per spec, a null check on its result is required.  There are
a number of places throughout xpcom/ where we know that we are passing
non-null pointers to placement new (and receiving them as a return
value), and we are therefore doing useless work performing these null
checks.

Therefore, we should be using an operator new overload that doesn't
require the null check.  MFBT has just such an overload, so use that.

Comment 1

[Nathan Froyd [:froydnj]]

Attachment: use non-null-checking operator new in xpcom/

Comment 2

[Eric Rahm [:erahm]]

Comment on attachment 8781177 [details] [diff] [review]
use non-null-checking operator new in xpcom/

Review of attachment 8781177 [details] [diff] [review]:
-----------------------------------------------------------------

This looks good, but you're not actually removing any null checks. Maybe update the commit comment?

Comment 3

[Nathan Froyd [:froydnj]]

(In reply to Eric Rahm [:erahm] from comment #2)
> This looks good, but you're not actually removing any null checks. Maybe
> update the commit comment?

The null checks are implicitly generated by the compiler prior to the patch.

Comment 4

[Eric Rahm [:erahm]]

(In reply to Nathan Froyd [:froydnj] from comment #3)
> (In reply to Eric Rahm [:erahm] from comment #2)
> > This looks good, but you're not actually removing any null checks. Maybe
> > update the commit comment?
> 
> The null checks are implicitly generated by the compiler prior to the patch.

Ah ok, reword with that language?

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/f8d33a975228
use non-null-checked operator new in xpcom/; r=erahm

Comment 6

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

Backed out because it might have caused 32-bit linux debug crashtest assertions like https://treeherder.mozilla.org/logviewer.html#?job_id=33957663&repo=mozilla-inbound
https://hg.mozilla.org/integration/mozilla-inbound/rev/3f52630cfbe8

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/57753be3e421
use non-null-checked operator new in xpcom/; r=erahm

Comment 8

[Nathan Froyd [:froydnj]]

That landing has the nsTArray parts removed, which are probably the most beneficial part of the patch, but also the part that seems to cause crashiness. :(

Comment 9

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/57753be3e421