Closed Bug 1719669 Opened 2 years ago Closed 2 years ago

Crash in [@ InvalidArrayIndex_CRASH | nsTArray_Impl<T>::RemoveElementsAt | mozilla::SelectionState::SaveSelection]


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

Windows 10



92 Branch
Tracking Status
firefox-esr78 --- unaffected
firefox89 --- unaffected
firefox90 --- unaffected
firefox91 --- fixed
firefox92 --- fixed


(Reporter: sefeng, Assigned: masayuki)




(Keywords: crash, regression)

Crash Data


(1 file)

Maybe Fission related. (DOMFissionEnabled=1)

Crash report:

MOZ_CRASH Reason: ElementAt(aIndex = 4294967295, aLength = 0)

Top 10 frames of crashing thread:

0 xul.dll InvalidArrayIndex_CRASH xpcom/ds/nsTArray.cpp:28
1 xul.dll nsTArray_Impl<RefPtr<mozilla::RangeItem>, nsTArrayInfallibleAllocator>::RemoveElementsAt xpcom/ds/nsTArray.h:2486
2 xul.dll mozilla::SelectionState::SaveSelection editor/libeditor/SelectionState.cpp:54
3 xul.dll mozilla::PlaceholderTransaction::EndPlaceHolderBatch editor/libeditor/PlaceholderTransaction.cpp:299
4 xul.dll mozilla::EditorBase::EndPlaceholderTransaction editor/libeditor/EditorBase.cpp:1231
5 xul.dll mozilla::EditorBase::DeleteSelectionAsAction editor/libeditor/EditorBase.cpp:4232
6 xul.dll mozilla::EditorBase::HandleKeyPressEvent editor/libeditor/EditorBase.cpp:4972
7 xul.dll mozilla::EditorEventListener::KeyPress editor/libeditor/EditorEventListener.cpp:621
8 xul.dll mozilla::EventListenerManager::HandleEventInternal dom/events/EventListenerManager.cpp:1305
9 xul.dll static mozilla::EventTargetChainItem::HandleEventTargetChain dom/events/EventDispatcher.cpp:586
Severity: -- → S2

Looks like some kind of integer underflow. Maybe this is a regression from bug 1714914?

Has Regression Range: --- → yes

Yeah, it seems so.

Assignee: nobody → masayuki
Priority: -- → P1
Hardware: Unspecified → All

The crash is caused by the infinite loop to delete unnecessary ranges.
The i is always equals or larger than 0 due to unsigned. So, if
Selection::RangeCount() is 0, i can be UINT32_MAX.

Instead, we can use nsTArray<RefPtr<RangeItem>>::TruncateLength().

Hmm, the block is not covered by any automated tests. I'd love somebody to tell us the STR.

Pushed by
Fix the reverse loop in `SelectionState::SaveSelection()` r=m_kato
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → 92 Branch

The patch landed in nightly and beta is affected.
:masayuki, is this bug important enough to require an uplift?
If not please set status_beta to wontfix.

For more information, please visit auto_nag documentation.

Flags: needinfo?(masayuki)

Comment on attachment 9230560 [details]
Bug 1719669 - Fix the reverse loop in SelectionState::SaveSelection() r=m_kato!

Beta/Release Uplift Approval Request

  • User impact if declined: Even though the number of crash reports is low, some users may meet this crash.
  • Is this code covered by automated tests?: No
  • Has the fix been verified in Nightly?: No
  • Needs manual test from QE?: No
  • If yes, steps to reproduce:
  • List of other uplifts needed: None
  • Risk to taking this patch: Low
  • Why is the change risky/not risky? (and alternatives if risky): Just fixing a mistake, and newly using existing API of nsTArray instead of removing items with a loop. (Unfortunately, we don't have STR for this crash.)
  • String changes made/needed: No
Flags: needinfo?(masayuki)
Attachment #9230560 - Flags: approval-mozilla-beta?

Comment on attachment 9230560 [details]
Bug 1719669 - Fix the reverse loop in SelectionState::SaveSelection() r=m_kato!

New 91 crasher, approved for uplift in 91 beta 4, thanks

Attachment #9230560 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
Crash Signature: [@ InvalidArrayIndex_CRASH | nsTArray_Impl<T>::RemoveElementsAt | mozilla::SelectionState::SaveSelection] → [@ InvalidArrayIndex_CRASH | nsTArray_Impl<T>::RemoveElementsAt | mozilla::SelectionState::SaveSelection] [@ InvalidArrayIndex_CRASH | mozilla::SelectionState::SaveSelection]
You need to log in before you can comment on or make changes to this bug.