пятница, мая 28, 2010

Неинициализированный указатель и вызов функции

С неинициализированными указателями на экземпляр класса связан один неприятный момент. Допустим, у нас есть класс CBase, в котором есть функция, которая не обращается к переменным класса. То есть указатель this не использует.

class CBase
{
int i;
public:
void f() { std::cout<<"CBase::f"<<std::endl;}
};


И если где-то дальше вы написали код вроде такого:

CBase* p = NULL;
p->f();


То по стандарту это undefined behavior. Однако очень часто функция отрабатывает нормально. Visual Studio 2008, например, генерит вполне себе работающий код. А чего, вызов этой функции был разрешен на этапе компиляции, указатель this не нужен. И довольно долго может создаваться ощущение, что все нормально работает. Можно даже привести более страшно выглядящий пример
((CBase*)0)->f();


Но на самом деле далеко не все нормально. Как только появляется обращение к this, то программа начинает "падать". Примеры:
class CBase
{
int i;
public:
virtual void f() { std::cout<<"CBase::f"<<std::endl;}
};

class CBase
{
int i;
public:
void f() { std::cout<<"CBase::f"<<std::endl; i=0;}
};


Все это приводит к очень интересным трудно отлавливаемым багам.

Ссылка по теме:
comp.lang.c++.moderated - Functions that don't use this, called with uninitialized pointers

30 коммент.:

Unknown комментирует...

Не совсем понятно, что не так в последнем примере?

Alena комментирует...

2Yorik.sar:

Не совсем понятно, что не так в последнем примере?

Там идет присвоение переменной. int i=0. То есть идет обращение к this.

Unknown комментирует...

А разве не происходит сокрытие новой локальное переменной i?

Alena комментирует...

2Logonoff:

А разве не происходит сокрытие новой локальное переменной i?

Угу, моя ошибка. Исправила.

Qehgt комментирует...

Только не ссылка, а указатель

Alena комментирует...

2Qehgt:

Только не ссылка, а указатель

Поправила, спасибо.

Laplander комментирует...

Был случай, когда из функции, в которой забыли написать return, успешно возвращался осмысленный итератор по вектору. Причем итератор объявлялся в этой функции как локальная переменная.

Unknown комментирует...

что мешает сделать метод f статическим?

Alena комментирует...

2Muxa:

что мешает сделать метод f статическим?

В данном конкретном случае ничего не мешает.

eao197 комментирует...

Ага, с этим поведением VC++ доводилось сталкиваться. Он там живет еще с 90-х. Спасало то, что приходилось программу таскать между разными компиляторами. В результате под Watcom-ом программа ломалась там, где под VC++ успешно работала.

Тогда у меня даже способ ловли особо хитрых багов был такой: если ошибка не желала ловиться под одним компилятором, нужно было попробовать скомпилировать ее другим компилятором :)

Vadim Atlygin комментирует...

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

Angel5a комментирует...

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

Unknown комментирует...

В gcc это вызовет segmentation fault, по крайней мере на данном примере. Вообще не приятно что нет предупреждений от компилятора, интересно как icc это проглотит.

Анонимный комментирует...

Надо же, сам топтался в прошлом году по этим-же граблям - только с gcc. Программа падала на вполне себе нормальном мьютексе. С тех пор как только начинает происходить что-то загадочное с кодом - сразу смотрю на this.

Анонимный комментирует...

2Plambir:
компилятор даже теоретически не может это обнаружить если указатель нулевой из-за провала фабричного метода.

Sergey Maslyakov комментирует...

Static code analysis дефект такого плана выловить не может. Точнее, может, но только простые и утрированные примеры.

Эту проблему должен был поймать если не сам девелопер, то хотя бы человек, делающий code review. Метод, не обращающийся к data members таекущего объекта должен быть статическим.

Alexei Eskenazi комментирует...

Строго говоря, именно такая ситуация особых проблем с диагностикой и отладкой чаще всего не доставляет (если есть навыки отладки и знание инструментальных средств).
Вот "повисшие" указатели на объекты, которые когда-то были в динамической памяти, могут доставить немало приятных минут...

Alena комментирует...

2evolvah:

Эту проблему должен был поймать если не сам девелопер, то хотя бы человек, делающий code review. Метод, не обращающийся к data members таекущего объекта должен быть статическим.

Это в данном синтетическом примере все очевидно. На самом деле все бывает не так. Много кода, его правят несколько человек. Есть if'ы и в какой-то несчастный момент ситуация складывается так, что указатель равен NULL, а if'ами мы все обращения к переменным обошли. После чего добавляем где-то в кода выражение вроде m_i=0; и в ужасе наблюдаем, что после этого программа "падает".

Sergey Maslyakov комментирует...

@Alena:

Да, но первая версия такой функции не будет содержать обращений к data members, верно? Вот тогда-то и надо сделать ее static. Когда кто-то другой попытается добавить обращение к data member, то получит ошибку от компилятора.

Это вещь того же масштаба, что и передача объектов по const reference, если передаваемый объект не должен быть изменен вызываемой функцией. Эдакая защита от неаккуратного программиста средствами самого языка.

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

Unknown комментирует...

Ничего странного в этом нет.
Класс в памяти это только его переменные-члены. Функции - это код, который генерится всегда, если к нем есть обращение. (В примере, фактически, статический метод).
Objective-C позволяет слать мессаджы на nil - то есть программа не упадет даже, если в этом методе будет обрашение к переменным класса.

Unknown комментирует...

Вы так говорите "программа начнет падать" как будто это что-то плохое.
Хуже если указатель не 0, а указывает на вполне себе выделенную память, но не принадлежащую объекту этого класса.

Unknown комментирует...

Плохо то, что это официально не разрешенное поведение, т.е. каждый компилер как хочет, так и делает. ИМХО стоило бы узаконить путь, как это делает MSVC - самый правильный, this - это просто один из параметров, используй как хочешь.

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

Врядли какая-нибуть проверка, кроме runtime, может это определить, да и runtime в случае С++ часто не будет работать.

Unknown комментирует...

2evolvah:
Вот тогда-то и надо сделать ее static.

Не всегда так получается, особенно если пишется generic код. Часто бывает интересно, например, в дебаге использовать динамические данные с удобным изменением, а в релиз закомилить уже полностью настроенные статические. При этом, чтобы часть данных осталась линамической (простой #ifdef в этом случае не поможет).

Анонимный комментирует...

Не понимаю, что не так. Ну да, обращения по мусорным указателям иногда дают сегфолт, а иногда нет. Я думал, все привыкли уже. За код типа ((CBase*)0)->f(); надо просто эцих с гвоздями на 72 часа.

Конечно, если бы компилятор мог автоматически в каждый метод добавить assert(this) - было бы чуточку приятнее, но мир не идеален :(

Alena комментирует...

2legolegs:

Я думал, все привыкли уже.

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

Kodt комментирует...

Ещё там есть такая подляна, как смещение базы.

struct A { int x; };
struct B {
int y;
void f() {
cout << (void*)this << endl;
if(this==NULL) return;
// безопасно, думаете вы?
y=123;
}
};
struct C : A, B {
void g() { f(); }
};

int main() {
((C*)NULL)->f(); // 1
((C*)NULL)->g(); // 2
}

Нулевые указатели всегда переходят в нулевые указатели, то есть up-casting: (B*)(C*)NULL == (B*)NULL, и наоборот, down-casting: (C*)(B*)NULL == (C*)NULL.
Но это, понятное дело, требует лишней проверки, а в С++ принято не платить за то, что не заказано.

Таким образом, в //1 ещё есть шанс, что компилятор выполнит up-casting указателя перед разыменованием (->). Но в //2, в теле функции C::g(), эти шансы стремительно исчезают.

Смещение базы возникает не только при множественном наследовании.
Например, компиляторы VC и gcc стараются поместить vfptr с нулевым смещением - ценой сдвига базы. (Да, это по Стандарту можно).

struct A {
int x; // чтобы база была не пустой
void f() { cout<<(void*)this; }
};
struct B : A {
virtual void g() {}
};
int main() { ((B*)0)->f(); } // 0x4

Kodt комментирует...

Забыл пояснить, в чём фокус со смещением базы.
Разница - в обработке пользовательского указателя (который может быть NULL) и this (который, как мы ожидаем, указывает на валидный объект - а следовательно, не должен быть NULL).
Даже пользовательские ссылки - и тех некоторые компиляторы (VC) проверяют на нулёвость (хотя и не обязаны). В отношении же this - или, более общо, левого операнда оператора (->) и (.), такие проверки не производятся.
http://codepad.org/fEhLsZyg

Alena комментирует...

2Kodt:

Забыл пояснить, в чём фокус со смещением базы.

Спасибо за хороший пример.

Inso комментирует...

gcc ведет себя примерно так же. Вообще то что это работает (в смысле вызов метода без обращения к this) оно и понятно, метод класса манглится и линкуется в в сишную функцию и принимает параметр this уже в рантайме. Если сишной функции послать неинициализированный указатель на структуру то она не упадет пока не обратится к ней. Получается странная философия, семантика такой простой вещи как опретора непрямого обращения в случае с p->func(); изменяется так, что очевидные ошибки с ним связанные в Си становятся неочевидными в С++ :)

Анонимный комментирует...

Проблема вовсе не в компиляторе а в программисте.
Это чистой воды статическая функция!
Если вы её не объявили таковой это Ваша ошибка как дизайнера класса.
Это все равно что ругать продавца за то, что кому продали топор. а он отрубил себе пальцы.