1312136 - Crash in InvalidArrayIndex_CRASH | nsMsgDBView::GetMsgHdrForViewIndex

Status: VERIFIED FIXED

(MailNews Core :: Backend, defect)

Description

[Calum Mackay]

This bug was filed from the Socorro interface and is 
report bp-574f3421-f1e5-4b34-b3fe-3ac5a2161021.
=============================================================

I've had this Daily crash (or similar) 5 times now, since the earliest instance with build 20160905030200. The latest, this one, with 20161020030210.

The other reports are as follows; same signature unless noted:

38108d1b-f113-488b-a0f6-a4a3a2161018

679d272f-adff-4d6c-a09a-64c762161016

8c453e4b-accc-47c4-bc1d-807df2160927

b1db78e6-0f33-4344-acc2-1782a2160905 Signature: InvalidArrayIndex_CRASH | nsMsgKeyArray::GetArray

Comment 1

[Calum Mackay]

crash stats suggest it affects Windows too.

Comment 2

[Calum Mackay]

oldest seems to be 20160821030227

Comment 3

[Calum Mackay]

yup, thanks Wayne; I did have mailsummaries temporarily installed, when I had that GetArray() crash, but I don't have it installed now, for the GetMsgHdrForViewIndex() crashes.

Comment 4

[Kent James (:rkent)]

This happens because in bug 1159244 invalid array indices were turned into crashes. The fact that we are getting crashes shows that we have some invalid array index issues in nsMsgDBView.

The issue is broader than just this one point though. My suggestion is that we replace most m_key[index] access in view code with a macro that checks array bounds, adds an NS_WARNING and then returns NS_ERROR_INVALID_ARG.

Comment 5

[alta88]

Attachment: index.patch

this happened in a test profile recently, so here's a patch.

there is a func IsValidIndex() that checks upper bound also, and the caller can return NS_MSG_INVALID_DBVIEW_INDEX if appropriate.  this is used in dozens of places in nsMsgDBView, apparently a few were missed.

Comment 6

[Kent James (:rkent)]

Comment on attachment 8805115 [details] [diff] [review]
index.patch

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

LGTM, but I did not compile and test. Simple though.

Comment 7

[Jorg K (CEST = GMT+2)]

https://hg.mozilla.org/comm-central/rev/dd6e3dbcec59187b8dc92e66931d8740e57a34e5
Landed on closed/busted tree, will follow-up with try run.

Comment 8

[Jorg K (CEST = GMT+2)]

Did I hear anyone say "uplift"?

Comment 9

[Jorg K (CEST = GMT+2)]

https://treeherder.mozilla.org/#/jobs?repo=try-comm-central&revision=f2fd71ad418ed4cc1b40b3033fccb756f81e2d92

Comment 10

[alta88]

Comment on attachment 8805115 [details] [diff] [review]
index.patch

follow Bug 1159244 landing in 51.

Comment 11

[Jorg K (CEST = GMT+2)]

Comment on attachment 8805115 [details] [diff] [review]
index.patch

Great, I'll see how my push goes and then uplift.

Comment 12

[Wayne Mery (:wsmwk)]

#3 crash for 50.0b2 and #4 for 50.0b3. please uplift to aurora, then we'll have it for 51 beta. thanks

Comment 13

[Jorg K (CEST = GMT+2)]

I have three bugs on the radar for Aurora uplift, this one, bug 1311672 and bug 1312314. I'm waiting for C-C to come back, so I can try these in a Daily first. Don't worry, this won't miss TB 51 beta.

Comment 14

[Jorg K (CEST = GMT+2)]

Aurora (TB 51):
https://hg.mozilla.org/releases/comm-aurora/rev/3a2c0f8c8896daf9e9a7ab9fd23eea1c9583ec13

Comment 16

[Wayne Mery (:wsmwk)]

no crashes starting TB51 beta, so v.fixed
(granted, we didn't automatically update most Mac users to 51 - but there have been enough Mac users there to conclude this is gone)

Comment 17

[Wayne Mery (:wsmwk)]

bp-38108d1b-f113-488b-a0f6-a4a3a2161018
 0 	XUL	InvalidArrayIndex_CRASH(unsigned long, unsigned long)	xpcom/glue/nsTArray.cpp:35
1 	XUL	nsMsgDBView::GetMsgHdrForViewIndex(unsigned int, nsIMsgDBHdr**)	/builds/slave/tb-c-cen-m64-ntly-000000000000/build/objdir-tb/x86_64/dist/include/nsTArray.h:1033
2 	XUL	nsMsgDBView::UpdateDisplayMessage(unsigned int)	/builds/slave/tb-c-cen-m64-ntly-000000000000/build/mailnews/base/src/nsMsgDBView.cpp:1081
3 	XUL	nsMsgDBView::ReloadMessage()	/builds/slave/tb-c-cen-m64-ntly-000000000000/build/mailnews/base/src/nsMsgDBView.cpp:1067
4 	XUL	NS_InvokeByIndex	xpcom/reflect/xptcall/md/unix/xptcinvoke_x86_64_unix.cpp:180