1053_60_cpp_antipatterns_ru/image2.png


Перед вами обновлённая коллекция вредных советов для C++ программистов, которая превратилась в целую электронную книгу. Всего их 60, и каждый сопровождается пояснением, почему на самом деле ему не стоит следовать. Всё будет одновременно и в шутку, и серьёзно. Как бы глупо ни смотрелся вредный совет, он не выдуман, а подсмотрен в реальном мире программирования.


Я буду публиковать советы по 5 штук, чтобы не утомить вас, так как мини-книга содержит много интересных отсылок на другие статьи, видео и т. д. Однако, если вам не терпится, здесь вы можете сразу перейти к её полному варианту: "60 антипаттернов для С++ программиста". В любом случае желаю приятного чтения.


Вредный совет N11. Во всём виноват компилятор


Если что-то не работает, то, скорее всего, глючит компилятор. Попробуйте поменять местами некоторые переменные и строки кода.


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


Конечно, в компиляторах тоже бывают ошибки, и с ними можно столкнуться. Однако в 99 % случаев, когда кто-то говорит, что "компилятор глючит", он неправ и на самом деле некорректен именно его код.


Чаще всего программист или не понимает какие-то тонкости языка C++, или столкнулся с неопределённым поведением. Давайте рассмотрим пару таких примеров.


Первая история берёт своё начало из обсуждения, происходившего на форуме linux.org.ru.


Программист жаловался на глюк в компиляторе GCC 8, но, как затем выяснилось, виной всему являлся некорректный код, приводящий к неопределённому поведению. Давайте рассмотрим этот случай.


Примечание. В оригинальной дискуссии переменная s имеет тип const char *s. При этом на целевой платформе автора тип char является беззнаковым. Поэтому для наглядности я сразу в коде использую указатель типа const unsigned char *.


int foo(const unsigned char *s)
{
  int r = 0;
  while(*s) {
    r += ((r * 20891 + *s *200) | *s ^ 4 | *s ^ 3) ^ (r >> 1);
    s++;
  }
  return r & 0x7fffffff;
}

Компилятор не генерирует код для оператора побитового И (&). Из-за этого функция возвращает отрицательные значения, хотя по задумке программиста этого происходить не должно.


Разработчик считает, что это глюк в компиляторе. Но на самом деле неправ программист, который написал такой код. Функция работает не так, как ожидается, из-за того, что в ней возникает неопределённое поведение.


Компилятор видит, что в переменной r считается некоторая сумма. Переполнения переменной r произойти не должно (с точки зрения компилятора). Иначе это неопределённое поведение, которое компилятор никак не должен рассматривать и учитывать. Итак, компилятор считает, что значение в переменной r после окончания цикла не может быть отрицательным. Следовательно, операция r & 0x7fffffff для сброса знакового бита является лишней, и компилятор решает просто возвращать из функции значение переменной r.


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


Чтобы исправить код, достаточно считать хэш, используя для этого беззнаковую переменную:


int foo(const unsigned char *s)
{
  unsigned r = 0;
  while(*s) {
    r += ((r * 20891 + *s *200) | *s ^ 4 | *s ^ 3) ^ (r >> 1);
    s++;
  }
  return (int)(r & 0x7fffffff);
}

Вторая история была ранее описана мной в статье "Во всём виноват компилятор". Однажды анализатор PVS-Studio выдал предупреждение на такой код:


TprintPrefs::TprintPrefs(IffdshowBase *Ideci,
                         const TfontSettings *IfontSettings)
{
  memset(this, 0, sizeof(this)); // This doesn't seem to
                                 // help after optimization.
  dx = dy = 0;
  isOSD = false;
  xpos = ypos = 0;
  align = 0;
  linespacing = 0;
  sizeDx = 0;
  sizeDy = 0;
  ...
}

Анализатор прав. А автор кода – нет.


Комментарий говорит нам: компилятор глючит при включении оптимизации и не обнуляет поля структуры.


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


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


Правильное вычисление размера класса должно выглядеть так:


memset(this, 0, sizeof(*this));

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


Не надо так писать. Я привёл этот пример только потому, что эти нюансы меркнут перед ошибкой вычисления размера структуры.


Вот так и рождаются легенды о глючных компиляторах и отважных программистах, которые с ними сражаются.


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


