Bug 52831

Summary: Crash in glDrawArrays with NaCl crystal model
Product: WebKit Reporter: Kenneth Russell <kbr>
Component: WebGLAssignee: Zhenyao Mo <zmo>
Status: RESOLVED FIXED    
Severity: Normal CC: cmarrin, gman, zmo
Priority: P2 Keywords: InRadar
Version: 528+ (Nightly build)   
Hardware: All   
OS: All   
URL: http://www.ibiblio.org/e-notes/webgl/models/NaClb5.html
Attachments:
Description Flags
Patch
none
revised patch: test added
none
Patch kbr: review+

Description Kenneth Russell 2011-01-20 12:14:47 PST
The NaCl crystal model in the link above is causing WebKit's WebGL implementation to crash in glDrawArrays, at least on an NVIDIA GeForce 8600M GT. In Safari the stack trace is as follows:

Thread 0 Crashed:  Dispatch queue: com.apple.main-thread
0   com.apple.GeForceGLDriver     	0x00000002000c24b9 glrCompExecuteKernel + 643065
1   GLEngine                      	0x000000011902ca70 glDrawArrays_ACC_Exec + 882
2   com.apple.WebCore             	0x00000001018685a1 WebCore::WebGLRenderingContext::drawArrays(unsigned int, int, int, int&) + 289
3   com.apple.WebCore             	0x00000001014bb969 WebCore::jsWebGLRenderingContextPrototypeFunctionDrawArrays(JSC::ExecState*) + 569
4   ???                           	0x00004afef92001b8 0 + 82458961772984
5   com.apple.JavaScriptCore      	0x00000001007d891c JSC::Interpreter::executeCall(JSC::ExecState*, JSC::JSObject*, JSC::CallType, JSC::CallData const&, JSC::JSValue, JSC::ArgList const&) + 780
6   ???                           	0x000000000000000a 0 + 10
7   ???                           	0x00000001188cf320 0 + 4706857760
8   com.apple.JavaScriptCore      	0x000000010083a440 JSC::JSFunction::~JSFunction() + 0
9   ???                           	0x9090909090909090 0 + 10416984888683040912

I assume the application is doing something illegal and the WebGL layer isn't catching it before handing the call down to OpenGL.

In Chromium it looks like the GPU process crashes and restarts.
Comment 1 Chris Marrin 2011-01-21 09:20:23 PST
<rdar://problem/8898613>
Comment 2 Zhenyao Mo 2011-02-02 15:32:48 PST
I'll have a look.  Can reproduce on my mac with ToT chromium and webkit.
Comment 3 Zhenyao Mo 2011-02-03 16:34:46 PST
Created attachment 81138 [details]
Patch
Comment 4 Zhenyao Mo 2011-02-03 18:48:43 PST
Created attachment 81169 [details]
revised patch: test added

Test is copied from khronos
Comment 5 Kenneth Russell 2011-02-03 19:09:03 PST
Comment on attachment 81169 [details]
revised patch: test added

What would happen if we instead disabled vertex attribute 0 as an array if the program isn't consuming it? The original reason for the simulation of vertex attribute 0 is that that attribute can't be set to a constant value in desktop GL. If the program was consuming it and it wasn't enabled as an array, draw calls would be completely ignored. If the program isn't consuming it at all, then if we silently disable it as an array behind the scenes, would draw calls using that program still generate output?
Comment 6 Zhenyao Mo 2011-02-04 05:58:02 PST
(In reply to comment #5)
> (From update of attachment 81169 [details])
> What would happen if we instead disabled vertex attribute 0 as an array if the program isn't consuming it? The original reason for the simulation of vertex attribute 0 is that that attribute can't be set to a constant value in desktop GL. If the program was consuming it and it wasn't enabled as an array, draw calls would be completely ignored. If the program isn't consuming it at all, then if we silently disable it as an array behind the scenes, would draw calls using that program still generate output?

On desktop GL we have to enable vertex attribute 0 to trigger the draw.  (An alternative is to call vertexPointer, which isn't exposed through GraphicContext3D).
Comment 7 Zhenyao Mo 2011-02-04 14:58:03 PST
Created attachment 81293 [details]
Patch
Comment 8 Zhenyao Mo 2011-02-04 15:00:12 PST
This seems to avoid crash in both attrib0+attrib1 and attrib1+attrib2 cases.  I've tested with multiple contexts in one page in Safari, and it seems to work fine.

Please review.

I am working on filing a bug report to Apple at the moment.
Comment 9 Kenneth Russell 2011-02-04 15:33:51 PST
Comment on attachment 81293 [details]
Patch

The workaround looks fine. It's unfortunate how complicated it is. The test of whether vertex attribute 0 has ever been used in this context before is disconcerting. Please do make sure a bug gets filed against Apple's OpenGL implementation so that this code might be removed at some point.
Comment 10 Zhenyao Mo 2011-02-04 17:25:08 PST
I just filed a bug report to Apple, tracking ID is 8962402.

Chris, is there anyway you could boost this bug a little bit so it gets solved sooner than later?  Even with this patch, I am worried that disaster might happen.
Comment 11 Zhenyao Mo 2011-02-07 09:31:17 PST
Committed r77821: <http://trac.webkit.org/changeset/77821>