V704. The expression is always false on newer compilers. Avoid using 'this == 0' comparison.
Анализатор обнаружил в коде выражение вида this == 0
. Данное выражение может оказаться работоспособным в ряде случаев, однако его использование крайне опасно по некоторым соображениям.
Рассмотрим простой пример:
class CWindow {
HWND handle;
public:
HWND GetSafeHandle() const
{
return this == 0 ? 0 : handle;
}
};
Вызов метода CWindow::GetSafeHandle()
для нулевого указателя this
по стандарту С++ ведёт к неопределённому поведению. Однако, поскольку во время работы метода не производится доступа к полям этого класса, метод может работать. С другой стороны, существует два возможных неблагоприятных сценария выполнения данного кода. Во-первых, согласно стандарту С++, указатель this
никогда не может быть нулевым; следовательно, компилятор может оптимизировать вызов метода, упростив его до:
return handle;
Во-вторых, предположим, что существует следующий код:
class CWindow {
.... // CWindow из предыдущего примера
};
class MyWindowAdditions {
unsigned long long x; // 8 bytes
};
class CMyWindow: public MyWindowAdditions, public CWindow {
....
};
....
void foo()
{
CMyWindow * nullWindow = NULL;
nullWindow->GetSafeHandle();
}
Этот код приведёт к чтению из памяти по адресу 0x00000008
. В этом можно убедиться, написав следующую строку:
std::cout << nullWindow->handle << std::endl;
Будет выведен адрес 0x00000008
, поскольку исходный указатель NULL (0x00000000
) был скорректирован таким образом, чтобы указывать на начало подобъекта класса CWindow
. Для этого его надо сместить на sizeof(MyWindowAdditions)
байт.
Теперь проверка this == 0
полностью теряет смысл. Указатель this
всегда по меньшей мере равен значению 0x00000008
.
С другой стороны, ошибка не проявит себя, если поменять местами базовые классы в объявлении CMyWindow
:
class CMyWindow: public CWindow, public MyWindowAdditions{
....
};
Всё это может приводить к крайне неочевидным ошибкам.
Исправление кода достаточно нетривиально. Корректным выходом в данном случае будет изменение метода класса на статический. Это повлечёт за собой большое количество изменений во всех местах, где встречался вызов метода.
class CWindow {
HWND handle;
public:
static HWND GetSafeHandle(CWindow * window)
{
return window == 0 ? 0 : window->handle;
}
};
Второй вариант — использование паттерна Null Object
, что тоже повлечёт за собой значительный объём работ.
class CWindow {
HWND handle;
public:
HWND GetSafeHandle() const
{
return handle;
}
};
class CNullWindow : public CWindow {
public:
HWND GetSafeHandle() const
{
return nullptr;
}
};
....
void foo(void)
{
CNullWindow nullWindow;
CWindow * windowPtr = &nullWindow;
// Выведет 0
std::cout << windowPtr->GetSafeHandle() << std::endl;
}
Примечание. Данный дефект опасен тем, что на его обработку почти никогда нет времени, поскольку "всё и так работает", а затраты на рефакторинг велики. Однако то, что работало годами, может неожиданно дать сбой при малейшем изменении условий: сборка под другую ОС, изменение версии компилятора (в том числе и его обновление) и так далее.
Например: начиная с версии 4.9.0, компилятор GCCнаучился выбрасывать проверку на неравенство нулю разыменованного выше по коду указателя (см. диагностику V595):
int wtf( int* to, int* from, size_t count ) {
memmove( to, from, count );
if( from != 0 ) // <= после оптимизации условие всегда истинно
return *from;
return 0;
}
Примеров проблемного кода из реальных приложений, оказавшегося "сломанным" из-за undefined behavior, достаточно много. Вот некоторые из них, чтобы подчеркнуть важность проблемы:
Пример N1. Уязвимость в ядре Linux
struct sock *sk = tun->sk; // initialize sk with tun->sk
....
if (!tun) // <= всегда false
return POLLERR; // if tun is NULL return error
Пример N2. Некорректная работа srandomdev():
struct timeval tv;
unsigned long junk; // <= Не инициализирована специально
gettimeofday(&tv, NULL);
// LLVM: аналог srandom() от неинициализированной переменной,
// т.е. tv.tv_sec, tv.tv_usec и getpid() не учитываются.
srandom((getpid() << 16) ^ tv.tv_sec ^ tv.tv_usec ^ junk);
Пример N3. Синтетический пример, очень наглядно показывающий и возможности компиляторов по агрессивной оптимизации в связи с undefined behavior, и новые возможности "прострелить себе ногу":
#include <stdio.h>
#include <stdlib.h>
int main() {
int *p = (int*)malloc(sizeof(int));
int *q = (int*)realloc(p, sizeof(int));
*p = 1;
*q = 2;
if (p == q)
printf("%d %d\n", *p, *q); // <= Clang r160635: Вывод: 1 2
}
Насколько нам известно, на момент выхода этой диагностики вызов проверки this == 0
ещё не проигнорирован ни одним компилятором, однако в стандарте С++ явным образом написано (§9.3.1/1): "If a nonstatic member function of a class X is called for an object that is not of type X, or of a type derived from X, the behavior is undefined.". Иными словами, результат вызова любой нестатической функции для класса с this == 0
не определён. Это лишь дело времени, когда компиляторы начнут вместо (this == 0)
подставлять false
на этапе компиляции.
Данная диагностика классифицируется как:
Взгляните на примеры ошибок, обнаруженных с помощью диагностики V704. |