- PVSM.RU - https://www.pvsm.ru -

Поиск и исправление багов в исходниках PHP

Поиск и исправление багов в исходниках PHP - 1

Честно предупреждаю: воспринимайте этот текст с определённой долей скептицизма. Я лишь недавно начал знакомство с внутренностями PHP, но хотел бы рассказать вам о том, что творится за кулисами бага #75237 [1].

Обнаружение бага

Всё началось с поиска строк кода для покрытия на митапе Chicago PHP UG [2], который был частью PHP TestFest 2017 [3].

Я нашёл непокрытую строку с помощью сайта gcov для [4] ext/json/json_encoder.c. Попытался написать код на PHP, который обработал бы эту строку [5].

Поиск и исправление багов в исходниках PHP - 2

Попытался и так и эдак, но ничто не сработало, и внезапно я посредством этого кода ненароком инициировал segfault [6]:

<?php
class Foo implements JsonSerializable {
  public function jsonSerialize() {
    return new self;
  }
}

# Segfault!
var_dump(json_encode(new Foo));

Но в PHP никогда не должно быть segfault. Основной код PHP должен бросать исключение и предоставлять для пользовательского пространства красивое сообщение об ошибке, прежде чем произойдёт segfault.

Чтобы узнать, какие версии PHP подвержены этому багу, я создал сниппет 3v4l [7] и увидел, что все активно поддерживаемые версии генерируют segfault.

Это случается только тогда, когда jsonSerialize() возвращает новый экземпляр самого себя. Если я возвращаю с помощью $this тот же экземпляр, код работает как нужно.

<?php
class Foo implements JsonSerializable {
  public function jsonSerialize() {
    return $this;
  }
}

# Works fine
var_dump(json_encode(new Foo));

Отправка отчёта о баге

Обо всех багах в PHP нужно сообщать посредством bugs.php.net [8]. Хотя я и пытался патчить баг самостоятельно, мне всё же пришлось создать отчёт, на который можно ссылаться в патче.

Так что я заполнил форму [9] — и возник баг #75237 [1].

Написание патча

Теперь самое трудное. Как начать патчить эту ошибку?

Обновление php-src и перекомпилирование

Сначала я получил все предыдущие изменения из основного Git-репозитория, чтобы быть уверенным, что я работаю с наиболее свежей версией php-src. Я удалённо вызвал команду upstream, указывающую на основной репозиторий php-src на GitHub [10].

$ git fetch upstream
$ git checkout master
$ git rebase upstream/master master

Затем создал новую ветку master для патча и назвал её в честь ID бага.

$ git checkout -b bug75237

Теперь нужно перекомпилироваться из исходного кода. Не буду вдаваться в подробности, у меня есть другая статья, где подробно описано, как компилировать PHP из исходного кода [11].

Когда я перекомпилирую PHP, я запускаю bash-скрипт под названием config, который я храню вне основной папки php-src. Он удаляет все существующие скомпилированные бинарники и прочие не находящиеся под версионным контролем файлы, собирает конфигурационный скрипт и запускает конфигурирование с нужными мне флагами, включёнными по умолчанию.

Вот мой скрипт, если захотите сделать для себя что-то подобное:

#!/bin/sh

make distclean
./vcsclean
./buildconf
./configure --enable-maintainer-zts 
        --enable-debug 
        --enable-cli 
        --with-zlib 
        --enable-mbstring 
        --with-openssl=/usr/local/opt/openssl 
        --with-libxml-dir=/usr/local/opt/libxml2 
        --with-sodium 
        "$@"

Почему здесь такие странные пути? Я работаю на Mac с некоторыми зависимостями, установленными с Homebrew [12], поэтому кое-где ради некоторых расширений пути ведут в /usr/local/opt.

Так что я просто запустил свой конфигурационный скрипт, а затем скомпилировал всё с помощью make.

$ ../config && make -j9

По адресу sapi/cli/php создался бинарник, после чего я проверил версию PHP и запустил приводящий к segfault код, чтобы удостовериться, что мастер-ветка php-src still всё ещё содержит баг.

$ sapi/cli/php --version
$ sapi/cli/php json_encode.php

Да! Опять segfault. Теперь воспользуемся отладчиком, чтобы пройти по С-коду, который строка за строкой исполняет наш PHP-скрипт.

Выполнение GDB

