Crash (access negative array index assert failure) when delete titlebox, change initial chordrest duration, and undo twice

• Mar 1, 2018 - 22:05
Reported version
3.0
Type
Functional
Severity
S2 - Critical
Status
closed
Project

Repro steps:
1. New Blank Score.
2. Delete title box.
3. Click the whole rest in measure 1.
4. Press a duration button other than whole-duration.
5. Press Ctrl->Z (undo) twice.
6. The second undo should restore the title box, but instead get a crash:

1 raise 0x7fffeaa4f860
2 abort 0x7fffeaa50ec9
3 QMessageLogger::fatal(const char *, ...) const 0x7fffeb4098e8
4 qt_assert_x(const char *, const char *, const char *, int) 0x7fffeb40452b
5 QList::operator[] qlist.h 549 0x555556380d5b
6 Ms::Score::doLayoutRange layout.cpp 3674 0x555556645164
7 Ms::Score::update cmd.cpp 221 0x5555567a1fb7
8 Ms::Score::undoRedo cmd.cpp 156 0x5555567a1b5c
9 Ms::ScoreView::startUndoRedo scoreview.cpp 3048 0x555555e15d94
10 Ms::MuseScore::undoRedo musescore.cpp 3816 0x555555fd7526
11 Ms::MuseScore::cmd musescore.cpp 4954 0x555555fde342
12 Ms::MuseScore::cmd musescore.cpp 4740 0x555555fdd024
13 Ms::MuseScore::qt_static_metacall moc_musescore.cpp 738 0x5555564c1b84
14 QMetaObject::activate(QObject *, int, int, void * *) 0x7fffeb63d6c6
15 QActionGroup::triggered(QAction *) 0x7ffff3a580c0
16 ?? 0x7ffff3a58bc9
17 QMetaObject::activate(QObject *, int, int, void * *) 0x7fffeb63d6c6
18 QAction::triggered(bool) 0x7ffff3a54533
19 QAction::activate(QAction::ActionEvent) 0x7ffff3a56db6
20 QAction::event(QEvent *) 0x7ffff3a57766
...

In stack level 5: QList::operator[], I observer int i is -2, which is obviously bad, cause I believe that means it is trying to access a negative array index. Hence the assert failure & crash.

In stack level 6: Ms::Score::doLayoutRange, I observe systemIndex is equal to -1. It trys to access _systems[systemIndex-1] and therefore crashes. Interestingly just above that access, there is a test for systemIndex == 0 in which case it sets lc.nextMeasure to the sane value of _measures.first(). The systemIndex variable is initialized to _system.indexOf(system), and according to QList docs, the return value of -1 means that system was "not found" in the Score's list of _systems). Measure m is of type Ms::VBox, which means it represents the initial measure, and appears valid. The system variable comes from that VBox's system, and it too looks like a valid system.

So what seems to me to have happened is that upon the second undo, the intial VBox is added back to the score's list of measures, because I see its address is the same as the score's first measure. But somehow m->system() is still pointing to a non-null system. I believe that m->systems() should instead actually point to NULL upon being readded to the score on the undo.


Comments

Status (old) patch (code needs review) fixed
Status fixed

Fixed in branch master, commit c64e557d20

fix #269919 undoRemoveElement detach parent system

The presence of a non-null parent system resulted in a crash in the intial layout after a redo of a deleted initial VBox. To fix this, I'm detaching the parent system during undo, so the VBox MeasureBase when undeleted won't point to the old system (which confused layout because layout couldn't find this old system in the new list of systems). Since a new parent system will be regenerated anyway on the initial layout after the redo operation, I believe it is safe to detach the parent system here during undo.