Closed Bug 1011327 Opened 6 years ago Closed 6 years ago

[Flame] FX OS crash in mozilla::layers::SharedBufferManagerParent::RecvDropGrallocBuffer(mozilla::layers::MaybeMagicGrallocBufferHandle const&)

Categories

(Core :: Graphics: Layers, defect, critical)

All
Gonk (Firefox OS)
defect
Not set
critical

Tracking

()

RESOLVED FIXED
mozilla32
tracking-b2g backlog

People

(Reporter: marcia, Assigned: sotaro)

Details

(Keywords: crash, Whiteboard: [b2g-crash])

Crash Data

Attachments

(1 file, 2 obsolete files)

This bug was filed from the Socorro interface and is 
report bp-42a24371-cdcf-42a1-8f9a-d6ec82140515.
=============================================================

Seen while running on Flame device using:

Gaia   3a1d67246a79e3632c3b3f2460a25291e7e2714c
SourceStamp e4843f4f08a7
BuildID 20140515040207
Version 32.0a1

STR:
1. Was swiping through various screens using edge gestures when the crash occurred.
blocking-b2g: --- → backlog
Keywords: steps-wanted
sotaro is this the missing null check?
Flags: needinfo?(sotaro.ikeda.g)
Yea, it seems mull check for a pointer of GraphicBuffer seems missing. The crash address is 0x34. From the following disassemble info, it seems to related to "buf->getPixelFormat()".


