Почему даже легендарные игры, такие как Wesnoth, скрывают ошибки на виду
programming cpp gamedev the-battle-for-wesnoth-bugs wesnoth-code-analysis open-source-game-debugging game-development-bugs analyze-open-source-game c++-game-developers

Почему даже легендарные игры, такие как Wesnoth, скрывают ошибки на виду

17 июля 2025 г.

В этой статье мы расскажем вам о нашем путешествии по землям Ирди. Наши приключения обещают великолепные сражения, победы и редкие награды за могучие артефакты! "Что, черт возьми, эти артефакты?" Вы можете спросить. Что ж, это ошибки, найденные в коде хорошо известной, очень вызывающей привыкание игры, битвы за Уэснот.

О проекте

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

Он сочетает в себе потрясающую пиксельную графику и умный игровой процесс со многими привлекательными механиками. Самое главное, что игра никогда не разбивала и не выбивала ошибку в течение шести месяцев, когда я играл в нее без перерыва (просто чтобы написать эту статью, конечно).

Если вам больше 30, и вам каким -то образом пропустил эту удивительную игру, но в 2004 году у вас был функциональный телефон, вы, возможно, помните такую игру, как «Древняя империя». Битва за Уэснот - это лучшая версия!

Что ж, не справедливо сравнить эту замечательную игру с мобильной игрой 50 МБ. В любом случае, я очень рекомендую это!

Игра была выпущена 18 июня 2003 года и получила более 70% положительных отзывов на Steam. Проект жив и пинает, и он постоянно развивается: разработчики только что выпустили обновление 1.19.12 5 июня. Между тем, репозиторий GitHub обновляется ежедневно с новыми изменениями.

Как жесткий программист, конечно, я построил игру из Source. Кстати, благодаря инструкциям вдумчивых разработчиков, процесс гладкий и беспрепятственный. Удивительно, сколько они вложили - несмотря на многочисленные зависимости проекта, игра построена только с несколькими командами. Все, что нам нужно сделать, это начать и наслаждаться!

Если вам интересно, вот коммит, который я использовал для клонирования проекта:Фебф638Полем

Неизбежный спойлер

Цель статьи не в том, чтобы обесценить работу программистов, участвующих в разработке этого продукта. Его цель состоит в том, чтобы популяризировать анализаторы статического кода, которые полезны даже для высококачественных и установленных проектов.

Интеграция PVS-Studio в проект на этапе анализа

Последняя версия PVS-Studio 7.37 была недавно выпущена. Релиз заполнен всевозможными изменениями. Мы упорно трудились, чтобы увеличить количество полезных диагностических правил и минимизировать ложные позитивы.

Тем не менее, есть проблема: даже если количество ложных срабатываний сокращается, предупреждения все еще могут запутать новых пользователей, которые еще не настроили анализатор. Это особенно верно для крупных проектов, где некоторые группы диагностических правил могут быть неактуальными или ненужными для пользователей.

Некоторые, возможно, только недавно начали исследовать сферу статического анализа. Чтобы обеспечить положительный опыт при первом использовании анализатора, вы можете рассмотреть следующие рекомендации:

  1. ЗавершитьНачальный процесс конфигурацииПолем Как упоминалось ранее, рекомендуется отключить любые ненужные группы диагностических правил и сохранить только необходимые. Например, если проект не работает со стандартами MISRA или AutoSAR, нет никаких оснований держать эти группы диагностических правил.
  2. Используйте настройки фильтрации, чтобы отобразитьНаиболее интересные предупрежденияПолем Это сделает рецензирование отчета быстрее и проще.
  3. Изучить, как настроить процесс анализа через*.pvsconfigфайл. Он упрощает настройку, повышает точность, ускоряет настройку и разблокируетДополнительные функцииПолем
  4. Упростить свой рабочий процесс с помощью отчетов, используяМеханизм массового подавлениячерез подавленные файлы. Этот механизм подавляет как конкретные предупреждения для конкретного файла, так и все предупреждения в целом, гарантируя получение только новых предупреждений (то есть базовая линия).
  5. ИспользуйтеПокрементный режим анализаВместо того, чтобы выполнять полный анализ каждый раз. В нем анализируются только измененные файлы и предупреждают только для нового кода.
  6. Интегрируйте полный анализ проекта в CI в течение ночных сборов. Если вы используете модель «Запросы на развлечение/слияние» в своем процессе разработки, вы также можетеинтегрироватьАнализ там.

