Regression: Crashes on certain tuplets due to failure to sanitize

• Apr 8, 2018 - 10:00
Reported version
2.2
Type
Functional
Severity
S2 - Critical
Status
closed
Project
Priority (old)
critical
Status (old)
closed
Component (old)
Code
Category (old)
bug report

See https://musescore.org/en/node/271131 and esp. @Marc Sabatella's analysis there


Comments

I would say that the crash is somehow expected: the problem is a tuplet in voice 2 in the second staff of measure 100 with a missing note.
In particular, the tuplet has a ratio of 16/16, but it contains only 15 notes (one was deleted) of duration 1/32.
When summing the note durations, we obtain 15/32; multiplying it by 1/16 (from the ratio) to find the possible baselength duration gives 15/512. This duration is the one causing trouble, since 512th notes are not suported in 2.2.

And the best way to solve the crash is exactly what Marc is proposing in https://musescore.org/en/node/271131 i.e. adding a check t.isValid().
If the fraction is shorter than 1/128 (among other cases of invalid durations), the output of function truncateToFraction is DurationType::V_INVALID, see here:
https://github.com/musescore/MuseScore/blob/00c1d417f7e59d578ba2798559a…
therefore in those cases t.fraction() gives "fraction" 0/0; the simple check t.isValid() prevents arithmetic operations on 0/0 indeterminate form.
In master shorter durations are supported, that's why it is not crashing for these files:
https://github.com/musescore/MuseScore/blob/287f99ddccdfe04551c2353a396…
It would crash only for properly tailored extremely short tuplet with non-reduced ratio and missing elements (i.e. for computed incomplete tuplet durations which multiplied by the inverse of tuplet ratio would give a denominator > 1024).
That's why the check t.isValid() should go also to master.

Status (old) patch (code needs review) patch (ready to commit)
Status patch (code needs review) PR created

See https://github.com/musescore/MuseScore/pull/3730
Since 2.3 release candidate appears to be imminent, I think this fix should go into 2.3.
(It reduces to a simple check that t is not a V_INVALID duration type, so no negative side efffects)