754669 - Update WebGL tests to r17794

Status: RESOLVED FIXED

(Core :: Graphics: CanvasWebGL, defect)

Description

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

Upstream is:
  https://cvs.khronos.org/svn/repos/registry/trunk/public/webgl/conformance-suites/1.0.1/

This has some important fixes since our last sync, but many of the important recently added tests are not there. If we want the most exhaustive tests, we'll have to switch back to trunk.

Try:
https://tbpl.mozilla.org/?tree=Try&rev=d9f145fc866d

Comment 1

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

Attachment: remove legacy cast to int32 in vertexAttribPointer

Needed to pass updated 1.0.1 tests.

Comment 2

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

https://tbpl.mozilla.org/?tree=Try&rev=c4e52cc8c081

Comment 3

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 626204 [details] [diff] [review]
remove legacy cast to int32 in vertexAttribPointer

r=me

Comment 4

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

The Mac oranges on this tryserver run are crashes while running conformance/misc/invalid-passed-params.html. Need a mac to test locally.

Comment 5

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

Can't reproduce on MacBook Air with 10.7. This suggests a NVIDIA-specific driver bug (the test slaves have NVIDIA).

Comment 6

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

Note that Mo at google mentioned that 10.7.4 regressed graphics drivers badly, breaking the webgl conformance suite.  Wonder if we're hitting something like that?

Comment 7

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

Nope, the crash occurs on both our 10.6 and 10.7 mac slaves.

Comment 8

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

Actually, Linux 32bit slaves crash too, in the same test. Not the 64bit ones though. Anyway, this adds more suspiscion of a NVIDIA driver bug.

Comment 9

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

Googlers -- Have you seen NVIDIA driver crashes in invalid-passed-params.html in recent revisions of 1.0.1 tests?

Comment 10

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

The last log line is from

  shouldGenerateGLError(context, context.NO_ERROR, "context.bindTexture(context.TEXTURE_2D, tex)");

So the crash occurs in the next line, which is

  shouldGenerateGLError(context, context.INVALID_VALUE, "context.texImage2D(context.TEXTURE_2D, 0, context.RGBA, -16, -16, 0, context.RGBA, context.UNSIGNED_BYTE, null)");

We should be catching negative sizes in texImage2D...

Comment 11

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

Yes we do check for negative sizes.

Comment 12

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

TL;DR: not a driver bug. Pure Mozilla bug. The reason why it was finicky to reproduce was stack corruption.

I was adding some printf debugging to push to tryserver, ran it locally just to test... and boom! Got the crash. The stack shows I was crashing in NSPR printf implementation:

(gdb) bt
#0  0x00007ffff7e92dc0 in cvt_s (ss=0x7fffffff7d80, str=0x4000 <Address 0x4000 out of bounds>, width=0, prec=-1, flags=0)
    at /hack/mozilla-central/nsprpub/pr/src/io/prprf.c:370
#1  0x00007ffff7e94429 in dosprintf (ss=0x7fffffff7d80, fmt=0x7ffff5a55fb2 ": width and height must be >= 0", ap=0x7fffffff8248)
    at /hack/mozilla-central/nsprpub/pr/src/io/prprf.c:998
#2  0x00007ffff7e94bc9 in PR_vsnprintf (out=0x7fffffff7e10 "PUO\312\377\177", outlen=1024, 
    fmt=0x7ffff5a55fb0 "%s: width and height must be >= 0", ap=0x7fffffff8248) at /hack/mozilla-central/nsprpub/pr/src/io/prprf.c:1202
#3  0x00007ffff3d5196c in mozilla::WebGLContext::GenerateWarning (this=0x7fffdec2d400, 
    fmt=0x7ffff5a55fb0 "%s: width and height must be >= 0", ap=0x7fffffff8248)
    at /hack/mozilla-central/content/canvas/src/WebGLContextUtils.cpp:56
#4  0x00007ffff3d51d8e in mozilla::WebGLContext::ErrorInvalidValue (this=0x7fffdec2d400, 
    fmt=0x7ffff5a55fb0 "%s: width and height must be >= 0") at /hack/mozilla-central/content/canvas/src/WebGLContextUtils.cpp:144
#5  0x00007ffff3d55796 in mozilla::WebGLContext::ValidateLevelWidthHeightForTarget (this=0x7fffdec2d400, target=3553, level=0, 
    width=-16, height=-16, info=0x7ffff5a50fcb "texImage2D") at /hack/mozilla-central/content/canvas/src/WebGLContextValidate.cpp:436
#6  0x00007ffff3d4b1d3 in mozilla::WebGLContext::TexImage2D_base (this=0x7fffdec2d400, target=3553, level=0, internalformat=6408, 
    width=-16, height=-16, srcStrideOrZero=0, border=0, format=6408, type=5121, data=0x0, byteLength=0, jsArrayType=-1, 
    srcFormat=mozilla::WebGLTexelConversions::Auto, srcPremultiplied=false)
    at /hack/mozilla-central/content/canvas/src/WebGLContextGL.cpp:5582


And the cause is a regression from bug 728017 which refactored how we do this validation: the new code is:

    if (width < 0 || height < 0) {
        ErrorInvalidValue("%s: width and height must be >= 0");
        return false;
    }

Spot the bug? Printf-like call with a %s but without matching extra argument!

This would explain why the stacks we got were useless.

Comment 13

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

Attachment: fix stack corruption

New try:
https://tbpl.mozilla.org/?tree=Try&rev=2e38b791857d

Comment 14

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

Green \o/

Comment 15

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

Landed the stack corruption fix on a CLOSED TREE as somehow, this preexisting stack corruption started causing crashes with my earlier push today.

https://hg.mozilla.org/integration/mozilla-inbound/rev/2a421f162401

Keep open for the other patches, which I'll land once the tree reopens.

Comment 16

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

Remaining patches:

http://hg.mozilla.org/integration/mozilla-inbound/rev/62ca6f4bd8f0
http://hg.mozilla.org/integration/mozilla-inbound/rev/19dbf7300434

Can close this bug now!

Comment 17

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/62ca6f4bd8f0
https://hg.mozilla.org/mozilla-central/rev/19dbf7300434

Comment 18

[Ed Morley [:emorley]]

And the first changeset:
https://hg.mozilla.org/mozilla-central/rev/2a421f162401