Конечно, мы обычно не выполняем все эти шаги, чтобы проверить проекты с открытым исходным кодом для написания статей. Обычно мы отключаем сторонние библиотеки и включаем только диагностические правила для общих целей и микро-оптимизации. Этот подход ясно показывает состояние кодовой базы проекта. После выбора наиболее интересных предупреждений мы можем поделиться с вами результатами.

Хорошо, давайте свернемся на рукава и исследуем ошибки - у меня есть для вас!

Результаты анализа

Фрагмент n1

inline double stod(std::string_view str) {
  trim_for_from_chars(str);
  double res;
  auto [ptr, ec] = utils::charconv::from_chars(str.data(), 
                                   str.data() + str.size(), res);

  if(ec == std::errc::invalid_argument) {
    throw std::invalid_argument("");                // <=
  } else  if(ec == std::errc::result_out_of_range) {
    throw std::out_of_range("");                    // <=
  }
  return res;
}

Мы начнем наше приключение с небольшой, но очень серьезной проблемы. Вы можете это заметить? Если нет, то вы, возможно, никогда не испытывали ситуаций, когда пользователь контактирует с технической поддержкой с проблемой. Вместо того, чтобы предоставлять четкое описание, пользователь отправляет что -то вроде: «Все сломано/остановлено/закрыто», а затем «Нет, он не показывает никаких ошибок».

Чтобы уточнить, фрагмент кода бросает два исключения без какого -либо сообщения. Это превращается в игру в угадание.

О, как сильно это больно ... моя команда испытала эту боль не так давно. Чтобы это не произошло снова, мы создали специальныйV1116Диагностическое правило для таких случаев.

Предупреждения PVS-Studio:

V1116[CWE-778] Создание объекта исключения без объяснительного сообщения может привести к недостаточной регистрации.charconv.hpp146

V1116[CWE-778] Создание объекта исключения без объяснительного сообщения может привести к недостаточной регистрации.charconv.hpp148

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

inline int stoi(std::string_view str) {
  trim_for_from_chars(str);
  int res;
  auto [ptr, ec] = utils::charconv::from_chars(
                       str.data(), str.data() + str.size(), res);

  if(ec == std::errc::invalid_argument) {
    throw std::invalid_argument("");               // <=
  } else if(ec == std::errc::result_out_of_range) {
    throw std::out_of_range("");                   // <=
  }
  return res;
}

Предупреждения PVS-Studio:

V1116[CWE-778] Создание объекта исключения без объяснительного сообщения может привести к недостаточной регистрации.charconv.hpp159

V1116[CWE-778] Создание объекта исключения без объяснительного сообщения может привести к недостаточной регистрации.charconv.hpp161

Фрагмент n2

std::string dsgettext (const char * domainname, const char *msgid)
{
  std::string msgval = dgettext (domainname, msgid);
  if (msgval == msgid) {
    const char* firsthat = std::strchr (msgid, '^');
    if (firsthat == nullptr)
      msgval = msgid;
    else
      msgval = firsthat + 1;
  }
  return msgval;
}

PVS-Studio Warning:V1048[CWE-1164] переменной «MSGVAL» была присвоена одинаковым значением.getText.cpp440

Этот фрагмент кода показывает то же значение, которое присваиваетсяmsgvalпеременная. Во -первых, происходит инициализация выражением:

dgettext (domainname, msgid);

Аdgettextфункция по сути является оберткой надdgettextфункция изboost::localeбиблиотека:

Namespace bl = boost::locale;
....
std::string dgettext(const char* domain, const char* msgid)
{
  return bl::dgettext(domain, msgid, get_manager().get_locale());
}

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

Если функция возвращаетсяmsgid, тогда не было найдено никакого фразного перевода. В таких случаях разработчики проверяют, началось ли исходное сообщение с^характер. Если это так, они представляют подстроение после этого символа в качестве вывода:

msgval = firsthat + 1;

Итак, теперь мы знаем, как работает код, но не ясно, почемуmsgvalпеременная пересчитанаmsgidценить. Поскольку это действие является избыточным, давайте немного упростим код:

