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)
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)
(deleted),
patch
|
nika
:
review+
|
Details | Diff | Splinter Review |
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)
Assignee | ||
Comment 1•6 years ago
|
||
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)
Assignee | ||
Comment 2•6 years ago
|
||
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 | ||
Comment 3•6 years ago
|
||
Attachment #9005555 -
Flags: review?(nika)
Assignee | ||
Updated•6 years ago
|
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
Assignee | ||
Comment 5•6 years ago
|
||
I have landed this ahead of getting r+; apologies for the forwardness, but I want to get it into the earliest possible Nightly.
Updated•6 years ago
|
Priority: -- → P1
Comment 6•6 years ago
|
||
bugherder |
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla63
Assignee | ||
Comment 7•6 years ago
|
||
Let's leave this open until we have confirmation in a day or two whether the crashes have stopped.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Updated•6 years ago
|
Attachment #9005555 -
Flags: review?(nika) → review+
Assignee | ||
Comment 8•6 years ago
|
||
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 ago → 6 years ago
Resolution: --- → FIXED
You need to log in
before you can comment on or make changes to this bug.
Description
•