Last Comment Bug 728656 - Crash @mozilla::gl::GLContext::InitExtensions
: Crash @mozilla::gl::GLContext::InitExtensions
Status: RESOLVED FIXED
[qa!:esr10]
: crash
Product: Core
Classification: Components
Component: Graphics (show other bugs)
: Trunk
: All Linux
: -- critical (vote)
: mozilla13
Assigned To: Mike Hommey [:glandium]
:
Mentors:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2012-02-19 02:03 PST by Mike Hommey [:glandium]
Modified: 2012-03-05 04:42 PST (History)
4 users (show)
See Also:
Crash Signature:
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---
affected
fixed
fixed
11+
verified
unaffected


Attachments
Avoid crashing when there are no GL extensions reported by the GL implementation (1.67 KB, patch)
2012-02-19 02:04 PST, Mike Hommey [:glandium]
jacob.benoit.1: review+
Details | Diff | Splinter Review
Avoid crashing when there are no GL extensions reported by the GL implementation (796 bytes, patch)
2012-02-19 23:02 PST, Mike Hommey [:glandium]
lukasblakk+bugs: approval‑mozilla‑aurora+
lukasblakk+bugs: approval‑mozilla‑beta+
lukasblakk+bugs: approval‑mozilla‑esr10+
Details | Diff | Splinter Review

Description Mike Hommey [:glandium] 2012-02-19 02:03:09 PST
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] 2012-02-19 02:04:49 PST
Created attachment 598640 [details] [diff] [review]
Avoid crashing when there are no GL extensions reported by the GL implementation
Comment 2 Benoit Jacob [:bjacob] (mostly away) 2012-02-19 15:34:06 PST
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] 2012-02-19 22:48:48 PST
(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] 2012-02-19 23:02:26 PST
Created attachment 598777 [details] [diff] [review]
Avoid crashing when there are no GL extensions reported by the GL implementation

Refreshed to only contain the relevant part
Comment 6 Ed Morley [:emorley] 2012-02-21 08:54:34 PST
https://hg.mozilla.org/mozilla-central/rev/356382604d2d
Comment 7 Mike Hommey [:glandium] 2012-02-25 01:47:49 PST
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 2012-02-27 16:10:59 PST
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 10 Simona B [:simonab] 2012-03-05 04:42:26 PST
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.

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