std::string dsgettext (const char * domainname, const char *msgid)
{
  std::string msgval = dgettext (domainname, msgid);
  if (msgval == msgid) {
    const char* firsthat = std::strchr (msgid, '^');
    if (firsthat != nullptr)
      msgval = firsthat + 1;
  }
  return msgval;
}

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

Фрагмент n3

void mp_create_game::sync_with_depcheck()
{
  ....
  auto& game_types_list = find_widget<menu_button>("game_types");
  game_types_list.set_value(
    std::distance(level_types_.begin(),
                  std::find_if(level_types_.begin(), 
                               level_types_.begin(), // <=
                               [&](const level_type_info& info)
                               {
                                 return info.first == new_level_index.first;
                               })));
  ....
}

PVS-Studio Warning:V539Рассмотрим проверку итераторов, которые передаются в качестве аргументов для функционирования «find_if».mp_create_game.cpp534

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

Вот как это работает. Аstd::find_ifФункция вызывается с двумя идентичными итераторами в качестве первых двух аргументов, оба указывая на начало -level_types_.begin()Полем Аstd::find_ifФункция вообще не повторяется. Он ничего не найдет и вернет второй аргумент, который, в данном случае, такой жеlevel_types_.begin()Запуск итератора.

Тогдаstd::distanceФункция будет рассчитать расстояние между первым аргументом,level_types_.begin()итератор иlevel_types_.begin()итератор возвращенstd::find_ifфункция В результате мы получаем0Полем

Фиксированный код выглядит следующим образом:

void mp_create_game::sync_with_depcheck()
{
  ....
  auto& game_types_list = find_widget<menu_button>("game_types");
  game_types_list.set_value(
    std::distance(level_types_.begin(),
                  std::find_if(level_types_.begin(), 
                               level_types_.end(),
                               [&](const level_type_info& info)
                               {
                                 return info.first == new_level_index.first;
                               })));
  ....
}

Фрагмент n4

template<typename T>
class enable_lua_ptr
{
public:
  ....
  enable_lua_ptr& operator=(enable_lua_ptr&& o)
  {
    self_ = std::move(o.self_);
    *self_ = static_cast<T*>(this);
  }                                  // <=
....
}

PVS-Studio Warning:V591[CWE-393] Невоидная функция должна вернуть значение.lua_ptr.hpp34

Как вы уже заметили, оператор движения не возвращает значение и бум! Мы ловим неопределенное поведение.

Фрагмент N5

std::string addon_info::display_icon() const
{
  std::string ret = icon;

  // make sure it's set to something when there are issues
  // otherwise display errors will spam the log while the 
  // add-ons manager is open
  if(ret.empty()){
    ret = "misc/blank-hex.png";
  } if(!image::exists(image::locator{ret}) && !ret.empty()) { // <=
    ERR_AC << "add-on '" << id << 
           "' has an icon which cannot be found: '" << ret << "'";
    ret = "misc/blank-hex.png";
  } else if(ret.find("units/") != std::string::npos 
         && ret.find_first_of('~') == std::string::npos) {
    // HACK: prevent magenta icons, because they look awful
    LOG_AC << "add-on '" << id << 
           "' uses a unit baseframe as icon without TC/RC specifications";
    ret += "~RC(magenta>red)";
  }
  return ret;
}

Предупреждения PVS-Studio:

V560[CWE-571] Часть условного выражения всегда верна :! ret.empty ().info.cpp240

V646[CWE-670] рассмотрим проверку логики приложения. Вполне возможно, что ключевое слово «иначе» отсутствует.info.cpp240

Согласно идее разработчика, скорее всего, если нет логотипа или изображения заголовка для конкретного дополнения в диспетчере Add-On, будет установлено изображение по умолчанию.

Анализатор побуждает нас посмотреть на эту проблему: вторая часть!ret.empty()всегда вернетсяtrueПолем Это происходит из -за предыдущей противоположной проверки с назначением строковой буквацииretпеременная.

Кроме того, анализатор намекает, что программист мог случайно пропуститьelseКлючевое слово между первым и вторым условиями. Похоже, это должно быть там: еслиiconпустая строка, функция вернетсяmisc/blank-hex.pngПолем То же самое должно произойти, еслиiconэто непустая строка, но файл не существует на этом пути. Однако!ret.empty()Проверка не нужна, поэтому мы можем просто удалить его.

