Crash by removing a score with frames when the last measure contains a pedal line/volta line

Jojo-Schmitz

• Jan 7, 2017 - 09:32

Reply

cadiz1

• Jan 7, 2017 - 09:49

I don't think so.
Only here pedal lines and volta lines AND frames (necessary condition)

Reply

Jojo-Schmitz

• Jan 7, 2017 - 11:58

Stack trace (self-build from latest master, 73a2e6a):
1 Ms::MeasureBase::prev measurebase.h 98 0xe07828
2 Ms::MeasureBase::prevMeasureMM measurebase.cpp 241 0x85114d
3 Ms::Score::doLayoutRange layout.cpp 3463 0x8a4d46
4 Ms::Score::update cmd.cpp 201 0x7b5ef5
5 Ms::Score::endCmd cmd.cpp 175 0x7b5ccd
6 Ms::Score::cmd cmd.cpp 3267 0x7c385a
7 Ms::ScoreView::cmd scoreview.cpp 3260 0x414574
8 Ms::MuseScore::cmd musescore.cpp 4987 0x4a0cb1
9 Ms::MuseScore::cmd musescore.cpp 4533 0x49e8b8
10 Ms::MuseScore::qt_static_metacall moc_musescore_IKSZSQWDH4YDZS.cpp 733 0x69a512
11 ZN11QMetaObject8activateEP7QObjectiiPPv 0x68a08942
12 Ms::ScoreTab::actionTriggered moc_scoretab_QR4BWIRYNXAVBM.cpp 193 0x6a0841
13 Ms::ScoreTab::qt_static_metacall moc_scoretab_QR4BWIRYNXAVBM.cpp 95 0x6a0513
14 ZN11QMetaObject8activateEP7QObjectiiPPv 0x68a08942
15 ZN12QActionGroup7hoveredEP7QAction 0x1daa5295
16 ?? 0x240de940
17 ZN7QAction8activateENS_11ActionEventE 0x1daa4248
18 ?? 0x28ce28
19 ?? 0x1fdb5e40
20 ZN7QThread18qt_static_metacallEP7QObjectN11QMetaObject4CallEiPPv 0x68a68380
...

In frame 3 (in method Ms::Score::doLayoutRange(), file layout.cpp, line 3463) we're dereferencing a nullptr we just got from tick2measure() in the previous statement.
There's also a debug output:
Debug: tick2measure 0 (max -1) not found (...\MuseScore\libmscore\utils.cpp:60, Ms::Measure* Ms::Score::tick2measure(int) const)

However, adding a volta or pedal line is not needed at all, steps 1, 4 and 5 are sufficient for getting a crash. Maybe in master we're talking about a different issue though?

Another issue (in master): it is not possible to delete the title frame

Reply

cadiz1

• Jan 7, 2017 - 12:06

"adding a volta or pedal line is not needed at all, steps 1, 4 and 5 are sufficient for getting a crash.
Maybe in master we're talking about a different issue though?"

Probably, or the most likely, knowing that my tests were almost exclusively with 2.0.3 and 2.1, because the 3.0 is so buggy, that it is discouraging to check further ...

EDIT: and for this: "Another issue (in master): it is not possible to delete the title frame"

Reply

cadiz1

• Jan 7, 2017 - 15:45

For the record, I observe a change on February 17, 2015.

- With this nighty (last one of February 16): no crash, the frame is preserved: c0a13fc

