Анализ кода ROOT, Структура анализа научных данных

Пока в Стокгольме проходила 118-я Нобелевская неделя, я сидел в нашем офисе, где мы разрабатываем статический анализатор PVS-Studio, и работал над аналитическим обзором проекта ROOT — фреймворка для обработки больших данных, используемого в научных исследованиях. Приза этот код, конечно, не получит, но авторы точно могут рассчитывать на подробный обзор наиболее интересных дефектов плюс бесплатную лицензию для самостоятельной тщательной проверки проекта.

Введение

ROOT — модульный научный программный инструментарий. Он предоставляет все функции, необходимые для обработки больших данных, статистического анализа, визуализации и хранения. В основном он написан на C++. ROOT родился в ЦЕРН, в центре исследований физики высоких энергий. Каждый день тысячи физиков используют приложения ROOT для анализа своих данных или моделирования.

PVS-Studio — инструмент для обнаружения программных ошибок и потенциальных уязвимостей в исходном коде программ, написанных на языках C, C++, C# и Java. Он работает на 64-разрядных платформах Windows, Linux и macOS и может анализировать исходный код, написанный для 32-разрядных, 64-разрядных и встроенных платформ ARM.

Дебют новой диагностики

V1046 Небезопасное совместное использование типов bool и int в операции &=. GSLMultiRootFinder.h 175

int AddFunction(const ROOT::Math::IMultiGenFunction & func) {
  ROOT::Math::IMultiGenFunction * f = func.Clone();
  if (!f) return 0;
  fFunctions.push_back(f);
  return fFunctions.size();
}
template<class FuncIterator>
bool SetFunctionList( FuncIterator begin, FuncIterator end) {
  bool ret = true;
  for (FuncIterator itr = begin; itr != end; ++itr) {
    const ROOT::Math::IMultiGenFunction * f = *itr;
    ret &= AddFunction(*f);
  }
  return ret;
}

Во-первых, вот замечательная ошибка, обнаруженная в бета-версии PVS-Studio, которую я использовал для этого обзора.

Ожидания. Функция SetFunctionList просматривает список итераторов. Если хотя бы один итератор недействителен, функция возвращает false или true в противном случае.

Реальность. Функция SetFunctionList может возвращать false даже для допустимых итераторов. Давайте разберемся, почему. Функция AddFunction возвращает количество допустимых итераторов в списке fFunctions. То есть добавление ненулевых итераторов приведет к постепенному увеличению размера списка: 1, 2, 3, 4 и так далее. Вот где ошибка вступает в игру:

ret &= AddFunction(*f);

Поскольку функция возвращает значение типа int, а не bool, операция '&=' будет возвращать false для четных значений, поскольку наименее значимое бит четного числа всегда устанавливается равным нулю. Вот как одна незаметная ошибка может нарушить возвращаемое значение SetFunctionsList, даже если его аргументы допустимы.

Ошибки в условных выражениях

V501 Слева и справа от оператора && одинаковые подвыражения: модуль && модуль rootcling_impl.cxx 3650

virtual void HandleDiagnostic(....) override
{
  ....
  bool isROOTSystemModuleDiag = module && ....;
  bool isSystemModuleDiag = module && module && module->IsSystem;
  if (!isROOTSystemModuleDiag && !isSystemModuleDiag)
    fChild->HandleDiagnostic(DiagLevel, Info);
  ....
}

Начнем с наименее опасной ошибки. Указатель module проверяется дважды. Одна из проверок, вероятно, избыточна, но все же было бы разумно исправить ее, чтобы избежать путаницы в будущем.

V501 Слева и справа от оператора || одинаковые подвыражения strchr(fHostAuth-›GetHost(), ‘*’). TAuthenticate.cxx 300

TAuthenticate::TAuthenticate(TSocket *sock, const char *remote,
                             const char *proto, const char *user)
{
  ....
  // If generic THostAuth (i.e. with wild card or user == any)
  // make a personalized memory copy of this THostAuth
  if (strchr(fHostAuth->GetHost(),'*') || strchr(fHostAuth->GetHost(),'*') ||
     fHostAuth->GetServer() == -1 ) {
    fHostAuth = new THostAuth(*fHostAuth);
    fHostAuth->SetHost(fqdn);
    fHostAuth->SetUser(checkUser);
    fHostAuth->SetServer(servtype);
  }
  ....
}

Строка fHostAuth-›GetHost() дважды сканируется на наличие символа «*». Одна из этих проверок, вероятно, предназначалась для поиска символа «?», поскольку эти два символа обычно используются для указания различных масок подстановочных знаков.

