Last Comment Bug 765198 - WebGL crash [@mozilla::WebGLContext::ReadPixels]
: WebGL crash [@mozilla::WebGLContext::ReadPixels]
Status: RESOLVED FIXED
[asan] webgl-test-needed
: assertion, crash, regression, testcase
Product: Core
Classification: Components
Component: Canvas: WebGL (show other bugs)
: 15 Branch
: All All
: -- critical (vote)
: mozilla16
Assigned To: Benoit Jacob [:bjacob] (mostly away)
:
Mentors:
Depends on: 767006
Blocks: 658170 748266
  Show dependency treegraph
 
Reported: 2012-06-15 04:41 PDT by Christoph Diehl [:posidron]
Modified: 2012-06-22 10:35 PDT (History)
4 users (show)
See Also:
Crash Signature:
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---
affected


Attachments
testcase (1.75 KB, text/html)
2012-06-15 04:41 PDT, Christoph Diehl [:posidron]
no flags Details
callstack (5.47 KB, text/plain)
2012-06-15 04:42 PDT, Christoph Diehl [:posidron]
no flags Details
check for null pixels in readPixels (1.12 KB, patch)
2012-06-15 08:07 PDT, Benoit Jacob [:bjacob] (mostly away)
bzbarsky: review+
Details | Diff | Splinter Review
callstack-new.txt (7.53 KB, text/plain)
2012-06-19 05:37 PDT, Christoph Diehl [:posidron]
no flags Details

Description Christoph Diehl [:posidron] 2012-06-15 04:41:55 PDT
Created attachment 633476 [details]
testcase
Comment 1 Christoph Diehl [:posidron] 2012-06-15 04:42:19 PDT
Created attachment 633477 [details]
callstack
Comment 2 Scoobidiver (away) 2012-06-15 06:11:44 PDT
On Windows: bp-4163d421-8c71-4a16-b481-777092120615
Comment 3 Loic 2012-06-15 07:55:13 PDT
It's a regression, you can add 'regression' keyword.

Regression range:

m-c
good=2012-06-02
bad=2012-06-03
http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=5199196b65ec&tochange=d0ebcaa7efb5

m-i
good=2012-06-01
bad=2012-06-02
http://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=50c9995aa7d0&tochange=9abc60f44fd5

Suspected bug:
Boris Zbarsky — Bug 748266. Switch the WebGL canvas context to new DOM bindings. r=peterv
Comment 4 Benoit Jacob [:bjacob] (mostly away) 2012-06-15 08:02:53 PDT
Many thanks for the report. The crash is trivial: the testcase calls readpixels with null |pixels| argument and we crash at:

Program received signal SIGSEGV, Segmentation fault.
0x00007ffff3e2e703 in mozilla::WebGLContext::ReadPixels (this=0x3a3f2b0, x=7, y=7, width=7, height=63, format=6406, type=32820, pixels=0x0, 
    rv=...) at /hack/mozilla-central/content/canvas/src/WebGLContextGL.cpp:3856
3856        void* data = pixels->mData;
(gdb) p pixels
$1 = (mozilla::dom::ArrayBufferView *) 0x0
Comment 5 Benoit Jacob [:bjacob] (mostly away) 2012-06-15 08:07:24 PDT
Created attachment 633529 [details] [diff] [review]
check for null pixels in readPixels

Per spec, 5.14.12: If pixels is null, an INVALID_VALUE error is generated.
Comment 6 Benoit Jacob [:bjacob] (mostly away) 2012-06-15 08:11:12 PDT
Confirming the testcase doesn't crash anymore with this patch.
Comment 7 Boris Zbarsky [:bz] (TPAC) 2012-06-15 13:33:23 PDT
> Per spec, 5.14.12: If pixels is null, an INVALID_VALUE error is generated.

This should probably have a test in the test suite, if there isn't one already; our old binding code threw NS_ERROR_FAILURE in that case....
Comment 8 Boris Zbarsky [:bz] (TPAC) 2012-06-15 13:33:46 PDT
Comment on attachment 633529 [details] [diff] [review]
check for null pixels in readPixels

r=me
Comment 9 Benoit Jacob [:bjacob] (mostly away) 2012-06-15 14:41:57 PDT
http://hg.mozilla.org/integration/mozilla-inbound/rev/82c5ff778cab
Comment 10 Ryan VanderMeulen [:RyanVM] 2012-06-16 06:51:03 PDT
https://hg.mozilla.org/mozilla-central/rev/82c5ff778cab
Comment 11 Christoph Diehl [:posidron] 2012-06-19 05:37:05 PDT
Created attachment 634365 [details]
callstack-new.txt

The testcase now produces an assertion failure:

JavaScript warning: file:///765198/testcase.html, line 40: WebGL: readPixels: null destination buffer
Assertion failure: !AccessCheck::callerIsChrome(), at /Users/cdiehl/Code/Mozilla/mc-asan/js/xpconnect/wrappers/XrayWrapper.cpp:770
Comment 12 Benoit Jacob [:bjacob] (mostly away) 2012-06-19 13:56:05 PDT
I can only reproduce this crash on Win7 and on Mac; I cannot reproduce on Linux and WinXP. So I will need a bit more time than normal to debug this.
Comment 13 Benoit Jacob [:bjacob] (mostly away) 2012-06-19 14:11:18 PDT
Actually, I can't reproduce anymore on my Mac since I updated Nightly from June 11's build to today's.

Can you still reproduce in current Nightly?
Comment 14 Christoph Diehl [:posidron] 2012-06-19 14:36:39 PDT
I can reproduce it with an ASAN enabled build (trunk).
Comment 15 Benoit Jacob [:bjacob] (mostly away) 2012-06-21 09:03:35 PDT
Can't easily debug on Windows at the moment due to bug 767006.
Comment 16 Benoit Jacob [:bjacob] (mostly away) 2012-06-22 08:28:26 PDT
I can't reproduce the crash anymore in a Windows debug build from today's mozilla-central. Last week, I could reproduce. Can you still reproduce a crash or is ASAN necessary to observe any issue?
Comment 17 Christoph Diehl [:posidron] 2012-06-22 08:30:39 PDT
Yes, an ASAN build seems to be necessary.
Comment 18 Benoit Jacob [:bjacob] (mostly away) 2012-06-22 08:33:19 PDT
Can you teach me how to make an ASAN build? And then, how to reproduce with it?
Comment 19 Christoph Diehl [:posidron] 2012-06-22 08:42:45 PDT
The steps for building are described here:
https://developer.mozilla.org/en/Building_Firefox_with_Address_Sanitizer

Once you have done that, you can just open the testcase with Firefox and you will see the result in the shell.
Comment 20 Christoph Diehl [:posidron] 2012-06-22 10:35:20 PDT
Fixed. The bug is indeed fixed with a build of today even with ASAN enabled.

Note You need to log in before you can comment on or make changes to this bug.