Last Comment Bug 653419 - WebGL crash [@ mozilla::WebGLContext::GetCanvasLayer(nsDisplayListBuilder*, mozilla::layers::CanvasLayer*, mozilla::layers::LayerManager*) ]
: WebGL crash [@ mozilla::WebGLContext::GetCanvasLayer(nsDisplayListBuilder*, m...
Status: VERIFIED FIXED
:
Product: Core
Classification: Components
Component: Canvas: WebGL (show other bugs)
: 2.0 Branch
: All Windows 7
: -- critical (vote)
: ---
Assigned To: Benoit Jacob [:bjacob] (mostly away)
:
: Milan Sreckovic [:milan]
Mentors:
https://cvs.khronos.org/svn/repos/reg...
Depends on:
Blocks: 655589 656215
  Show dependency treegraph
 
Reported: 2011-04-28 06:39 PDT by Virtual_ManPL [:Virtual] - (ni? me)
Modified: 2016-04-15 14:49 PDT (History)
7 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---
fixed


Attachments
handle null gl pointer (1017 bytes, patch)
2011-04-28 06:55 PDT, Benoit Jacob [:bjacob] (mostly away)
joe: review-
Details | Diff | Splinter Review
correctly mark WebGL context creation as successful (1.20 KB, patch)
2011-04-28 07:00 PDT, Benoit Jacob [:bjacob] (mostly away)
no flags Details | Diff | Splinter Review
correctly mark WebGL context creation as successful (1.38 KB, patch)
2011-04-28 07:13 PDT, Benoit Jacob [:bjacob] (mostly away)
joe: review+
asa: approval‑mozilla‑aurora+
dveditz: approval2.0-
Details | Diff | Splinter Review
check for null gl ptr in SetDimensions NOP path (804 bytes, patch)
2011-04-28 07:15 PDT, Benoit Jacob [:bjacob] (mostly away)
joe: review+
asa: approval‑mozilla‑aurora+
dveditz: approval2.0-
Details | Diff | Splinter Review

Description Virtual_ManPL [:Virtual] - (ni? me) 2011-04-28 06:39:26 PDT
User-Agent:       Mozilla/5.0 (Windows NT 6.1; Win64; x64; rv:6.0a1) Gecko/20110427 Firefox/6.0a1
Build Identifier: 

Crashlogs:
https://crash-stats.mozilla.com/report/index/551ae92e-7c08-4a77-ad99-c3bc62110428
https://crash-stats.mozilla.com/report/index/d53c4e0b-232f-4202-bbb2-971e52110428
https://crash-stats.mozilla.com/report/index/ceabb40e-799a-4271-9aa9-110d52110428
https://crash-stats.mozilla.com/report/index/fc63c5ef-f2aa-466a-b4c1-cf5c82110428

Graphic info
Adapter Description - NVIDIA GeForce 8600 GT
Vendor ID - 10de
Device ID - 0402
Adapter RAM - 256
Adapter Drivers - nvd3dumx,nvwgf2umx,nvwgf2umx nvd3dum,nvwgf2um,nvwgf2um
Driver Version - 8.17.12.7061
Driver Date - 4-7-2011
Direct2D Enabled - true
DirectWrite Enabled - true (6.1.7601.17563)
WebGL Renderer - NVIDIA Corporation -- GeForce 8600 GT/PCI/SSE2 -- 3.3.0
GPU Accelerated Windows - 1/1 Direct3D 10

Reproducible: Always
Comment 1 Benoit Jacob [:bjacob] (mostly away) 2011-04-28 06:55:01 PDT
Created attachment 528837 [details] [diff] [review]
handle null gl pointer
Comment 2 Benoit Jacob [:bjacob] (mostly away) 2011-04-28 07:00:41 PDT
Created attachment 528838 [details] [diff] [review]
correctly mark WebGL context creation as successful
Comment 3 Benoit Jacob [:bjacob] (mostly away) 2011-04-28 07:13:33 PDT
Created attachment 528846 [details] [diff] [review]
correctly mark WebGL context creation as successful
Comment 4 Benoit Jacob [:bjacob] (mostly away) 2011-04-28 07:15:22 PDT
Created attachment 528847 [details] [diff] [review]
check for null gl ptr in SetDimensions NOP path
Comment 5 Benoit Jacob [:bjacob] (mostly away) 2011-04-28 07:57:37 PDT
OK, I don't fully understand this bug, here's what I know.

There are at least 2 bugs here.

1) The first bug is the crash. It is a read at address 0 at this line of code:

void* native_surface = gl->GetNativeData(gl::GLContext::NativeImageSurface);

Since GetNativeData is a virtual method, that crash almost certainly means that the gl pointer is null. The 'handle null gl pointer' patch is a bruteforce fix for it, the 'check for null gl ptr in SetDimensions NOP path' patch is an attempt at a smart fix (see below).

2) The second bug is that the AppNotes entries for WebGL are unreliable because of a bug, which is fixed by the 'correctly mark WebGL context creation as successful' patch. So in any case I would like this patch to go it, it will at least allow us to get accurate crash reports.