V517 Обнаружено использование шаблона if (A) {…} else if (A) {…}. Существует вероятность наличия логической ошибки. Проверить строки: 163, 165. TProofMonSenderML.cxx 163

Int_t TProofMonSenderML::SendSummary(TList *recs, const char *id)
{
  ....
  if (fSummaryVrs == 0) {
    if ((dsn = recs->FindObject("dataset"))) recs->Remove(dsn);
  } else if (fSummaryVrs == 0) {
    // Only the first records
    xrecs = new TList;
    xrecs->SetOwner(kFALSE);
    TIter nxr(recs);
    TObject *o = 0;
    while ((o = nxr())) {
       if (!strcmp(o->GetName(), "vmemmxw")) break;
       xrecs->Add(o);
    }
  }
  ....
}

Переменная fSummaryVrs дважды сравнивается с нулем, поэтому выполнение никогда не достигает кода в ветви else-if. И там довольно много кода…

V523 Оператор тогда эквивалентен оператору еще. TKDTree.cxx 805

template <typename  Index, typename Value>
void TKDTree<Index, Value>::UpdateRange(....)
{
  ....
  if (point[fAxis[inode]]<=fValue[inode]){
    //first examine the node that contains the point
    UpdateRange(GetLeft(inode),point, range, res);
    UpdateRange(GetRight(inode),point, range, res);
  } else {
    UpdateRange(GetLeft(inode),point, range, res);
    UpdateRange(GetRight(inode),point, range, res);
  }
  ....
}

Один и тот же блок кода, который является клоном копирования и вставки, выполняется независимо от условия. Я предполагаю, что есть путаница между словами слева и справа.

В проекте полно таких подозрительных мест:

V523 Оператор "then" эквивалентен оператору "else". TContainerConverters.cxx 51
V523 Оператор "then" эквивалентен оператору "else". TWebFile.cxx 1310
V523 Оператор "then" эквивалентен оператору "else". МетодMLP.cxx 423
V523 Оператор "then" эквивалентен оператору "else". RooAbsCategory.cxx 394

V547 Выражение ‘!file_name_value.empty()’ всегда ложно. SelectionRules.cxx 1423

bool SelectionRules::AreAllSelectionRulesUsed() const {
  for(auto&& rule : fClassSelectionRules){
    ....
    std::string file_name_value;
    if (!rule.GetAttributeValue("file_name", file_name_value))
     file_name_value.clear();
    if (!file_name_value.empty()) {                  // <=
      // don't complain about defined_in rules
      continue;
    }
    const char* attrName = nullptr;
    const char* attrVal = nullptr;
    if (!file_name_value.empty()) {                  // <=
      attrName = "file name";
      attrVal = file_name_value.c_str();
    } else {
      attrName = "class";
      if (!name.empty()) attrVal = name.c_str();
    }
    ROOT::TMetaUtils::Warning(0,"Unused %s rule: %s\n", attrName, attrVal);
  }
  ....
}

Вероятно, это не ошибка; анализатор просто нашел код, который можно упростить. Поскольку возвращаемое значение file_name_value.empty() уже проверено в начале цикла, вторую проверку на дублирование можно удалить, тем самым избавившись от большого количества ненужного кода.

V590 Рассмотрите возможность проверки «!file1 || с ‹= 0 || с == '*' || c != '('' выражение. Выражение избыточно или содержит опечатку. TTabCom.cxx 840

TString TTabCom::DetermineClass(const char varName[])
{
  ....
  c = file1.get();
  if (!file1 || c <= 0 || c == '*' || c != '(') {
    Error("TTabCom::DetermineClass", "variable \"%s\" not defined?",
        varName);
    goto cleanup;
  }
  ....
}

Вот проблемная часть условного выражения, выданная анализатором:

if (.... || c == '*' || c != '(') {
  ....
}

