Для получения триального ключа
заполните форму ниже
Team License (базовая версия)
Enterprise License (расширенная версия)
* Нажимая на кнопку, вы даете согласие на обработку
своих персональных данных. См. Политику конфиденциальности

Запросите информацию о ценах
Новая лицензия
Продление лицензии
--Выберите валюту--
USD
EUR
GBP
RUB
* Нажимая на кнопку, вы даете согласие на обработку
своих персональных данных. См. Политику конфиденциальности

Бесплатная лицензия PVS-Studio для специалистов Microsoft MVP
* Нажимая на кнопку, вы даете согласие на обработку
своих персональных данных. См. Политику конфиденциальности

Для получения лицензии для вашего открытого
проекта заполните, пожалуйста, эту форму
* Нажимая на кнопку, вы даете согласие на обработку
своих персональных данных. См. Политику конфиденциальности

Мне интересно попробовать плагин на:
* Нажимая на кнопку, вы даете согласие на обработку
своих персональных данных. См. Политику конфиденциальности

Ваше сообщение отправлено.

Мы ответим вам на


Если вы так и не получили ответ, пожалуйста, проверьте папку
Spam/Junk и нажмите на письме кнопку "Не спам".
Так Вы не пропустите ответы от нашей команды.

>
>
>
break и fallthrough

break и fallthrough

27 Янв 2018

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

0554_Chromium_2_break_ru/image1.png

Многие годы я изучал ошибки в программах и сейчас могу с уверенностью заявить, что в C, а вслед за ним и в C++, оператор switch сделан неправильно. Я понимаю, что возможность не писать break, сделанная, чтобы передать управление дальше, позволяет писать изящные алгоритмы. Но всё равно огромное количество ошибок убедило меня, что был выбран неправильный подход. Понятно, что теперь уже поздно. Просто хотелось сказать, что правильным решением было бы обязательно писать слово break или обратное ключевое слово, например, fallthrough. Сколько бы сил, времени и денег было сэкономлено. Конечно, этот недостаток не сравнится с Null References: The Billion Dollar Mistake, но всё равно большой ляп.

Ладно, пофилософствовали и хватит. Язык C++ такой, какой есть. Однако, это не значит, что можно расслабиться и ничего не предпринимать для повышения качества и надёжности кода. Проблема "пропущенного break" - это большая проблема, и её нельзя недооценивать. Даже в высококачественном проекте Chromium прячутся ошибки этого типа.

Давайте посмотрим, что я заметил в процессе разбора отчета, выданного PVS-Studio. Как я писал в вводной статье, отчёт я смотрел достаточно бегло, поэтому могут быть и другие, незамеченные мной ошибки. Тем не менее, найденных ошибок мне будет достаточно, чтобы продемонстрировать, что всё это не отдельные случайные ляпы, а устойчивый паттерн ошибки. Читателям следует отнестись к этому паттерну серьезно и постараться использовать меры по его предотвращению.

Первая ошибка взята непосредственно из кода проекта Chromium.

