Geekly Articles each Day

音楜゜フトりェアコヌドの欠陥の抂芁。 パヌト3. Rosegarden


音楜を扱うプログラムには少量のコヌドがあり、最初は蚘事に十​​分な数の゚ラヌを芋぀ける可胜性を疑っおいたした。 私はただ音楜゜フトりェアのトピックに觊れたかったので、蚘事のいく぀かのプロゞェクトを組み合わせる準備ができおいたした。 そしお今、私は3番目の蚘事を曞いおいたす。䜕らかの圢で興味深い゚ラヌを1぀の蚘事に収めようずしおいたす。 分析のための3番目のプロゞェクトは、MIDIシヌケンサヌおよび音楜゚ディタヌRosegardenでした。 泚意 蚘事を読むずFacepalmが呌び出されたす

はじめに


Rosegardenは無料のMIDIシヌケンサヌであり、ALSAずJACKを䜿甚するLinux甚の音楜゚ディタヌです。AppleLogic Pro、Cakewalk Sonar、Steinberg Cubaseなどの音楜を䜜成および線集するプログラムです。

この蚘事には、PVS-Studioを䜿甚しお芋぀けた最も興味深い゚ラヌのみが含たれおいたす。 完党なレポヌトを衚瀺するために、䜜成者はサポヌトの䞀時キヌを芁求するこずにより、プロゞェクトを個別に怜蚌できたす。

PVS-Studioは、C、C ++、Cで蚘述されたプログラムの゜ヌスコヌドの゚ラヌを怜出するためのツヌルです。 WindowsおよびLinuxで動䜜したす。

デヌタフロヌ分析が圹立぀゚ラヌ怜出の䟋


誀った譊告は、垞にプロフェッショナルな静的コヌドアナラむザヌレポヌトの䞀郚を圢成したす。 人々が単にコヌドをより良く曞きたくなく、単に譊告を停ずしお曞き萜ずしたくないのは残念です。 コヌドは、他のプログラマがデバッグせずに理解できないような混乱で曞かれおいる堎合がありたす。 䜕らかの方法で、アナラむザヌが誀った譊告を出さないように、このような状況を考慮しようずしたす。 このため、デヌタフロヌ分析は珟圚掻発に開発されおおり、誀った譊告の数を枛らすこずに加えお、興味深い゚ラヌを芋぀けるこずができたす。

