Bug 52831 – Crash in glDrawArrays with NaCl crystal model

• Component: WebGL
• Status: RESOLVED FIXED

[Kenneth Russell]

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.

[Chris Marrin]

<rdar://problem/8898613>

[Zhenyao Mo]

I'll have a look.  Can reproduce on my mac with ToT chromium and webkit.

[Zhenyao Mo]

Created attachment 81138 [details]
Patch

[Zhenyao Mo]

Created attachment 81169 [details]
revised patch: test added

Test is copied from khronos

[Kenneth Russell]

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?

[Zhenyao Mo]

(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).

[Zhenyao Mo]

Created attachment 81293 [details]
Patch

[Zhenyao Mo]

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.

[Kenneth Russell]

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.

[Zhenyao Mo]

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.

[Zhenyao Mo]

Committed r77821: <http://trac.webkit.org/changeset/77821>