Fix for #278916 causes crashes

• Apr 30, 2022 - 22:52
Reported version
3.2
Type
Functional
Frequency
Once
Severity
S2 - Critical
Reproducibility
Always
Status
active
Regression
Yes
Workaround
No
Project

I’ve got crashes on a piece of mine. Specifically, when

  • always, when uploading it to mu͒.com without removing the parts
  • always, trying to convert two instances of it at the same time using the -j option (for example, a normal one and one that starts with an even page number)
  • sometimes, when loading it in an interactive session after having worked on some files already

gdb bt for Debian’s 3.2.3:

Thread 1 "musescore3" received signal SIGSEGV, Segmentation fault.
0x00007ffff58e7270 in QVariant::QVariant(QVariant const&) () from /usr/lib/x86_64-linux-gnu/libQt5Core.so.5
(gdb) bt
#0  0x00007ffff58e7270 in QVariant::QVariant(QVariant const&) () from /usr/lib/x86_64-linux-gnu/libQt5Core.so.5
#1  0x0000555555f8735d in Ms::MStyle::value (this=<optimized out>, idx=<optimized out>)
    at /usr/include/c++/10/array:55
#2  0x000055555603ea04 in Ms::Score::styleV (idx=Ms::Sid::NOSTYLE, this=<optimized out>)
    at ./libmscore/score.h:838
#3  Ms::ScoreElement::styleValue (this=0x555557688a60, pid=<optimized out>, sid=<optimized out>)
    at ./libmscore/scoreElement.cpp:841
#4  0x000055555603ec54 in Ms::ScoreElement::initElementStyle (this=this@entry=0x555557688a60,
    ss=ss@entry=0x555556821c20 <Ms::textLineSegmentStyle>) at ./libmscore/scoreElement.cpp:211

Code in question:

void ScoreElement::initElementStyle(const ElementStyle* ss)
      {
      _elementStyle = ss;
      size_t n      = _elementStyle->size();
      delete[] _propertyFlagsList;
      _propertyFlagsList = new PropertyFlags[n];
      for (size_t i = 0; i < n; ++i)
            _propertyFlagsList[i] = PropertyFlags::STYLED;
      for (const StyledProperty& spp : *_elementStyle)
//            setProperty(spp.pid, styleValue(spp.pid, spp.sid));
            setProperty(spp.pid, styleValue(spp.pid, getPropertyStyle(spp.pid)));
      }

This code is unchanged since 2018, so it applies to most 3.x versions, including those newer than Debian’s.

Supporting code:

Sid ScoreElement::getPropertyStyle(Pid id) const
      {
      for (const StyledProperty& p : *_elementStyle) {
            if (p.pid == id)
                  return p.sid;
            }
      return Sid::NOSTYLE;
      }

The crash occurs because getPropertyStyle returns Sid::NOSTYLE, which is -1.

Looking at https://musescore.org/en/node/278916 the code itself seems to have fixed something, but I cannot see, from looking at the code, what/how… as far as I can see, getPropertyStyle would just return the same as spp.sid is, always? (Because both access _elementStyle.)

In the same file, I see this:

QVariant ScoreElement::propertyDefault(Pid pid) const
      {
      Sid sid = getPropertyStyle(pid);
      if (sid != Sid::NOSTYLE)
            return styleValue(pid, sid);
//      qDebug("<%s>(%d) not found in <%s>", propertyQmlName(pid), int(pid), name());
      return QVariant();
      }

This makes me think that changing the line to…

            setProperty(spp.pid, propertyDefault(spp.pid));

might fix the bug… if passing just a new QVariant to setProperty is the correct thing to do here.

The question is, why does it return Sid::NOSTYLE in the first place?

More detail:

(gdb) frame 4
#4  0x000055555603ec54 in Ms::ScoreElement::initElementStyle (this=this@entry=0x55555768d3d0, 
    ss=ss@entry=0x555556821c20 <Ms::textLineSegmentStyle>) at ./libmscore/scoreElement.cpp:211
211                 setProperty(spp.pid, styleValue(spp.pid, getPropertyStyle(spp.pid)));
(gdb) print spp
$68 = (const Ms::StyledProperty &) @0x5555568a7aa0: {sid = Ms::Sid::textLinePosAbove, pid = Ms::Pid::OFFSET}
(gdb) print ss
$69 = (const Ms::ElementStyle *) 0x555556821c20 <Ms::textLineSegmentStyle>
(gdb) print *ss
$70 = std::vector of length 2, capacity 2 = {{sid = Ms::Sid::textLinePosAbove, pid = Ms::Pid::OFFSET}, {
    sid = Ms::Sid::textLineMinDistance, pid = Ms::Pid::MIN_DISTANCE}}
(gdb) print spp.pid
$71 = Ms::Pid::OFFSET
(gdb) print getPropertyStyle(spp.pid)
$72 = Ms::Sid::NOSTYLE
(gdb) print _elementStyle
$73 = (const Ms::ElementStyle *) 0x555556821c20 <Ms::textLineSegmentStyle>
(gdb) print *_elementStyle
$74 = std::vector of length 2, capacity 2 = {{sid = Ms::Sid::textLinePosAbove, pid = Ms::Pid::OFFSET}, {
    sid = Ms::Sid::textLineMinDistance, pid = Ms::Pid::MIN_DISTANCE}}

Scratch the above. WHY does getPropertyStyle not return Ms::Sid::textLinePosAbove but Ms::Sid::NOSTYLE? Oh wait, it’s not a ScoreElement, it’s an Ms::TextLineSegment… HAH! and that has:

Sid TextLineSegment::getPropertyStyle(Pid pid) const
      {
      if (pid == Pid::OFFSET) {
            if (spanner()->anchor() == Spanner::Anchor::NOTE)
                  return Sid::NOSTYLE;
            else
                  return spanner()->placeAbove() ? Sid::textLinePosAbove : Sid::textLinePosBelow;
            }
      return TextLineBaseSegment::getPropertyStyle(pid);
      }

A breakpoint later:

(gdb) print _spanner->_anchor
$81 = Ms::Spanner::Anchor::NOTE

Well, that’s that. So this… somewhat… applies to note-anchored textlines?


Comments

Maybe this could be a fix? I’ll test it…

Description: Fix crash in some files
 where getPropertyStyle(spp.pid) returns Sid::NOSTYLE
 .
 replace styleValue(x, getPropertyStyle(x)) with safePropertyStyleValue(x)
Author: mirabilos <m@mirbsd.org>
Bug: https://musescore.org/en/node/331690
Forwarded: yes
 
--- a/libmscore/ottava.cpp
+++ b/libmscore/ottava.cpp
@@ -405,7 +405,7 @@ QVariant Ottava::propertyDefault(Pid pid
             case Pid::END_TEXT:
                   return QString("");
             case Pid::PLACEMENT:
-                  return styleValue(Pid::PLACEMENT, getPropertyStyle(Pid::PLACEMENT));
+                  return safePropertyStyleValue(Pid::PLACEMENT);
 
             default:
                   return TextLineBase::propertyDefault(pid);
--- a/libmscore/scoreElement.cpp
+++ b/libmscore/scoreElement.cpp
@@ -207,8 +207,7 @@ void ScoreElement::initElementStyle(cons
       for (size_t i = 0; i < n; ++i)
             _propertyFlagsList[i] = PropertyFlags::STYLED;
       for (const StyledProperty& spp : *_elementStyle)
-//            setProperty(spp.pid, styleValue(spp.pid, spp.sid));
-            setProperty(spp.pid, styleValue(spp.pid, getPropertyStyle(spp.pid)));
+            setProperty(spp.pid, safePropertyStyleValue(spp.pid));
       }
 
 //---------------------------------------------------------
@@ -737,7 +736,7 @@ void ScoreElement::styleChanged()
       for (const StyledProperty& spp : *_elementStyle) {
             PropertyFlags f = propertyFlags(spp.pid);
             if (f == PropertyFlags::STYLED)
-                  setProperty(spp.pid, styleValue(spp.pid, getPropertyStyle(spp.pid)));
+                  setProperty(spp.pid, safePropertyStyleValue(spp.pid));
             }
       }
 
@@ -856,5 +855,17 @@ QVariant ScoreElement::styleValue(Pid pi
                   return score()->styleV(sid);
             }
       }
-}
 
+//---------------------------------------------------------
+//   styleValueSafe
+//---------------------------------------------------------
+
+QVariant ScoreElement::safePropertyStyleValue(Pid pid) const
+      {
+      Sid sid = getPropertyStyle(pid);
+      if (sid != Sid::NOSTYLE)
+            return styleValue(pid, sid);
+      return propertyDefault(pid);
+      }
+
+}
--- a/libmscore/scoreElement.h
+++ b/libmscore/scoreElement.h
@@ -215,6 +215,7 @@ class ScoreElement {
       virtual PropertyFlags propertyFlags(Pid) const;
       bool isStyled(Pid pid) const;
       QVariant styleValue(Pid, Sid) const;
+      QVariant safePropertyStyleValue(Pid) const;
 
       virtual void setPropertyFlags(Pid, PropertyFlags);
 
--- a/libmscore/textbase.cpp
+++ b/libmscore/textbase.cpp
@@ -2656,13 +2656,13 @@ void TextBase::styleChanged()
       for (const StyledProperty& spp : *_elementStyle) {
             PropertyFlags f = _propertyFlagsList[i];
             if (f == PropertyFlags::STYLED)
-                  setProperty(spp.pid, styleValue(spp.pid, getPropertyStyle(spp.pid)));
+                  setProperty(spp.pid, safePropertyStyleValue(spp.pid));
             ++i;
             }
       for (const StyledProperty& spp : *textStyle(tid())) {
             PropertyFlags f = _propertyFlagsList[i];
             if (f == PropertyFlags::STYLED)
-                  setProperty(spp.pid, styleValue(spp.pid, getPropertyStyle(spp.pid)));
+                  setProperty(spp.pid, safePropertyStyleValue(spp.pid));
             ++i;
             }
       }

@Jojo-Schmitz you know, the best thing about reproducers is when they don’t reproduce on another system which is supposedly mostly identical, version-wise.

But I was able to see it in valgrind, with even the easiest call:

