Static analysis to find null-checks after infallible malloc

NEW
Unassigned

Status

()

Core
Rewriting and Analysis
2 years ago
2 years ago

People

(Reporter: dmajor, Unassigned)

Tracking

42 Branch
Points:
---

Firefox Tracking Flags

(firefox42 affected)

Details

(Reporter)

Description

2 years ago
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

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

Comment 2

2 years ago
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.
(Reporter)

Comment 3

2 years ago
> 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.
You need to log in before you can comment on or make changes to this bug.