The interpreter engine for the core JavaScript language, independent of the browser's object model. File ONLY core JavaScript language bugs in this category. For bugs involving browser objects such as "window" and "document", use the "DOM" component. For bugs involving calls between JavaScript and C++, use the "XPConnect" component.

Created attachment 481883[details][diff][review]
Fix malloc(), calloc() callers.
Grep and I noticed a few places in TraceMonkey where allocation functions are used without verifying that non-NULL has been returned. This fixes those locations:
ethogram_construct(), js_dtobasestr(), js_DumpOpMeters(), TraceRecorder::compile() [debug], TypedArrayTemplate::copyFromWithOverlap().
In the case of malloc failure in js_dtobasestr(), the code also dereferenced an uninitialized address. The fix checks for malloc failure and removes an unnecessary indentation level.
TypedArrayTemplate::copyFromWithOverlap(), and its caller ::copyFrom(), were made fallible.
js_ConcatStringsZ() was removed on approval from cdleary.

Good catch! But since prevention is better than cure, can we take a leaf out of posix_memalign()'s book and change the signature of our allocators to this?
bool js_calloc(size_t bytes, void** p);
That makes it much harder to forget to check for failure.

>+ if (!copyFrom(tarray))
>+ return false;
Two places suggests making copyFrom take a leading cx and do its own reporting.
> That makes it much harder to forget to check for failure.
People can ignore a return value no matter what. I don't see why we should pay the cost of an out param here.
Rather, let's have a static analysis to check for allocator return values not null-checked. And separately, get to the infallible (small) alloc promised land, which will reduce the bug surface quite a bit.
/be

(In reply to comment #3)
>
> And separately, get to the infallible (small) alloc promised
> land, which will reduce the bug surface quite a bit.
Oh, I thought that was never going to happen in js/. How will it happen? Where can I sign up?

(In reply to comment #4)
> (In reply to comment #3)
> >
> > And separately, get to the infallible (small) alloc promised
> > land, which will reduce the bug surface quite a bit.
>
> Oh, I thought that was never going to happen in js/.
Why not?
> How will it happen? Where can I sign up?
It's part of the e10s plan, which will not be implemented fully (i.e., reliably) for Firefox 4 / Mozilla 2. See bug 599791 comment 19 and bug 521309 comment 11, among others. Cc'ing cjones.
/be

(In reply to comment #3)
> People can ignore a return value no matter what.
While this is certainly true, we could partially help ourselves here by adding __attribute__((warn_unused_result)) (macroized) to some of these functions (although not the ones that return a useful value like a maybe-NULL pointer). With this, at least on gcc-using platforms, we'd get compiler warnings calling such functions without using their return values.

(In reply to comment #8)
> (In reply to comment #3)
> > People can ignore a return value no matter what.
>
> While this is certainly true, we could partially help ourselves here by adding
> __attribute__((warn_unused_result)) (macroized) to some of these functions
> (although not the ones that return a useful value like a maybe-NULL pointer).
> With this, at least on gcc-using platforms, we'd get compiler warnings calling
> such functions without using their return values.
Good idea -- file a bug?
/be