tglase@tglase:~ $ valgrind mscore3 -o TebePoem.pdf Bortnjanski\ --\ Tebe\ poem.mscz                                                                                                                    
==583== Memcheck, a memory error detector                                                                                                                                                              
==583== Copyright (C) 2002-2017, and GNU GPL'd, by Julian Seward et al.                                                                                                                                
==583== Using Valgrind-3.16.1 and LibVEX; rerun with -h for copyright info                                                                                                                             
==583== Command: mscore3 -o TebePoem.pdf Bortnjanski\ --\ Tebe\ poem.mscz                                                                                                                              
==583==                                                                                                                                         
convert <Bortnjanski -- Tebe poem.mscz>...                                                                                                                                                             
==583== Conditional jump or move depends on uninitialised value(s)                                                                                                                                     
==583==    at 0x9CBE16: Ms::Articulation::anchorGroup(Ms::SymId) (in /usr/bin/mscore3)                                                                                                                 
==583==    by 0x9CD2AE: Ms::Articulation::getPropertyStyle(Ms::Pid) const (in /usr/bin/mscore3)                                                                                                        
==583==    by 0xBF2C43: Ms::ScoreElement::initElementStyle(std::vector<Ms::StyledProperty, std::allocator<Ms::                                                                                         StyledProperty> > const*) (in /usr/bin/mscore3)                                                                                                                                                        
==583==    by 0x9CBCDF: Ms::Articulation::Articulation(Ms::Score*) (in /usr/bin/mscore3)                                                                                                               
==583==    by 0xA0E25C: Ms::ChordRest::readProperties(Ms::XmlReader&) (in /usr/bin/mscore3)                                                                                                            
==583==    by 0x9F36BA: Ms::Chord::readProperties(Ms::XmlReader&) (in /usr/bin/mscore3)                                                                                                                
==583==    by 0x9F3DAA: Ms::Chord::read(Ms::XmlReader&) (in /usr/bin/mscore3)                                                                                                                          
==583==    by 0xAB19B2: Ms::Measure::readVoice(Ms::XmlReader&, int, bool) (in /usr/bin/mscore3)                                                                                                        
==583==    by 0xAB4144: Ms::Measure::read(Ms::XmlReader&, int) (in /usr/bin/mscore3)                                                                                                                   
==583==    by 0xBB09BD: Ms::Score::readStaff(Ms::XmlReader&) (in /usr/bin/mscore3)                                                                                                                     
==583==    by 0xC216DB: Ms::Score::read(Ms::XmlReader&) (in /usr/bin/mscore3)                                                                                                                          
==583==    by 0xC22CCF: Ms::MasterScore::read(Ms::XmlReader&) (in /usr/bin/mscore3)                                                                                                                    
==583==                                                                                                                                         
==583== Conditional jump or move depends on uninitialised value(s)                                                                                                                                     
==583==    at 0x9CBE23: Ms::Articulation::anchorGroup(Ms::SymId) (in /usr/bin/mscore3)                                                                                                                 
==583==    by 0x9CD2AE: Ms::Articulation::getPropertyStyle(Ms::Pid) const (in /usr/bin/mscore3)                                                                                                        
==583==    by 0xBF2C43: Ms::ScoreElement::initElementStyle(std::vector<Ms::StyledProperty, std::allocator<Ms::                                                                                         StyledProperty> > const*) (in /usr/bin/mscore3)                                                                                                                                                        
==583==    by 0x9CBCDF: Ms::Articulation::Articulation(Ms::Score*) (in /usr/bin/mscore3)                                                                                                               
==583==    by 0xA0E25C: Ms::ChordRest::readProperties(Ms::XmlReader&) (in /usr/bin/mscore3)                                                                                                            
==583==    by 0x9F36BA: Ms::Chord::readProperties(Ms::XmlReader&) (in /usr/bin/mscore3)                                                                                                                
==583==    by 0x9F3DAA: Ms::Chord::read(Ms::XmlReader&) (in /usr/bin/mscore3)                                                                                                                          
==583==    by 0xAB19B2: Ms::Measure::readVoice(Ms::XmlReader&, int, bool) (in /usr/bin/mscore3)                                                                                                        
==583==    by 0xAB4144: Ms::Measure::read(Ms::XmlReader&, int) (in /usr/bin/mscore3)                                                                                                                   
==583==    by 0xBB09BD: Ms::Score::readStaff(Ms::XmlReader&) (in /usr/bin/mscore3)                                                                                                                     
==583==    by 0xC216DB: Ms::Score::read(Ms::XmlReader&) (in /usr/bin/mscore3)                                                                                                                          
==583==    by 0xC22CCF: Ms::MasterScore::read(Ms::XmlReader&) (in /usr/bin/mscore3)                                                                                                                    
==583==                                                                                                                                         
==583== Conditional jump or move depends on uninitialised value(s)                                                                                                                                     
==583==    at 0x9CBE2B: Ms::Articulation::anchorGroup(Ms::SymId) (in /usr/bin/mscore3)                                                                                                                 
==583==    by 0x9CD2AE: Ms::Articulation::getPropertyStyle(Ms::Pid) const (in /usr/bin/mscore3)                                                                                                        
==583==    by 0xBF2C43: Ms::ScoreElement::initElementStyle(std::vector<Ms::StyledProperty, std::allocator<Ms::                                                                                         StyledProperty> > const*) (in /usr/bin/mscore3)                                                                                                                                                        
==583==    by 0x9CBCDF: Ms::Articulation::Articulation(Ms::Score*) (in /usr/bin/mscore3)                                                                                                               
==583==    by 0xA0E25C: Ms::ChordRest::readProperties(Ms::XmlReader&) (in /usr/bin/mscore3)
==583==    by 0x9F36BA: Ms::Chord::readProperties(Ms::XmlReader&) (in /usr/bin/mscore3)
==583==    by 0x9F3DAA: Ms::Chord::read(Ms::XmlReader&) (in /usr/bin/mscore3)
==583==    by 0xAB19B2: Ms::Measure::readVoice(Ms::XmlReader&, int, bool) (in /usr/bin/mscore3)
==583==    by 0xAB4144: Ms::Measure::read(Ms::XmlReader&, int) (in /usr/bin/mscore3)
==583==    by 0xBB09BD: Ms::Score::readStaff(Ms::XmlReader&) (in /usr/bin/mscore3)
==583==    by 0xC216DB: Ms::Score::read(Ms::XmlReader&) (in /usr/bin/mscore3)
==583==    by 0xC22CCF: Ms::MasterScore::read(Ms::XmlReader&) (in /usr/bin/mscore3)
==583== 
==583== Conditional jump or move depends on uninitialised value(s)
==583==    at 0x9CBE35: Ms::Articulation::anchorGroup(Ms::SymId) (in /usr/bin/mscore3)
==583==    by 0x9CD2AE: Ms::Articulation::getPropertyStyle(Ms::Pid) const (in /usr/bin/mscore3)
==583==    by 0xBF2C43: Ms::ScoreElement::initElementStyle(std::vector<Ms::StyledProperty, std::allocator<Ms::StyledProperty> > const*) (in /usr/bin/mscore3)
==583==    by 0x9CBCDF: Ms::Articulation::Articulation(Ms::Score*) (in /usr/bin/mscore3)
==583==    by 0xA0E25C: Ms::ChordRest::readProperties(Ms::XmlReader&) (in /usr/bin/mscore3)
==583==    by 0x9F36BA: Ms::Chord::readProperties(Ms::XmlReader&) (in /usr/bin/mscore3)
==583==    by 0x9F3DAA: Ms::Chord::read(Ms::XmlReader&) (in /usr/bin/mscore3)
==583==    by 0xAB19B2: Ms::Measure::readVoice(Ms::XmlReader&, int, bool) (in /usr/bin/mscore3)
==583==    by 0xAB4144: Ms::Measure::read(Ms::XmlReader&, int) (in /usr/bin/mscore3)
==583==    by 0xBB09BD: Ms::Score::readStaff(Ms::XmlReader&) (in /usr/bin/mscore3)
==583==    by 0xC216DB: Ms::Score::read(Ms::XmlReader&) (in /usr/bin/mscore3)
==583==    by 0xC22CCF: Ms::MasterScore::read(Ms::XmlReader&) (in /usr/bin/mscore3)
==583== 
==583== Use of uninitialised value of size 8
==583==    at 0xB3B2D5: Ms::MStyle::value(Ms::Sid) const (in /usr/bin/mscore3)
==583==    by 0xBF28F9: Ms::ScoreElement::styleValue(Ms::Pid, Ms::Sid) const (in /usr/bin/mscore3)
==583==    by 0xBF2C53: Ms::ScoreElement::initElementStyle(std::vector<Ms::StyledProperty, std::allocator<Ms::StyledProperty> > const*) (in /usr/bin/mscore3)
==583==    by 0x9CBCDF: Ms::Articulation::Articulation(Ms::Score*) (in /usr/bin/mscore3)
==583==    by 0xA0E25C: Ms::ChordRest::readProperties(Ms::XmlReader&) (in /usr/bin/mscore3)
==583==    by 0x9F36BA: Ms::Chord::readProperties(Ms::XmlReader&) (in /usr/bin/mscore3)
==583==    by 0x9F3DAA: Ms::Chord::read(Ms::XmlReader&) (in /usr/bin/mscore3)
==583==    by 0xAB19B2: Ms::Measure::readVoice(Ms::XmlReader&, int, bool) (in /usr/bin/mscore3)
==583==    by 0xAB4144: Ms::Measure::read(Ms::XmlReader&, int) (in /usr/bin/mscore3)
==583==    by 0xBB09BD: Ms::Score::readStaff(Ms::XmlReader&) (in /usr/bin/mscore3)
==583==    by 0xC216DB: Ms::Score::read(Ms::XmlReader&) (in /usr/bin/mscore3)
==583==    by 0xC22CCF: Ms::MasterScore::read(Ms::XmlReader&) (in /usr/bin/mscore3)
==583== 
==583== Use of uninitialised value of size 8
==583==    at 0x6FC01E0: QVariant::QVariant(QVariant const&) (in /usr/lib/x86_64-linux-gnu/libQt5Core.so.5.15.2)
==583==    by 0xB3B35C: Ms::MStyle::value(Ms::Sid) const (in /usr/bin/mscore3)
==583==    by 0xBF28F9: Ms::ScoreElement::styleValue(Ms::Pid, Ms::Sid) const (in /usr/bin/mscore3)
==583==    by 0xBF2C53: Ms::ScoreElement::initElementStyle(std::vector<Ms::StyledProperty, std::allocator<Ms::StyledProperty> > const*) (in /usr/bin/mscore3)
==583==    by 0x9CBCDF: Ms::Articulation::Articulation(Ms::Score*) (in /usr/bin/mscore3)
==583==    by 0xA0E25C: Ms::ChordRest::readProperties(Ms::XmlReader&) (in /usr/bin/mscore3)
==583==    by 0x9F36BA: Ms::Chord::readProperties(Ms::XmlReader&) (in /usr/bin/mscore3)
==583==    by 0x9F3DAA: Ms::Chord::read(Ms::XmlReader&) (in /usr/bin/mscore3)
==583==    by 0xAB19B2: Ms::Measure::readVoice(Ms::XmlReader&, int, bool) (in /usr/bin/mscore3)
==583==    by 0xAB4144: Ms::Measure::read(Ms::XmlReader&, int) (in /usr/bin/mscore3)
==583==    by 0xBB09BD: Ms::Score::readStaff(Ms::XmlReader&) (in /usr/bin/mscore3)
==583==    by 0xC216DB: Ms::Score::read(Ms::XmlReader&) (in /usr/bin/mscore3)
==583== 
==583== Conditional jump or move depends on uninitialised value(s)
==583==    at 0x9CBE59: Ms::Articulation::anchorGroup(Ms::SymId) (in /usr/bin/mscore3)
==583==    by 0x9CD2AE: Ms::Articulation::getPropertyStyle(Ms::Pid) const (in /usr/bin/mscore3)
==583==    by 0xBF2C43: Ms::ScoreElement::initElementStyle(std::vector<Ms::StyledProperty, std::allocator<Ms::StyledProperty> > const*) (in /usr/bin/mscore3)
==583==    by 0x9CBCDF: Ms::Articulation::Articulation(Ms::Score*) (in /usr/bin/mscore3)
==583==    by 0xA0E25C: Ms::ChordRest::readProperties(Ms::XmlReader&) (in /usr/bin/mscore3)
==583==    by 0x9F36BA: Ms::Chord::readProperties(Ms::XmlReader&) (in /usr/bin/mscore3)
==583==    by 0x9F3DAA: Ms::Chord::read(Ms::XmlReader&) (in /usr/bin/mscore3)
==583==    by 0xAB19B2: Ms::Measure::readVoice(Ms::XmlReader&, int, bool) (in /usr/bin/mscore3)
==583==    by 0xAB4144: Ms::Measure::read(Ms::XmlReader&, int) (in /usr/bin/mscore3)
==583==    by 0xBB0F23: Ms::Score::readStaff(Ms::XmlReader&) (in /usr/bin/mscore3)
==583==    by 0xC216DB: Ms::Score::read(Ms::XmlReader&) (in /usr/bin/mscore3)
==583==    by 0xC22CCF: Ms::MasterScore::read(Ms::XmlReader&) (in /usr/bin/mscore3)
==583== 
Trying to construct an instance of an invalid type, type id: 117855344
==583== Conditional jump or move depends on uninitialised value(s)
==583==    at 0xB3B2DC: Ms::MStyle::value(Ms::Sid) const (in /usr/bin/mscore3)
==583==    by 0xBF2A03: Ms::ScoreElement::styleValue(Ms::Pid, Ms::Sid) const (in /usr/bin/mscore3)
==583==    by 0xBF2C53: Ms::ScoreElement::initElementStyle(std::vector<Ms::StyledProperty, std::allocator<Ms::StyledProperty> > const*) (in /usr/bin/mscore3)
==583==    by 0xB665F5: Ms::TextLine::createLineSegment() (in /usr/bin/mscore3)
==583==    by 0xA9D9AD: Ms::SLine::readProperties(Ms::XmlReader&) (in /usr/bin/mscore3)
==583==    by 0xB68BA2: Ms::TextLineBase::readProperties(Ms::XmlReader&) (in /usr/bin/mscore3)
==583==    by 0xB66D87: Ms::TextLineBase::read(Ms::XmlReader&) (in /usr/bin/mscore3)
==583==    by 0xC297C0: Ms::ConnectorInfoReader::read() (in /usr/bin/mscore3)
==583==    by 0xC298D3: Ms::ConnectorInfoReader::readConnector(std::unique_ptr<Ms::ConnectorInfoReader, std::default_delete<Ms::ConnectorInfoReader> >, Ms::XmlReader&) (in /usr/bin/mscore3)
==583==    by 0xB21410: Ms::Spanner::readSpanner(Ms::XmlReader&, Ms::Element*, int) (in /usr/bin/mscore3)
==583==    by 0xAC06E1: Ms::Note::readProperties(Ms::XmlReader&) (in /usr/bin/mscore3)
==583==    by 0xAC0F22: Ms::Note::read(Ms::XmlReader&) (in /usr/bin/mscore3)
==583== 
invalid style value -1 no style
no sys staff
        to <TebePoem.pdf>
