Bug 21061

Created attachment 3739 [details]
Use memcpy() instead of strcpy()

Хм, посмотрел на код. Почему-то не помогла замена на strncpy(). Не очень понял всех тонкостей кода и как это работает. Но увидел как конкатенируются строки в ф-ции strPlus() и сделал по аналогии -- использовал memcpy(). После этого программа падать не стала.

Кстати, если собрать программу более старым компилятором -- gcc4.4-4.4.0-alt3, то она не падает, а вот уже с 4.4.0-alt6 начинает падать.

После https://bugzilla.altlinux.org/show_bug.cgi?id=20842 мне этот вариант не очень по душе. То что замена на strncpy не помогла, очень подозрительно.

М-да. Ну всё понятно. И почему strcpy обламывается, и strncpy не помогает. Досточтимый апстримовый изобретатель велосипедов не осилил написать корректную реализацию строк в стиле языка Паскаль. Даже не знаю за что хвататься, за пистолет или за текстовый редактор. Могу лишь сказать, что приведённый патч не работает. Почему - см. файл value.h, определение макроса StringChars.

(В ответ на комментарий №3)
> Почему - см. файл value.h, определение макроса StringChars.

Ты про s+1? Я так и не понял зачем это.

Кстати, автору я отписал и свой (пусть и неправильный патч) приложил. Посмотрим, что он ответит и как будет это исправлять.

(В ответ на комментарий №4)
> (В ответ на комментарий №3)
> > Почему - см. файл value.h, определение макроса StringChars.
> 
> Ты про s+1? Я так и не понял зачем это.
Я ещё сам до конца не разобрался, что, собственно, происходит, но мне уже не хочется продолжать.
s+1 - это такая чёрная магия. Товарищ хранит сначала структуру-дескриптор строки (тип под названием String), а ПРЯМО следом за ней - байтики, из которых состоит строка (см. реализацию NewString() ). Поскольку в StringChars передаётся String*, то s+1 указывает на позицию ЗА дескриптором. Но вообще говоря, s+1 вовсе необязательно будет указывать на непосредственно следующий за дескрипотором _байт_. А памяти между тем выделяется впритык. Код, который валится, всего-то навсего пытается разместить в такой структуре пустую строку. strcpy и strncpy (в силу своих реализация) пытаются записать нулевой байт, но передаваемый им указатель лежит за пределами выделенной области памяти, и поэтому проверка границ благополучно сваливается. А memcpy никакого нулевого байта не пишет, и в данном конкретном коде просто копирует ровно 0 байт, с неизменно превосходным результатом. Зато когда memcpy попытается скопировать больше 0 байт, произойдёт то же самое.

> Кстати, автору я отписал и свой (пусть и неправильный патч) приложил.
> Посмотрим, что он ответит и как будет это исправлять.

(В ответ на комментарий №5)
> (В ответ на комментарий №4)
> > (В ответ на комментарий №3)
> > > Почему - см. файл value.h, определение макроса StringChars.
> > 
> > Ты про s+1? Я так и не понял зачем это.
> Я ещё сам до конца не разобрался, что, собственно, происходит, но мне уже не
> хочется продолжать.

Ты мог бы объяснить автору на нормальном английском в чем тут проблема? Потому что:

>> May be you can apply my patch or made better fix for this?

>I don't understand why this works though -- strcpy should write
>precisely the same sequence of bytes as memcpy, along with the trailing
>'\0' character into a buffer which is allocated to be the length of the
>string plus one bytes long.

>Sounds like there's some kind of compiler bug we've uncovered here, and
>I'm afraid it will affect more than just nickle.

Это цитата Keith Packard, автора nickle.

> Зато когда memcpy попытается скопировать больше 0 байт, произойдёт то же самое.

У меня не происходило. Наверное, повезло?

(В ответ на комментарий №6)
> Ты мог бы объяснить автору на нормальном английском в чем тут проблема?
Для начала неплохо бы на самом деле разобраться, в чём проблема. Пока я вижу проблему в голове автора, но вряд ли он это признает :)

> > Зато когда memcpy попытается скопировать больше 0 байт, произойдёт то же самое.
> 
> У меня не происходило. Наверное, повезло?
Не знаю. Может, и я где-то ошибся с анализом. Отлаживать надо. Но я не мейнтейнер этого пакета и мне лень с ним возиться.

Лёш, с удовольствием тебе его отдам :)

??? Он мне на фиг не сдался. Лучше вон php-coder'у, он даже патчи к нему пишет :)

Тогда проще сразу в orphaned отправить...  (/me runs!!!)

(В ответ на комментарий №9)
> ??? Он мне на фиг не сдался. Лучше вон php-coder'у, он даже патчи к нему пишет
> :)

Мне эта программа интересна только из-за того, что она падает. Когда это будет исправлено она мне станет не интересна. (Впрочем, если мне выдадут права на NMU, то я самолично исправлю это падение.)

добавил nmu на php-coder

(В ответ на комментарий №3)
> Могу лишь сказать, что приведённый патч не работает.

Алексей, а ты можешь попробовать пересобрать nickle с последней версией gcc? (4.4.1-alt1) После пересборки с ним у меня не падает, как раньше, и даже патчи не понадобились. (Похоже, что это был действительно баг компилятора...)

nickle-2.68-alt1.1 -> sisyphus:

* Sun Aug 23 2009 Slava Semushin <php-coder@altlinux> 2.68-alt1.1

- NMU
- Rebuild with gcc 4.4.1-alt1 to fix crash after start (Closes: #21061)