SegFault when append two scores that don't contain any actual Measures

• Sep 21, 2015 - 03:35
Type
Functional
Severity
S4 - Minor
Status
closed
Project
MuseScore

If joining album where first two scores contain no actual Measure objects (e.g. only contains non-measure MeasureBase objects like vbox-only.mscx), then mscore will crash.

The segfault happens in Score::appendScore when de-referencing a null Measure* m at line "Segment* seg = m->getSegment(Segment::Type::KeySig, tickLen)" which was null because tick2measure(tickLen) returns null if can't find a measure. I will submit a quick fix which will only adjust key signatures if that measure exists.

Before someone says this isn't a bug, I will say it is because a score without measures is a valid score that mscore will save and load, and which the album feature will accept as input. Also, I can suggest one plausible use case example: a composer has a title page (which is a .mscz containing just a frame and and image) and a bio page (which is a .mscz of a page of text which is reused and always attached at the beginning of any albums created).

Note: if use completely empty score without any MeasureBase objects (like attached empty.mscx), then mscore will appropriately refuse and popup an error when appending empty.mscx to empty.mscx, or when appending vbox-only.mscx to empty.mscx, although will crash if try append empty.mscx to vbox-only.mscx.

Attachment Size
vbox-only.mscx 3.17 KB
empty.mscx 3.04 KB

Comments

• Sep 21, 2015 - 06:06

I'm submitting the fix with two minor changes:
- I'm changing the variable named "tickLen" inside appendScore into "tickOfAppend". I don't consider "tickLen" to be a very useful name in this function because the number of ticks in the first score increases after appending the second score. I've changed the name to "tickOfAppend" so the interpretation of the variable makes sense both before and after appending the second score.
- TestAlbum::album01() I've changed album from a Album* into an Album object, so its QList object gets properly deconstructed at end of each test function.
- I'm adding a constructor to AlbumItem with filepath as input arg. This just makes writing tests functions easier, since I can write consecutive lines of album.append(new AlbumItem("file.mscz")).

Reply

• Sep 22, 2015 - 15:45
Status (old) patch (code needs review) ⇒ fixed

Fixed in branch master, commit 1104742718

fix #78521 prevent segfault when append scores without any measures

in Score::appendScore, before trying to adjust key signatures at first appended measure, this fix makes sure that the measure actually exists first. This prevents the mscore crash due to null pointer dereference.

In addition, I've changed the variable name 'tickLen' into 'tickOfAppend' so the variable name makes sense before and after the append occurs.

In addition, I've added two constructors to AlbumItem. This allows album test functions to use fewer lines of code, since can append a new albumitem with a path in one line.

The test case I added creates an ablum from two scores of a vbox only and an empty score to exercise ability to append scores with no actual measures.

Reply

• Sep 22, 2015 - 16:38

Fixed in branch 2.0.3, commit eea14002d5

fix #78521 prevent segfault when append scores without any measures

in Score::appendScore, before trying to adjust key signatures at first appended measure, this fix makes sure that the measure actually exists first. This prevents the mscore crash due to null pointer dereference.

In addition, I've changed the variable name 'tickLen' into 'tickOfAppend' so the variable name makes sense before and after the append occurs.

In addition, I've added two constructors to AlbumItem. This allows album test functions to use fewer lines of code, since can append a new albumitem with a path in one line.

The test case I added creates an ablum from two scores of a vbox only and an empty score to exercise ability to append scores with no actual measures.

Reply