... success!
==583== 
==583== HEAP SUMMARY:
==583==     in use at exit: 7,911,082 bytes in 57,583 blocks
==583==   total heap usage: 658,189 allocs, 600,606 frees, 171,440,490 bytes allocated
==583== 
==583== LEAK SUMMARY:
==583==    definitely lost: 77,304 bytes in 342 blocks
==583==    indirectly lost: 860,240 bytes in 4,889 blocks
==583==      possibly lost: 27,072 bytes in 149 blocks
==583==    still reachable: 6,946,466 bytes in 52,203 blocks
==583==         suppressed: 0 bytes in 0 blocks
==583== Rerun with --leak-check=full to see details of leaked memory
==583== 
==583== Use --track-origins=yes to see where uninitialised values come from
==583== For lists of detected and suppressed errors, rerun with: -s
==583== ERROR SUMMARY: 27 errors from 8 contexts (suppressed: 0 from 0)

My previous reproducer involved a -j JSON file conversion where it was converted multiple times (PDF, part PDFs, …).

Attachment Size
Bortnjanski -- Tebe poem.mscz 43.08 KB

Bad news: the fix isn’t eliminating all invalid memory accesses either:

==3441== Memcheck, a memory error detector
==3441== Copyright (C) 2002-2017, and GNU GPL'd, by Julian Seward et al.
==3441== Using Valgrind-3.16.1 and LibVEX; rerun with -h for copyright info
==3441== Command: mscore3 -o /tmp/x.pdf free/Bortnjanski\ --\ Tebe\ poem.mscx
==3441==
load default style </home/tglase/Misc/Vendor/mirsol/music/Mscore3/vocal-resv.mss>
==3461== Warning: invalid file descriptor 1024 in syscall close()
==3461== Warning: invalid file descriptor 1025 in syscall close()
==3461== Warning: invalid file descriptor 1026 in syscall close()
==3461== Warning: invalid file descriptor 1027 in syscall close()
==3461==    Use --log-fd=<number> to select an alternative log fd.
==3461== Warning: invalid file descriptor 1028 in syscall close()
==3461== Warning: invalid file descriptor 1029 in syscall close()
convert <free/Bortnjanski -- Tebe poem.mscx>...
==3441== Conditional jump or move depends on uninitialised value(s)
==3441==    at 0x9CD036: Ms::Articulation::anchorGroup(Ms::SymId) (articulation.cpp:358)
==3441==    by 0x9CE4CE: getPropertyStyle (articulation.cpp:516)
==3441==    by 0x9CE4CE: Ms::Articulation::getPropertyStyle(Ms::Pid) const (articulation.cpp:509)
==3441==    by 0xBF45BD: Ms::ScoreElement::safePropertyStyleValue(Ms::Pid) const (scoreElement.cpp:865)
==3441==    by 0xBF4695: Ms::ScoreElement::initElementStyle(std::vector<Ms::StyledProperty, std::allocator<Ms::StyledProperty> > const*) (scoreElement.cpp:210)
==3441==    by 0x9CCEFF: Ms::Articulation::Articulation(Ms::Score*) (articulation.cpp:45)
==3441==    by 0xA0F39C: Ms::ChordRest::readProperties(Ms::XmlReader&) (chordrest.cpp:255)
==3441==    by 0x9F47FA: Ms::Chord::readProperties(Ms::XmlReader&) (chord.cpp:1045)
==3441==    by 0x9F4EEA: Ms::Chord::read(Ms::XmlReader&) (chord.cpp:1022)
==3441==    by 0xAB2F52: Ms::Measure::readVoice(Ms::XmlReader&, int, bool) (measure.cpp:2127)
==3441==    by 0xAB57CE: Ms::Measure::read(Ms::XmlReader&, int) (measure.cpp:1969)
==3441==    by 0xBB3D4D: Ms::Score::readStaff(Ms::XmlReader&) (scorefile.cpp:300)
==3441==    by 0xC2312B: Ms::Score::read(Ms::XmlReader&) (read301.cpp:45)
==3441==
==3441== Conditional jump or move depends on uninitialised value(s)
==3441==    at 0x9CD043: Ms::Articulation::anchorGroup(Ms::SymId) (articulation.cpp:358)
==3441==    by 0x9CE4CE: getPropertyStyle (articulation.cpp:516)
==3441==    by 0x9CE4CE: Ms::Articulation::getPropertyStyle(Ms::Pid) const (articulation.cpp:509)
==3441==    by 0xBF45BD: Ms::ScoreElement::safePropertyStyleValue(Ms::Pid) const (scoreElement.cpp:865)
==3441==    by 0xBF4695: Ms::ScoreElement::initElementStyle(std::vector<Ms::StyledProperty, std::allocator<Ms::StyledProperty> > const*) (scoreElement.cpp:210)
==3441==    by 0x9CCEFF: Ms::Articulation::Articulation(Ms::Score*) (articulation.cpp:45)
==3441==    by 0xA0F39C: Ms::ChordRest::readProperties(Ms::XmlReader&) (chordrest.cpp:255)
==3441==    by 0x9F47FA: Ms::Chord::readProperties(Ms::XmlReader&) (chord.cpp:1045)
==3441==    by 0x9F4EEA: Ms::Chord::read(Ms::XmlReader&) (chord.cpp:1022)
==3441==    by 0xAB2F52: Ms::Measure::readVoice(Ms::XmlReader&, int, bool) (measure.cpp:2127)
==3441==    by 0xAB57CE: Ms::Measure::read(Ms::XmlReader&, int) (measure.cpp:1969)
==3441==    by 0xBB3D4D: Ms::Score::readStaff(Ms::XmlReader&) (scorefile.cpp:300)
==3441==    by 0xC2312B: Ms::Score::read(Ms::XmlReader&) (read301.cpp:45)
==3441==
==3441== Conditional jump or move depends on uninitialised value(s)
==3441==    at 0x9CD04B: Ms::Articulation::anchorGroup(Ms::SymId) (articulation.cpp:358)
==3441==    by 0x9CE4CE: getPropertyStyle (articulation.cpp:516)
==3441==    by 0x9CE4CE: Ms::Articulation::getPropertyStyle(Ms::Pid) const (articulation.cpp:509)
==3441==    by 0xBF45BD: Ms::ScoreElement::safePropertyStyleValue(Ms::Pid) const (scoreElement.cpp:865)
==3441==    by 0xBF4695: Ms::ScoreElement::initElementStyle(std::vector<Ms::StyledProperty, std::allocator<Ms::StyledProperty> > const*) (scoreElement.cpp:210)
==3441==    by 0x9CCEFF: Ms::Articulation::Articulation(Ms::Score*) (articulation.cpp:45)
==3441==    by 0xA0F39C: Ms::ChordRest::readProperties(Ms::XmlReader&) (chordrest.cpp:255)
==3441==    by 0x9F47FA: Ms::Chord::readProperties(Ms::XmlReader&) (chord.cpp:1045)
==3441==    by 0x9F4EEA: Ms::Chord::read(Ms::XmlReader&) (chord.cpp:1022)
==3441==    by 0xAB2F52: Ms::Measure::readVoice(Ms::XmlReader&, int, bool) (measure.cpp:2127)
==3441==    by 0xAB57CE: Ms::Measure::read(Ms::XmlReader&, int) (measure.cpp:1969)
==3441==    by 0xBB3D4D: Ms::Score::readStaff(Ms::XmlReader&) (scorefile.cpp:300)
==3441==    by 0xC2312B: Ms::Score::read(Ms::XmlReader&) (read301.cpp:45)
==3441==
==3441== Conditional jump or move depends on uninitialised value(s)
==3441==    at 0x9CD055: Ms::Articulation::anchorGroup(Ms::SymId) (articulation.cpp:358)
==3441==    by 0x9CE4CE: getPropertyStyle (articulation.cpp:516)
==3441==    by 0x9CE4CE: Ms::Articulation::getPropertyStyle(Ms::Pid) const (articulation.cpp:509)
==3441==    by 0xBF45BD: Ms::ScoreElement::safePropertyStyleValue(Ms::Pid) const (scoreElement.cpp:865)
==3441==    by 0xBF4695: Ms::ScoreElement::initElementStyle(std::vector<Ms::StyledProperty, std::allocator<Ms::StyledProperty> > const*) (scoreElement.cpp:210)
==3441==    by 0x9CCEFF: Ms::Articulation::Articulation(Ms::Score*) (articulation.cpp:45)
==3441==    by 0xA0F39C: Ms::ChordRest::readProperties(Ms::XmlReader&) (chordrest.cpp:255)
==3441==    by 0x9F47FA: Ms::Chord::readProperties(Ms::XmlReader&) (chord.cpp:1045)
==3441==    by 0x9F4EEA: Ms::Chord::read(Ms::XmlReader&) (chord.cpp:1022)
==3441==    by 0xAB2F52: Ms::Measure::readVoice(Ms::XmlReader&, int, bool) (measure.cpp:2127)
==3441==    by 0xAB57CE: Ms::Measure::read(Ms::XmlReader&, int) (measure.cpp:1969)
==3441==    by 0xBB3D4D: Ms::Score::readStaff(Ms::XmlReader&) (scorefile.cpp:300)
==3441==    by 0xC2312B: Ms::Score::read(Ms::XmlReader&) (read301.cpp:45)
==3441==
==3441== Conditional jump or move depends on uninitialised value(s)
==3441==    at 0xBF45C1: Ms::ScoreElement::safePropertyStyleValue(Ms::Pid) const (scoreElement.cpp:866)
==3441==    by 0xBF4695: Ms::ScoreElement::initElementStyle(std::vector<Ms::StyledProperty, std::allocator<Ms::StyledProperty> > const*) (scoreElement.cpp:210)
==3441==    by 0x9CCEFF: Ms::Articulation::Articulation(Ms::Score*) (articulation.cpp:45)
==3441==    by 0xA0F39C: Ms::ChordRest::readProperties(Ms::XmlReader&) (chordrest.cpp:255)
==3441==    by 0x9F47FA: Ms::Chord::readProperties(Ms::XmlReader&) (chord.cpp:1045)
==3441==    by 0x9F4EEA: Ms::Chord::read(Ms::XmlReader&) (chord.cpp:1022)
==3441==    by 0xAB2F52: Ms::Measure::readVoice(Ms::XmlReader&, int, bool) (measure.cpp:2127)
==3441==    by 0xAB57CE: Ms::Measure::read(Ms::XmlReader&, int) (measure.cpp:1969)
==3441==    by 0xBB3D4D: Ms::Score::readStaff(Ms::XmlReader&) (scorefile.cpp:300)
==3441==    by 0xC2312B: Ms::Score::read(Ms::XmlReader&) (read301.cpp:45)
==3441==    by 0xC2471F: Ms::MasterScore::read(Ms::XmlReader&) (read301.cpp:275)
==3441==    by 0xC24B0D: Ms::MasterScore::read301(Ms::XmlReader&) (read301.cpp:327)
==3441==
==3441== Use of uninitialised value of size 8
==3441==    at 0xB3CA25: Ms::MStyle::value(Ms::Sid) const (style.cpp:2145)
==3441==    by 0xBF4239: styleV (score.h:838)
==3441==    by 0xBF4239: Ms::ScoreElement::styleValue(Ms::Pid, Ms::Sid) const (scoreElement.cpp:855)
==3441==    by 0xBF45FF: Ms::ScoreElement::safePropertyStyleValue(Ms::Pid) const (scoreElement.cpp:867)
==3441==    by 0xBF4695: Ms::ScoreElement::initElementStyle(std::vector<Ms::StyledProperty, std::allocator<Ms::StyledProperty> > const*) (scoreElement.cpp:210)
==3441==    by 0x9CCEFF: Ms::Articulation::Articulation(Ms::Score*) (articulation.cpp:45)
==3441==    by 0xA0F39C: Ms::ChordRest::readProperties(Ms::XmlReader&) (chordrest.cpp:255)
==3441==    by 0x9F47FA: Ms::Chord::readProperties(Ms::XmlReader&) (chord.cpp:1045)
==3441==    by 0x9F4EEA: Ms::Chord::read(Ms::XmlReader&) (chord.cpp:1022)
==3441==    by 0xAB2F52: Ms::Measure::readVoice(Ms::XmlReader&, int, bool) (measure.cpp:2127)
==3441==    by 0xAB57CE: Ms::Measure::read(Ms::XmlReader&, int) (measure.cpp:1969)
==3441==    by 0xBB3D4D: Ms::Score::readStaff(Ms::XmlReader&) (scorefile.cpp:300)
==3441==    by 0xC2312B: Ms::Score::read(Ms::XmlReader&) (read301.cpp:45)
==3441==
==3441== Use of uninitialised value of size 8
==3441==    at 0x6FBF1E0: QVariant::QVariant(QVariant const&) (in /usr/lib/x86_64-linux-gnu/libQt5Core.so.5.15.2)
==3441==    by 0xB3CAAC: Ms::MStyle::value(Ms::Sid) const (style.cpp:2149)
==3441==    by 0xBF4239: styleV (score.h:838)
==3441==    by 0xBF4239: Ms::ScoreElement::styleValue(Ms::Pid, Ms::Sid) const (scoreElement.cpp:855)
==3441==    by 0xBF45FF: Ms::ScoreElement::safePropertyStyleValue(Ms::Pid) const (scoreElement.cpp:867)
==3441==    by 0xBF4695: Ms::ScoreElement::initElementStyle(std::vector<Ms::StyledProperty, std::allocator<Ms::StyledProperty> > const*) (scoreElement.cpp:210)
==3441==    by 0x9CCEFF: Ms::Articulation::Articulation(Ms::Score*) (articulation.cpp:45)
==3441==    by 0xA0F39C: Ms::ChordRest::readProperties(Ms::XmlReader&) (chordrest.cpp:255)
==3441==    by 0x9F47FA: Ms::Chord::readProperties(Ms::XmlReader&) (chord.cpp:1045)
==3441==    by 0x9F4EEA: Ms::Chord::read(Ms::XmlReader&) (chord.cpp:1022)
==3441==    by 0xAB2F52: Ms::Measure::readVoice(Ms::XmlReader&, int, bool) (measure.cpp:2127)
==3441==    by 0xAB57CE: Ms::Measure::read(Ms::XmlReader&, int) (measure.cpp:1969)
==3441==    by 0xBB3D4D: Ms::Score::readStaff(Ms::XmlReader&) (scorefile.cpp:300)
==3441==
no sys staff
  to </tmp/x.pdf>
