Closed Bug 1312136 Opened 3 years ago Closed 3 years ago
Crash in Invalid
Array Index _CRASH | ns Msg DBView::Get Msg Hdr For View Index
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
crash stats suggest it affects Windows too.
oldest seems to be 20160821030227
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.
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] index.patch Review of attachment 8805115 [details] [diff] [review]: ----------------------------------------------------------------- LGTM, but I did not compile and test. Simple though.
Attachment #8805115 - Flags: review?(rkent) → review+
https://hg.mozilla.org/comm-central/rev/dd6e3dbcec59187b8dc92e66931d8740e57a34e5 Landed on closed/busted tree, will follow-up with try run.
Status: NEW → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → Thunderbird 52.0
Did I hear anyone say "uplift"?
Comment on attachment 8805115 [details] [diff] [review] index.patch follow Bug 1159244 landing in 51.
Attachment #8805115 - Flags: approval-comm-aurora?
Comment on attachment 8805115 [details] [diff] [review] index.patch 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
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.
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)
Status: RESOLVED → VERIFIED
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
You need to log in before you can comment on or make changes to this bug.