#1
|
|||
|
|||
Арифметика указателей UB в throw_realloc_debug()
Nil A написал(а) к Vitaliy Aksyonov в Oct 23 21:35:16 по местному времени:
Нello, Vitaliy! Во-первых, зачем по-умолчанию включены отладки GTНROWLOG и GTНROWDEBUG? Я иногда видел какие-то трейсы про память пишутся в golded.log, но реально, хоть что-то когда-нибудь присылал сюда в эху, или куда-то, какие-то отладочные трейсы памяти? Отладка GTНROW_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 throwreallocdebug(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/x8664-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() превращается. Вобщем-то бага то и нет, но всё равно, я бы почиНИЛ throwrealloc_debug(), пододвинул throwptrtodl() туда, где он реально используется. Т.е. вот такой фикс бы влил, но.. у меня нет на гитхабе аккаунта. --- a/goldlib/gall/gmemdbg.cpp +++ b/goldlib/gall/gmemdbg.cpp @@ -386,7 +386,6 @@ void throwrealloc_debug(void* __oldptr, size_t __size, const char _file, int { void* _ptr; - Throw* dl = throwptrtodl(_oldptr); if(size == 0) { @@ -399,6 +398,7 @@ void throwrealloc_debug(void* __oldptr, size_t __size, const char _file, int } else { + Throw* dl = throwptrtodl(_oldptr); ptr = throw_malloc_debug(__size,__file,_line); if(dl->nbytes < size) size = dl->nbytes; Best Regards, Nil --- GoldED+/LNX 1.1.5 |
#2
|
|||
|
|||
Re: Арифметика указателей UB в throw_realloc_debug()
Vitaliy Aksyonov написал(а) к Nil A в Oct 23 22:59:40 по местному времени:
Привет, Nil! 13 Oct 23 21:35, ты писал(а) мне: NA> Во-первых, зачем по-умолчанию включены отладки GTНROW_LOG и NA> GTНROW_DEBUG? Я иногда видел какие-то трейсы про память пишутся в NA> golded.log, но реально, хоть что-то когда-нибудь присылал сюда в эху, NA> или куда-то, какие-то отладочные трейсы памяти? Подозреваю, это сделали, чтобы было проще собирать отчеты об ошибках. Коряво, конечно. Для релизной сборки это все должно быть отключено. Оно и на производительность влияет не самым благоприятным образом. Я посмотрю, как это удобно сделать отключаемым для релизной сборки. Основная проблема, что там целая куча систем сборки, начиная от cmake, заканчивая пачкой makefiles для разных систем. NA> Отладка GTНROW_DEBUG в голдеде сделана таким образом, что они NA> аллоцируют памяти больше на структурку Throw, и прячут её в начале. NA> Перед адресом и после обкладывают магическими цифирками BEFOREVAL и NA> AFTERVAL, чтобы потом на недоезды и заезды проверить. Также они NA> двухсвязный список хранят, чтобы потом потерянную память напечатать. А NA> потом пришёл Гугл и запили ASAN билды, и такое делать в ручную уже NA> стало не нужно, так что выключить по-умолчанию я бы рекомендовал. Мало того, realloc совсем не обязательно должен выделять память в новом месте. В случае с уменьшением куска, это вполне может быть тот же кусок. Другое дело, что в таком виде это вряд ли используется. NA> Решилось мне прогнать UBSAN билд, и прямо на запуске, где происходит NA> сканирование всех областей (баз эх), случается UB [...skipped...] NA> Вобщем-то бага то и нет, но всё равно, я бы почиНИЛ NA> throwrealloc_debug(), пододвинул throwptrtodl() туда, где он реально NA> используется. Т.е. вот такой фикс бы влил, но.. у меня нет на гитхабе NA> аккаунта. Создал Pull Request с твоей правкой. Ожидайте на линии, Ваш звонок очень важен для нас. :)) Best regards, Vitaliy Aksyonov. ... Были бы мозги, было б сотрясение. --- GoldED+/LNX 1.1.5-b20231007 |
#3
|
|||
|
|||
Re: Арифметика указателей UB в throw_realloc_debug()
Semen Panevin написал(а) к Vitaliy Aksyonov в Oct 23 08:27:46 по местному времени:
Доброго здоровьица тебе, Vitaliy! Monday October 16 2023 22:59, Vitaliy Aksyonov писал Nil A: NA>> Во-первых, зачем по-умолчанию включены отладки GTНROW_LOG и NA>> GTНROW_DEBUG? Я иногда видел какие-то трейсы про память пишутся в NA>> golded.log, но реально, хоть что-то когда-нибудь присылал сюда в NA>> эху, или куда-то, какие-то отладочные трейсы памяти? VA> Подозреваю, это сделали, чтобы было проще собирать отчеты об ошибках. VA> Коряво, конечно. Для релизной сборки это все должно быть отключено. VA> Оно и на производительность влияет не самым благоприятным образом. VA> Я посмотрю, как это удобно сделать отключаемым для релизной сборки. VA> Основная проблема, что там целая куча систем сборки, начиная от cmake, VA> заканчивая пачкой makefiles для разных систем. Ну там же есть глобальный Makefile в корне. Где всякие PLATFORM=xxx, ICONV=1 и прочие OLDSНIFTFN=1. Напрашивается добавить туда же DEBUG=1 или чё-нить типа того, ИМХО. С наилучшими пожеланиями, Семён. ... От правды далеко не убежишь (с) Sage --- GoldED+/LNX 1.1.5-b20231008 (Linux 6.1.53-gentoo-r1 iF6M10) |
#4
|
|||
|
|||
Re: Арифметика указателей UB в throw_realloc_debug()
Vitaliy Aksyonov написал(а) к Semen Panevin в Oct 23 23:47:46 по местному времени:
Привет, Semen! 17 Oct 23 08:27, ты писал(а) мне: NA>>> Во-первых, зачем по-умолчанию включены отладки GTНROW_LOG и NA>>> GTНROW_DEBUG? Я иногда видел какие-то трейсы про память пишутся NA>>> в golded.log, но реально, хоть что-то когда-нибудь присылал сюда NA>>> в эху, или куда-то, какие-то отладочные трейсы памяти? VA>> Подозреваю, это сделали, чтобы было проще собирать отчеты об VA>> ошибках. Коряво, конечно. Для релизной сборки это все должно быть VA>> отключено. Оно и на производительность влияет не самым VA>> благоприятным образом. VA>> Я посмотрю, как это удобно сделать отключаемым для релизной VA>> сборки. Основная проблема, что там целая куча систем сборки, VA>> начиная от cmake, заканчивая пачкой makefiles для разных систем. SP> Ну там же есть глобальный Makefile в корне. Где всякие PLATFORM=xxx, SP> ICONV=1 и прочие OLDSНIFTFN=1. Напрашивается добавить туда же SP> DEBUG=1 или чё-нить типа того, ИМХО. Не все так просто. Там уже есть и DEBUG, и NDEBUG. Надо внимательно смотреть, чтобы не отломать что-то. Best regards, Vitaliy Aksyonov. ... Скажи мне с кем ты пьёшь и я скажу кто ты. --- GoldED+/LNX 1.1.5-b20231007 |
#5
|
|||
|
|||
Арифметика указателей UB в throw_realloc_debug()
Nil A написал(а) к Vitaliy Aksyonov в Oct 23 08:53:10 по местному времени:
Нello, Vitaliy! Monday October 16 2023 22:59, from Vitaliy Aksyonov -> Nil A: NA>> Во-первых, зачем по-умолчанию включены отладки GTНROW_LOG и NA>> GTНROW_DEBUG? Я иногда видел какие-то трейсы про память пишутся в NA>> golded.log, но реально, хоть что-то когда-нибудь присылал сюда в NA>> эху, или куда-то, какие-то отладочные трейсы памяти? VA> Подозреваю, это сделали, чтобы было проще собирать отчеты об ошибках. VA> Коряво, конечно. Для релизной сборки это все должно быть отключено. VA> Оно и на производительность влияет не самым благоприятным образом. VA> Мало того, realloc совсем не обязательно должен выделять память в VA> новом месте. В случае с уменьшением куска, это вполне может быть тот VA> же кусок. Другое дело, что в таком виде это вряд ли используется. Этот GTНROW_DEBUG ловит только аллокации в куче и какие-то минимальные заезды, тогда как тулы, в лице ASAN билдов, и valgrind, ловят и заезды на стеке тоже, коих в этот эхотаге $(grep strcpy) и маленькая тележка ;-) Best Regards, Nil --- GoldED+/LNX 1.1.5 |
#6
|
|||
|
|||
Re: Арифметика указателей UB в throw_realloc_debug()
Vitaliy Aksyonov написал(а) к Nil A в Oct 23 09:26:16 по местному времени:
Привет, Nil! 17 Oct 23 08:53, ты писал(а) мне: NA>>> Во-первых, зачем по-умолчанию включены отладки GTНROW_LOG и NA>>> GTНROW_DEBUG? Я иногда видел какие-то трейсы про память пишутся NA>>> в golded.log, но реально, хоть что-то когда-нибудь присылал сюда NA>>> в эху, или куда-то, какие-то отладочные трейсы памяти? VA>> Подозреваю, это сделали, чтобы было проще собирать отчеты об VA>> ошибках. Коряво, конечно. Для релизной сборки это все должно быть VA>> отключено. Оно и на производительность влияет не самым VA>> благоприятным образом. VA>> Мало того, realloc совсем не обязательно должен выделять память в VA>> новом месте. В случае с уменьшением куска, это вполне может быть VA>> тот же кусок. Другое дело, что в таком виде это вряд ли VA>> используется. NA> Этот GTНROW_DEBUG ловит только аллокации в куче и какие-то минимальные NA> заезды, тогда как тулы, в лице ASAN билдов, и valgrind, ловят и заезды NA> на стеке тоже, коих в этот эхотаге $(grep strcpy) и маленькая тележка NA> ;-) Я это прекрасно понимаю. :) Плюс текущего подхода, что не надо распространять сборку с санитайзером и все равно есть шанс поймать подобные ошибки у пользователя. Часто бывает, что ошибка воспроизводится только при крайне специфичных условиях. А вообще, конечно, надо этот легаси выпиливать. Best regards, Vitaliy Aksyonov. ... Федерация Исключительного Дружеского Общения. --- GoldED+/LNX 1.1.5-b20231007 |