Фрагмент n6

class install_dependencies : public modal_dialog
{
public:
  explicit install_dependencies(const addons_list& addons)
    : modal_dialog(window_id()), addons_(addons)   // <=
  {}
....
private:
  virtual const std::string& window_id() const override;
....
}

PVS-Studio Warning:V1099[CWE-908] Использование функции «window_id» ненициализированного полученного класса при инициализации базового класса «modal_dialog» приведет к неопределенному поведению.install_dependencies.hpp29

Благодаря этому фрагменту кода, я могу рассказать вам больше о неопределенном поведении.

Как мы можем видеть выше,install_dependenciesкласс получен изmodal_dialogсорт. Вinstall_dependenciesконструктор, базовый класс инициализируется со значением, возвращаемым (подождите ...) Нестатическийwindow_idфункция Итак, это произойдет:

  1. Выполнение списка инициализации:
    • призыв кinstall_dependencies::window_idметод;
    • конструктор вызовmodal_dialogсорт;
    • инициализацияaddons_член данных;
  2. Выполнение тела конструктораinstall_dependenciesсорт.

Это означает, что функция объекта, чей класс еще не был инициализирован, будет вызвана! Это нарушает следующееправило стандарта:

Функции членов (включая виртуальные функции членов, [class.virtual]) можно вызвать для строящегося объекта

Аналогичным образом, строящийся объект может быть операндом оператора TypeID ([expr.typeid]) или Dynamic_CASC ([expr.dynamic.cast]).

Однако, если эти операции выполняются в CTOR-инициализаторе (или в функции, которая прямо или косвенно называется из CTOR-инициализатора), прежде чем все мем-инициализаторы для базовых классов будут завершены, программа имеет неопределенное поведение.

Но подождите, еще не все! Как вы могли заметить,window_idФункция члена является виртуальной и переопределенной вinstall_dependenciesсорт. Проблемы могут возникнуть позже, когда программист пишет полученный класс, гдеwindow_idбудет переопределен.

Когда создается объект этого производного класса иinstalled_dependenciesКонструктор выполняется, пока нет информации о существовании нового переопределения. Итак,installed_dependencies::window_idФункция всегда будет вызвана в списке инициализации. Это может отличаться от первоначального намерения разработчиков. Вы можете прочитать больше об этомздесьПолем

Фрагмент n7

double readonly_context_impl::power_projection(const map_location& loc, 
                                               const move_map& dstsrc) const
{
  map_location used_locs[6];
  int ratings[6];
  std::fill_n(ratings, 0, 6);   // <=
  int num_used_locs = 0;
....
}

PVS-Studio Warning:V575[CWE-628] элементы функции 'fill_n' 0 '. Осмотрите второй аргумент.Contexts.cpp987

Скорее всего, разработчики хотели заполнитьratingsМассив с нулями, но что -то пошло не так. Итак, когдаstd::fill_nФункция называется, массив не будет инициализирован.

Проблема в том, что второй и третий аргументы были смешаны, когдаstd::fill_nбыл вызван. Как мы знаем - или простоЧитать в документации- Второй параметр указывает, сколько раз, какое значение должно быть записано на массив, а третий представляет значение. Другими словами, текущие выходы кода6вratingsмассив ровно нулевой раз.

Мы можем исправить это, обменяя0и6аргументы или путем явного инициализации при создании массива.

int ratings[6]{}; // explicitly initializes the array with zeros

Фрагмент n8

std::pair<map_location,map_location> move_to_targets_phase::
                     choose_move(std::vector<target>& targets)
{
  std::vector<target>::iterator best_target = best_rated_target->tg;
  ....
  if(best != units_.end()) {
    LOG_AI << "Could not make good move, staying still";

    //this sounds like the road ahead might be dangerous,
    //and that's why we don't advance.
    //create this as a target, attempting to rally units around
    targets.emplace_back(best->get_location(), best_target->value);
    best_target = targets.end() - 1;                               // <=
    return std::pair(best->get_location(), best->get_location());
  }
  LOG_AI << "Could not find anywhere to move!";
  return std::pair<map_location,map_location>();
}

PVS-Studio Warning:V1001[CWE-563] переменная 'BEST_TARGE' присваивается, но не используется в конце функции.ca_move_to_targets.cpp604

