Closed Bug 1312136 Opened 6 years ago Closed 6 years ago

Crash in InvalidArrayIndex_CRASH | nsMsgDBView::GetMsgHdrForViewIndex


(MailNews Core :: Backend, defect)

Not set


(thunderbird50 unaffected, thunderbird51 fixed, thunderbird52 fixed)

Thunderbird 52.0
Tracking Status
thunderbird50 --- unaffected
thunderbird51 --- fixed
thunderbird52 --- fixed


(Reporter: calum.mackay, Assigned: alta88)



(Keywords: crash, topcrash-thunderbird)

Crash Data


(1 file)

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:




b1db78e6-0f33-4344-acc2-1782a2160905 Signature: InvalidArrayIndex_CRASH | nsMsgKeyArray::GetArray
crash stats suggest it affects Windows too.
oldest seems to be 20160821030227
Depends on: 1302478
Component: General → Backend
Product: Thunderbird → MailNews Core
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.
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.
Attached patch index.patchSplinter Review
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.
Assignee: nobody → alta88
Attachment #8805115 - Flags: review?(rkent)
Comment on attachment 8805115 [details] [diff] [review]

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

LGTM, but I did not compile and test. Simple though.
Attachment #8805115 - Flags: review?(rkent) → review+
Keywords: checkin-needed
Landed on closed/busted tree, will follow-up with try run.
Closed: 6 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Target Milestone: --- → Thunderbird 52.0
Did I hear anyone say "uplift"?
Comment on attachment 8805115 [details] [diff] [review]

follow Bug 1159244 landing in 51.
Attachment #8805115 - Flags: approval-comm-aurora?
Comment on attachment 8805115 [details] [diff] [review]

Great, I'll see how my push goes and then uplift.
Attachment #8805115 - Flags: approval-comm-aurora? → approval-comm-aurora+
#3 crash for 50.0b2 and #4 for 50.0b3. please uplift to aurora, then we'll have it for 51 beta. thanks
Blocks: 1314348
Flags: needinfo?(jorgk)
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.
Flags: needinfo?(jorgk)
See Also: → 1308923
Duplicate of this bug: 1314348
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)

 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
You need to log in before you can comment on or make changes to this bug.