Прежде чем подозревать компилятор, полезно:


  1. Попросить опытных коллег провести обзор вашего кода;
  2. Внимательно посмотреть, не выдаёт ли компилятор на ваш код предупреждения, и попробовать такие ключи, как -Wall, -pedantic;
  3. Проверить код статическим анализатором, таким как PVS-Studio;
  4. Проверить код динамическим анализатором;
  5. Если вы знаете ассемблер, то изучить ассемблерный листинг, сгенерированный компилятором для кода, и подумать, почему он мог таким получиться;
  6. Воспроизвести ошибку как можно в более коротком коде и задать вопрос на сайте Stack Overflow.

Вредный совет N12. Будьте смелыми с argv


Не мешкайте и не тормозите. Сразу берите и используйте аргументы командной строки. Например, так: char buf[100]; strcpy(buf, argv[1]);. Проверки делают только параноики, не уверенные в себе и людях.


Дело не только в том, что буфер может быть переполнен. Обработка данных без предварительной проверки открывает ящик Пандоры, полный уязвимостей.


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


  1. Стреляем в ногу, обрабатывая входные данные;
  2. CWE-20: Improper Input Validation;
  3. Taint-анализ (taint checking);
  4. V1010. Unchecked tainted data is used in expression.

Вредный совет N13. Undefined behavior — просто страшилка


Undefined behavior – это страшилка на ночь для детей. На самом деле его не существует. Если программа работает, как вы ожидали, значит она правильная. И обсуждать здесь нечего, точка. Everything is fine.


1053_60_cpp_antipatterns_ru/image9.png


Наслаждайтесь! :)


  1. Неопределённое поведение.
  2. Что каждый программист на C должен знать об Undefined Behavior. Часть 1, часть 2, часть 3.
  3. Глубина кроличьей норы или собеседование по C++ в компании PVS-Studio.
  4. Undefined behavior ближе, чем вы думаете.
  5. Неопределённое поведение, пронесённое сквозь года.
  6. Разыменовывание нулевого указателя приводит к неопределённому поведению.
  7. Неопределённое поведение и правда не определено.
  8. With Undefined Behavior, Anything is Possible.
  9. Philosophy behind Undefined Behavior.
  10. Почему перенос при целочисленном переполнении — не очень хорошая идея.
  11. Пример проявления неопределённого поведения из-за отсутствия return.
  12. YouTube. C++Now 2018: John Regehr "Closing Keynote: Undefined Behavior and Compiler Optimizations".
  13. YouTube. Towards optimization-safe systems: analyzing the impact of undefined behavior.
  14. А дальше вводите в Google "Undefined behavior" и продолжайте :)

Вредный совет N14. double == double


Смело сравнивайте числа с плавающей точкой с помощью оператора ==. Раз есть такой оператор, значит им нужно пользоваться.


Сравнивать-то можно… Вот только у такого сравнения есть нюансы, которые нужно знать и учитывать. Рассмотрим пример:


double A = 0.5;
if (A == 0.5)               // True
  foo();

double B = sin(M_PI / 6.0);
if (B == 0.5)               // ????
  foo();

Первое сравнение A == 0.5 истинно. Второе сравнение B == 0.5 может быть как истинно, так и ложно. Результат выражения B == 0.5 зависит от используемого процессора, версии и настроек компилятора. Например, когда я пишу эту статью, мой компилятор создал код, который вычисляет значение переменной B, равное 0.49999999999999994.


Более корректно этот код можно написать следующим образом:


double b = sin(M_PI / 6.0);
if (std::abs(b - 0.5) < DBL_EPSILON)
  foo();

В данном случае сравнение с погрешностью DBL_EPSILON верно, так как результат функции sin лежит в диапазоне [-1, 1]. C numeric limits interface:


DBL_EPSILON — difference between 1.0 and the next representable value for double respectively.

Если мы работаем со значениями больше нескольких единиц, то такие погрешности, как FLT_EPSILON, DBL_EPSILON, могут оказаться слишком малы. И наоборот при работе со значениями типа 0.00001 эти погрешности слишком велики. Каждый раз следует выбирать погрешность, адекватную диапазону возможных значений.


Возникает вопрос. Как же все-таки сравнить две переменных типа double?


double a = ...;
double b = ...;
if (a == b) // how?
{
}

Одного единственно правильного ответа нет. В большинстве случаев можно сравнить две переменных типа double, написав код следующего вида:


if (std::abs(a - b) <= DBL_EPSILON * std::max(std::abs(a),
                                              std::abs(b)))
{
}

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


А можно ли все-таки точно сравнивать значения в формате с плавающей точкой?


В некоторых случаях да. Но эти ситуации весьма ограничены. Сравнивать можно в том случае, если это и есть, по сути, одно и то же значение.