... success!
==3441==
==3441== HEAP SUMMARY:
==3441==     in use at exit: 8,166,287 bytes in 64,048 blocks
==3441==   total heap usage: 654,136 allocs, 590,088 frees, 246,789,943 bytes allocated
==3441==
==3441== LEAK SUMMARY:
==3441==    definitely lost: 73,224 bytes in 343 blocks
==3441==    indirectly lost: 546,598 bytes in 3,357 blocks
==3441==      possibly lost: 61,154 bytes in 156 blocks
==3441==    still reachable: 7,485,311 bytes in 60,192 blocks
==3441==         suppressed: 0 bytes in 0 blocks
==3441== Rerun with --leak-check=full to see details of leaked memory
==3441==
==3441== Use --track-origins=yes to see where uninitialised values come from
==3441== For lists of detected and suppressed errors, rerun with: -s
==3441== ERROR SUMMARY: 35 errors from 7 contexts (suppressed: 0 from 0)

Maybe someone else has a better idea?

Looks like the getPropertyStyle in the first place also accesses uninitialised memory.

I wonder if the _elementStyle, which is set to the ss argument of the initElementStyle function, is maybe not correctly initialised in the first place?

cough

static const ElementStyle articulationStyle {
      { Sid::articulationMinDistance, Pid::MIN_DISTANCE },
//      { Sid::articulationOffset, Pid::OFFSET },
      { Sid::articulationAnchorDefault, Pid::ARTICULATION_ANCHOR },
      };
 