(gdb) disassemble /m
Dump of assembler code for function mozilla::layers::SharedBufferManagerParent::RecvDropGrallocBuffer(mozilla::layers::MaybeMagicGrallocBufferHandle const&):
173	{
   0xb4fdbdc8 <+0>:	push	{r0, r1, r2, r3, r4, lr}
   0xb4fdbdca <+2>:	mov	r4, r0

174	#ifdef MOZ_HAVE_SURFACEDESCRIPTORGRALLOC
175	  NS_ASSERTION(handle.type() == MaybeMagicGrallocBufferHandle::TGrallocBufferRef, "We shouldn't interact with the real buffer!");
176	  int bufferKey = handle.get_GrallocBufferRef().mKey;
=> 0xb4fdbdcc <+4>:	ldr	r2, [r1, #4]
   0xb4fdbdd2 <+10>:	str	r2, [sp, #4]

177	  sp<GraphicBuffer> buf = GetGraphicBuffer(bufferKey);
   0xb4fdbdce <+6>:	add	r0, sp, #8
   0xb4fdbdd0 <+8>:	mov	r1, r4
   0xb4fdbdd4 <+12>:	bl	0xb4fdbb68

178	  MutexAutoLock lock(mBuffersMutex);
   0xb4fdbdd8 <+16>:	add.w	r0, r4, #416	; 0x1a0

179	  NS_ASSERTION(mBuffers.count(bufferKey) == 1, "How can you drop others buffer");
180	  mBuffers.erase(bufferKey);
181	
182	  int bpp = 0;
183	  bpp = android::bytesPerPixel(buf->getPixelFormat());
   0xb4fdbdee <+38>:	ldr	r0, [r3, #52]	; 0x34
   0xb4fdbdf0 <+40>:	blx	0xb4c42058
---Type <return> to continue, or q <return> to quit---
   0xb4fdbdf4 <+44>:	ldr	r3, [sp, #8]

184	  if (bpp > 0)
   0xb4fdbdf6 <+46>:	cmp	r0, #0
   0xb4fdbdf8 <+48>:	ble.n	0xb4fdbe08 <mozilla::layers::SharedBufferManagerParent::RecvDropGrallocBuffer(mozilla::layers::MaybeMagicGrallocBufferHandle const&)+64>

185	    GrallocReporter::sAmount -= buf->getStride() * buf->getHeight() * bpp;
   0xb4fdbdfa <+50>:	ldr	r1, [pc, #60]	; (0xb4fdbe38 <mozilla::layers::SharedBufferManagerParent::RecvDropGrallocBuffer(mozilla::layers::MaybeMagicGrallocBufferHandle const&)+112>)
   0xb4fdbdfc <+52>:	ldr	r2, [r3, #44]	; 0x2c
   0xb4fdbdfe <+54>:	add	r1, pc
   0xb4fdbe00 <+56>:	ldr	r3, [r3, #48]	; 0x30
   0xb4fdbe02 <+58>:	muls	r3, r2
   0xb4fdbe04 <+60>:	muls	r0, r3
   0xb4fdbe06 <+62>:	b.n	0xb4fdbe18 <mozilla::layers::SharedBufferManagerParent::RecvDropGrallocBuffer(mozilla::layers::MaybeMagicGrallocBufferHandle const&)+80>

186	  else // Specical case for BSP specific formats(mainly YUV formats, count it as normal YUV buffer)
187	    GrallocReporter::sAmount -= buf->getStride() * buf->getHeight() * 3 / 2;
   0xb4fdbe08 <+64>:	ldr	r1, [pc, #48]	; (0xb4fdbe3c <mozilla::layers::SharedBufferManagerParent::RecvDropGrallocBuffer(mozilla::layers::MaybeMagicGrallocBufferHandle const&)+116>)
   0xb4fdbe0a <+66>:	ldr	r2, [r3, #48]	; 0x30
   0xb4fdbe0c <+68>:	ldr	r0, [r3, #44]	; 0x2c
   0xb4fdbe0e <+70>:	add	r1, pc
   0xb4fdbe10 <+72>:	muls	r0, r2
   0xb4fdbe12 <+74>:	movs	r3, #3
   0xb4fdbe14 <+76>:	muls	r0, r3
---Type <return> to continue, or q <return> to quit---
   0xb4fdbe16 <+78>:	lsrs	r0, r0, #1
   0xb4fdbe18 <+80>:	ldrd	r2, r3, [r1]
   0xb4fdbe1c <+84>:	subs	r2, r2, r0
   0xb4fdbe1e <+86>:	sbc.w	r3, r3, #0
   0xb4fdbe24 <+92>:	strd	r2, r3, [r1]

188	
189	#endif
190	  return true;
   0xb4fdbe22 <+90>:	add	r0, sp, #12
   0xb4fdbe28 <+96>:	bl	0xb4c59c66
   0xb4fdbe2c <+100>:	add	r0, sp, #8
   0xb4fdbe2e <+102>:	bl	0xb4d4ad84

191	}
   0xb4fdbe32 <+106>:	movs	r0, #1
   0xb4fdbe34 <+108>:	add	sp, #16
   0xb4fdbe36 <+110>:	pop	{r4, pc}
   0xb4fdbe38 <+112>:			; <UNDEFINED> instruction: 0x01529196
   0xb4fdbe3c <+116>:	cmpeq	r2, r6, lsl #3

End of assembler dump.
Flags: needinfo?(sotaro.ikeda.g)
Assignee: nobody → sotaro.ikeda.g
OS: Android → Gonk (Firefox OS)
Version: 29 Branch → Trunk
Update nits.
Attachment #8424033 - Attachment is obsolete: true
Attachment #8424038 - Flags: review?(nical.bugzilla)
Comment on attachment 8424038 [details] [diff] [review]
patch v2 - Add checks around SharedBufferManagerParent

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

The assertion below looks wrong, please clarify.

::: gfx/layers/ipc/SharedBufferManagerParent.cpp
@@ +194,5 @@
>  #ifdef MOZ_HAVE_SURFACEDESCRIPTORGRALLOC
>    NS_ASSERTION(handle.type() == MaybeMagicGrallocBufferHandle::TGrallocBufferRef, "We shouldn't interact with the real buffer!");
>    int bufferKey = handle.get_GrallocBufferRef().mKey;
>    sp<GraphicBuffer> buf = GetGraphicBuffer(bufferKey);
> +  MOZ_ASSERT(!buf.get());

Did you mean to check MOZ_ASSERT(buf.get()) ?
(In reply to Nicolas Silva [:nical] from comment #5)
> Comment on attachment 8424038 [details] [diff] [review]
> patch v2 - Add checks around SharedBufferManagerParent
> 
> Review of attachment 8424038 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> The assertion below looks wrong, please clarify.
> 
> ::: gfx/layers/ipc/SharedBufferManagerParent.cpp
> @@ +194,5 @@
> >  #ifdef MOZ_HAVE_SURFACEDESCRIPTORGRALLOC
> >    NS_ASSERTION(handle.type() == MaybeMagicGrallocBufferHandle::TGrallocBufferRef, "We shouldn't interact with the real buffer!");
> >    int bufferKey = handle.get_GrallocBufferRef().mKey;
> >    sp<GraphicBuffer> buf = GetGraphicBuffer(bufferKey);
> > +  MOZ_ASSERT(!buf.get());
> 
> Did you mean to check MOZ_ASSERT(buf.get()) ?

Yes, it's my mistake.
Apply the comment.
Attachment #8424038 - Attachment is obsolete: true
Attachment #8424038 - Flags: review?(nical.bugzilla)
Attachment #8425433 - Flags: review?(nical.bugzilla)
Attachment #8425433 - Flags: review?(nical.bugzilla) → review+
https://hg.mozilla.org/mozilla-central/rev/182479098f34
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla32
Keywords: steps-wanted
blocking-b2g: backlog → ---
You need to log in before you can comment on or make changes to this bug.