Для отладки кода я использовал GDB [13]. Он спроектирован для компилируемых языков вроде С и не работает с обычными PHP-файлами. Но PHP написан на С, так что с помощью gdb --args можно заставить GDB выполнить скомпилированный бинарник с PHP-скриптом.

$ gdb --args sapi/cli/php json_encode.php

Запустив GDB, я установил контрольную точку для паузы исполнения программы, а затем прошёл по каждой строке С-кода. О подробностях я рассказал в вышеприведённом видео, но если вкратце: оказалось, что всё дело в функции php_json_encode_serializable_object(), поэтому я установил здесь контрольную точку и запустил программу с помощью run.

> break php_json_encode_serializable_object
> run

Когда GDB доходит до контрольной точки и ставит исполнение кода на паузу, можно написать:

  • n и перепрыгнуть на следующую строку,
  • s и перейти в область видимости / фрейм следующей строки.
  • c и продолжить выполнение до следующей контрольной точки.

После ввода команд я понял, что код входит в бесконечную рекурсию между php_json_encode_zval() и php_json_encode_serializable_object(). Это приводит к переполнению стека [14] и segfault, потому что у программы кончается память.

Меня интересовало выражение if внутри php_json_encode_serializable_object().

if ((Z_TYPE(retval) == IS_OBJECT) &&
        (Z_OBJ(retval) == Z_OBJ_P(val))) {
        //
} else {
        //
}

Там обрабатывался случай, когда возвращаемым jsonSerialize() значением является тот же экземпляр его самого, но никто не ловил ситуацию, когда возвращался новый экземпляр.

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

if ((Z_TYPE(retval) == IS_OBJECT) &&
        // Make sure the objects are not the same instance
        (Z_OBJ(retval) != Z_OBJ_P(val)) &&
        // Check that the class names of the objects are the same
        zend_string_equals(ce->name, Z_OBJCE(retval)->name)) {
        // Throw an exception
        zend_throw_exception_ex(NULL, 0, "%s::jsonSerialize() cannot return a new instance of itself", ZSTR_VAL(ce->name));

        // Garbage collection
        zval_ptr_dtor(&retval);
        zval_ptr_dtor(&fname);
        PHP_JSON_HASH_APPLY_PROTECTION_DEC(myht);

        // Return fail
        return FAILURE;
} else if ((Z_TYPE(retval) == IS_OBJECT) &&
        (Z_OBJ(retval) == Z_OBJ_P(val))) {
        //
} else {
        //
}

Я сделал много проб и ошибок, чтобы прийти к этому варианту, и мне очень помог сайт phpinternalsbook.com [15], особенно при работе со zval [16].

Обновление: Сара Голмон [17], долгое время являющаяся ключевым разработчиком PHP и одним из релиз-менеджеров PHP 7.2, дала замечательный комментарий к моему патчу [18]: «Если пытаешься просто сопоставить класс, то можешь сравнить только ce. Так будет корректнее, чем сравнивать имена, и (номинально) более эффективно».

Благодаря предложению Сары мы можем изменить строку с zend_string_equals() в выражении if:

if ((Z_TYPE(retval) == IS_OBJECT) &&
        (Z_OBJ(retval) != Z_OBJ_P(val)) &&
        // This line changes
        ce == Z_OBJCE(retval)) {
        // Same as above
} else if ((Z_TYPE(retval) == IS_OBJECT) &&
        (Z_OBJ(retval) == Z_OBJ_P(val))) {
        //
} else {
        //
}

Тестирование

Прежде чем применить патч, нужно написать тест, подтверждающий, что ошибка действительно исправлена.

Опять же без подробностей, но если вам нужно больше информации, то я создал руководство из шести частей по написанию тестов для исходного кода PHP [19].

$ vi ext/json/tests/bug75237.phpt
--TEST--
Bug #75237 (jsonSerialize() - Returning new instance of self causes segfault)
--SKIPIF--
<?php if (!extension_loaded("json")) die("skip ext/json required"); ?>
--FILE--
<?php
class Foo implements JsonSerializable {
  public function jsonSerialize() {
    return new self;
  }
}

try {
  var_dump(json_encode(new Foo));
} catch (Exception $e) {
  echo $e->getMessage();
}
?>
--EXPECT--
Foo::jsonSerialize() cannot return a new instance of itself