//---------------------------------------------------------
//   Articulation
//---------------------------------------------------------
 
Articulation::Articulation(Score* s) 
   : Element(s, ElementFlag::MOVABLE)
      {
      initElementStyle(&articulationStyle);

cough… cough…

libmscore/style.h:typedef std::vector<StyledProperty> ElementStyle;

What does std::vector initialise and how? Are perhaps all other values uninitialised? Or even… not allocated? Is the size used correct wrt. the actual size? Hm well, it does use…

_elementStyle->size()

… but… I don’t know CFrustFrust enough. Meh. (And I have mentally switched to the Tₑχ/LᴬTᴇΧ side of my project yesternight already…)

In reply to by Jojo-Schmitz

The “cpp” tag is not the “code” tag, though. Both “code” and “pre” are documented (in the help you can open when you report a new issue) but do not work, whereas “cpp” and “xml” work, but have wrong syntax highlighting for things that aren’t C++ or XML.

So your propoposed code change does not fix the issue?

What exactly does reproduce the crash, step by step?`

"cpp" in not the "code" tag, but a code tag.

Perhaps, but I was talking about the “code” and “pre” tag. I need one that works but has no syntax highlighting. The code and pre tags do not work, period.

The proposed change at least hides the crash from me. It seems to fix part of the problem, but not the complete problem space spanned by the underlying issue, as the valgrind output shows.

You can see/reproduce that with the score I attached by running “valgrind mscore3 -o foo.pdf foo.mscz” (FSVO foo being the score name).

Note that running under Valgrind is very slow.

So that doesn't reproduce the crash you're talking about in the initial post(s) but `just' shows a memory leak, or am I missing something?

You’re missing something. Valgrind does not just show leaks. These are not leak reports.

Specifically, both Conditional jump or move depends on uninitialised value(s) and Use of uninitialised value of size 8 are certain crashes that are just mitigated by accident because they use memory that happens to be at the accessed location at this point in time, but does not always have to be (for example, from a previous allocation of a different object (which means it involves memory corruption) or uninitialised stack space).

So these point out places where the program will crash if the conditions are right (and otherwise may misbehave and/or corrupt its memory, including creating data corruption, including in the scores when saved).

Let’s look at one I could figure out quickly:

==3441== Conditional jump or move depends on uninitialised value(s)
==3441==    at 0x9CD036: Ms::Articulation::anchorGroup(Ms::SymId) (articulation.cpp:358)
==3441==    by 0x9CE4CE: getPropertyStyle (articulation.cpp:516)

We have:

  356 Articulation::AnchorGroup Articulation::anchorGroup(SymId symId)
  357       {
  358       switch (symId) {

So the read access to symId accesses uninitialised memory. This is passed in from the caller, so now we look at…

  516                   switch (anchorGroup(_symId)) {

… so the thing passed in is _symId, which is initialised…

   42 Articulation::Articulation(Score* s)
   43    : Element(s, ElementFlag::MOVABLE)
   44       {
   45       initElementStyle(&articulationStyle);
   46       _symId         = SymId::noSym;

… too late. The read access occurs in initElementStyle (line 45) but the memory place is initialised only in line 46, afterwards. (Note that, in the valgrind backtrace, articulation.cpp:45 is indeed there, four frames down.)

Is initElementStyle always called first? If so, maybe my first hunch was correct after all and it should use propertyDefault instead?