Fixed NULL dereference in glClear....

Fixed NULL dereference in glClear.

6 crashes on Windows XP
1 crash on Windows 7

This was Canary 7.0.530.0 using ANGLE r429.

0x011f523b 	[libGLESv2.dll 	- context.cpp:2412] 	gl::Context::clear(unsigned int)
0x011e5f41 	[libGLESv2.dll 	- libglesv2.cpp:611] 	glClear
0x020c400d 	[chrome.dll 	- gl_context.cc:33] 	gfx::GLContext::InitializeCommon()
0x020bb757 	[chrome.dll 	- gl_context_egl.cc:138] 	gfx::NativeViewEGLContext::Initialize()
0x020b8946 	[chrome.dll 	- gl_context_win.cc:502] 	gfx::GLContext::CreateViewGLContext(HWND__ *,bool)
0x01c4f6f8 	[chrome.dll 	- gpu_processor_win.cc:35] 	gpu::GPUProcessor::Initialize(HWND__ *,gfx::Size const &,gpu::GPUProcessor *,unsigned int)
0x01d6e669 	[chrome.dll 	- gpu_command_buffer_stub.cc:88] 	GpuCommandBufferStub::OnInitialize(int,void * *)

It's crashing upon creation of the GL context, possibly the first context. Device lost would possibly explain the XP crashes. The Windows 7 box might have run out of video memory.

I also checked another couple of NULL dereference crashes.

[apatrick1, 2010-09-22 21:28:15]

Another patch for you Daniel.

Thanks,

Al

[dgkoch, 2010-09-24 00:15:51]

Hi Al,

These look reasonable.  

One request though, lets start adding ERR() traces to all these unexpected
behaviour checks we're adding.  If it's possible to grab the debug.txt that ends
up beside the executable, we might start being able to track down the exact
source of the problems once we've added enough traces.  This might help us to
better understand what is occurring in these cases.

Daniel

http://codereview.appspot.com/2197046/diff/1/src/libGLESv2/Context.cpp
File src/libGLESv2/Context.cpp (right):

http://codereview.appspot.com/2197046/diff/1/src/libGLESv2/Context.cpp#newcod...
src/libGLESv2/Context.cpp:1618: {
add an ERR() trace here

http://codereview.appspot.com/2197046/diff/1/src/libGLESv2/Context.cpp#newcod...
src/libGLESv2/Context.cpp:1628: {
and here

http://codereview.appspot.com/2197046/diff/1/src/libGLESv2/Context.cpp#newcod...
src/libGLESv2/Context.cpp:2429: {
and here

[apatrick1, 2010-10-04 19:45:03]

Hi Daniel,

It's been a while but I finally got back on ANGLE. I added ERR as requested.

In terms of getting back the ANGLE debug log, we won't be able to get it with
crash reports since turning it on slows things down considerably and we can't
really turn it on by default. If someone files a reproducible bug it is possible
they will be willing to rerun with a command line switch to enable logging.
That'll need changes to both Chrome and ANGLE. I'll get on it.

Al

[dgkoch, 2010-10-05 01:59:08]

LGTM.

Note that I wasn't actually suggesting that the user be able to run with full
debug traces on, as yes it will be really slow -- although that might be a very
useful option too.

I believe that ERR messages will always be written to debug.txt even in release
mode, and thus once all unexpected paths are instrumented properly we might be
able to pin-point the cause of a later crash.

[apatrick, 2010-10-05 02:24:17]

In that case I'll make the log file name configurable. It is likely to
clobber one of our own log files if the ERR macro is used! Any thoughts on
how to expose that? I could add a new entry point to libEGL.dll?

Alternatively, I could add an error callback so different projects can
customize where the log output goes. It would be cool if we could integrate
it with Chrome's existing logging system and get the ANGLE logs interspersed
with log output from the Chrome GPU process.

Al

On Mon, Oct 4, 2010 at 6:59 PM, <daniel@transgaming.com> wrote:

> LGTM.
>
> Note that I wasn't actually suggesting that the user be able to run with
> full debug traces on, as yes it will be really slow -- although that
> might be a very useful option too.
>
> I believe that ERR messages will always be written to debug.txt even in
> release mode, and thus once all unexpected paths are instrumented
> properly we might be able to pin-point the cause of a later crash.
>
>
> http://codereview.appspot.com/2197046/
>

[dgkoch, 2010-10-05 02:34:23]

If you want to go ahead and rename the debug file to something unique
(angle-debug.txt?) feel free.  I'm not sure that making it run-time configurable
is really necessary, and seems like it might be a bit complex. 

If you wanted an error call back, I'd suggest looking at
http://www.opengl.org/registry/specs/ARB/debug_output.txt  as this extension
adds exactly that.

Hope this helps,
Daniel

On 2010-10-04, at 10:24 PM, Alastair Patrick wrote:

> In that case I'll make the log file name configurable. It is likely to clobber
one of our own log files if the ERR macro is used! Any thoughts on how to expose
that? I could add a new entry point to libEGL.dll?
> 
> Alternatively, I could add an error callback so different projects can
customize where the log output goes. It would be cool if we could integrate it
with Chrome's existing logging system and get the ANGLE logs interspersed with
log output from the Chrome GPU process.
> 
> Al
> 
> On Mon, Oct 4, 2010 at 6:59 PM, <daniel@transgaming.com> wrote:
> LGTM.
> 
> Note that I wasn't actually suggesting that the user be able to run with
> full debug traces on, as yes it will be really slow -- although that
> might be a very useful option too.
> 
> I believe that ERR messages will always be written to debug.txt even in
> release mode, and thus once all unexpected paths are instrumented
> properly we might be able to pin-point the cause of a later crash.
> 
> 
> http://codereview.appspot.com/2197046/
> 

---
                        Daniel Koch -+- daniel@transgaming.com
Senior Graphics Architect -+- TransGaming Inc.  -+- www.transgaming.com