Crash when adding new time signature in the measure of a clef change

• Dec 28, 2018 - 17:26
Reported version
3.0
Priority
P0 - Critical
Type
Functional
Frequency
Many
Severity
S2 - Critical
Reproducibility
Always
Status
closed
Regression
No
Workaround
No
Project
MuseScore

OS: Windows 10 (10.0), Arch.: x86_64, MuseScore version (64-bit): 3.0.0.4838, revision: a1152b9

Steps:
1) Create new score for at least two instruments (necessary condition)
2) Insert a new clef in first staff let's say measure 3
3) Insert a new time signature in the same measure 3

Result: crash

See:

Video.gif

Comments

• Dec 28, 2018 - 17:49
Title Crash when adding new time signature in the measure of a change clef ⇒ Crash when adding new time signature in the measure of a clef change

Reply

• Dec 29, 2018 - 07:12

Verified that also happens for single instrument with multiple staves (example piano grand staff score).

Crash occurs in line 1588 of undo.cpp in std::vector InsertRemoveMeasures::getCourtesyClefs(Measure* m) for call to clef->isClef()

Reply

• Dec 29, 2018 - 07:13

Call stack for reference:

MuseScore3.exe!Ms::ScoreElement::isClef() Line 266  C++
MuseScore3.exe!Ms::InsertRemoveMeasures::getCourtesyClefs(Ms::Measure * m) Line 1588    C++
MuseScore3.exe!Ms::InsertRemoveMeasures::removeMeasures() Line 1725 C++
MuseScore3.exe!Ms::RemoveMeasures::redo(Ms::EditData * __formal) Line 729   C++
MuseScore3.exe!Ms::UndoStack::push(Ms::UndoCommand * cmd, Ms::EditData * ed) Line 245   C++
MuseScore3.exe!Ms::Score::undo(Ms::UndoCommand * cmd, Ms::EditData * ed) Line 3423  C++
MuseScore3.exe!Ms::Score::undoRemoveMeasures(Ms::Measure * m1, Ms::Measure * m2) Line 4814  C++
MuseScore3.exe!Ms::Score::rewriteMeasures(Ms::Measure * fm, Ms::Measure * lm, const Ms::Fraction & ns, int staffIdx) Line 471   C++
MuseScore3.exe!Ms::Score::rewriteMeasures(Ms::Measure * fm, const Ms::Fraction & ns, int staffIdx) Line 554 C++
MuseScore3.exe!Ms::Score::cmdAddTimeSig(Ms::Measure * fm, int staffIdx, Ms::TimeSig * ts, bool local) Line 775  C++
MuseScore3.exe!Ms::Measure::drop(Ms::EditData & data) Line 1460 C++
MuseScore3.exe!Ms::ScoreView::dropEvent(QDropEvent * event) Line 537    C++

Reply

• Dec 29, 2018 - 08:36

For reference that above call stack was from a grand-staff single-instrument. Below is from two single-staff instruments:

MuseScore3.exe!Ms::ScoreElement::isClef() Line 266  C++
MuseScore3.exe!Ms::InsertRemoveMeasures::getCourtesyClefs(Ms::Measure * m) Line 1588    C++
MuseScore3.exe!Ms::InsertRemoveMeasures::removeMeasures() Line 1725 C++
MuseScore3.exe!Ms::RemoveMeasures::redo(Ms::EditData * __formal) Line 729   C++
MuseScore3.exe!Ms::UndoStack::push(Ms::UndoCommand * cmd, Ms::EditData * ed) Line 245   C++
MuseScore3.exe!Ms::Score::undo(Ms::UndoCommand * cmd, Ms::EditData * ed) Line 3423  C++
MuseScore3.exe!Ms::Score::undoRemoveMeasures(Ms::Measure * m1, Ms::Measure * m2) Line 4814  C++
MuseScore3.exe!Ms::Score::rewriteMeasures(Ms::Measure * fm, Ms::Measure * lm, const Ms::Fraction & ns, int staffIdx) Line 471   C++
MuseScore3.exe!Ms::Score::rewriteMeasures(Ms::Measure * fm, const Ms::Fraction & ns, int staffIdx) Line 554 C++
MuseScore3.exe!Ms::Score::cmdAddTimeSig(Ms::Measure * fm, int staffIdx, Ms::TimeSig * ts, bool local) Line 775  C++
MuseScore3.exe!Ms::Measure::drop(Ms::EditData & data) Line 1460 C++
MuseScore3.exe!Ms::ScoreView::dropEvent(QDropEvent * event) Line 537    C++

They are identical call stacks.

In both cases I was inserting to meas 3 first staff first a bass clef then a 3/4 time sig.

Reply

• Dec 29, 2018 - 08:40

In InsertRemoveMeasures::getCourtesyClefs(), the value of st is 1 when the crash occurs.

So it seems pretty simple. The code is assuming that every staff has a clef change. But that assumption is wrong if there aren't clef changes on every staff. That is why clef is null. And that is why should do if (clef && clef->isClef()) instead of if (clef->isClef())

Reply

• Jan 8, 2019 - 00:23
Status PR created ⇒ fixed

Fixed in branch master, commit 641ac7ad25

fix #280814 add timesig to meas w/some clefs

Fixes a crash when adding a timesig to a measure that has at least one clef but where not all staves have clef for that measure. Crash was result of a NULL defreference on the first staff that didn't have a clef in that clef segment.

Fix is simple to make sure to test that clef is non-NULL before dereferencing it.

Reply

Fix version
3.0.1