Here's what I don't know:

The AppNotes say:
AdapterVendorID: 10de, AdapterDeviceID: 0402, AdapterDriverVersion: 8.17.12.7061
D2D? D2D+
DWrite? DWrite+
D3D10 Layers? D3D10 Layers+
WebGL? EGL? EGL-
WGL? GL Context? GL Context+
WGL+
WebGL+
WebGL-

There are 2 ways to interprete that.

1) one way is to say that the last WebGL- is produced by the above-discussed bug i.e. there never was a WebGL context creation failure, there just was a SetDimensions call that returned early here:
    if (mWidth == width && mHeight == height)
        return NS_OK;
The bug (null gl pointer) could the be explained in the scenario of a race condition between destroying and resizing a WebGL context.

2) another way is to take the WebGL- seriously, despite the above-discussed bug which can produce bogus WebGL-. One thing that pleads for this is the EGL-. Something is really wrong on this machine: despite using a recent NVIDIA driver, it failed to create a ANGLE EGL context. So it tried WGL as fallback. That worked once and, under this hypothesis, failed the second time. It is still not clear how we ended up calling GetCanvasLayer on a failed WebGL context object.
Comment 6 Benoit Jacob [:bjacob] (mostly away) 2011-04-28 08:00:14 PDT
Ah! Didn't see that this is a 64bit build. That explains why EGL fails.