Анализатор обнаружил, что в конце функции локальныйbest_targetПервой переменной присваивается значение, но никогда не используется потом. Скорее всего, это указывает на ошибку: оценка по имени переменной, она, по -видимому, является важной частью локальной логики ИИ.

На данный момент это может быть преднамеренным, и мир не готов к искусственному искусственному искусству на уровне Джарвиса, как в фильмах «Железного человека». Тем не менее, я не могу ни подтвердить, ни опровергнуть это. Итак, мой совет разработчикам - поближе познакомиться с этим фрагментом кода.

Фрагмент N9

do{
  ....
} while((action_result && action_result->is_ok()) || !action_result);

PVS-Studio Warning:V728[CWE-571] Чрезмерная проверка может быть упрощена. '||' Оператор окружен противоположными выражениями '! Action_Result' 'и' action_Result '.Recruitment.cpp439

Показанный фрагмент кода не содержит ошибки, но анализатор рекомендует нам упростить выражение в условии цикла следующим образом:

while(!action_result || action_result->is_ok);

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

Фрагмент N10

bool mouse_handler::mouse_button_event(const SDL_MouseButtonEvent& event, 
                                                          uint8_t button,
                                                        map_location loc, 
                                                              bool click)
{
  static const std::array<const std::string, 6> buttons = {       // <=
    "",
    "left",         // SDL_BUTTON_LEFT
    "middle",         // SDL_BUTTON_MIDDLE
    "right",        // SDL_BUTTON_RIGHT
    "mouse4",       // SDL_BUTTON_X1
    "mouse5"        // SDL_BUTTON_X2
  };

  if (gui().view_locked() 
    || button < SDL_BUTTON_LEFT || button > buttons.size()) {     // <=
    return false;
  } else if (event.state > SDL_PRESSED || !pc_.get_map().on_board(loc)) {
    return false;
  }

  if(game_lua_kernel* lk = pc_.gamestate().lua_kernel_.get()) {
    lk->mouse_button_callback(loc, buttons[button],               // <=
              (event.state == SDL_RELEASED ? "up" : "down"));
  ....
  }
....
}

Когда -то давно - только GitHub знает, когда - массивstd::arrayтип названногоbuttonsбыл создан. Все шло гладко, но был записан чек до того, как был использован массив. Это должно было ограничить индекс при доступе кbuttonsМассив, но что -то, очевидно, пошло не так. Это связано с общей ошибкой, известной какОшибка "Off-One"Полем

Ну, что сделано, сделано ... Легенда говорит, что день наступит, когда второй параметрmouse_button_eventфункционироватьbutton, будет иметь ценность6Полем

PVS-Studio Warning:V557Возможен массив. Индекс кнопки «кнопка» указывает за пределы массива.mouse_events.cpp624

Когда это произойдет и проверка пройдет, будет предпринята попытка извлечь строку изbuttonsмассив с использованием6индекс. Затем произойдет что -то ужасное и неуклонно разрушительное, что никогда не было замечено в этом фрагменте кода.

Тем не менее, всегда есть надежда, что герой придет и сохранит этот код от возможных сумасшедших последствий, изменив>оператор вbutton > buttons.size()выражение к>=!

То же анализатор предупреждение:V557Возможен массив. Индекс кнопки «кнопка» указывает за пределы массива.mouse_events.cpp633

Заключение

Пришло время завершить статью. Было несколько предупреждений, поэтому благодарность всем разработчикам - PAST и PRESICT - которые внесли свой вклад в проект! Код прост в прочитании и хорошо организованном. Понятно, что разработчикам глубоко любят свою игру. Они разрабатывали и улучшали его в течение почти 20 лет, всегда придумывая новые функции.

На самом деле, проект напоминает мнеГерои мощи и магии, чудесный фэнтезийный мир и волшебное приключение на основе поворота-я тоже зацепил эту игру. Ну, вы знаете, что борьба с огнем: я собираюсь сыграть в World of Warcraft.

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

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

И как традиция, я рекомендую вамПопробуйте наш анализатор PVS-StudioПолем Мы предлагаембесплатная лицензияДля проектов с открытым исходным кодом.

Береги себя и хорошего дня!

Оригинал

Recent Post

Categories

PREVIOUS ARTICLE
NEXT ARTICLE