Closed Bug 1152972 Opened 6 years ago Closed 6 years ago

crash in rx::IndexRangeCache::ComputeRange(unsigned int, void const*, int)

Categories

(Core :: Graphics, defect)

x86
Windows NT
defect
Not set
critical

Tracking

()

RESOLVED WONTFIX
Tracking Status
firefox37 --- wontfix
firefox38 --- unaffected

People

(Reporter: away, Assigned: milan)

Details

(Keywords: crash, Whiteboard: gfx-noted)

Crash Data

Attachments

(1 file)

This bug was filed from the Socorro interface and is 
report bp-0f483195-ad7b-4096-a594-47b212150404.
=============================================================

#16 crash on 37.0.1 release. Always on Win7Sp1.

Frame 	Module 	Signature 	Source
0 	libglesv2.dll 	rx::IndexRangeCache::ComputeRange(unsigned int, void const*, int) 	gfx/angle/src/libglesv2/renderer/IndexRangeCache.cpp
1 	libglesv2.dll 	gl::ValidateDrawElements(gl::Context*, unsigned int, int, unsigned int, void const*, int, rx::Range<unsigned int>*) 	gfx/angle/src/libglesv2/validationES.cpp
2 	libglesv2.dll 	glDrawElements 	gfx/angle/src/libglesv2/libGLESv2.cpp
I don't see this on pre-release channels >37, but it's possible this only shows up on release (since we didn't see it when 37 was pre-release).

Milan do you know of anything that may have fixed this? Otherwise let's assume it will persist when 38 rolls out.
Flags: needinfo?(milan)
Flags: needinfo?(milan) → needinfo?(jmuizelaar)
I think we did some uplifts to 38 with ANGLE, but Jeff knows better.
Yes, there was an ANGLE update in 38
Flags: needinfo?(jmuizelaar)
If we're not seeing this past 37.0.1, it was likely the ANGLE update, and I don't think we'd propose the uplift to 37 at this point.
Whiteboard: gfx-noted
(In reply to Robert Kaiser (:kairo@mozilla.com) from comment #5)
> Are any of the following related or are they a different bug?
> https://crash-stats.mozilla.com/report/
> list?signature=rx%3A%3ACopyNativeVertexData%3Cunsigned+short%2C+int%2C+0%3E%2
> 8unsigned+char+const%2A%2C+unsigned+int%2C+unsigned+int%2C+unsigned+char%2A%2
> 9
> https://crash-stats.mozilla.com/report/
> list?signature=rx%3A%3ACopyNativeVertexData%3Cfloat%2C+int%2C+0%3E%28unsigned
> +char+const%2A%2C+unsigned+int%2C+unsigned+int%2C+unsigned+char%2A%29

It is suspicious to me that those too show up only on Win7SP1. I can't tell whether they're the same issue as this bug though (but those two definitely look related to each other).

By the way, I filed bug 1156300 about the broken links for these libGLESv2 files.
OK, yeah, this is a simple missing null pointer check in the previous version of ANGLE, fixed with the one we're using in 38.  It's a one liner, but specific to 37.  I'll attach it to this bug anyway, and we can have a conversation about whether it warrants going into 37.0.* or not.
The version of ANGLE that has gl::Error getData(const uint8_t **outData) function should be OK, and the version of ANGLE that has void* Buffer11::getData() has a problem.  But if I clone mozilla-release, I get the "good one".  Am I looking in the wrong place?
Probably the 38.0.5 stuff from Sylvestre's mail. Try rewinding m-r a bit?
Approval Request Comment
[Feature/regressing bug #]:
[User impact if declined]: Largest 37.0.1 Google Maps crash.
[Describe test coverage new/current, TreeHerder]:
[Risks and why]: None, this function allows early returns and we're just checking null pointers.
[String/UUID change made/needed]:

If we decide to do another 37.* release, this patch should stop the crash.
Attachment #8596780 - Flags: review?(jgilbert)
Attachment #8596780 - Flags: approval-mozilla-release?
Comment on attachment 8596780 [details] [diff] [review]
Don't derefence null pointer. r=jgilberg

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

Isn't 38 release now?
Attachment #8596780 - Flags: review?(jgilbert) → review+
(In reply to Milan Sreckovic [:milan] from comment #8)
> But if I clone mozilla-release, I get the "good one".  Am I looking in the wrong place?

As dmajor says, that's probably because we already uplifted 38 to the mozilla-release repository. If you look at the FIREFOX_37_0_2_RELEASE tag, you'll find the version that we have on releases right now.

I think we want to avoid doing further 37.0.x releases, but if we do, we'll probably do that off the relbranch that this tag is on.
Comment on attachment 8596780 [details] [diff] [review]
Don't derefence null pointer. r=jgilberg

Let's do it!
Attachment #8596780 - Flags: approval-mozilla-release? → approval-mozilla-release+
Should be it 38 beta 8
Comment on attachment 8596780 [details] [diff] [review]
Don't derefence null pointer. r=jgilberg

Wait - 38 doesn't need this fix. I believe Milan was requesting m-r approval in case we do another 37.0.x
Attachment #8596780 - Flags: approval-mozilla-release+ → approval-mozilla-release?
ok, sorry, my bad. AFAIK, we are not planning to do a new 37 .
Right, we don't have a driver right now; this was just going to be in line to get picked up in case we do have a new 37.
I take it that 38+ are not affected. I have marked 37 as wontfix and am resolving this bug. Please reopen if a fix is still required.
Assignee: nobody → milan
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → WONTFIX
Attachment #8596780 - Flags: approval-mozilla-release? → approval-mozilla-release-
You need to log in before you can comment on or make changes to this bug.