- With this one, the first one (or close) of February 17, I receive the crash: 4c18e20
So, maybe a side effect of this commit/nightly? (for fix: #44836: Deleting a bar causes slurs in following bar(s) to disappear)

EDIT: by the way, note that if a frame (eg horizontal, is "really" inserted inside the score), there is no crash. Only when the frames are "before" or "after" the score, from what I see by now.

Reply

cadiz1

• Jan 21, 2017 - 10:35

I can't reproduce (as before February 17, 2015, the frame is preserved) with the current 3.0 dev.
Solved by : e8621b8
Not of course with 2.0.3 and 2.1 dev.

Reply

Jojo-Schmitz

• Jan 24, 2017 - 17:05

Similar or even same stack trace in https://musescore.org/en/node/165476

Reply

ericfontainejazz

• Jan 25, 2017 - 12:37

In my case that jojo linked to above in #6, I was staring with an Empty Score. The problem was doLayoutRange() trying to dereference the first() measure, which is NULL. And the solution I proposed was to simply test at the beginning of doLayoutRange() that fire() is not NULL.

Another way to create a SEGFAULT with the similar stack trace as OP is:

New Defautl Score
Click on title HFrame and press Delete (but observe how that title is still visaully present)
Ctrl->A, Delete

Resulting stack trace:

1   Ms::MeasureBase::prev                                            measurebase.h     98   0xdda05c   
2   Ms::MeasureBase::prevMeasureMM                                   measurebase.cpp   241  0x83bcab   
3   Ms::Score::doLayoutRange                                         layout.cpp        3395 0x88a78a   
4   Ms::Score::update                                                cmd.cpp           201  0x79add1   
5   Ms::Score::endCmd                                                cmd.cpp           175  0x79aba5   
6   Ms::Score::cmd                                                   cmd.cpp           3269 0x7a8402   
7   Ms::ScoreView::cmd                                               scoreview.cpp     3260 0x414535   
8   Ms::MuseScore::cmd                                               musescore.cpp     4978 0x49fd30   
9   Ms::MuseScore::cmd                                               musescore.cpp     4524 0x49d994   
10  Ms::MuseScore::qt_static_metacall                                moc_musescore.cpp 733  0x69ab98   
11  ZN11QMetaObject8activateEP7QObjectiiPPv                                                 0x68a08842 
12  Ms::ScoreTab::actionTriggered                                    moc_scoretab.cpp  193  0x6a0eb7   
13  Ms::ScoreTab::qt_static_metacall                                 moc_scoretab.cpp  95   0x6a0b89   
14  ZN11QMetaObject8activateEP7QObjectiiPPv                                                 0x68a08842 
15  ZN12QActionGroup7hoveredEP7QAction                                                      0x1d8c5295 
16  ??                                                                                      0x20946d90 
17  ZN7QAction8activateENS_11ActionEventE                                                   0x1d8c4248 
18  ??                                                                                      0x1d1ecdb8 
19  ??                                                                                      0x1fcb0470 
20  ZN7QThread18qt_static_metacallEP7QObjectN11QMetaObject4CallEiPPv                        0x68a67dd0 
...

As i pointed out, the preivous line in doLayoutRange

if (m == 0)
m = first();

never actually checks that first() might be null. As I said doLayoutRange should exit if first() is NULL. I'd be happy to make a PR for that....but I don't know if I should make a seperate issue.

As cadiz says in #6, I can't reporduce the OP's testcase in 3.0dev which apparently was fixed with e8621b8.

Reply

ericfontainejazz

• Jan 25, 2017 - 12:39

As cadiz says in #6, I can't reporduce the OP's testcase in 3.0dev which apparently was fixed with e8621b8.

Well actually...I can produce the exact same stack trace with OP's test file if I first delete the measure and then delete the frame. That causes first() to be NULL and thus the null dereference in doLayoutRange().

Reply

ericfontainejazz

• Feb 12, 2017 - 19:46

(I'm noting that my bug also happens when generating thumbnails when starting musescore if the score causing the problem is in the recent scores list)

1 Ms::MeasureBase::prev measurebase.h 98 0xdd7288
2 Ms::MeasureBase::prevMeasureMM measurebase.cpp 241 0x82b7a7
3 Ms::Score::doLayoutRange layout.cpp 3403 0x87a3b8
4 Ms::Score::update cmd.cpp 219 0x78a783
5 Ms::readScore file.cpp 2154 0x596fff
6 Ms::createThumbnail file.cpp 2722 0x59bc3a
7 Ms::MuseScore::extractThumbnail file.cpp 2749 0x59bed9
8 Ms::ScoreBrowser::genScoreItem scoreBrowser.cpp 109 0x65523f
9 Ms::ScoreBrowser::setScores scoreBrowser.cpp 229 0x65671d
10 Ms::Startcenter::updateRecentScores startcenter.cpp 121 0x654aab
11 Ms::Startcenter::Startcenter startcenter.cpp 66 0x654886
12 Ms::MuseScore::showStartcenter startcenter.cpp 29 0x654448
13 main musescore.cpp 5834 0x4a6d31

Reply

ericfontainejazz

• Feb 12, 2017 - 19:56

My suggested fix at the beginning of doLayoutRange to exit if first() is NULL fixes my bug. I'll submit PR for that, unless someone can suggest a better way to than simply avoiding doLayoutRange.

Reply

ericfontainejazz

• Feb 13, 2017 - 06:46

The OP's issue results from lastMeasure() being NULL, which get's deferenced if etick is <0 at the beginning of doLayoutRange():

if (etick < 0)
etick = lastMeasure()->endTick();

Reply

ericfontainejazz

• Feb 13, 2017 - 07:43

Disregard my two comments above. It turns out what I suggested prevents the crash but causes the score to look gigantic.

Reply

ericfontainejazz

• Feb 13, 2017 - 19:06

I'm getting a very similar stack trace when trying to load a score created in an earlier musescore (2012). Very suspicious I notice that doLayoutRange tries to dereference m when it is NULL. Even though there an earlier test

      if (m == 0)
            m = first();

So that means that someone first() is failing to return a the valid first measure base object. So somehow _first is never getting set properly to begin with...maybe that is where the error in all these lie.

1 Ms::MeasureBase::prev measurebase.h 98 0xdd7288
2 Ms::MeasureBase::prevMeasureMM measurebase.cpp 241 0x82b7a7
3 Ms::Score::doLayoutRange layout.cpp 3403 0x87a3b8
4 Ms::Score::update cmd.cpp 219 0x78a783
5 Ms::readScore file.cpp 2154 0x596fff
6 Ms::MuseScore::readScore file.cpp 329 0x587180
7 Ms::MuseScore::openScore file.cpp 310 0x58704c
8 Ms::MuseScore::loadFiles file.cpp 291 0x586d0e
9 Ms::MuseScore::cmd musescore.cpp 4680 0x49d382
10 Ms::MuseScore::cmd musescore.cpp 4535 0x49ca18
11 Ms::MuseScore::qt_static_metacall moc_musescore.cpp 733 0x68a140
12 ZN11QMetaObject8activateEP7QObjectiiPPv 0x68a08842
13 ZN12QActionGroup7hoveredEP7QAction 0x1f265295
14 ?? 0x2233abe0
15 ZN7QAction8activateENS_11ActionEventE 0x1f264248
16 ?? 0x1e9dc808
17 ?? 0x22493840

Attachment	Size
Yesterdays_Date_Fontaine_leadsheet.mscz	5.61 KB

Reply

Jojo-Schmitz

• Feb 13, 2017 - 19:14

.

Reply

Jojo-Schmitz

• Feb 13, 2017 - 19:13

I've seens this stack trace several times and when looking into it I also saw first() to return NULL

Reply

Git Message

• Feb 14, 2017 - 14:42

Status (old)

active

⇒

fixed

Fixed in branch fix-154551, commit 317c70190a

fix #154551: Crash by removing a score with frames when the last measure contains a pedal line/volta line

Reply

Jojo-Schmitz

• Feb 14, 2017 - 14:55

Status (old)

fixed

⇒

active

I think that fix was an accident? Not applied to master nor 2.1

Reply

Jojo-Schmitz

• Feb 14, 2017 - 15:08

Status (old)

active

⇒

patch (code needs review)

see https://github.com/musescore/MuseScore/pull/2996

Reply

ericfontainejazz

• Feb 14, 2017 - 20:39

Ok, well I guess that PR won't fix the two problems I brought up...so I guess I'll make a separate issue for them.

Reply

Git Message

• Feb 16, 2017 - 11:28

Status (old)

patch (code needs review)

⇒

fixed

Fixed in branch 2.1, commit 317c70190a

fix #154551: Crash by removing a score with frames when the last measure contains a pedal line/volta line

Reply

Git Message

• Feb 16, 2017 - 11:28

Fixed in branch 2.1, commit 30ba005160

Merge pull request #2996 from lasconic/fix-154551

Fix #154551

Reply

ericfontainejazz

• Feb 18, 2017 - 01:29

Folks, now I see the scores load, but similar to what I said in #13 here, "causes the score to look gigantic." See pics here: https://musescore.org/en/node/174806

Reply

ericfontainejazz

• Feb 18, 2017 - 01:37

I also notice that there is a still a crash with empty score when doLayoutRange() tries to dereference a NULL measure. I've spun that off into a separate issue: #174811: Removing all measures in a score leads to all pages disappearing. Leaves empty page instead.

Reply

ericfontainejazz

• Feb 18, 2017 - 02:26

I also spun off the error I mentioned in comment #14 into a separate issue as well: #174821: crash when doLayoutRange() trying to dereference measure because for some reason first() returns null. (I don't know if these are related or not, but null measure dereference in both cases occurs in doLayoutRange(), which is why I posted to this thread.)

Reply

System Message

• Mar 4, 2017 - 02:30

Status (old)

fixed

⇒

closed

Automatically closed -- issue fixed for 2 weeks with no activity.

Reply

Crash by removing a score with frames when the last measure contains a pedal line/volta line

Comments