I suggest taking the 2nd and 3rd patch; perhaps we can pass on the 1st patch for now.
Comment 7 Benoit Jacob [:bjacob] (mostly away) 2011-04-28 08:00:45 PDT
@ Virtual_ManPL: 64bit builds on Windows are not supported yet.
Comment 8 Virtual_ManPL [:Virtual] - (ni? me) 2011-04-28 09:29:39 PDT
(In reply to comment #7)
> @ Virtual_ManPL: 64bit builds on Windows are not supported yet.

I know that "officially" 64bit builds of Firefox for Windows aren't supported yet, but someone need to test them and report bugs, so you guys will have less job to do in the future and we will have fewer bugs :)
Comment 9 Raul Malea 2011-04-28 10:26:24 PDT
Confirm this bug. Tested in Firefox 4 in default profile.

My reports:

https://crash-stats.mozilla.com/report/index/bp-05197bea-07b7-4e1e-a05e-eb0152110428

https://crash-stats.mozilla.com/report/index/1a3aac84-4f87-4c72-9a2f-ec2442110428

Mozilla/5.0 (Windows NT 6.1; rv:2.0) Gecko/20100101 Firefox/4.0 ID:20110318052756

Graphics:

Descrierea adaptorului NVIDIA GeForce 8600 GT 
ID furnizor 10de
ID dispozitiv 0402
RAM pentru adaptor 512
Drivere pentru adaptor  nvd3dum nvwgf2um,nvwgf2um
Versiune driver  8.17.12.6724
Dată driver 2-23-2011
Direct2D activat  true
DirectWrite activa  ttrue (6.1.7601.17563, font cache n/a)
Motor de afișare WebGL Google Inc. -- ANGLE -- OpenGL ES 2.0 (ANGLE 0.0.0.541)
Accelerare GPU Windows 1/1 Direct3D 10
      
      
      
        
        

My addons: 
Abduction! 3.0.10
About startup 0.1.5
about:me 0.5
about:response 0.1
Add-on Collector 2.0.1
Add-on Compatibility Reporter 0.8.3
AddonFox 1.1.7
Adobe Acrobat 10.0.1.434
Adobe Contribute Toolbar 6.0 [DISABLED]
Blog Comment Autofill 
British English Dictionary 1.19.1
Bugzilla Tweaks 1.10
ChatZilla 0.9.86.1
Converter 1.0.9
Crash Report Helper 1.2 [DISABLED]
Default 4.0 [DISABLED]
Dicționar românesc de corectare ortografică. 1.11
Download Statusbar 0.9.8
DownloadHelper 4.8.6
DownThemAll! 2.0.2
F1 by Mozilla Labs 0.8.3
FFixer 2.3.1
FileList V3 
fireblur 1260925626
Firefox 4 Menu Button with icon+(Tabs in Titlebar) 
Firefox B 1260925626 [DISABLED]
Flagfox 4.1.2
FlashGames Maximizer 3.1.3.86 [DISABLED]
FlashGot 1.2.9.3
FoxTab 1.4.2
German Dictionary 2.0.2
Google Earth Plugin 1.0.0.1
Google Shortcuts 2.1.6
Google Talk Plugin 1.9.2.0
Google Talk Plugin Video Accelerator 0.1.43.5
Google Update 1.3.21.53
Greasefire 1.0.4 [DISABLED]
Greasemonkey 0.9.2
Hard blockers counter 1.0.1 [DISABLED]
IE Tab 2 (FF 3.6+) 2.12.21.1
IE Tab Plug-in 2.2.0.1
ImTranslator 4.01
Java Deployment Toolkit 6.0.240.7 6.0.240.7
Java(TM) Platform SE 6 U24 6.0.240.7
Massive Extender 1.0b5
Microsoft Office 2010 14.0.4730.1010
Microsoft Office 2010 14.0.4761.1000
Minefield - Better Add-on Bar/Status Bar  [DISABLED]
Movie Torrent Search Linker (for IMDB, Rotten Tomatoes and Yahoo Movies) 0.3.7.9
Mozilla Labs: Prospector - Home Dash 10 [DISABLED]
Mozilla QA Companion 1.2.3
MozMill Crowd 0.1.2
Nightly and Aurora 1303328422 [DISABLED]
Nightly Tester Tools 3.1.6
NoScript 2.1.0.2
NVIDIA 3D Vision 7.17.12.6099
NVIDIA 3D VISION 7.17.12.6099
Personas 1.6.2
Picasa 3.0.0.0
Power Twitter 1.60
Prism for Firefox 1.0b4 [DISABLED]
Puppy Dogs... 1291695684 [DISABLED]
QuickTime Plug-in 7.6.9 7.6.9.0
Read It Later 2.1.1
RealJukebox NS Plugin 12.0.1.647
RealNetworks(tm) RealPlayer Chrome Background Extension Plug-In (32-bit)  12.0.1.647
RealPlayer Version Plugin 12.0.1.647
RealPlayer(tm) G2 LiveConnect-Enabled Plug-In (32-bit)  12.0.1.647
RealPlayer(tm) HTML5VideoShim Plug-In (32-bit)  12.0.1.647
ReminderFox 1.9.9.3.1
Sage 1.4.10
Shockwave Flash 10.2.159.1
Shockwave for Director 11.5.9.620
Show Just Image 2 2.5.3
Silverlight Plug-In 5.0.60401.0
startup.service 0.2
Status-4-Evar 2011.04.06.18
StumbleUpon 3.81
Stylish 1.1.2
Test Pilot 1.1
TortoiseSVN Menu 0.2.5
Ubiquity 0.6 [DISABLED]
URL Fixer 2.0.3
Userscripts Updater 
Windows Live™ Photo Gallery 15.4.3508.1109
WiseStamp 2.2.1
Yahoo Application State Plugin 1.0.0.7
Yoono 7.6.5
YouTube Enhancer 2010-12-18
Comment 10 Benoit Jacob [:bjacob] (mostly away) 2011-04-28 10:39:49 PDT
Raul: this seems like a separate bug, it would be a good idea to file it separately. Indeed your Firefox survives the line that's crashy in the original report here. That means that gl!=null in your case. The line that's crashy for you is
    HRESULT hr = mTexture->Map(0, D3D10_MAP_WRITE_DISCARD, 0, &map);
in CanvasLayerD3D10::Updated(). Could well be that the mTexture pointer was null.
Comment 11 Raul Malea 2011-04-28 10:43:14 PDT
Also confirmed in Nightly. Mozilla/5.0 (Windows NT 6.1; rv:6.0a1) Gecko/20110428 Firefox/6.0a1 ID:20110428030557

Crash reports:

https://crash-stats.mozilla.com/report/index/8257dd4e-5e84-4b40-8707-5053d2110428

https://crash-stats.mozilla.com/report/index/aaf756f6-7d42-48b8-8f48-8697e2110428
Comment 12 Raul Malea 2011-04-28 10:44:22 PDT
(In reply to comment #10)
> Raul: this seems like a separate bug, it would be a good idea to file it
> separately. Indeed your Firefox survives the line that's crashy in the original
> report here. That means that gl!=null in your case. The line that's crashy for
> you is
>     HRESULT hr = mTexture->Map(0, D3D10_MAP_WRITE_DISCARD, 0, &map);
> in CanvasLayerD3D10::Updated(). Could well be that the mTexture pointer was
> null.

Oh! Not same bug? Sorry.
Comment 13 Raul Malea 2011-04-28 10:46:40 PDT
And also, crash report for my Nighlty is different bug?
Comment 14 novacrystallis 2011-04-28 14:33:13 PDT
I was the one that had problems originally on this and reported on MozillaZine. Here are my crash reports. Had earlier updated it to Catalyst 11.4 drivers.

https://crash-stats.mozilla.com/report/index/bp-41306687-9555-4e25-acfc-d77b72110427
https://crash-stats.mozilla.com/report/index/bp-6bcabeaf-8ada-48f7-9309-c1e222110428
Comment 16 Johnathan Nightingale [:johnath] 2011-05-03 14:45:20 PDT
Definitely want to fix this, but Firefox 5 isn't supported on win64, so this doesn't need tracking-firefox5
Comment 17 Joe Drew (not getting mail) 2011-05-05 12:53:16 PDT
Comment on attachment 528837 [details] [diff] [review]
handle null gl pointer

Review of attachment 528837 [details] [diff] [review]:

I'm not really in favour of adding null checks at seemingly arbitrary points in the code, and, as Benoit says in comment 6, we probably don't need this patch.
Comment 19 Benoit Jacob [:bjacob] (mostly away) 2011-05-06 13:27:57 PDT
Comment on attachment 528847 [details] [diff] [review]
check for null gl ptr in SetDimensions NOP path

These two patches are low-risk; one fixes a serious crasher (not just on Win64), the other one improves the quality of crash reports. Recommending for both Firefox 4 and Aurora/5.
Comment 20 Virtual_ManPL [:Virtual] - (ni? me) 2011-05-08 07:33:43 PDT
Verified on
Mozilla/5.0 (Windows NT 6.1; Win64; x64; rv:6.0a1) Gecko/20110507 Firefox/6.0a1

But now I get other crash.
bug #655589
Comment 21 Benoit Jacob [:bjacob] (mostly away) 2011-05-08 07:43:40 PDT
Replying on bug 655589: it seems that you're still not using a build of Firefox that has these fixes.
Comment 22 Virtual_ManPL [:Virtual] - (ni? me) 2011-05-10 13:15:29 PDT
Odd, it still happens in
Mozilla/5.0 (Windows NT 6.1; Win64; x64; rv:6.0a1) Gecko/20110510 Firefox/6.0a1

https://crash-stats.mozilla.com/report/index/bp-e2f2976c-4de2-4960-a3d5-54a9d2110510
Comment 23 Benoit Jacob [:bjacob] (mostly away) 2011-05-10 14:39:20 PDT
Can you please go to about:buildconfig and paste the "built from" link?
Comment 24 Virtual_ManPL [:Virtual] - (ni? me) 2011-05-10 14:47:56 PDT
Sure.

http://hg.mozilla.org/mozilla-central/rev/e0f6db50231f
Comment 25 Benoit Jacob [:bjacob] (mostly away) 2011-05-10 15:15:21 PDT
Looks like a separate bug, though it could have caused the first bug that we fixed here.

You're crashing at:

if ((value = GetParsedAttr(nsGkAtoms::height)) &&
    value->Type() == nsAttrValue::eInteger)
{
    size.height = value->GetIntegerValue();  // <-- crash, deref null
} 

If that crash were caused by value being null, then you should have crashed above, at value->Type() already. Indeed, neither of the methods here is virtual.

So I rather think that GetIntegerValue() was inlined here (you're running an optimized build) and you're crashing inside of it:

inline PRInt32
nsAttrValue::GetIntegerValue() const
{
  NS_PRECONDITION(Type() == eInteger, "wrong type");
  return (BaseType() == eIntegerBase)
         ? GetIntInternal()
         : GetMiscContainer()->mInteger;
}

The crash could well be explained if for example GetMiscContainer() were returning null.

In any case please file this as a separate bug, against Canvas:2D, and please CC me.
Comment 26 Benoit Jacob [:bjacob] (mostly away) 2011-05-10 15:16:31 PDT
Well, you can file as Canvas:WebGL if you want. I dont know what component is appropriate for generic HTMLCanvasElement code.
Comment 27 JP Rosevear [:jpr] 2011-05-12 14:45:18 PDT
Please land for Aurora by Monday May 16 or the approval will potentially be lost.  Please mark as status-firefox5 fixed when you do.
Comment 29 Daniel Veditz [:dveditz] 2011-06-03 16:37:24 PDT
Comment on attachment 528847 [details] [diff] [review]
check for null gl ptr in SetDimensions NOP path

Since we don't plan another 2.0-based release I assume you don't still want these approvals

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