Арифметика указателей UB в throw_realloc_debug()

[Nil A]

Hello, Vitaliy!

Во-первых, зачем по-умолчанию включены отладки GTHROW_LOG и GTHROW_DEBUG? Я
иногда видел какие-то трейсы про память пишутся в golded.log, но реально, хоть
что-то когда-нибудь присылал сюда в эху, или куда-то, какие-то отладочные
трейсы памяти?

Отладка GTHROW_DEBUG в голдеде сделана таким образом, что они аллоцируют памяти
больше на структурку Throw, и прячут её в начале. Перед адресом и после
обкладывают магическими цифирками BEFOREVAL и AFTERVAL, чтобы потом на недоезды
и заезды проверить. Также они двухсвязный список хранят, чтобы потом потерянную
память напечатать. А потом пришёл Гугл и запили ASAN билды, и такое делать в
ручную уже стало не нужно, так что выключить по-умолчанию я бы рекомендовал.


Решилось мне прогнать UBSAN билд, и прямо на запуске, где происходит
сканирование всех областей (баз эх), случается UB

/home/fido/src/golded-plus/goldlib/gall/gmemdbg.cpp:130:53: runtime error:
pointer index expression with base 0x000000000000 overflowed to
0xffffffffffffffd4
#0 0xae8fbc in throw_ptrtodl(void const*)
/home/fido/src/golded-plus/goldlib/gall/gmemdbg.cpp:130
#1 0xae6a1c in throw_realloc_debug(void*, unsigned long, char const*, int)
/home/fido/src/golded-plus/goldlib/gall/gmemdbg.cpp:389
#2 0x94880f in SquishArea::refresh()
/home/fido/src/golded-plus/goldlib/gmb3/gmosqsh2.cpp:71
#3 0x94b7f1 in SquishArea::raw_scan(int, int)
/home/fido/src/golded-plus/goldlib/gmb3/gmosqsh2.cpp:153
#4 0x94fd9b in SquishArea::scan_area()
/home/fido/src/golded-plus/goldlib/gmb3/gmosqsh2.cpp:333
#5 0x7ebee5 in Area::ScanArea()
/home/fido/src/golded-plus/golded3/gescan.cpp:71
#6 0x7efc77 in AreaList::AreaScan(int, unsigned int, int, int&, int&, char
const*) /home/fido/src/golded-plus/golded3/gescan.cpp:281
#7 0x7b236f in Reader() /home/fido/src/golded-plus/golded3/geread.cpp:171
#8 0x6c63bb in main /home/fido/src/golded-plus/golded3/gemain.cpp:54
#9 0x7f7f53f1ef44 in __libc_start_main
(/lib/x86_64-linux-gnu/libc.so.6+0x21f44)
#10 0x407c98
(/home/fido/src/golded-plus/build_asan/golded3/golded+0x407c98)

Тут хотят вычитать целиком .SQI индекс файл в память. Зовут функцию
throw_realloc(), где оригинальный указатель в NULL. Так то норм, звать realloc
с nullptr, тогда он просто в malloc() превращается.

Вобщем-то бага то и нет, но всё равно, я бы почиНИЛ throw_realloc_debug(),
пододвинул throw_ptrtodl() туда, где он реально используется. Т.е. вот такой
фикс бы влил, но.. у меня нет на гитхабе аккаунта.

