1186212 - Static analysis to find null-checks after infallible malloc

Status: NEW

(Developer Infrastructure :: Source Code Analysis, defect)

Description

[(Away)]

There are a bunch of places in the tree where we do an infallible allocation even though the code can gracefully handle a null result. I suspect that many of them were originally null-safe but got converted to an infallible allocation through refactorings and mass-conversions.

I propose that we find callers of moz_xmalloc (and similar) that do a null-check, and convert them to fallible calls. This can help prevent "OOM | large" crashes since moz_xmalloc calls tend to have a variable size.

I don't think we need to do this for cases like |new SomeClass()| since those are usually of little consequence for OOMs.

Comment 1

[(no longer active)]

Oh, not handling this for |new SomeClass()| would eliminate tons of false positives that I was worried about in bug 1159604...

Comment 2

[Benjamin Smedberg]

The original intent of moz_xmalloc was that we very often get null-checking incorrect. So I don't think we should do this without a subject-matter expert checking each case.

In some ways I'd prefer that we use infallible by default, remove all the null checks, and make people explicitly opt in to fallible if they are confident in their error handling.

Comment 3

[(Away)]

> The original intent of moz_xmalloc was that we very often get null-checking
> incorrect. So I don't think we should do this without a subject-matter
> expert checking each case.

That's another reason why I'm not pushing to change callsites like |new SomeClass()|. I bet those are full of hazards. I think that looking at the relatively few places that call moz_xmalloc directly (with review from experts) would be a good bang per buck.