Шестая проверка Chromium, послесловие04.02.2019 11:19

строгий единорог


В начале 2018 года в нашем блоге появился цикл статей, посвящённый шестой проверке исходного кода проекта Chromium. Цикл включает в себя 8 статей, посвященных ошибкам и рекомендациям по их предотвращению. Две статьи вызвали бурное обсуждение, и до сих пор на почту мне изредка приходят комментарии касательно тем, затронутых в них. Пожалуй, следует дать некоторые дополнительные пояснения и, как говорится, расставить точки над i.
Прошёл год с момента написания цикла статей, посвященного очередной проверке исходных кодов проекта Chromium:

  1. Chromium: шестая проверка проекта и 250 багов
  2. Красивый Chromium и корявый memset
  3. break и fallthrough
  4. Chromium: утечки памяти
  5. Chromium: опечатки
  6. Chromium: использование недостоверных данных
  7. Почему важно проверять, что вернула функция malloc
  8. Chromium: другие ошибки


Статьи, посвящённые memset и malloc вызывали и продолжают вызывать дискуссии, которые кажутся мне странными. Видимо возникло некоторое недопонимание из-за того, что я недостаточно чётко сформулировал свои мысли. Я решил вернуться к этим статьям и внести некоторые уточнения.

memset


Начнём со статьи про memset, так как здесь всё просто. Возникли споры касательно того, как лучше инициализировать структуры. Достаточно много программистов написало, что лучше давать рекомендацию писать не: 

HDHITTESTINFO hhti = {};


А так: 

HDHITTESTINFO hhti = { 0 };


Аргументы:

  1. Конструкцию {0} легче заметить при чтении кода, чем {}.
  2. Конструкция {0} более интуитивно понятна, чем {}. То есть 0 подсказывает, что структура заполняется нулями.


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

По поводу заметности. Мне кажется, это дело вкуса и привычки. Не думаю, что наличие 0 внутри фигурных скобок принципиально меняет ситуацию.

А вот со вторым аргументом я вообще не согласен. Запись вида {0} даёт повод неправильно воспринять код. Например, можно посчитать, что, если заменить 0 на 1, все поля будут инициализированы единицами. Поэтому такой стиль написания скорее вреден, чем полезен.

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

V1009. Check the array initialization. Only the first element is initialized explicitly.

Анализатор обнаружил возможную ошибку, связанную с тем, что при объявлении массива указано значение только для одного элемента. Таким образом, остальные элементы будут инициализированы неявно нулём или конструктором по умолчанию.

Рассмотрим пример подозрительного кода: 

int arr[3] = {1};


Возможно, программист ожидал, что arr будет состоять из одних единиц, но это не так. Массив будет состоять из значений 1, 0, 0.

Корректный код: 

int arr[3] = {1, 1, 1};


Подобная путаница может произойти из-за схожести с конструкцией arr = {0}, которая инициализирует весь массив нулями.

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

Также не рекомендуется пренебрегать наглядностью кода.

Например, код для кодирования значений цвета записан следующим образом: 

int White[3] = { 0xff, 0xff, 0xff };
int Black[3] = { 0x00 };
int Green[3] = { 0x00, 0xff };


Благодаря неявной инициализации все цвета заданы правильно, но лучше переписать код более наглядно: 

int White[3] = { 0xff, 0xff, 0xff };
int Black[3] = { 0x00, 0x00, 0x00 };
int Green[3] = { 0x00, 0xff, 0x00 };


malloc


Прежде чем читать далее, прошу освежить в памяти содержимое статьи «Почему важно проверять, что вернула функция malloc». Эта статья породила много обсуждений и критики. Вот некоторые из дискуссий: reddit.com/r/cpp, reddit.com/r/C_Programming, habr.com. Изредка мне пишут по поводу этой статьи на почту и сейчас.

Статья критикуется читателями по следующим пунктам:

1. Если malloc вернула NULL, то лучше сразу завершить работу программы, чем писать кучу if-ов и пытаться как-то обработать нехватку памяти, из-за которой часто выполнение программы всё равно невозможно.

Я вовсе не призывал до последнего бороться с последствиями нехватки памяти, пробрасывая ошибку всё выше и выше. Если для приложения допустимо завершить работу без предупреждения, то пусть так и будет. Для этого достаточно всего одной проверки сразу после malloc или использования xmalloc (см. следующий пункт).

Я возражал и предупреждал об отсутствии проверок, из-за которых программа продолжает работать «как ни в чем не бывало». Это совсем другое. Это опасно, так как приводит к неопределённому поведению, порче данных и так далее.

