Должен ли я проверять параметры своих функций?

Это перевод Should I check the parameters to my function? Автор: Ларри Остерман.

У меня только что был интересный разговор с одним из тестеров в моей группе.

Он только что закончил отчёт о серии багов в наших компонентах, потому что они не проверяли, не передают ли им мусорные указатели. Вместо этого они просто возбуждали исключение $C0000005, и приложение вылетало (*).

Эти функции возвращали ошибку E_POINTER, если им передавали nil.

Но он считал, что они должны также проверять не-nil указатели на допустимость и также возвращать E_POINTER, если эти указатели не указывают на допустимую память.

Этот момент был темой многих дискуссий у нас в Microsoft на протяжении многих лет. И у нас есть две школы:

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

Вторая школа говорит: “GIGO – если приложение передало нам мусор, то что плохого, если оно вылетит?”.

Я твёрдо принадлежу второму лагерю (не удивительно, если вы знаете меня). Для этого есть много причин. Самая большая - безопасность. Дело вот в чём: как вы обычно проверяете указатель на допустимость? Вызовом функций IsBadReadPtr или IsBadWritePtr, так? Майкл Ховард называет эти функции “CrashMyApplication” и “CorruptMemoryAndCrashMySystem” соответственно. Проблема в том, что функции IsBadReadPtr/IsBadWritePtr делают в точности то, что сказано в их описании: они читают или пишут в указанную память, обернув эту запись в обработчик исключений. Если при этом возникает исключение, то они возвращают ошибку, если нет - то успех.

С этим подходом есть две проблемы. Во-первых, на самом деле IsBadReadPtr/IsBadWritePtr проверяют не доступность памяти вообще, а то, что память была доступна в момент вызова функции. Если у вас мусорная ссылка случайно указывает на память другого потока, то IsBadReadPtr вернёт успех, но этот поток может освободить память сразу после того, как IsBadReadPtr вернёт управление. Что означает, что любые ваши проверки, которые вы делаете на основе этой информации, ненадёжны (и это указано в документации по IsBadWritePtr/IsBadReadPtr).

Вторая проблема ещё хуже. Что случится, если адрес памяти, передаваемый в IsBadReadPtr, случайно окажется указывающим на guard-страницу стека (**)? Ну, IsBadReadPtr поймает это исключение на guard-странице и обработает его (потому что IsBadReadPtr обрабатывает все исключения). Поэтому системный обработчик исключений не увидит это исключение. Что означает, что при дальнейшей работе приложения стек этого потока не будет расти выше текущего предела. Вызвав IsBadReadPtr в вашей функции, вы превратили легко идентифицируемую проблему в плавающий баг переполнения стека, который может возникнуть много позже от места его введения (минуты или даже часы работы программы).

Ещё одна проблема с агрессивной проверкой параметров функции: что произойдёт, если приложение не проверяет код возврата функции? Это означает, что если у них в коде есть баг, который приводит к передаче мусора в вашу функцию, то, поскольку они не проверяют возвращаемое значение, то они никогда про него не узнают. Кроме того, у них будет новый баг - та самая порча памяти. Если бы функция выбросила исключение - они узнали бы о проблеме сразу же, на месте. Никаких подводных камней и плавающих багов порчи памяти.

Теперь, принимая во внимание сказанное, если вы присоединитесь к школе два, то у вас всё ещё есть проблема – вы не можете доверять параметрам вызывающего. Вообще. Это означает, что вам нужно быть аккуратным при доступе к этим буферам, чтобы у вас не произошло каких-нибудь плохих вещей (типа, вы захватили критическую секцию, потом пишете в буфер, но происходит исключение, а критическую секцию вы не отпускаете).

Ещё одной вещью, которую стоит иметь в виду: бывают ситуации, когда крешить пользовательские приложения НЕ хорошо. К примеру, когда вы используете RPC, RPC использует обработку исключений, чтобы передать RPC-ошибки назад в вызвавшее приложение (в отличие от кодов возврата API). Так что иногда у вас нет выбора, кроме как ловить исключения. Другой вариант - это когда у вас уже есть написанный код, который использует IsBadReadPtr. Быть может, что вы не сможете убрать эту проверку, потому что у вас может быть код, который зависит от этого поведения (если весь вызывающий код находится в ваших руках - то нет проблем, если же эта функция может вызываться внешним кодом (типа функции в DLL), и код уже побывал в реальной эксплуатации, то у вас нет выбора).

Так что, обычно использовать IsBadXxxPtr для проверки параметров своих функций является плохой идеей. Ваши пользователи могут проклясть вас за то, что ваше приложение вылетело, но в долгосрочной перспективе это хорошая идея (***).

Примечания переводчика:
(*) $C0000005 - это код исключения Access Violation. Хотя в Delphi вы часто можете увидеть, как это исключение обрабатывается, но это противоречит обычным практикам: если ты не знаешь, что делать с исключением - игнорируй его. Таким образом, в обычных программах исключение Access Violation раскручивается до самого верха и остаётся необработанным. А необработанное исключение в потоке приводит к закрытию процесса. При этом к процессу подключается WER и собирает данные об исключении, отправляя их на сервер отчётов Microsoft (и которые вы потом можете собрать и проанализировать, после чего исправить ошибку).

В Delphi же почти все исключения в VCL-приложениях молчаливо заворачиваются в глобальный try/except, который просто показывает сообщение. С одной стороны такое поведение не даёт диагностировать проблему, с другой стороны оно, видимо, является наследием тех времён, когда WER просто не было, и когда программа вылетала, то не было никого, кто мог бы обработать этот вылет. Сейчас есть, но Delphi довольно консервативна. Так что имеем то, что имеем. Возможно, именно этим объясняется популярность утилит диагностики типа EurekaLog и madExcept.

(**) Guard-страница - это страница на плавающей границе стека. Когда системный обработчик исключений верхнего уровня видит исключение, произошедшее на guard-странице стека, он увеличивает размер стека. Guard-страница перемещается, а на её место выделяется память. Таким образом стек динамически растёт по мере требований.

(***) Если вы обрабатываете Access Violation, то вы обрабатываете исключение, с которым вы (вообще-то) не знаете, что делать. Вы не знаете, почему оно возникло. Это исключение однозначно говорит, что у вас в программе есть баг. Если вы продолжите работать, сделав вид, что у вас ничего не произошло - это может работать во многих случаях, но в некоторых вы в итоге можете испортить данные пользователя.

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

У меня несколько раз были такие проблемы в Delphi: возникает AV в каком нибудь rtl.bpl в среде, я закрываю окно об ошибке, появляется ещё одно (другое), я иду дальше, в надежде, что я сумею как-то сохранить изменения в своём проекте. В итоге я получаю испорченные файлы, и мне приходится брать предыдущий вариант и по памяти восстанавливать мои изменения. Да, такое случается крайне редко, но перевешивает ли удобство отсутствия необходимости перезапуска приложения по AV эти риски?

Не будет ли лучше показать сообщение "В программе возникла ошибка", как это делает WER?