musescore / MuseScore

MuseScore is an open source and free music notation software. For support, contribution, bug reports, visit MuseScore.org. Fork and make pull requests!
https://musescore.org
Other
12.31k stars 2.66k forks source link

Potential crash when having last system with only one measure and then performing operation that causes that measure to move to the previous system #24705

Open cbjeukendrup opened 2 months ago

cbjeukendrup commented 2 months ago

Issue type

Crash or freeze

Description with steps to reproduce

It's as the title says; an easy instance is the following:

  1. Create 1-instrument empty score with standard settings; note that there are three systems
  2. Add a system break between the pre-last and last measure, so that a new fourth system containing only that last measure is constructed
  3. Remove that system break, so that the last measure moves back to the third system

Now, the fourth system contains no measures anymore, so it should be deleted. But that is apparently not what happens; the layout code doesn't handle this case correctly.

In Address Sanitizer builds, a crash occurs, because systems.back()->measures() is empty but we call .back() on it: https://github.com/musescore/MuseScore/blob/b86e6421b3155c4841129336bd624552d1ea0f00/src/engraving/rendering/score/pagelayout.cpp#L84 In release builds, most likely no crash will occur, but .back() reads potentially garbage data, which could cause problems. So, although it doesn't cause a crash immediately, it would be good to fix.

Supporting files, videos and screenshots

https://github.com/user-attachments/assets/9738bdf0-96b7-4879-8067-61a78f4d439f

What is the latest version of MuseScore Studio where this issue is present?

4.5 (master)

Regression

I was unable to check

Operating system

*

Additional context

No response

Checklist

cbjeukendrup commented 2 months ago

https://github.com/musescore/MuseScore/issues/24648 appears to be the same problem, but its reproduction steps surprisingly don't involve a situation where any system should become empty. Adding a simple emptiness check is easy enough, but it would be better to investigate this a bit deeper to find out whether this emptiness is valid at all.