Closed Bug 1487541 Opened 6 years ago Closed 6 years ago

Crash in InvalidArrayIndex_CRASH | nsTArray_Impl<T>::RemoveElementsAt | nsSHistory::AddEntry

Categories

(Core :: DOM: Navigation, defect, P1)

Unspecified
Windows 10
defect

Tracking

()

RESOLVED FIXED
mozilla63
Tracking Status
firefox-esr52 --- unaffected
firefox-esr60 --- unaffected
firefox61 --- unaffected
firefox62 --- unaffected
firefox63 --- fixed

People

(Reporter: calixte, Assigned: n.nethercote)

References

(Blocks 1 open bug)

Details

(Keywords: crash, regression)

Crash Data

Attachments

(1 file)

This bug was filed from the Socorro interface and is report bp-ce78c9b8-bcf2-4c50-852d-27c440180830. ============================================================= Top 10 frames of crashing thread: 0 mozglue.dll MOZ_CrashPrintf mfbt/Assertions.cpp:67 1 xul.dll InvalidArrayIndex_CRASH xpcom/ds/nsTArray.cpp:26 2 xul.dll void nsTArray_Impl<nsCOMPtr<nsISHTransaction>, nsTArrayInfallibleAllocator>::RemoveElementsAt xpcom/ds/nsTArray.h:2389 3 xul.dll nsSHistory::AddEntry docshell/shistory/nsSHistory.cpp:663 4 xul.dll nsresult nsDocShell::AddToSessionHistory docshell/base/nsDocShell.cpp:12223 5 xul.dll bool nsDocShell::OnNewURI docshell/base/nsDocShell.cpp:11524 6 xul.dll nsDSURIContentListener::DoContent docshell/base/nsDSURIContentListener.cpp:196 7 xul.dll bool nsDocumentOpenInfo::TryContentListener uriloader/base/nsURILoader.cpp:759 8 xul.dll nsDocumentOpenInfo::OnStartRequest uriloader/base/nsURILoader.cpp:306 9 xul.dll void mozilla::net::HttpChannelChild::DoOnStartRequest netwerk/protocol/http/HttpChannelChild.cpp:765 ============================================================= There are 8 crashes (from 7 installations) in nightly 63 with buildid 20180830111745. In analyzing the backtrace, the regression may have been introduced by patch [1] to fix bug 1486358. [1] https://hg.mozilla.org/mozilla-central/rev?node=e39b65db5b40
Flags: needinfo?(n.nethercote)
In all of these crashes we get into AddEntry() with mTransactions.Length() == 0 but mIndex is positive, e.g 3, 7, 22. Then we call mTransactions.TruncateLength(mIndex + 1) and the bounds check fails because TruncateLength() requires its argument to be less than or equal to the length. So, things have gone awry prior to the call to AddEntry(). There are two possibilities. - Bug 1486358 introduced a defect. - Bug 1486358 exposed a latent defect. The old code would not have aborted with mLength==0 and positive mIndex. (AddEntry() would have exited having given mLength a bogus value of mIndex+1, though.) I'm leaning towards the latter, but that doesn't really help. I can't see how to fix this so I'll probably have to back bug 1486358 out. After that I might add some diagnostic asserts at various places to check that mLength and mIndex are reasonable, to determine if this really is a latent defect.
Flags: needinfo?(n.nethercote)
I did some digging, by adding a bunch of assertions that checked the transactions indices and length, and I found a latent defect: mRequestedIndex can get out of bounds, because in PurgeHistory() we update mIndex if necessary, but we don't do likewise for mRequestedIndex. And mRequestedIndex gets assigned to mIndex in UpdateIndex(). So this may explain how mIndex is getting out of bounds. I am not certain that this latent defect is causing the crashes, but it's my best theory at the moment. I will land a patch fixing that and see what happens to the crash rates in Nightly. If they stop, good; if not, I will back out bug 1486358's patches.
Assignee: nobody → n.nethercote
Status: NEW → ASSIGNED
Pushed by nnethercote@mozilla.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/57f23dc56b70 Keep mRequestedIndex in bounds in PurgeHistory(). r=me
I have landed this ahead of getting r+; apologies for the forwardness, but I want to get it into the earliest possible Nightly.
Priority: -- → P1
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla63
Let's leave this open until we have confirmation in a day or two whether the crashes have stopped.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Attachment #9005555 - Flags: review?(nika) → review+
Nightly 20180830111745 and 20180830220110 had these crashes. Nightly 20180831100058 had these patches, and there have been no crashes in that build or later Nightly builds. Time to declare victory!
Status: REOPENED → RESOLVED
Closed: 6 years ago6 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: