пятница, мая 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 комментариев:

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

    ОтветитьУдалить
  2. 2Yorik.sar:

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

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

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

    ОтветитьУдалить
  4. 2Logonoff:

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

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

    ОтветитьУдалить
  5. Только не ссылка, а указатель

    ОтветитьУдалить
  6. 2Qehgt:

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

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

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

    ОтветитьУдалить
  8. что мешает сделать метод f статическим?

    ОтветитьУдалить
  9. 2Muxa:

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

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

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

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

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

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

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

    ОтветитьУдалить
  14. Анонимный28/5/10 21:39

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

    ОтветитьУдалить
  15. Анонимный28/5/10 22:50

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

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

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

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

    ОтветитьУдалить
  18. 2evolvah:

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

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

    ОтветитьУдалить
  19. @Alena:

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

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

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

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

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

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

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

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

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

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

    ОтветитьУдалить
  24. Анонимный1/6/10 01:49

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

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

    ОтветитьУдалить
  25. 2legolegs:

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

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

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

    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

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

    ОтветитьУдалить
  28. 2Kodt:

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

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

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

    ОтветитьУдалить
  30. Анонимный30/10/10 01:02

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

    ОтветитьУдалить