728656 - Crash @mozilla::gl::GLContext::InitExtensions

Status: RESOLVED FIXED

(Core :: Graphics, defect)

Description

[Mike Hommey [:glandium]]

I got a couple reports in Debian with the following stack trace:
#0  __strlen_sse2 () at ../sysdeps/x86_64/multiarch/../strlen.S:31
#1  0x00007ffff73c5876 in *__GI___strdup (s=0x0) at strdup.c:42
#2  0x00007ffff5748e96 in mozilla::gl::GLContext::InitExtensions (this=0x7fffc90f7800)
    at /tmp/buildd/iceweasel-9.0.1/gfx/thebes/GLContext.cpp:448
#3  0x00007ffff574a507 in mozilla::gl::GLContext::InitWithPrefix (this=0x7fffc90f7800, prefix=<value optimized out>, 
    trygl=<value optimized out>) at /tmp/buildd/iceweasel-9.0.1/gfx/thebes/GLContext.cpp:374
#4  0x00007ffff5757d72 in mozilla::gl::GLContextGLX::Init (format=<value optimized out>, display=0x7ffff6d96000, 
    drawable=<value optimized out>, cfg=<value optimized out>, vinfo=<value optimized out>, shareContext=0x7fffcaab0800, 
    deleteDrawable=<value optimized out>, pixmap=0x7fffcb5b6d80)
    at /tmp/buildd/iceweasel-9.0.1/gfx/thebes/GLContextProviderGLX.cpp:730

The code looks like this (in that particular version):

 443 void
 444 GLContext::InitExtensions()
 445 {
 446     MakeCurrent();
 447     const GLubyte *extensions = fGetString(LOCAL_GL_EXTENSIONS);
 448     char *exts = strdup((char *)extensions);

The problem is that fGetString(LOCAL_GL_EXTENSIONS) returns NULL, and strdup crashes when given a NULL argument.

Comment 1

[Mike Hommey [:glandium]]

Attachment: Avoid crashing when there are no GL extensions reported by the GL implementation

Comment 2

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

Comment on attachment 598640 [details] [diff] [review]
Avoid crashing when there are no GL extensions reported by the GL implementation

Review of attachment 598640 [details] [diff] [review]:
-----------------------------------------------------------------

r=me with this caveat:

::: dom/base/nsGlobalWindowCommands.cpp
@@ +66,5 @@
>  #include "nsIClipboardDragDropHookList.h"
>  
>  using namespace mozilla;
>  
> +static const char sSelectAllString[] = "cmd_selectAll";

That unrelated hunk should be handled separately.

Comment 3

[Mike Hommey [:glandium]]

(In reply to Benoit Jacob [:bjacob] from comment #2)
> > +static const char sSelectAllString[] = "cmd_selectAll";
> 
> That unrelated hunk should be handled separately.

That wasn't meant to be there.

Comment 4

[Mike Hommey [:glandium]]

Attachment: Avoid crashing when there are no GL extensions reported by the GL implementation

Refreshed to only contain the relevant part

Comment 5

[Mike Hommey [:glandium]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/356382604d2d

Comment 6

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/356382604d2d

Comment 7

[Mike Hommey [:glandium]]

Comment on attachment 598777 [details] [diff] [review]
Avoid crashing when there are no GL extensions reported by the GL implementation

[Approval Request Comment]
User impact if declined: Firefox may crash when the system GL libraries provide no extensions
Risk to taking this patch (and alternatives if risky): It's a simple NULL check. No risk.
String changes made by this patch: None

Comment 8

[Lukas Blakk [:lsblakk] use ?needinfo]

Comment on attachment 598777 [details] [diff] [review]
Avoid crashing when there are no GL extensions reported by the GL implementation

[Triage Comment]
please land this today if possible (02/27/12) for tomorrow's go-to-build on beta5 and also land on mozilla-esr10 branch before Thursday March 1, 2012 in preparation for March 2 go-to-build on esr. 

See https://wiki.mozilla.org/Release_Management/ESR_Landing_Process for details

Comment 9

[Mike Hommey [:glandium]]

http://hg.mozilla.org/releases/mozilla-aurora/rev/6061fc798602
http://hg.mozilla.org/releases/mozilla-beta/rev/db9e90094ab8
http://hg.mozilla.org/releases/mozilla-esr10/rev/7b68d8316c3d

Comment 10

[Simona Badau, Desktop QA]

Mozilla/5.0 (Windows NT 6.1; rv:10.0.3) Gecko/20100101 Firefox/10.0.3

No new crash reports having the signature: [@ strlen | je_strdup | mozilla::gl::GLContext::InitExtensions()] appear in Socorro after the patch landed.
Marking this as Verified on Firefox 10.0.3 ESR.