Open Bug 1186212 Opened 5 years ago Updated 2 years ago

Static analysis to find null-checks after infallible malloc

Categories

(Firefox Build System :: Source Code Analysis, defect)

42 Branch
defect
Not set

Tracking

(firefox42 affected)

Tracking Status
firefox42 --- affected

People

(Reporter: dmajor, Unassigned)

References

Details

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.
Oh, not handling this for |new SomeClass()| would eliminate tons of false positives that I was worried about in bug 1159604...
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.
> 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.
Product: Core → Firefox Build System
You need to log in before you can comment on or make changes to this bug.