Я запустил тест и проверил его прохождение, а затем прогнал все тесты в ext/json, чтобы удостовериться, что ничего не сломал.

$ make test TESTS=ext/json/tests/bug75237.phpt
$ make test TESTS=ext/json/tests/

Все тесты были пройдены, так что я был готов к применению патча.

Отправка PR и обновление бага

Затем я залил патч в свой форк на GitHub...

$ git add ext/json/json_encoder.c ext/json/tests/bug75237.phpt
$ git commit -m "Fix bug 75237 - (jsonSerialize() - Returning new instance of self causes segfault)"
$ git push origin 

… и создал PR [20] в основном репозитории php-src.

Наконец, я вернулся к своему отчёту об ошибке [1] и обновил его ссылкой на только что созданный PR.

Ожидание отклика и внесение изменений

После отправки PR прошло немного времени, и я получил отклик [21] от нескольких сотрудников [22], работающих над PHP. Один из них помог понять, что я некорректно сравниваю имена классов [23] (я это исправил в видео и кусках кода в статье, но оставил неизменённым исходный PR, так что вы можете проанализировать первый вариант).

В конце концов Niki P закрыл PR [24], потому что мы выявили более глубокую, более общую проблему, связанную с PHP. У этого языка нет общей защиты от переполнения стека, а значит, segfault может быть создан во многих [25] других контекстах [26].

Вот как об этом высказалась Сара:

Удачи!

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

Надеюсь, вам понравился рассказ о моём приключении с багом. Желаю удачи в поисках и исправлении собственных багов в исходном коде PHP!

Автор: AloneCoder

Источник [29]


Сайт-источник PVSM.RU: https://www.pvsm.ru

Путь до страницы источника: https://www.pvsm.ru/open-source/265924

Ссылки в тексте:

[1] бага #75237: https://bugs.php.net/bug.php?id=75237

[2] Chicago PHP UG: https://www.meetup.com/preview/Chicago-PHP-User-Group/events/242500371

[3] PHP TestFest 2017: https://phptestfest.org/

[4] gcov для: http://gcov.php.net/PHP_7_2/lcov_html/ext/json/json_encoder.c.gcov.php

[5] эту строку: https://github.com/php/php-src/blob/cb9d81ef4f07f82835273800b0cb3d6a67816050/ext/json/json_encoder.c#L462

[6] segfault: https://en.wikipedia.org/wiki/Segmentation_fault

[7] сниппет 3v4l: https://3v4l.org/tLMv6

[8] bugs.php.net: https://bugs.php.net/

[9] заполнил форму: https://bugs.php.net/report.php

[10] репозиторий php-src на GitHub: https://github.com/php/php-src

[11] как компилировать PHP из исходного кода: https://www.sammyk.me/compiling-php-from-source-writing-tests-for-php-source

[12] Homebrew: https://brew.sh/

[13] GDB: https://www.gnu.org/software/gdb/

[14] переполнению стека: https://en.wikipedia.org/wiki/Stack_overflow

[15] phpinternalsbook.com: http://www.phpinternalsbook.com/

[16] zval: http://www.phpinternalsbook.com/php7/internal_types/zvals/basic_structure.html

[17] Сара Голмон: https://twitter.com/SaraMG

[18] замечательный комментарий к моему патчу: https://github.com/php/php-src/pull/2763#discussion_r143798219

[19] руководство из шести частей по написанию тестов для исходного кода PHP: https://www.sammyk.me/compiling-php-from-source-writing-tests-for-php-source#all-posts-in-this-series

[20] создал PR: https://github.com/php/php-src/pull/2763

[21] отклик: https://github.com/php/php-src/pull/2763#issuecomment-331129755

[22] нескольких сотрудников: https://github.com/php/php-src/pull/2763#issuecomment-331735935

[23] я некорректно сравниваю имена классов: https://github.com/php/php-src/pull/2763/files#r140304003

[24] Niki P закрыл PR: https://github.com/php/php-src/pull/2763#issuecomment-334430022

[25] многих: https://bugs.php.net/bug.php?id=64196

[26] контекстах: https://bugs.php.net/bug.php?id=74977

[27] pic.twitter.com/RdvutMbH58: https://t.co/RdvutMbH58

[28] September 15, 2017: https://twitter.com/SaraMG/status/908742735754092544?ref_src=twsrc%5Etfw

[29] Источник: https://habrahabr.ru/post/340242/