Closed Bug 1379995 Opened 7 years ago Closed 7 years ago

Crash in mozilla::ScopedDrawHelper::ScopedDrawHelper

Categories

(Core :: Graphics: CanvasWebGL, defect, P3)

x86
Windows 10
defect

Tracking

()

RESOLVED FIXED
mozilla57
Tracking Status
firefox-esr52 --- unaffected
firefox55 --- unaffected
firefox56 --- fixed
firefox57 --- fixed

People

(Reporter: marcia, Assigned: jerry)

References

(Blocks 1 open bug)

Details

(Keywords: crash, Whiteboard: gfx-noted)

Crash Data

Attachments

(4 files)

This bug was filed from the Socorro interface and is 
report bp-c1230620-8679-42fa-a151-c969d0170710.
=============================================================

Seen while looking at crash stats - crashes started using 20170708030206: http://bit.ly/2u9WwSu

Possible regression range based on Build ID: https://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=a0b5515b13ebfd3220a06fd1cd31f98a9557f031&tochange=06c1a0dc0fd8719ffa2d9aa2de75f32de5657102
All the crash reasons are EXCEPTION_ACCESS_VIOLATION_READ with small address 0x3C, this crash might be caused by null accessing to attribData in [1].

[1] https://hg.mozilla.org/mozilla-central/annotate/06c1a0dc0fd8/dom/canvas/WebGLContextDraw.cpp#l431
Has Regression Range: --- → yes
Has STR: --- → no
Priority: -- → P3
Whiteboard: gfx-noted
There is a STR in Bug 1379939 comment 0 .
Has STR: no → yes
Blocks: 1388995
Assignee: nobody → hshih
Status: NEW → ASSIGNED
Attachment #8900233 - Attachment is obsolete: true
Attachment #8900233 - Flags: review?(jgilbert)
Comment on attachment 8900233 [details]
Bug 1379995 - reset the mBufferFetchingIsVerified flag after the webgl deleteBuffer call.

https://reviewboard.mozilla.org/r/171608/#review177242
Attachment #8900233 - Flags: review?(jgilbert) → review+
Comment on attachment 8900234 [details]
Bug 1379995 - test case for webgl drawArray() call.

https://reviewboard.mozilla.org/r/171610/#review177244
Attachment #8900234 - Flags: review?(jgilbert) → review+
Pushed by hshih@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/a36632b95683
reset the mBufferFetchingIsVerified flag after the webgl deleteBuffer call. r=jgilbert
https://hg.mozilla.org/integration/autoland/rev/8d1350135a04
test case for webgl drawArray() call. r=jgilbert
https://hg.mozilla.org/mozilla-central/rev/a36632b95683
https://hg.mozilla.org/mozilla-central/rev/8d1350135a04
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla57
Please request Beta approval on this when you get a chance.
Flags: needinfo?(hshih)
Comment on attachment 8900233 [details]
Bug 1379995 - reset the mBufferFetchingIsVerified flag after the webgl deleteBuffer call.

request for attachment 8900233 [details] and attachment 8900234 [details]

Approval Request Comment
[Feature/Bug causing the regression]:
It's a user reported bug.

[User impact if declined]:
We can write a "simple webgl code" to "crash" the firefox.

[Is this code covered by automated tests?]:
Yes, it pass our gl mochitest.

[Has the fix been verified in Nightly?]:
Yes, I can't reproduce the issue with user's bug STR.

[Needs manual test from QE? If yes, steps to reproduce]: 
no.

[List of other uplifts needed for the feature/fix]:
none.

[Is the change risky?]:
low risky.

[Why is the change risky/not risky?]:
It just clear one flag to make webgl context to check the buffer status again when we delete buffer.

[String changes made/needed]:
none.
Flags: needinfo?(hshih)
Attachment #8900233 - Flags: approval-mozilla-beta?
After this change, the theme "Compact Dark" no longer changes the colour of developer panel. Just wanted to confirm if it is intentional behavior because the bug title looks like fixing a crash issue.
Flags: needinfo?(hshih)
No, that will be a regression. Could you please show how to reproduce the issue? If you could also provide the picture comparison, that will be wonderful.
Flags: needinfo?(hshih) → needinfo?(61.1p57)
Comment on attachment 8900233 [details]
Bug 1379995 - reset the mBufferFetchingIsVerified flag after the webgl deleteBuffer call.

I would like to pending the beta uplift request. There might be a regression with this fix. Please check comment 15.
Attached image screenshot.png
STR:
1. Go to about:addons, themes
2. Open dev panel
3. Change theme to dark
Flags: needinfo?(61.1p57)
I try to remove my patch locally(attachment 8900233 [details]), but I still can't see the color changing for the theme "Compact Dark". So, I think this is not related to this fix.
Flags: needinfo?(61.1p57)
I create Bug 1394315 for comment 15 and 18.
Attached image mozregression.png
Wired, I just used mozregression and it pointed to this bug
Flags: needinfo?(61.1p57)
Here is a branch which remove the fix in this bug. If you could build firefox locally, you can try this.

https://github.com/JerryShih/gecko-dev/tree/test-theme
Flags: needinfo?(61.1p57)
Well, for reason I can't tell mozregression stopped bisection at mozilla-central and pointed to this bug. I will try it again after updating to a newer version.
Flags: needinfo?(61.1p57)
Comment on attachment 8900233 [details]
Bug 1379995 - reset the mBufferFetchingIsVerified flag after the webgl deleteBuffer call.

Let's uplift the fix. It's not a high volume crash on beta, but I also don't see how this could cause problems with the theme. So let's give this a try.
Attachment #8900233 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
You need to log in before you can comment on or make changes to this bug.