2. Не рассказано про решение, которое заключается в написании функций-врапперов для выделения памяти с последующей проверкой или использование уже существующих функций, таких как xmalloc.

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

Функция xmalloc не является частью стандартной библиотеки C (см. «What is the difference between xmalloc and malloc?»). Однако эта функция может быть объявлена в других библиотеках, например, в GNU utils library (GNU libiberty).

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

void* xmalloc(size_t s)
{
  void* p = malloc(s);
  if (!p) {
    fprintf (stderr, "fatal: out of memory (xmalloc(%zu)).\n", s);
    exit(EXIT_FAILURE);
  }
  return p;
}


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

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

3. Больше всего комментариев было следующего вида: «на практике malloc никогда не возвращает NULL».

К счастью, не я один понимаю, что это неправильный подход. Мне очень понравился вот этот комментарий в мою поддержку:

По опыту обсуждения подобной темы складывается ощущение, что в «Интернетах» есть две секты. Приверженцы первой свято уверены в том, что под Linux-ом malloc никогда не возвращает NULL. Приверженцы второй свято уверены в том, что если память в программе выделить не удалось, но ничего уже в принципе сделать нельзя, нужно только падать. Переубедить их никак нельзя. Особенно когда эти две секты пересекаются. Можно только принять это как данность. Причем не суть важно, на каком профильном ресурсе идёт обсуждение.

Я подумал и решил последовать совету и не буду пытаться переубеждать :). Будем надеяться, что эти группы разработчиков пишут только некритичные программы. Если, например, какие-то данные испортятся в игре или игра упадёт, это не страшно.

Единственное, что важно — чтобы так не делали разработчики библиотек, баз данных и т.д.

Воззвание к разработчикам библиотек и ответственного кода


Если вы разрабатываете библиотеку или иной ответственный код, то всегда проверяйте значение указателя, который вернула функция malloc/realloc, и возвращайте вовне код ошибки, если память выделить не удалось.

В библиотеках нельзя вызвать функцию exit, если не удалось выделить память. По этой же причине нельзя использовать xmalloc. Для многих приложений недопустимо просто взять и аварийно завершить их работу. Из-за этого, например, может быть испорчена база данных. Могут быть потеряны данные, которые считались много часов. Из-за этого программа может быть подвержена уязвимостям «отказ в обслуживании», когда вместо того, чтобы как-то корректно обработать возрастающую нагрузку, работа многопоточного приложения просто завершается.

Нельзя предположить, как и в каких проектах будет использована библиотека. Поэтому следует исходить из того, что приложение может решать очень ответственные задачи. И просто «убить» его, вызвав exit, никуда не годится. Скорее всего, такая программа написана с учётом возможности нехватки памяти и может что-то предпринять в этом случае. Например, некая CAD система из-за сильной фрагментации памяти не может выделить достаточный буфер памяти для очередной операции. Но это не повод ей завершиться в аварийном режиме с потерей данных. Программа может дать возможность сохранить проект и перезапустить себя в штатном режиме.

Ни в коем случае нельзя полагаться на то, что malloc всегда может выделить память. Неизвестно, на какой платформе и как будет использоваться библиотека. Если на одной платформе нехватка памяти — это экзотика, то на другой это может быть весьма частой ситуацией.

Нельзя надеяться, что если malloc вернёт NULL, то программа упадёт. Произойти может всё что угодно. Как я описывал в статье, программа может писать данные вовсе не по нулевому адресу. В результате могут быть испорчены некие данные, что ведёт к непредсказуемым последствиям. Даже memset опасен. Если заполнение данных идёт в обратном порядке, то вначале портятся некие данные, и только потом программа упадёт. Но падение может произойти слишком поздно. Если в момент работы функции memset в параллельных потоках будут использованы испорченные данные, то последствия могут быть фатальны. Можно получить испорченную транзакцию в базе данных или отправку команды на удаление «ненужных» файлов. Может успеть произойти всё что угодно. Предлагаю читателю пофантазировать самостоятельно, к чему может привести использование мусора в памяти.

Таким образом, у библиотеки есть только один правильный вариант работы с функциями malloc. Нужно СРАЗУ проверить, что вернула функция, и если это — NULL, то вернуть статус ошибки.

Дополнительные ссылки


  1. OOM handling.
  2. Fun with NULL pointers: part 1, part 2.
  3. What Every C Programmer Should Know About Undefined Behavior: part 1, part 2, part 3.

tsz9kmyjtteajhd4x1au60rsrvq.png

Если хотите поделиться этой статьей с англоязычной аудиторией, то прошу использовать ссылку на перевод: Andrey Karpov. Sixth Chromium Check, Afterword.

© Habrahabr.ru