Проверка на символ звездочки не повлияет на результат условия. Эта часть всегда будет истинна для любого символа, кроме ‘(‘. Вы можете легко проверить это сами, нарисовав таблицу истинности.

Еще два предупреждения об условиях со странной логикой:

V590 Попробуйте проверить это выражение. Выражение является избыточным или содержит опечатку. TFile.cxx 3963
V590 Попробуйте проверить это выражение. Выражение является избыточным или содержит опечатку. TStreamerInfoActions.cxx 3084

V593 Рассмотрим выражение вида A = B ‹ C. Выражение рассчитывается следующим образом: A = (B ‹ C). TProofServ.cxx 1903

Int_t TProofServ::HandleSocketInput(TMessage *mess, Bool_t all)
{
  ....
  if (Int_t ret = fProof->AddWorkers(workerList) < 0) {
    Error("HandleSocketInput:kPROOF_GETSLAVEINFO",
          "adding a list of worker nodes returned: %d", ret);
  }
  ....
}

Этот баг проявляется только в случае некорректного поведения программы. Предполагается, что переменная ret хранит код возврата функции AddWorkers и записывает это значение в журнал в случае ошибки. Но это не работает так, как задумано. В условии отсутствуют дополнительные круглые скобки, задающие желаемый порядок оценки. На самом деле переменная ret хранит не код возврата, а результат логического сравнения, то есть либо 0, либо 1.

Еще похожая проблема:

V593 Рассмотрим выражение типа «A = B ‹ C». Выражение рассчитывается следующим образом: «A = (B ‹ C)». TProofServ.cxx 3897

V768 Константа перечисления kCostComplexityPruning используется как переменная логического типа. МетодDT.cxx 283

enum EPruneMethod {kExpectedErrorPruning=0, kCostComplexityPruning, kNoPruning};
void TMVA::MethodDT::ProcessOptions()
{
  ....
  if (fPruneStrength < 0) fAutomatic = kTRUE;
  else fAutomatic = kFALSE;
  if (fAutomatic && fPruneMethod==!DecisionTree::kCostComplexityPruning){
    Log() << kFATAL
          <<  "Sorry automatic pruning strength determination is ...." << Endl;
  }
  ....
}

Хм… Зачем инвертировать постоянное значение kCostComplexityPruning? Я подозреваю, что символ отрицания — это опечатка, которая теперь искажает логику выполнения.

Ошибки обработки указателя

V522 Может иметь место разыменование нулевого указателя pre. TSynapse.cxx 61

void TSynapse::SetPre(TNeuron * pre)
{
  if (pre) {
    Error("SetPre","this synapse is already assigned to a pre-neuron.");
    return;
  }
  fpre = pre;
  pre->AddPost(this);
}

Я сделал все, что мог, пытаясь понять этот странный код, и, кажется, идея заключалась в том, чтобы избежать присвоения нового значения полю fpre. Если да, то программист случайно проверяет не тот указатель. Текущая реализация приводит к разыменованию нулевого указателя, если вы передаете значение nullptr в функцию SetPre.

Я думаю, что этот фрагмент должен быть исправлен следующим образом:

void TSynapse::SetPre(TNeuron * pre)
{
  if (fpre) {
    Error("SetPre","this synapse is already assigned to a pre-neuron.");
    return;
  }
  fpre = pre;
  pre->AddPost(this);
}

Это, однако, не предотвратило бы передачу нулевого указателя в функцию, но, по крайней мере, эта версия более логически непротиворечива, чем исходная.

Слегка измененный клон этого кода можно найти в другом месте:

V522 Может иметь место разыменование нулевого указателя «post». TSynapse.cxx 74

V595 Указатель N использовался до того, как он был проверен на соответствие nullptr. Проверить строки: 484, 488. Scanner.cxx 484

bool RScanner::shouldVisitDecl(clang::NamedDecl *D)
{
   if (auto M = D->getOwningModule()) {                      // <= 2
      return fInterpreter.getSema().isModuleVisible(M);
   }
   return true;
}
bool RScanner::VisitNamespaceDecl(clang::NamespaceDecl* N)
{
 if (fScanType == EScanType::kOnePCM)
  return true;
 if (!shouldVisitDecl(N))                                    // <= 1
  return true;
 if((N && N->isImplicit()) || !N){                           // <= 3
    return true;
 }
 ....
}

Это чрезвычайно опасный фрагмент кода! Указатель N не проверяется на значение null до первого разыменования. Более того, вы не можете увидеть, как это происходит здесь, потому что разыменование происходит внутри функции shouldVisitDecl.

Эта диагностика традиционно генерирует кучу релевантных предупреждений. Вот несколько примеров:

V595 Указатель «файл» использовался до того, как он был проверен на соответствие nullptr. Проверить строки: 141, 153. TFileCacheRead.cxx 141
V595 Указатель «fFree» использовался до того, как он был проверен на соответствие nullptr. Контрольные строки: 2029, 2038. TFile.cxx 2029
V595 Указатель tbuf использовался до того, как он был проверен на соответствие nullptr. Проверить строки: 586, 591. TGText.cxx 586
V595 Указатель «fPlayer» использовался до того, как он был проверен на соответствие nullptr. Строки проверки: 3425, 3430. TProof.cxx 3425
V595 Указатель «gProofServ» использовался до того, как он был проверен на соответствие nullptr. Строки проверки: 1192, 1194. TProofPlayer.cxx 1192
V595 Указатель projDataTmp использовался до того, как он был проверен на соответствие nullptr. Проверить строки: 791, 804. RooSimultaneous.cxx 791

Следующий — не баг, а еще один пример того, как макросы поощряют написание ошибочного или избыточного кода.

V571 Периодическая проверка. Условие if (fCanvasImp) уже проверено в строке 799. TCanvas.cxx 800

#define SafeDelete(p) { if (p) { delete p; p = 0; } }
void TCanvas::Close(Option_t *option)
{
  ....
  if (fCanvasImp)
    SafeDelete(fCanvasImp);
  ....
}

Указатель fCanvasImp проверяется дважды, причем одна из проверок уже реализована в макросе SafeDelete. Одна из проблем с макросами заключается в том, что по ним трудно ориентироваться внутри кода, поэтому многие программисты не проверяют их содержимое перед использованием.

Ошибки обработки массива

V519 Переменной Линия[Курсор] дважды подряд присваиваются значения. Возможно, это ошибка. Проверить строки: 352, 353. Editor.cpp 353

size_t find_last_non_alnum(const std::string &str,
                             std::string::size_type index = std::string::npos) {
  ....
  char tmp = Line.GetText()[Cursor];
  Line[Cursor] = Line[Cursor - 1];
  Line[Cursor] = tmp;
  ....
}

Элементу Line[Cursor] присваивается новое значение, которое сразу же перезаписывается. Это выглядит не так…

V557 Возможен переполнение массива. Индекс ivar указывает за границу массива. BasicMinimizer.cxx 130

bool BasicMinimizer::SetVariableValue(unsigned int ivar, double val) {
  if (ivar > fValues.size() ) return false;
  fValues[ivar] = val;
  return true;
}

Совершение этой ошибки при проверке индексов массива является недавней тенденцией; мы видим это почти в каждом третьем проекте. В то время как индексация массива внутри цикла проста — вы обычно используете оператор «‹» для сравнения индекса с размером массива — проверки, подобные показанной выше, требуют оператора «›=», а не «›». В противном случае вы рискуете проиндексировать один элемент за границей массива.

Эта ошибка несколько раз клонировалась в коде:

V557 Возможно переполнение массива. Индекс «ivar» указывает за границу массива. BasicMinimizer.cxx 186
V557 Возможно переполнение массива. Индекс «ivar» указывает за границу массива. BasicMinimizer.cxx 194
V557 Возможно переполнение массива. Индекс «ivar» указывает за границу массива. BasicMinimizer.cxx 209
V557 Возможно переполнение массива. Индекс «ivar» указывает за границу массива. BasicMinimizer.cxx 215
V557 Возможно переполнение массива. Индекс «ivar» указывает за границу массива. BasicMinimizer.cxx 230

V621 Рассмотрите возможность проверки оператора for. Возможно, что цикл будет выполняться некорректно или вообще не будет выполняться. TDataMember.cxx 554

Int_t TDataMember::GetArrayDim() const
{
 if (fArrayDim<0 && fInfo) {
    R__LOCKGUARD(gInterpreterMutex);
    TDataMember *dm = const_cast<TDataMember*>(this);
    dm->fArrayDim = gCling->DataMemberInfo_ArrayDim(fInfo);
    // fArrayMaxIndex should be zero
    if (dm->fArrayDim) {
       dm->fArrayMaxIndex = new Int_t[fArrayDim];
       for(Int_t dim = 0; dim < fArrayDim; ++dim) {
          dm->fArrayMaxIndex[dim] = gCling->DataMemberInfo_MaxIndex(fInfo,dim);
       }
    }
 }
 return fArrayDim;
}

В цикле for разработчики, очевидно, имели в виду сравнение переменной dim с dm-›fArrayDim, а не с fArrayDim. . Значение fArrayDim отрицательное, что гарантируется условием в начале функции. Следовательно, этот цикл никогда не будет выполняться.

V767 Подозрительный доступ к элементу текущего массива по постоянному индексу внутри цикла. TClingUtils.cxx 3082

llvm::StringRef ROOT::TMetaUtils::DataMemberInfo__ValidArrayIndex(....)
{
  ....
  while (current!=0) {
    // Check the token
    if (isdigit(current[0])) {
       for(i=0;i<strlen(current);i++) {
          if (!isdigit(current[0])) {
             if (errstr) *errstr = current;
             if (errnum) *errnum = NOT_INT;
             return llvm::StringRef();
          }
       }
    } else { // current token is not a digit
      ....
    }
    ....
  }
  ....
}

Этот код анализирует и проверяет некоторую строку. Если первый символ текущейстроки (т. е. с индексом 0) был распознан как число, цикл будет проходить по всем остальным символам, чтобы убедиться, что все они являются числами. Ну, по крайней мере, это идея. Проблема в том, что счетчик i не используется в цикле. Условие следует переписать так, чтобы оно проверяло current[i], а не current[0].

Утечка памяти

V773 Выход из функции без отпускания указателя optionlist. Возможна утечка памяти. TDataMember.cxx 355

void TDataMember::Init(bool afterReading)
{
  ....
  TList *optionlist = new TList();       //storage for options strings
  for (i=0;i<token_cnt;i++) {
     if (strstr(tokens[i],"Items")) {
        ptr1 = R__STRTOK_R(tokens[i], "()", &rest);
        if (ptr1 == 0) {
           Fatal("TDataMember","Internal error, found \"Items....",GetTitle());
           return;
        }
        ptr1 = R__STRTOK_R(nullptr, "()", &rest);
        if (ptr1 == 0) {
           Fatal("TDataMember","Internal error, found \"Items....",GetTitle());
           return;
        }
        ....
     }
     ....
  }
  ....
  // dispose of temporary option list...
  delete optionlist;
  ....
}

Указатель optionList не освобождается перед возвратом из функции. Не знаю, нужно ли такое освобождение в данном конкретном случае, но когда мы сообщаем о таких ошибках, разработчики обычно их исправляют. Все зависит от того, хотите ли вы, чтобы ваша программа продолжала работать в случае ошибки. Таких дефектов у ROOT куча, поэтому я бы посоветовал авторам самим перепроверить проект.

память снова

V597 Компилятор мог удалить вызов функции memset, который используется для очистки буфера x. Функцию memset_s() следует использовать для удаления личных данных. TMD5.cxx 366

void TMD5::Transform(UInt_t buf[4], const UChar_t in[64])
{
  UInt_t a, b, c, d, x[16];
  ....
  // Zero out sensitive information
  memset(x, 0, sizeof(x));
}

Многие думают, что комментарий не попадет в бинарный файл после компиляции, и они абсолютно правы :D. Некоторые могут не знать, что компилятор также удалит функцию memset. И это обязательно произойдет. Если рассматриваемый буфер больше не используется в коде, компилятор оптимизирует вызов функции. Технически это разумное решение, но если в буфере хранились какие-то приватные данные, то эти данные там и останутся. Это классическая слабость безопасности CWE-14.

Разное

V591 Непустая функция должна возвращать значение. LogLikelihoodFCN.h 108

LogLikelihoodFCN & operator = (const LogLikelihoodFCN & rhs) {
  SetData(rhs.DataPtr() );
  SetModelFunction(rhs.ModelFunctionPtr() );
  fNEffPoints = rhs.fNEffPoints;
  fGrad = rhs.fGrad;
  fIsExtended = rhs.fIsExtended;
  fWeight = rhs.fWeight;
  fExecutionPolicy = rhs.fExecutionPolicy;
}

Перегруженный оператор не имеет возвращаемого значения. Это еще одна недавняя тенденция.

V596 Объект создан, но не используется. Ключевое слово throw может отсутствовать: throw runtime_error(FOO); RTensor.hxx 363

template <typename Value_t, typename Container_t>
inline RTensor<Value_t, Container_t> RTensor<Value_t, Container_t>::Transpose()
{
  if (fLayout == MemoryLayout::RowMajor) {
    fLayout = MemoryLayout::ColumnMajor;
  } else if (fLayout == MemoryLayout::ColumnMajor) {
    fLayout = MemoryLayout::RowMajor;
  } else {
    std::runtime_error("Memory layout is not known.");
  }
  ....
}

Проблема в том, что программист случайно пропустил ключевое слово throw, что предотвратило создание исключения в случае возникновения ошибки.

Таких предупреждений было всего два. Вот второй:

V596 Объект создан, но не используется. Ключевое слово throw может отсутствовать: throw runtime_error(FOO); Лес.hxx 137

V609 Деление на ноль. Диапазон знаменателя [0..100]. TGHtmlImage.cxx 340

const char *TGHtml::GetPctWidth(TGHtmlElement *p, char *opt, char *ret)
{
  int n, m, val;
  ....
  if (n < 0 || n > 100) return z;
  if (opt[0] == 'h') {
    val = fCanvas->GetHeight() * 100;
  } else {
    val = fCanvas->GetWidth() * 100;
  }
  if (!fInTd) {
    snprintf(ret, 15, "%d", val / n);  // <=
  } else {
    ....
  }
  ....
}

Это похоже на примеры обработки массивов, рассмотренные ранее. Переменная n ограничена диапазоном от 0 до 100. Но есть ветвь, выполняющая деление на переменную n, которая может иметь значение 0. Я думаю, пределы диапазона n должны быть установлены следующим образом:

if (n <= 0 || n > 100) return z;

V646 Попробуйте проверить логику приложения. Возможно, отсутствует ключевое слово else. TProofServ.cxx 729

TProofServ::TProofServ(Int_t *argc, char **argv, FILE *flog)
       : TApplication("proofserv", argc, argv, 0, -1)
{
  ....
  if (!logmx.IsDigit()) {
    if (logmx.EndsWith("K")) {
      xf = 1024;
      logmx.Remove(TString::kTrailing, 'K');
    } else if (logmx.EndsWith("M")) {
      xf = 1024*1024;
      logmx.Remove(TString::kTrailing, 'M');
    } if (logmx.EndsWith("G")) {
      xf = 1024*1024*1024;
      logmx.Remove(TString::kTrailing, 'G');
    }
  }
  ....
}

Анализатор сообщает о странном формате оператора if с отсутствующим ключевым словом else . Судя по тому, как выглядит этот код, его нужно исправить.

Еще пара предупреждений такого типа:

V646 Рассмотрите возможность проверки логики приложения. Возможно, отсутствует ключевое слово else. TFormula_v5.cxx 3702
V646 Рассмотрите возможность проверки логики приложения. Возможно, отсутствует ключевое слово else. RooAbsCategory.cxx 604

V663 Возможен бесконечный цикл. Условие cin.eof() недостаточно для выхода из цикла. Рассмотрите возможность добавления вызова функции cin.fail() в условное выражение. МетодKNN.cxx 602

void TMVA::MethodKNN::ReadWeightsFromStream(std::istream& is)
{
  ....
  while (!is.eof()) {
    std::string line;
    std::getline(is, line);
    if (line.empty() || line.find("#") != std::string::npos) {
       continue;
    }
    ....
  }
  ....
}

При работе с классом std::istream вызова функции eof() недостаточно для завершения цикла. Функция eof() всегда будет возвращать false, если данные не могут быть прочитаны, и в этом коде нет других точек завершения. Чтобы гарантировать завершение цикла, требуется дополнительная проверка значения, возвращаемого функцией fail():

while (!is.eof() && !is.fail())
{ 
....
}

В качестве альтернативы его можно переписать следующим образом:

while (is)
{ 
....
}

V678 Объект используется как аргумент собственного метода. Рассмотрите возможность проверки первого фактического аргумента функции Копировать. TFormLeafInfo.cxx 2414

TFormLeafInfoMultiVarDim::TFormLeafInfoMultiVarDim(
  const TFormLeafInfoMultiVarDim& orig) : TFormLeafInfo(orig)
{
   fNsize = orig.fNsize;
   fSizes.Copy(fSizes);   // <=
   fCounter2 = orig.fCounter2?orig.fCounter2->DeepCopy():0;
   fSumOfSizes = orig.fSumOfSizes;
   fDim = orig.fDim;
   fVirtDim = orig.fVirtDim;
   fPrimaryIndex = orig.fPrimaryIndex;
   fSecondaryIndex = orig.fSecondaryIndex;
}

Давайте закончим статью этой милой опечаткой. Функцию Копировать следует вызывать с параметром orig.fSizes, а не fSizes.

Вывод

Около года назад мы проверили проект NCBI Genome Workbench — еще одну программу, используемую в научных исследованиях, связанных с анализом генома. Я упоминаю об этом, потому что качество научного программного обеспечения чрезвычайно важно, но разработчики склонны его недооценивать.

Кстати, на днях вышла macOS 10.15 Catalina, где прекратили поддержку 32-битных приложений. К счастью, PVS-Studio предлагает большой набор диагностик, специально предназначенных для выявления ошибок, сопровождающих перенос программ на 64-битные системы. Подробнее в этом посте от команды PVS-Studio.