592737 - vertexAttrib3fv crash [@JSObject::getClass]

Status: RESOLVED FIXED

(Core :: Graphics: CanvasWebGL, defect)

Description

[Christoph Diehl [:posidron]]

Attachment: callstack

vertexAttrib3fv(0, null);

The parameter 'null' causes a null pointer dereference.

Required parameter type: Float32Array | sequence

Comment 1

[Benoit Jacob [:bjacob] (mostly away)]

Attachment: Fix bug 592737

OK, let helper_isInt32Array and helper_isFloat32Array check if obj is null, and return false in that case.

Comment 2

[Vladimir Vukicevic [:vlad] [:vladv] (needinfo me, slow to respond)]

Comment on attachment 471170 [details] [diff] [review]
Fix bug 592737

This actually isn't enough -- JS_IsArrayObject will crash if given a null argument, so we'd just move the crash one branch down in the if() statement.  Better would be to just null check the arg right after it's pulled out of the args array.  In some cases this involves adding an additional brahc, in others it means changing if (!JSVAL_IS_OBJECT(x)) -> if (JSVAL_IS_PRIMITIVE(x))

Comment 3

[Benoit Jacob [:bjacob] (mostly away)]

Attachment: Fix bug 592737

OK, here's the new patch.

By the way, these were the last occurences of !JSVAL_IS_OBJECT in this file.

This should prevent crashing, but I wonder if that is spec-compliant: in case of a null object, shouldn't these methods just generate ERROR_INVALID_VALUE and let the JS continue?

Comment 4

[Vladimir Vukicevic [:vlad] [:vladv] (needinfo me, slow to respond)]

Comment on attachment 471204 [details] [diff] [review]
Fix bug 592737

looks fine; not sure about the GL error though -- maybe?

Comment 5

[Benoit Jacob [:bjacob] (mostly away)]

Anyway, that would be nontrivial to fix as we can't generate GL errors from quickstubs. Sweeping this one under the carpet...

Will land the fix tomorrow morning (don't want to spend tonight babysitting tinderbox...)

Comment 6

[Benoit Jacob [:bjacob] (mostly away)]

Please mark as blocking so I can land this tomorrow.

Comment 7

[Benoit Jacob [:bjacob] (mostly away)]

http://hg.mozilla.org/mozilla-central/rev/886665dec3cb