--- a/goldlib/gall/gmemdbg.cpp
+++ b/goldlib/gall/gmemdbg.cpp
@@ -386,7 +386,6 @@ void* throw_realloc_debug(void* __oldptr, size_t __size,
const char* __file, int
{

void* _ptr;
- Throw* dl = throw_ptrtodl(__oldptr);

if(__size == 0)
{
@@ -399,6 +398,7 @@ void* throw_realloc_debug(void* __oldptr, size_t __size,
const char* __file, int
}
else
{
+ Throw* dl = throw_ptrtodl(__oldptr);
_ptr = throw_malloc_debug(__size,__file,__line);
if(dl->nbytes < __size)
__size = dl->nbytes;


Best Regards, Nil

[Vitaliy Aksyonov]

Привет, Nil!

13 Oct 23 21:35, ты писал(а) мне:

NA> Во-первых, зачем по-умолчанию включены отладки GTHROW_LOG и
NA> GTHROW_DEBUG? Я иногда видел какие-то трейсы про память пишутся в
NA> golded.log, но реально, хоть что-то когда-нибудь присылал сюда в эху,
NA> или куда-то, какие-то отладочные трейсы памяти?

Подозреваю, это сделали, чтобы было проще собирать отчеты об ошибках. Коряво,
конечно. Для релизной сборки это все должно быть отключено. Оно и на
производительность влияет не самым благоприятным образом.

Я посмотрю, как это удобно сделать отключаемым для релизной сборки. Основная
проблема, что там целая куча систем сборки, начиная от cmake, заканчивая пачкой
makefiles для разных систем.

NA> Отладка GTHROW_DEBUG в голдеде сделана таким образом, что они
NA> аллоцируют памяти больше на структурку Throw, и прячут её в начале.
NA> Перед адресом и после обкладывают магическими цифирками BEFOREVAL и
NA> AFTERVAL, чтобы потом на недоезды и заезды проверить. Также они
NA> двухсвязный список хранят, чтобы потом потерянную память напечатать. А
NA> потом пришёл Гугл и запили ASAN билды, и такое делать в ручную уже
NA> стало не нужно, так что выключить по-умолчанию я бы рекомендовал.

Мало того, realloc совсем не обязательно должен выделять память в новом месте.
В случае с уменьшением куска, это вполне может быть тот же кусок. Другое дело,
что в таком виде это вряд ли используется.

NA> Решилось мне прогнать UBSAN билд, и прямо на запуске, где происходит
NA> сканирование всех областей (баз эх), случается UB

[...skipped...]

NA> Вобщем-то бага то и нет, но всё равно, я бы почиНИЛ
NA> throw_realloc_debug(), пододвинул throw_ptrtodl() туда, где он реально
NA> используется. Т.е. вот такой фикс бы влил, но.. у меня нет на гитхабе
NA> аккаунта.

Создал Pull Request с твоей правкой. Ожидайте на линии, Ваш звонок очень важен
для нас. :))

Best regards,
Vitaliy Aksyonov.

... Были бы мозги, было б сотрясение.

[Semen Panevin]

Доброго здоровьица тебе, Vitaliy!

Monday October 16 2023 22:59, Vitaliy Aksyonov писал Nil A:

NA>> Во-первых, зачем по-умолчанию включены отладки GTHROW_LOG и
NA>> GTHROW_DEBUG? Я иногда видел какие-то трейсы про память пишутся в
NA>> golded.log, но реально, хоть что-то когда-нибудь присылал сюда в
NA>> эху, или куда-то, какие-то отладочные трейсы памяти?

VA> Подозреваю, это сделали, чтобы было проще собирать отчеты об ошибках.
VA> Коряво, конечно. Для релизной сборки это все должно быть отключено.
VA> Оно и на производительность влияет не самым благоприятным образом.

VA> Я посмотрю, как это удобно сделать отключаемым для релизной сборки.
VA> Основная проблема, что там целая куча систем сборки, начиная от cmake,
VA> заканчивая пачкой makefiles для разных систем.
Ну там же есть глобальный Makefile в корне. Где всякие PLATFORM=xxx, ICONV=1 и
прочие OLD_SHIFT_FN=1. Напрашивается добавить туда же DEBUG=1 или чё-нить типа
того, ИМХО.

С наилучшими пожеланиями, Семён.

... От правды далеко не убежишь (с) Sage

[Vitaliy Aksyonov]

Привет, Semen!

17 Oct 23 08:27, ты писал(а) мне:

NA>>> Во-первых, зачем по-умолчанию включены отладки GTHROW_LOG и
NA>>> GTHROW_DEBUG? Я иногда видел какие-то трейсы про память пишутся
NA>>> в golded.log, но реально, хоть что-то когда-нибудь присылал сюда
NA>>> в эху, или куда-то, какие-то отладочные трейсы памяти?

VA>> Подозреваю, это сделали, чтобы было проще собирать отчеты об
VA>> ошибках. Коряво, конечно. Для релизной сборки это все должно быть
VA>> отключено. Оно и на производительность влияет не самым
VA>> благоприятным образом.

VA>> Я посмотрю, как это удобно сделать отключаемым для релизной
VA>> сборки. Основная проблема, что там целая куча систем сборки,
VA>> начиная от cmake, заканчивая пачкой makefiles для разных систем.
SP> Ну там же есть глобальный Makefile в корне. Где всякие PLATFORM=xxx,
SP> ICONV=1 и прочие OLD_SHIFT_FN=1. Напрашивается добавить туда же
SP> DEBUG=1 или чё-нить типа того, ИМХО.

Не все так просто. Там уже есть и DEBUG, и NDEBUG. Надо внимательно смотреть,
чтобы не отломать что-то.

Best regards,
Vitaliy Aksyonov.

... Скажи мне с кем ты пьёшь и я скажу кто ты.

[Nil A]

Hello, Vitaliy!

Monday October 16 2023 22:59, from Vitaliy Aksyonov -> Nil A:

NA>> Во-первых, зачем по-умолчанию включены отладки GTHROW_LOG и
NA>> GTHROW_DEBUG? Я иногда видел какие-то трейсы про память пишутся в
NA>> golded.log, но реально, хоть что-то когда-нибудь присылал сюда в
NA>> эху, или куда-то, какие-то отладочные трейсы памяти?

VA> Подозреваю, это сделали, чтобы было проще собирать отчеты об ошибках.
VA> Коряво, конечно. Для релизной сборки это все должно быть отключено.
VA> Оно и на производительность влияет не самым благоприятным образом.

VA> Мало того, realloc совсем не обязательно должен выделять память в
VA> новом месте. В случае с уменьшением куска, это вполне может быть тот
VA> же кусок. Другое дело, что в таком виде это вряд ли используется.

Этот GTHROW_DEBUG ловит только аллокации в куче и какие-то минимальные заезды,
тогда как тулы, в лице ASAN билдов, и valgrind, ловят и заезды на стеке тоже,
коих в этот эхотаге $(grep strcpy) и маленькая тележка ;-)

Best Regards, Nil

[Vitaliy Aksyonov]

Привет, Nil!

17 Oct 23 08:53, ты писал(а) мне:

NA>>> Во-первых, зачем по-умолчанию включены отладки GTHROW_LOG и
NA>>> GTHROW_DEBUG? Я иногда видел какие-то трейсы про память пишутся
NA>>> в golded.log, но реально, хоть что-то когда-нибудь присылал сюда
NA>>> в эху, или куда-то, какие-то отладочные трейсы памяти?
VA>> Подозреваю, это сделали, чтобы было проще собирать отчеты об
VA>> ошибках. Коряво, конечно. Для релизной сборки это все должно быть
VA>> отключено. Оно и на производительность влияет не самым
VA>> благоприятным образом.

VA>> Мало того, realloc совсем не обязательно должен выделять память в
VA>> новом месте. В случае с уменьшением куска, это вполне может быть
VA>> тот же кусок. Другое дело, что в таком виде это вряд ли
VA>> используется.

NA> Этот GTHROW_DEBUG ловит только аллокации в куче и какие-то минимальные
NA> заезды, тогда как тулы, в лице ASAN билдов, и valgrind, ловят и заезды
NA> на стеке тоже, коих в этот эхотаге $(grep strcpy) и маленькая тележка
NA> ;-)

Я это прекрасно понимаю. :) Плюс текущего подхода, что не надо распространять
сборку с санитайзером и все равно есть шанс поймать подобные ошибки у
пользователя. Часто бывает, что ошибка воспроизводится только при крайне
специфичных условиях. А вообще, конечно, надо этот легаси выпиливать.

Best regards,
Vitaliy Aksyonov.

... Федерация Исключительного Дружеского Общения.