Пример, где допустимо точное сравнение:


// -1 - признак, что значение переменной не было установлено
double val = -1.0;
if (Foo1())
  val = 123.0;
if (val == -1.0) // OK
{
}

В данном случае сравнение со значением -1 допустимо, так как именно точно таким же значением мы инициализировали переменную ранее.


Такое сравнение будет работать даже в случае, если число не может быть представлено конечной дробью. Следующий код распечатает "V == 1.0/3.0":


double V = 1.0/3.0;
if (V == 1.0/3.0)
{
  std::cout << "V == 1.0/3.0" << std::endl;
} else {
  std::cout << "V != 1.0/3.0" << std::endl;
}

Однако надо быть очень бдительным. Достаточно заменить тип переменной V на float и условие станет ложным:


float V = 1.0/3.0;
if (V == 1.0/3.0)
{
  std::cout << "V == 1.0/3.0" << std::endl;
} else {
  std::cout << "V != 1.0/3.0" << std::endl;
}

Этот код уже печатает "V != 1.0/3.0". Почему? Значение переменной V равно 0.333333, а значение 1.0/3.0 равно 0.333333333333333. Перед сравнением переменная V, имеющая тип float, расширяется до типа double. Происходит сравнение:


if (0.333333000000000 == 0.333333333333333)

Эти числа естественно не равны. В общем, будьте аккуратны.


Кстати, анализатор PVS-Studio может найти все операторы == и !=, у которых операнды имеют тип с плавающей точкой, чтобы вы могли ещё раз проверить этот код. См. диагностику V550 — Suspicious precise comparison.


Дополнительные ресурсы:


  1. Bruce Dawson. Comparing floating point numbers, 2012 Edition.
  2. RSDN. Про сравнение double (RU).
  3. Андрей Карпов. 64-битные программы и вычисления с плавающей точкой.
  4. Wikipedia. Floating point.
  5. CodeGuru Forums. C++ General: How is floating point representated?
  6. Boost. Floating-point comparison algorithms.

Вредный совет N15. memmove — лишняя функция


memmove — лишняя функция. Всегда и везде используйте memcpy.


Роль функций одинакова. Но есть важное отличие: когда области памяти, переданные через первые два параметра, частично перекрываются, memmove гарантирует, что результат копирования — правильный, а в случае с memcpy произойдёт неопределённое поведение.


1053_60_cpp_antipatterns_ru/image10.png


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


  • memmove – проблем с копированием перекрывающихся областей не возникнет и содержимое будет скопировано правильно;
  • memcpy – возникнет проблема. Исходные значения этих двух байтов будут перезаписаны и не сохранены. Поэтому последние два байта последовательности будут точно такими же, что и два первых.

См. также дискуссию на Stack Overflow: memcpy() vs memmove().


Раз функции ведут себя так по-разному, что стало поводом шутить на эту тему? Оказывается, авторы многих проектов невнимательно читали документацию про эти функции. Дополнительно невнимательных программистов спасало то, что в старых версиях glibc функция memcpy была псевдонимом memmove. Заметка на эту тему: Glibc change exposing bugs.


А вот как это описывается в Linux manual page:


Failure to observe the requirement that the memory areas do not overlap has been the source of significant bugs. (POSIX and the C standards are explicit that employing memcpy() with overlapping areas produces undefined behavior.) Most notably, in glibc 2.13 a performance optimization of memcpy() on some platforms (including x86-64) included changing the order in which bytes were copied from src to dest.

This change revealed breakages in a number of applications that performed copying with overlapping areas. Under the previous implementation, the order in which the bytes were copied had fortuitously hidden the bug, which was revealed when the copying order was reversed. In glibc 2.14, a versioned symbol was added so that old binaries (i.e., those linked against glibc versions earlier than 2.14) employed a memcpy() implementation that safely handles the overlapping buffers case (by providing an "older" memcpy() implementation that was aliased to memmove(3)).

Об этой мини-книге


Автор: Карпов Андрей Николаевич. E-Mail: karpov [@] viva64.com.


Более 15 лет занимается темой статического анализа кода и качества программного обеспечения. Автор большого количества статей, посвящённых написанию качественного кода на языке C++. С 2011 по 2021 год удостаивался награды Microsoft MVP в номинации Developer Technologies. Один из основателей проекта PVS-Studio. Долгое время являлся CTO компании и занимался разработкой С++ ядра анализатора. Основная деятельность на данный момент — управление командами, обучение сотрудников и DevRel активность.


Ссылки на полный текст:



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

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