int GetFieldTypeGroupMetric(....) {
  ....
  switch (AutofillType(field_type).group()) {
    ....
    case ADDRESS_HOME_LINE3:
      group = GROUP_ADDRESS_LINE_3;
      break;
    case ADDRESS_HOME_STREET_ADDRESS:
      group = GROUP_STREET_ADDRESS;
    case ADDRESS_HOME_CITY:
      group = GROUP_ADDRESS_CITY;
      break;
    case ADDRESS_HOME_STATE:
      group = GROUP_ADDRESS_STATE;
      break;
    ....
}

Независимо от того, нужно ли автоматически заполнить некое поле "Street Address", или поле "Сity", в любом случае будет выбрана константа GROUP_ADDRESS_CITY. Т.е. где-то вместо названия улицы автоматически будет подставляться название города.

Причина в том, что пропущен оператор break. В результате, после присваивания:

group = GROUP_STREET_ADDRESS;

Переменной group тут же будет присвоено новое значение:

group = GROUP_ADDRESS_CITY;

Анализатор PVS-Studio замечает это двойное присваивание и выдаёт предупреждение: V519 The 'group' variable is assigned values twice successively. Perhaps this is a mistake. Check lines: 145, 147. autofill_metrics.cc 147

Вторая ошибка также относится к коду Chromium и выглядит схожим образом.

void GLES2Util::GetColorFormatComponentSizes(...., int* a) {
  ....
  // Sized formats.
  switch (internal_format) {
    case GL_ALPHA8_EXT:
      *a = 8;
    case GL_ALPHA16F_EXT:
      *a = 16;
    case GL_ALPHA32F_EXT:
      *a = 32;
    case GL_RGB8_OES:
    case GL_SRGB8:
    case GL_RGB8_SNORM:
    case GL_RGB8UI:
    case GL_RGB8I:
      *r = 8;
      *g = 8;
      *b = 8;
      break;
    case GL_RGB565:
  ....
}

Здесь забыто 2 или 3 оператора break. Я не знаю, как точно должен работать этот код, поэтому воздержусь от комментариев по поводу того, как следует исправить ошибку. Анализатор PVS-Studio выдаёт для этого кода два предупреждения:

  • V519 CWE-563 The '* a' variable is assigned values twice successively. Perhaps this is a mistake. Check lines: 1385, 1387. gles2_cmd_utils.cc 1387
  • V519 CWE-563 The '* a' variable is assigned values twice successively. Perhaps this is a mistake. Check lines: 1387, 1389. gles2_cmd_utils.cc 1389

Третья ошибка из кода Chromium.

gfx::ColorSpace VideoColorSpace::ToGfxColorSpace() const {
  ....
  switch (primaries) {
  ....
  case PrimaryID::SMPTEST431_2:
    primary_id = gfx::ColorSpace::PrimaryID::SMPTEST431_2;
    break;
  case PrimaryID::SMPTEST432_1:
    primary_id = gfx::ColorSpace::PrimaryID::SMPTEST432_1;
  case PrimaryID::EBU_3213_E:
    primary_id = gfx::ColorSpace::PrimaryID::INVALID;
    break;
  }
  ....
}

Точно та же картина, что и раньше. Предупреждение PVS-Studio: V519 CWE-563 The 'primary_id' variable is assigned values twice successively. Perhaps this is a mistake. Check lines: 106, 109. video_color_space.cc 109

Четвёртая ошибка из кода Chromium. В этот раз нам уже поможет предупреждение V796, а не V519. Диагностика V519 выявляет пропущенный break косвенным образом, когда замечает повторное присваивание. Диагностика V796 разработана специально для поиска пропущенных операторов break.

void RecordContextLost(ContextType type,
                       CommandBufferContextLostReason reason) {
  switch (type) {
    ....
    case MEDIA_CONTEXT:
      UMA_HISTOGRAM_ENUMERATION("GPU.ContextLost.Media",
        reason, CONTEXT_LOST_REASON_MAX_ENUM);
      break;
    case MUS_CLIENT_CONTEXT:
      UMA_HISTOGRAM_ENUMERATION("GPU.ContextLost.MusClient",
        reason, CONTEXT_LOST_REASON_MAX_ENUM);
      break;
    case UI_COMPOSITOR_CONTEXT:
      UMA_HISTOGRAM_ENUMERATION("GPU.ContextLost.UICompositor",
        reason, CONTEXT_LOST_REASON_MAX_ENUM);
    case CONTEXT_TYPE_UNKNOWN:
      UMA_HISTOGRAM_ENUMERATION("GPU.ContextLost.Unknown",
        reason, CONTEXT_LOST_REASON_MAX_ENUM);
      break;
  }
}

После выполнения ветки "UI_COMPOSITOR_CONTEXT" управление передаётся в ветку "CONTEXT_TYPE_UNKNOWN". По всей видимости, это приводит где-то к неверной обработке... А вот не знаю я, на что это повлияет. Но break здесь пропущен, по всей видимости, случайно, а не намеренно.

Предупреждение PVS-Studio: V796 CWE-484 It is possible that 'break' statement is missing in switch statement. command_buffer_metrics.cc 125

Пятая ошибка в коде Chromium, из-за которой страдает средняя клавиша мыши.

void SystemInputInjectorMus::InjectMouseButton(
  ui::EventFlags button, bool down)
{
  ....
  int modifier = ui::MODIFIER_NONE;
  switch (button) {
    case ui::EF_LEFT_MOUSE_BUTTON:
      modifier = ui::MODIFIER_LEFT_MOUSE_BUTTON;
      break;
    case ui::EF_RIGHT_MOUSE_BUTTON:
      modifier = ui::MODIFIER_RIGHT_MOUSE_BUTTON;
      break;
    case ui::EF_MIDDLE_MOUSE_BUTTON:
      modifier = ui::MODIFIER_MIDDLE_MOUSE_BUTTON;
    default:
      LOG(WARNING) << "Invalid flag: " << button
                   << " for the button parameter";
      return;
  }
  ....
}

Неправильно обрабатывается нажатие на среднюю клавишу мыши. После корректного действия:

modifier = ui::MODIFIER_MIDDLE_MOUSE_BUTTON;

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

Предупреждение PVS-Studio: V796 CWE-484 It is possible that 'break' statement is missing in switch statement. system_input_injector_mus.cc 78

Здесь читатель вправе сказать: "Всё понятно, достаточно!". Однако, мне попалась на глаза ещё парочка таких ошибок в используемых библиотеках, так что давайте посмотрим и на них. Я хочу убедительно показать, что этот вид ошибок повсеместен.

Шестая ошибка живёт в коде библиотеки Angle, используемой в Chromium.

void State::getIntegerv(const Context *context,
                        GLenum pname, GLint *params)
{
  ....
  switch (pname)
  {
    ....
    case GL_DEBUG_GROUP_STACK_DEPTH:
      *params = static_cast<GLint>(mDebug.getGroupStackDepth());
       break;
    case GL_MULTISAMPLE_EXT:
      *params = static_cast<GLint>(mMultiSampling);
       break;
    case GL_SAMPLE_ALPHA_TO_ONE_EXT:
      *params = static_cast<GLint>(mSampleAlphaToOne);      // <=
    case GL_COVERAGE_MODULATION_CHROMIUM:
      *params = static_cast<GLint>(mCoverageModulation);
       break;
    case GL_ATOMIC_COUNTER_BUFFER_BINDING:
    ....
}

Предупреждение PVS-Studio: V519 CWE-563 The '* params' variable is assigned values twice successively. Perhaps this is a mistake. Check lines: 2044, 2046. state.cpp 2046

Седьмая ошибка живёт в коде библиотеки SwiftShader, используемой в Chromium.

GL_APICALL void GL_APIENTRY glInvalidateSubFramebuffer(....)
{
  ....
  switch(target)
  {
  case GL_DRAW_FRAMEBUFFER:
  case GL_FRAMEBUFFER:
    framebuffer = context->getDrawFramebuffer();
  case GL_READ_FRAMEBUFFER:
    framebuffer = context->getReadFramebuffer();
    break;
  default:
    return error(GL_INVALID_ENUM);
  }
  ....
}

Предупреждение PVS-Studio: V519 CWE-563 The 'framebuffer' variable is assigned values twice successively. Perhaps this is a mistake. Check lines: 3879, 3881. libglesv3.cpp 3881

Семь - это красивое число. На нём и остановимся. Возможно, есть другие ошибки, но их поиск я оставляю авторам Chromium и библиотек. Мне было скучно внимательно смотреть предупреждения V519. Диагностика V519 даёт немало бестолковых ложных срабатываний, связанных с неаккуратным написанием кода или с макросами. А настраивать анализатор для такого большого проекта – это уже работа, требующая оплаты (да, это был тонкий намёк для Google).

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

Рекомендация

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

  • [[gnu::fallthrough]];
  • [[clang::fallthrough]];
  • __attribute__((fallthrough));
  • BOOST_FALLTHROUGH;
  • и так далее.

К сожалению, всё это было как-то не универсально. К счастью, у меня для всех C++ программистов есть хорошая новость. В стандарте C++17 наконец утвердили стандартный метод подсказать компилятору, что программист специально планирует передать управление дальше. Это атрибут [[fallthrough]]. Анализаторы, естественно, тоже будут пользоваться этой подсказкой. Кстати, рекомендую посмотреть нашу статью "C++17" про то, что нового появилось в этом стандарте.

Чуть подробнее про атрибут [[fallthrough]].

Этот атрибут показывает, что оператор break внутри блока case отсутствует намеренно (т.е. управление передается в следующий блок case), и поэтому соответствующее предупреждение компилятора или статического анализатора кода выдаваться не должно.

Пример использования:

switch (i)
{
case 10:
  f1();
  break;
case 20:
  f2();
  [[fallthrough]]; // Предупреждение будет подавлено
case 30:
  f3();
  break;
case 40:
  f4();
  break;
}

Если Вы уже перешли на C++17, нет причин не использовать [[fallthrough]]. Включите предупреждения в своём компиляторе, чтобы он информировал о пропущенном break. А там, где оператор break в действительности не нужен, пишите [[fallthrough]]. Также рекомендую описать всё это в используемом в вашей компании стандарте кодирования.

Компиляторы Clang и GCC начинают предупреждать о пропущенном break, если указать им флаг:

-Wimplicit-fallthrough

Если добавить [[fallthrough]], то предупреждение пропадает.

С MSVC сложнее. Начиная с Visual C++ 2017 RTM, он, вроде, должен выводить предупреждение C4468, если указан флаг /W4. Подробнее: Compiler Warnings by compiler version (см. C4468). Но что-то у меня последняя версия Visual Studio со всеми последними обновлениями упорно молчит. Впрочем, я долго не экспериментировал и, возможно, сделал что-то неправильно. В любом случае, если не сейчас, то в ближайшее время этот механизм будет работать и в Visual C++.

Спасибо всем за внимание. Желаю всем безбажного кода. Да, и не забудьте попробовать проверить с помощью PVS-Studio ваши рабочие проекты.

Популярные статьи по теме
Эффект последней строки

Дата: 31 Май 2014

Автор: Андрей Карпов

Я изучил множество ошибок, возникающих в результате копирования кода. И утверждаю, что чаще всего ошибки допускают в последнем фрагменте однотипного кода. Ранее я не встречал в книгах описания этого …
Бесплатный PVS-Studio для тех, кто развивает открытые проекты

Дата: 22 Дек 2018

Автор: Андрей Карпов

В канун празднования нового 2019 года команда PVS-Studio решила сделать приятный подарок всем контрибьюторам open-source проектов, хостящихся на GitHub, GitLab или Bitbucket. Им предоставляется возмо…
PVS-Studio для Java

Дата: 17 Янв 2019

Автор: Андрей Карпов

В седьмой версии статического анализатора PVS-Studio мы добавили поддержку языка Java. Пришло время немного рассказать, как мы начинали делать поддержку языка Java, что у нас получилось и какие дальн…
Главный вопрос программирования, рефакторинга и всего такого

Дата: 14 Апр 2016

Автор: Андрей Карпов

Вы угадали, ответ - "42". Здесь приводится 42 рекомендации по программированию, которые помогут избежать множества ошибок, сэкономить время и нервы. Автором рекомендаций выступает Андрей Карпов - тех…
Характеристики анализатора PVS-Studio на примере EFL Core Libraries, 10-15% ложных срабатываний

Дата: 31 Июл 2017

Автор: Андрей Карпов

После большой статьи про проверку операционной системы Tizen мне было задано много вопросов о проценте ложных срабатываний и о плотности ошибок (сколько ошибок PVS-Studio выявляет на 1000 строк кода)…
PVS-Studio ROI

Дата: 30 Янв 2019

Автор: Андрей Карпов

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

Дата: 21 Ноя 2018

Автор: Андрей Карпов

Краткое описание технологий, используемых в инструменте PVS-Studio, которые позволяют эффективно обнаруживать большое количество паттернов ошибок и потенциальных уязвимостей. Статья описывает реализа…
Статический анализ как часть процесса разработки Unreal Engine

Дата: 27 Июн 2017

Автор: Андрей Карпов

Проект Unreal Engine развивается - добавляется новый код и изменятся уже написанный. Неизбежное следствие развития проекта - появление в коде новых ошибок, которые желательно выявлять как можно раньш…
Как PVS-Studio оказался внимательнее, чем три с половиной программиста

Дата: 22 Окт 2018

Автор: Андрей Карпов

PVS-Studio, как и другие статические анализаторы кода, часто выдаёт ложные срабатывания. Но не стоит спешить считать странные срабатывания ложными. Это короткая история о том, как PVS-Studio вновь ок…
Как и почему статические анализаторы борются с ложными срабатываниями

Дата: 20 Мар 2017

Автор: Андрей Карпов

В своей предыдущей статье я писал, что мне не нравится подход, при котором статические анализаторы кода оцениваются с помощью синтетических тестов. В статье приводился пример, воспринимаемый анализат…

Комментарии (0)

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