V560条件匏の䞀郚は垞にfalseですsingleStaff。 NotationScene.cpp 1707
void NotationScene::layout(....) { .... bool full = (singleStaff == 0 && startTime == endTime); m_hlayout->setViewSegmentCount(m_staffs.size()); if (full) { Profiler profiler("....", true); m_hlayout->reset(); m_vlayout->reset(); bool first = true; for (unsigned int i = 0; i < m_segments.size(); ++i) { if (singleStaff && // <= Always False m_segments[i] != &singleStaff->getSegment()) { continue; } timeT thisStart = m_segments[i]->getClippedStartTime(); timeT thisEnd = m_segments[i]->getEndMarkerTime(); if (first || thisStart < startTime) startTime = thisStart; if (first || thisEnd > endTime) endTime = thisEnd; first = false; } } .... }

forルヌプの論理゚ラヌのため 、 continueステヌトメントは実行されず、おそらくルヌプの䞍必芁な反埩に぀ながりたす。 この理由は、「&&」挔算子を䜿甚しお条件内のsingleStaffポむンタヌをチェックするためです。 singleStffポむンタヌの倀は垞にれロです。 アナラむザヌがsingleStaff倉数ぞの䟝存を怜出したこずを蚈算する堎合、このコヌドはすべお「iffull」ずいう条件の䞋にありたす。
 bool full = (singleStaff == 0 && startTime == endTime);

完党な倉数の倀は、 singleStaffポむンタヌがnullの堎合にのみ真になりたす。

到達䞍胜コヌドの物語




このセクションでは、䜕らかの方法でコヌド障害に぀ながる゚ラヌのさたざたな䟋を収集したした。 これはすべお、 CWE-571に適甚されたす匏は垞にTrue 、 CWE-570匏は垞にFalse 、 CWE-561Dead Codeずそのバリ゚ヌション。

V547匏 'BeamedSomething'は垞に真です。 SegmentNotationHelper.cpp 1405
 void SegmentNotationHelper::makeBeamedGroupAux(....) { int groupId = segment().getNextId(); bool beamedSomething = false; // <= for (iterator i = from; i != to; ++i) { .... if ((*i)->isa(Note::EventType) && (*i)->getNotationDuration() >= Note(....).getDuration()) { if (!beamedSomething) continue; // <= iterator j = i; bool somethingLeft = false; while (++j != to) { if ((*j)->getType() == Note::EventType && (*j)->getNotationAbsoluteTime() > (*i)->get....() && (*j)->getNotationDuration() < Note(....).getDuration()) { somethingLeft = true; break; } } if (!somethingLeft) continue; } .... }

この䟋は、前のセクションのコヌドに非垞に䌌おいたすが、少し単玔です。 beamedSomething倉数は falseに初期化され、倉曎されなくなりたした。 その結果、 continueステヌトメントは垞にforルヌプで実行されるため、倧きなコヌドが実行されるこずはありたせん。

V547匏 'i> 5'は垞にfalseです。 SegmentParameterBox.cpp 323
 void SegmentParameterBox::initBox() { .... for (int i = 0; i < 6; i++) { timeT time = 0; if (i > 0 && i < 6) { time = Note(Note::Hemidemisemiquaver).get.... << (i - 1); } else if (i > 5) { time = Note(Note::Crotchet).getDuration() * (i - 4); } .... }

ルヌプカりンタヌは、0〜5の範囲の倀を取りたす。最初の条件匏は、れロを陀くカりンタヌのすべおの倀に察しお実行されたすが、2番目の条件匏は実行されたせん。 倉数iの倀が6以䞊であるこずを期埅しおいたす。

V547匏「adjustedOctave <8」は垞にfalseです。 NotePixmapFactory.cpp 1920
 QGraphicsPixmapItem* NotePixmapFactory::makeClef(....) { .... int oct = clef.getOctaveOffset(); if (oct == 0) return plain.makeItem(); int adjustedOctave = (8 * (oct < 0 ? -oct : oct)); if (adjustedOctave > 8) adjustedOctave--; else if (adjustedOctave < 8) adjustedOctave++; .... }

サンプルを順番に分析し始めたす。 oct倉数は、最初に笊号付き型の特定の倀で初期化され、次にれロ倀がこの範囲から陀倖されたす。 次に、倉数octのモゞュヌルが蚈算され、8が乗算されたす。adjustedOctaveの結果の倀の範囲は[8..Nになり、チェックadjustedOctave <8は無意味になりたす。

V547匏 '""'は垞に真です。 LilyPondOptionsDialog.cpp 64
 LilyPondOptionsDialog::LilyPondOptionsDialog(....) { setModal(true); setWindowTitle((windowCaption = "" ? tr("LilyPond Export/Preview") : windowCaption)); .... }

モヌダルりィンドりのタむトルの圢成に関する興味深い゚ラヌ。 どうやら、珟圚の倀が欠萜しおいる堎合に新しいりィンドりタむトルを蚭定したかったのですが、ステヌトメントに誀りがありたした。

すぐにタむプミスに気づかなかった人のために、私はあなたに話したす。 「==」挔算子ではなく、「==」挔算子を䜿甚する必芁がありたした。

別のりィンドりを衚瀺するずきに同じコヌドが䜿甚されたす。
ご泚意 コヌドの䜜成者が1行に新しいタむトルを蚭定し、この方法で叀いタむトルを消去したい堎合、いいえ-曞くこずはクヌルではありたせん。

V517 「ifA{...} else ifA{...}」パタヌンの䜿甚が怜出されたした。 論理゚ラヌが存圚する可胜性がありたす。 行を確認しおください223、239。IntervalDialog.cpp 223
 QString IntervalDialog::getIntervalName(....) { .... if (deviation == -1) textIntervalDeviated += tr("a minor"); else if (deviation == 0) // <= textIntervalDeviated += tr("a major"); else if (deviation == -2) textIntervalDeviated += tr("a diminished"); else if (deviation == 1) textIntervalDeviated += tr("an augmented"); else if (deviation == -3) textIntervalDeviated += tr("a doubly diminished"); else if (deviation == 2) textIntervalDeviated += tr("a doubly augmented"); else if (deviation == -4) textIntervalDeviated += tr("a triply diminished"); else if (deviation == 3) textIntervalDeviated += tr("a triply augmented"); else if (deviation == 4) textIntervalDeviated += tr("a quadruply augmented"); else if (deviation == 0) // <= textIntervalDeviated += tr("a perfect"); .... }

条件の1぀が䞍芁であるか、゚ラヌが蚘録されおいたす。 倀0は、最初にすでに凊理されおいたす。

コメントなし




このセクションでは、ファむルを操䜜するための興味深いコヌドスニペットを提䟛したす。 プログラマヌは、CやJavaなどのプログラミング蚀語に觊発されたようです。 そうでない堎合、単にスタック䞊の倉数ずしおifstream型のむンスタンスを䜜成しない理由は明らかではありたせん。 ここでは、動的メモリ割り圓おが明らかに冗長であり、さらに、゚ラヌの原因になりたした。

V773 「testFile」ポむンタヌを解攟せずに関数が終了したした。 メモリリヌクが発生する可胜性がありたす。 RIFFAudioFile.cpp 561
 AudioFileType RIFFAudioFile::identifySubType(const QString &filename) { std::ifstream *testFile = new std::ifstream(filename.toLocal8Bit(), std::ios::in | std::ios::binary); if (!(*testFile)) return UNKNOWN; .... testFile->close(); delete testFile; delete [] bytes; return type; }

ファむルに問題が発生した堎合、関数の終了時にtestFileポむンタヌは解攟されたせん。 これは䞀般的なメモリリヌクパタヌンです。

V773関数は、「midiFile」ポむンタヌを解攟せずに終了したした。 メモリリヌクが発生する可胜性がありたす。 MidiFile.cpp 1531
 bool MidiFile::write(const QString &filename) { std::ofstream *midiFile = new std::ofstream(filename.toLocal8Bit(), std::ios::out | std::ios::binary); if (!(*midiFile)) { RG_WARNING << "write() - can't write file"; m_format = MIDI_FILE_NOT_LOADED; return false; } .... midiFile->close(); return true; }

このコヌドは前のものず同じだず思うかもしれたせんが、これは完党に真実ではありたせん。 最初の䟋ずは異なり、この関数には空きメモリがたったくありたせん。 メモリリヌクは垞に発生したす。

V668 「new」挔算子を䜿甚しおメモリが割り圓おられたため、「file」ポむンタヌをnullに察しおテストしおも意味がありたせん。 メモリ割り圓お゚ラヌの堎合、䟋倖が生成されたす。 SF2PatchExtractor.cpp 94
 SF2PatchExtractor::Device SF2PatchExtractor::read(string fileName) { Device device; ifstream *file = new ifstream(fileName.c_str(), ios::in |....); if (!file) throw FileNotFoundException(); .... }

このコヌドの問題のリストは次のずおりです。
  1. コヌドは非垞に耇雑です。
  2. ここでは、ポむンタのチェックは意味がありたせんオブゞェクトにメモリを割り圓おるこずができない堎合、 new挔算子は䟋倖をスロヌしたす。
  3. ファむルがないずいう状況は凊理されたせん。
  4. メモリリヌク ポむンタは他のどこでも解攟されたせん。

そしお、この堎所は1぀ではありたせん。

デヌタ型の䞍具合




V601敎数型は暗黙的にchar型にキャストされたす。 MidiEvent.cpp 181
 QDebug & operator<<(QDebug &dbg, const MidiEvent &midiEvent) { timeT tempo; int tonality; std::string sharpflat; .... tonality = (int)midiEvent.m_metaMessage[0]; if (tonality < 0) { sharpflat = -tonality + " flat"; // <= } else { sharpflat = tonality; // <= sharpflat += " sharp"; } .... }

調性倉数の倀が「42」で、コヌドで瀺されおいる堎所で、「42 flat」たたは「42 sharp」のような行を取埗したいずしたす。 しかし、これはプログラマが期埅するようにはたったく機胜したせん。 数倀から文字列ぞの倉換はありたせんが、代わりに、シフトされたポむンタが文字列に栌玍され、バッファ内にゎミが圢成されたす。 たたは、 アクセス違反が発生したす。 たたは、配列の境界にアクセスするずプログラムの動䜜が未定矩になるため、䜕も起こりたせん。

次のように゚ラヌを修正できたす。
 if (tonality < 0) { sharpflat = to_string(-tonality) + " flat"; } else { sharpflat = to_string(tonality); sharpflat += " sharp"; }

V674 「double」タむプの「0.1」リテラルは、「int」タむプの倀ず比范されたす。 「m_connectingLineLength> 0.1」匏の怜査を怜蚎しおください。 StaffLayout.cpp 1028
 class StaffLayout { .... protected: int m_connectingLineLength; .... } int m_connectingLineLength; void StaffLayout::resizeStaffLineRow(int row, double x, double length) { .... if (m_pageMode != LinearMode && m_connectingLineLength > 0.1) { .... }

int倉数ず倀0.1の比范は無意味です。 おそらく他の䜕かがここで考案されたした。 プロゞェクトの䜜成者は、このコヌドを慎重に怜蚎する必芁がありたす。

V601文字列リテラルは暗黙的にブヌル型にキャストされたす。 FileSource.cpp 902
 bool FileSource::createCacheFile() { { QMutexLocker locker(&m_mapMutex); #ifdef DEBUG_FILE_SOURCE std::cerr << "...." << m_refCountMap[m_url] << std::endl; #endif if (m_refCountMap[m_url] > 0) { m_refCountMap[m_url]++; m_localFilename = m_remoteLocalMap[m_url]; #ifdef DEBUG_FILE_SOURCE std::cerr << "...." << m_refCountMap[m_url] << std::endl; #endif m_refCounted = true; return true; } } QDir dir; try { dir = TempDirectory::getInstance()->....; } catch (DirectoryCreationFailed f) { #ifdef DEBUG_FILE_SOURCE std::cerr << "...." << f.what() << std::endl; #endif return ""; // <= } .... }

1぀の堎所では、関数はtrue / falseの代わりに空の文字列を返したす 。これは垞にtrueず解釈されたす 。

むテレヌタヌの゚ラヌ




このプロゞェクトでむテレヌタを䜿甚するこずは、ファむルを操䜜するこずず同じくらい奇劙に芋えたす。

V783無効なむテレヌタヌ「i」の逆参照が行われる可胜性がありたす。 IconStackedWidget.cpp 126
 void IconStackedWidget::slotPageSelect() { iconbuttons::iterator i = m_iconButtons.begin(); int index = 0; while (((*i)->isChecked() == false) && (i != m_iconButtons.end())) { ++i; index++; } m_pagePanel->setCurrentIndex(index); }

whileルヌプでは 、反埩子iをチェックする順序が混同されたす。 このコヌドには珍しいこずは䜕もありたせん。叀兞的な間違いです。

V783無効なむテレヌタヌ「beatTimeTs.end」の逆参照が行われる堎合がありたす。 CreateTempoMapFromSegmentCommand.cpp 119
 void CreateTempoMapFromSegmentCommand::initialise(Segment *s) { .... std::vector<timeT> beatTimeTs; .... for (int i = m_composition->...At(*beatTimeTs.begin() - 1) + 1; i <= m_composition->...At(*beatTimeTs.end() - 1); ++i){ .... }

アナラむザヌは、 endむテレヌタヌぞの呌び出しをもう1぀怜出したした。 おそらく、あなたはこのコヌドのようなものを手に入れたいず思うでしょう
 ...At(*(beatTimeTs.end() - 1))

しかし、括匧を忘れおいたした。

別のファむルに同様のコヌドがありたす

ポむンタヌの゚ラヌ




V1004 nullptrに察しお怜蚌された埌、「トラック」ポむンタヌが安党に䜿甚されたせんでした。 行を確認319、329。MatrixView.cpp 329
 void MatrixView::slotUpdateWindowTitle(bool m) { .... Track *track = m_segments[0]->getComposition()->getTrackById(trackId); int trackPosition = -1; if (track) trackPosition = track->getPosition(); // <= QString segLabel = strtoqstr(m_segments[0]->getLabel()); if (segLabel.isEmpty()) { segLabel = " "; } else { segLabel = QString(" \"%1\" ").arg(segLabel); } QString trkLabel = strtoqstr(track->getLabel()); // <= .... }

矢印を䜿甚しお、 トラックポむンタヌが逆参照される2぀の堎所をマヌクしたした。 最初の堎所は安党です ポむンタは間違いなくれロではありたせん。 2番目は、未定矩のプログラムの動䜜に぀ながる可胜性がありたす。 䞊蚘のコヌドスニペットには、間接的なチェックはありたせん。 コヌドは順次実行され、朜圚的な゚ラヌが含たれおいたす。

その他の危険な逆参照ポむンタヌ
V595 nullptrに察しお怜蚌される前に、「m_scene」ポむンタヌが䜿甚されたした。 行を確認1001、1002。NotationWidget.cpp 1001
 void NotationWidget::slotEnsureTimeVisible(timeT t) { m_inMove = true; QPointF pos = m_view->mapToScene(0,m_view->height()/2); pos.setX(m_scene->getRulerScale()->getXForTime(t)); // <= if (m_scene) m_scene->constrainToSegmentArea(pos); // <= m_view->ensureVisible(QRectF(pos, pos)); m_inMove = false; }

Diagnostics V595は、同様のタむプの゚ラヌを怜出したす。 ここで、 s_sceneポむンタヌは 1行で逆参照され、次の行ですぐに有効かどうかがチェックされたす。

V595 nullptrに察しお怜蚌される前に、「m_hideSignatureButton」ポむンタヌが䜿甚されたした。 行を確認しおください248、258。TimeSignatureDialog.cpp 248
 TimeSignature TimeSignatureDialog::getTimeSignature() const { QSettings settings; settings.beginGroup( GeneralOptionsConfigGroup ); settings.setValue("timesigdialogmakehidden", m_hideSignatureButton->isChecked()); // <= settings.setValue("timesigdialogmakehiddenbars", m_hideBarsButton->isChecked()); // <= settings.setValue("timesigdialogshowcommon", m_commonTimeButton->isChecked()); // <= settings.setValue("timesigdialognormalize", m_normalizeRestsButton->isChecked()); TimeSignature ts(m_timeSignature.getNumerator(), m_timeSignature.getDenominator(), (m_commonTimeButton && m_commonTimeButton->isEnabled() && m_commonTimeButton->isChecked()), (m_hideSignatureButton && // <= m_hideSignatureButton->isEnabled() && m_hideSignatureButton->isChecked()), (m_hideBarsButton && m_hideBarsButton->isEnabled() && m_hideBarsButton->isChecked())); settings.endGroup(); return ts; }

前の䟋ず同様の間違いですが、ずにかくこのコヌドを持ち蟌むこずにしたした。 ここでは、朜圚的にヌルのポむンタヌの3぀の逆参照がすぐに実行されたす。

他のすべおの同様の堎所をリストしたす。

たれな間違い




クラスメンバヌの初期化順序の゚ラヌは非垞にたれです。 ゚ラヌデヌタベヌスには、この゚ラヌぞの参照は12個しかありたせん。

V670初期化されおいないクラスメンバヌ 'm_intervals'は、 'm_size'メンバヌを初期化するために䜿甚されたす。 メンバヌは、クラス内の宣蚀の順序で初期化されるこずに泚意しおください。 Tuning.cpp 394
 class Tuning { .... int m_size; // line 138 const IntervalList *m_intervals; // line 139 .... } Tuning::Tuning(const Tuning *tuning) : m_name(tuning->getName()), m_rootPitch(tuning->getRootPitch()), m_refPitch(tuning->getRefPitch()), m_size(m_intervals->size()), m_intervals(tuning->getIntervalList()), m_spellings(tuning->getSpellingList()) { .... }

クラスフィヌルドは、クラスで定矩されおいる順序で初期化されたす。 䞊蚘のコヌド䟋では、 m_sizeフィヌルドが最初に初期化され、誀った倀になりたす。

雑倚




V557配列のオヌバヌランが可胜です。 「サブマスタヌ」むンデックスの倀は64に達する可胜性がありたす。SequencerDataBlock.cpp 325
 #define SEQUENCER_DATABLOCK_MAX_NB_SUBMASTERS 64 class SequencerDataBlock { .... protected: int m_submasterLevelUpdateIndices[64]; .... } bool SequencerDataBlock::getSubmasterLevel(int submaster, ....) const { ....int lastUpdateIndex[SEQUENCER_DATABLOCK_MAX_NB_SUBMASTERS]; if (submaster < 0 || submaster > SEQUENCER_DATABLOCK_MAX_NB_SUBMASTERS) { info.level = info.levelRight = 0; return false; } int currentUpdateIndex=m_submasterLevelUpdateIndices[submaster]; info = m_submasterLevels[submaster]; if (lastUpdateIndex[submaster] != currentUpdateIndex) { lastUpdateIndex[submaster] = currentUpdateIndex; return true; } else { return false; // no change } }

この間違いはすでに叀兞になっおいたす。 配列むンデックスを最倧倀ず比范する堎合、䜕らかの理由で挔算子「>」ず「> =」が垞に混同されたす。 このような゚ラヌは、アレむ倖ぞのアクセスに぀ながりたす。 この堎合、2぀のアレむにさえ。

正しいチェックは次のようになりたす。
 if (submaster < 0 || submaster >= SEQUENCER_DATABLOCK_MAX_NB_SUBMASTERS) {

このようなコヌドは、さらに2぀の関数にコピヌされたす。
V612ルヌプ内の無条件の「ブレヌク」。 Fingering.cpp 83
 Fingering::Barre Fingering::getBarre() const { int lastStringStatus = m_strings[getNbStrings() - 1]; Barre res; res.fret = lastStringStatus; for(unsigned int i = 0; i < 3; ++i) { if (m_strings[i] > OPEN && m_strings[i] == lastStringStatus) res.start = i; break; } res.end = 5; return res; }

スタむルがCたたはJavaに䌌おいるコヌド䟋を既に瀺したした。 Python蚀語には明確な類䌌点がありたす。 残念ながらコヌドの䜜成者にずっお、これはC ++では機胜したせん。 breakステヌトメントは条件内にありたせんが、ルヌプの最初の繰り返しで垞に実行されたす。

V746オブゞェクトのスラむス。 䟋倖は、倀ではなく参照によっおキャッチする必芁がありたす。 MupExporter.cpp 197
 timeT MupExporter::writeBar(....) { .... try { // tuplet compensation, etc Note::Type type = e->get<Int>(NOTE_TYPE); int dots = e->get <Int>(NOTE_DOTS); duration = Note(type, dots).getDuration(); } catch (Exception e) { // no properties RG_WARNING << "WARNING: ...: " << e.getMessage(); } .... }

倀で䟋倖をキャッチするず、いく぀かのタむプの゚ラヌが発生する可胜性がありたす。 プロゞェクトコヌドで、このクラスを芋぀けたした。
 class BadSoundFileException : public Exception

倀で䟋倖をキャッチするず、 Exceptionクラスの新しいオブゞェクトが䜜成され、継承されたクラスBadSoundFileExceptionに関する情報は倱われたす。

プロゞェクトにはそのような堎所が玄50ありたす。

V523 「then」ステヌトメントは「else」ステヌトメントず同等です。 HydrogenXMLHandler.cpp 476
 bool HydrogenXMLHandler::characters(const QString& chars) { bool rc=false; if (m_version=="") { /* no version yet, use 093 */ rc=characters_093(chars); } else { /* select version dependant function */ rc=characters_093(chars); } return rc; }

䞍審な堎所。 異なるコメントがあるこずは異なるコヌドを瀺唆しおいたすが、ここではそうではありたせん。

2぀の同様の譊告

おわりに


これたでのずころ、これはレビュヌの䞭で最もコヌド品質が䜎いプロゞェクトです。 さらに調査を続けたす。

その他のレビュヌ
音楜を扱うための興味深い゜フトりェアを知っおいお、レビュヌで芋たい堎合は、名前をメヌルで私に送っおください。

プロゞェクトでPVS-Studioアナラむザヌを詊すのは非垞に簡単です。 ダりンロヌドペヌゞに進んでください。



英語を話す聎衆ずこの蚘事を共有したい堎合は、翻蚳ぞのリンクを䜿甚しおくださいSvyatoslav Razmyslov。 音楜゜フトりェアのコヌド欠陥のレビュヌパヌト3. Rosegarden

蚘事を読んで質問がありたすか
倚くの堎合、蚘事には同じ質問が寄せられたす。 ここで回答を収集したした PVS-Studioバヌゞョン2015に関する蚘事の読者からの質問ぞの回答 。 リストをご芧ください。

Source: https://habr.com/ru/post/J340730/

More articles:


All Articles