Предлагаем вашему вниманию цикл статей, посвященных рекомендациям по написанию качественного кода на примере ошибок, найденных в проекте Chromium. Это первая часть, которая будет посвящена функции memset.Господа программисты, с функцией memset надо что-то делать в C++ программах! Вернее, даже сразу понятно что делать — её надо прекратить использовать. В своё время я написал статью "Самая опасная функция в мире С/С++". Я думаю, несложно догадаться, что речь в статье идёт как раз о memset.
Однако, не буду голословным и ещё раз на примерах продемонстрирую опасность этой функции. Код проекта Chromium и используемых в нём библиотек очень качественный. Разработчики Google уделяют много внимания тестам и использованию различного инструментария для выявления дефектов. Например, Google разработал такие инструменты, как AddressSanitizer, ThreadSanitizer и MemorySanitizer.
В результате, ошибок, связанных с функций memset, очень мало, но печально, что они всё-таки есть. Очень качественный проект, но всё равно они есть!
Давайте посмотрим, что я заметил в процессе разбора отчета, выданного PVS-Studio. Как я писал в вводной статье, отчёт я смотрел достаточно бегло, поэтому могут быть и другие, незамеченные мной ошибки. Тем не менее, найденных дефектов нам будет достаточно для обсуждения функции malloc.
Неправильно вычисленный размер буфера
Первый тип ошибок связан с неправильным вычислением размера буфера. Или, другими словами, проблема в том, что возникает путаница между размером массива в байтах и количеством элементов в массиве. Подобные ошибки можно классифицировать как CWE-682: Incorrect Calculation.
Первый пример ошибки взят непосредственно из кода проекта Chromium. Обратите внимание, что массивы text и unmodified_text состоят из юникодных символов.
#if defined(WIN32)
typedef wchar_t WebUChar;
#else
typedef unsigned short WebUChar;
#endif
static const size_t kTextLengthCap = 4;
class WebKeyboardEvent : public WebInputEvent {
....
WebUChar text[kTextLengthCap];
WebUChar unmodified_text[kTextLengthCap];
....
}; В результате, заполняется нулями только половина элементов в этих массивах:
WebKeyboardEvent* BuildCharEvent(const InputEventData& event)
{
WebKeyboardEvent* key_event = new WebKeyboardEvent(....);
....
memset(key_event->text, 0, text_length_cap);
memset(key_event->unmodified_text, 0, text_length_cap);
....
}Предупреждения PVS-Studio:
- V512 CWE-682 A call of the 'memset' function will lead to underflow of the buffer 'key_event->text'. event_conversion.cc 435
- V512 CWE-682 A call of the 'memset' function will lead to underflow of the buffer 'key_event->unmodified_text'. event_conversion.cc 436
Второй пример ошибки взят из используемой в Chromium библиотеки WebRTC. Ошибка аналогична предыдущей: не учтено, что элементы массива имеют тип int64_t.
class VCMRttFilter {
....
enum { kMaxDriftJumpCount = 5 };
....
int64_t _jumpBuf[kMaxDriftJumpCount];
int64_t _driftBuf[kMaxDriftJumpCount];
....
};
void VCMRttFilter::Reset() {
_gotNonZeroUpdate = false;
_avgRtt = 0;
_varRtt = 0;
_maxRtt = 0;
_filtFactCount = 1;
_jumpCount = 0;
_driftCount = 0;
memset(_jumpBuf, 0, kMaxDriftJumpCount);
memset(_driftBuf, 0, kMaxDriftJumpCount);
}Здесь вообще обнуляется только первый элемент массива и один байт во втором элементе.
Предупреждение PVS-Studio: V512 CWE-682 A call of the 'memset' function will lead to underflow of the buffer '_jumpBuf'. rtt_filter.cc 52
Рекомендация
Способ избежать таких ошибок — не использовать более memset. Можно быть очень аккуратным, но рано или поздно ошибки всё равно просочатся в ваш проект. Это в проекте Chromium картина хорошая. В других проектах — это очень распространенная проблема (proof).
Да, отказаться от memset в коде на языке C невозможно. Однако, если речь идёт о C++, то давайте забудем про эту функцию. Не используйте в C++ коде функцию memset. Не используйте и точка.
На что заменить вызов memset?
Во-первых, можно использовать функцию std::fill. В этом случае заполнение массива будет выглядеть так:
fill(begin(key_event->text), end(key_event->text), 0);Во-вторых, часто вообще не надо использовать вызов специальных функций. Как правило, функция memset служит для инициализации локальных массивов и структур. Классика:
HDHITTESTINFO hhti;
memset(&hhti, 0, sizeof(hhti));Но ведь можно написать гораздо проще и надёжнее:
HDHITTESTINFO hhti = {};Если речь идёт о конструкторе:
class C
{
int A[100];
public:
C() { memset(A, 0, sizeof(A)); }
};То можно написать:
class C
{
int A[100] = {};
public:
C() { }
};Неправильные ожидания от memset
Иногда забывают, что второй аргумент задаёт значение одного единственного байта, который используется для заполнения буфера. С толку сбивает то, что второй аргумент функции memset имеет тип int. В результате возникают ошибки, которые можно классифицировать как CWE-628: Function Call with Incorrectly Specified Arguments.
Рассмотрим пример подобной ошибки, которую я заметил в движке V8, используемом в проекте Chromium.
void i::V8::FatalProcessOutOfMemory(
const char* location, bool is_heap_oom)
{
....
char last_few_messages[Heap::kTraceRingBufferSize + 1];
char js_stacktrace[Heap::kStacktraceBufferSize + 1];
i::HeapStats heap_stats;
....
memset(last_few_messages, 0x0BADC0DE,
Heap::kTraceRingBufferSize + 1);
memset(js_stacktrace, 0x0BADC0DE,
Heap::kStacktraceBufferSize + 1);
memset(&heap_stats, 0xBADC0DE,
sizeof(heap_stats));
....
}Предупреждения PVS-Studio:
- V575 CWE-628 The 'memset' function processes value '195936478'. Inspect the second argument. api.cc 327
- V575 CWE-628 The 'memset' function processes value '195936478'. Inspect the second argument. api.cc 328
- V575 CWE-628 The 'memset' function processes value '195936478'. Inspect the second argument. api.cc 329
Программист решил заполнить блоки памяти значением 0x0BADC0DE, чтобы при отладке было легче понять что к чему. Однако, области памяти будут заполнены байтом со значением 0xDE.
То, что делает программист в коде, является низкоуровневой операцией и здесь без memset обойтись сложнее, чем в ситуациях, описанных ранее. Размер буферов не кратен 4 байтам, поэтому не получится как раньше использовать std::fill. Придётся написать и использовать собственную функцию.
void Fill_0x0BADC0DE(void *buf, const size_t size)
{
const unsigned char badcode[4] = { 0xDE, 0xC0, 0xAD, 0x0B };
size_t n = 0;
generate_n(static_cast<char *>(buf), size,
[&] { if (n == 4) n = 0; return badcode[n++]; });
}Рекомендация
Какой-то специальной рекомендации здесь нет. Зато мы вновь убедились, что функция memset на самом деле здесь не нужна, так как не решает поставленную перед программистом задачу.
Ошибка затирания приватных данных
Функцию memset используют для затирания приватных данных после того, как они перестают быть нужны. Это неправильно. Если буфер с приватными данными после вызова функции memset никак не используется, то компилятор вправе удалить вызов этой функции. Этот дефект классифицируется как CWE-14: Compiler Removal of Code to Clear Buffers.
Я уже предвижу возражения, что вызов memset компилятору нельзя убирать. Можно. И он это делает с целью оптимизации. Чтобы разобраться в теме, предлагаю внимательно изучить следующую статью "Безопасная очистка приватных данных".
Давайте посмотрим, как выглядят такие ошибки на практике. Начнём мы с библиотеки WebRTC, используемой в Chromium.
void AsyncSocksProxySocket::SendAuth() {
....
char * sensitive = new char[len];
pass_.CopyTo(sensitive, true);
request.WriteString(sensitive); // Password
memset(sensitive, 0, len);
delete [] sensitive;
DirectSend(request.Data(), request.Length());
state_ = SS_AUTH;
}Предупреждение PVS-Studio: V597 CWE-14 The compiler could delete the 'memset' function call, which is used to flush 'sensitive' object. The RtlSecureZeroMemory() function should be used to erase the private data. socketadapters.cc 677
Функция memset с вероятностью близкой к 100% будет удалена компилятором в Release версии.
Аяяй! Пароль останется болтаться где-то в памяти и, теоретически, может быть куда-то отправлен. Я серьезно, такое действительно бывает.
В этой же библиотеке я встретил ещё 3 подобных ошибки. Описывать я их не буду, так как они однотипны. Приведу только соответствующие сообщения анализатора:
- V597 CWE-14 The compiler could delete the 'memset' function call, which is used to flush 'sensitive' object. The RtlSecureZeroMemory() function should be used to erase the private data. httpcommon.cc 721
- V597 CWE-14 The compiler could delete the 'memset' function call, which is used to flush 'sensitive' object. The RtlSecureZeroMemory() function should be used to erase the private data. httpcommon.cc 766
- V597 CWE-14 The compiler could delete the 'memset' function call, which is used to flush 'sensitive' object. The RtlSecureZeroMemory() function should be used to erase the private data. httpcommon.cc 917
Рекомендация
Никогда не используйте функцию memset для затирания приватных данных!
Следует использовать специализированные функции очистки памяти, которые не могут быть удалены компилятором в процессе оптимизации кода.
Примечание. Это касается не только С++ программистов, но C программистов тоже.
В Visual Studio, например, можно использовать RtlSecureZeroMemory. Начиная с C11 существует функция memset_s. В случае необходимости вы можете создать свою собственную безопасную функцию. В интернете достаточно много примеров, как её сделать. Вот некоторые из вариантов.
Вариант N1.
errno_t memset_s(void *v, rsize_t smax, int c, rsize_t n) {
if (v == NULL) return EINVAL;
if (smax > RSIZE_MAX) return EINVAL;
if (n > smax) return EINVAL;
volatile unsigned char *p = v;
while (smax-- && n--) {
*p++ = c;
}
return 0;
}Вариант N2.
void secure_zero(void *s, size_t n)
{
volatile char *p = s;
while (n--) *p++ = 0;
}В случае проекта Chromium, наверное, рационально использовать функцию OPENSSL_cleanse.
Заключение
Если Вы пишите программу на C++ и захотели написать вызов функции memset, то остановитесь. Скорее всего, вы отлично обойдётесь без этой опасной функции.
Если хотите поделиться этой статьей с англоязычной аудиторией, то прошу использовать ссылку на перевод: Andrey Karpov. Nice Chromium and clumsy memset.
Комментарии (45)
datacompboy
26.01.2018 11:44«Малявата будет». Слишком мелко посты надроблены, никакого мяса.
Впрочем, добавить в этот пост можно было бы только проблемы с игнором malloc, с которым я уже могу поспорить. В последнее время развелось overcommitment для памяти, так что память выделяется, вот только пользовать её не получится когда она закончилась. То есть падение уже и так оттянуто до момента юза… :/ Так что проверять возврат маллока обычно бесполезное действие.
Andrey2008 Автор
26.01.2018 11:54Так что проверять возврат маллока обычно бесполезное действие.
О! У меня отдельная статья на эту тему есть. Буду громить такой подход в хвост и гриву. Есть 4 причины, почему это неправильно. Но придётся немого подождать, это 6-ая часть.datacompboy
26.01.2018 12:34Я знаю минусы этого подхода, да. Но вообще, в моём понимании переход с возврата ошибки на исключение вызван именно этим.
linuxover
26.01.2018 12:02Следует использовать специализированные функции очистки памяти, которые не могут быть удалены компилятором в процессе оптимизации кода.
а у memset аттрибут какой-то проставлен, что ее можно удалять? или напротив у "специализированных функций" надо проставить аттрибут что их нельзя удалять?
можете показать как правильно написать функцию чтобы компилятор ее вызов железно не удалил, там где удаляет memset? и наоборот?
Andrey2008 Автор
26.01.2018 12:08а у memset аттрибут какой-то проставлен, что ее можно удалять?
Нет. Это просто функция. Она меняет объекты, которые затем не используются. Значит её можно удалить. Так компилятор поступает и с другими функциями. memset ничем не особенная функция. Просто один из методов оптимизации.
можете показать как правильно написать функцию чтобы компилятор ее вызов железно не удалил, там где удаляет memset?
Надо использовать специальные функции. Про них говорится в статье. Или написать свою, используя volatile. Специальные атрибуты мне не известны. Возможно, кто-то что-то на эту тему напишет здесь.linuxover
26.01.2018 12:11Надо использовать специальные функции
так я спрашиваю: специальная функция от memset отличается чем? почему memset компилятор выбросит, а не memset (специальную функцию) нет?
void special_foo(void *buf, size_t len) { // bla }
чем
special_fooдолжна отmemsetотличаться?Andrey2008 Автор
26.01.2018 12:19специальная функция от memset отличается чем?
Тем, что она специальная. А как это достигается, прикладного программиста волновать не должно. Может использоваться какое-то непереносимое расширение компилятора.
Если говорить про самодельные функции, то в них всё вертится вокруг volatile. Раз что-то volatile, то это нельзя оптимизировать и выбрасывать. Но вообще самоделки — это плохо. Это на крайний случай.
linuxover
26.01.2018 12:15используя volatile
на чем? на параметрах? если параметр — массив, который удаляется, а
special_foo— функция, которая вызывается перед его удалением, то какvolatileпоможет проблеме?Andrey2008 Автор
26.01.2018 12:24volatile object — an object whose type is volatile-qualified, or a subobject of a volatile object, or a mutable subobject of a const-volatile object. Every access (read or write operation, member function call, etc.) made through a glvalue expression of volatile-qualified type is treated as a visible side-effect for the purposes of optimization (that is, within a single thread of execution, volatile accesses cannot be optimized out or reordered with another visible side effect that is sequenced-before or sequenced-after the volatile access. This makes volatile objects suitable for communication with a signal handler, but not with another thread of execution, see std::memory_order). Any attempt to refer to a volatile object through a non-volatile glvalue (e.g. through a reference or pointer to non-volatile type) results in undefined behavior.
И из CWE-14: Potential Mitigations. Phase: Implementation. Store the sensitive data in a «volatile» memory location if available.linuxover
26.01.2018 14:29это то я читал, но у нас есть массив который НЕ volatile
есть конец scope
foo() { char *bla; // что-то тут bar(bla); }
и теоретически отбрасывая вызов
bar(char *str)на основании "данный вызов модифицирует отбрасываемыйstr" можно отбрасывать иbar(volatile char *str), признакvolatileс этой проблемой вроде не пересекается.
То есть слово volatile в прототипеbar, поскольку решает не совсем ту проблему что описана в статье, то и не дает гарантий. Как-то так.Cryvage
26.01.2018 14:59Что мешает сделать так?
foo() { char *bla; volatile char *kostyl = bla; bar(kostyl); }linuxover
26.01.2018 15:18то что большинство нужных оптимизаций при этом будут убраны, это тоже плохо.
я просто думал может автор знает какой-то путь что «и волки сыты и овцы целы».
PS: а если такой код располагается в КОНЦЕ функции и выбрасывается по критерию «модифицируется отбрасываемый массив», то опять же по идее этот код тоже не гарантирует невмешательства оптимизатора.Andrey2008 Автор
26.01.2018 15:23путь что «и волки сыты и овцы целы»
Это использовать такие функции, как memset_s. Компилятор знает, что от него хотят и обязательно затрёт память.linuxover
26.01.2018 15:47ну вот вопрос и стоял: компилятор их по названию различает или по другому критерию?
Andrey2008 Автор
26.01.2018 15:53Это зависит от реализации. Вот такая статья, например, попалась: A glance at compiler internals: Keep my memset.
olegator99
26.01.2018 13:36Поддерживаю политику отказа от memset в плюсовом коде — это действительно тот еще рассадник ошибок.
Однако, лучше инициализировать массив конструкцией вида
int a[10000] = {0};.
При ревью кода конструкцию видаint a[10000] = {};можно не заметить, а если в контексте задачи инициализация массива нулями не требуется, то это незаметная конструкция сильно ударит по производительности кода...khim
26.01.2018 14:40Вопрос привычки исключительно. И вообще — склолько лишних символов вам нужно «для заметности»? Там и так знак равенства лишний, чтобы заметнее было…
olegator99
26.01.2018 15:47С одной стороны вопрос, конечно, субъективный.
Но с другой стороны, предпочитаю, что бы код максимально сам себя описывал.
То, что конструкция '={0}' протрет массив нулями, на мой взгляд, интуитивно понятнее, чем '={}'.khim
26.01.2018 16:43+1То, что конструкция '={0}' протрет массив нулями, на мой взгляд, интуитивно понятнее, чем '={}'.
Ага, конечно. То, что вы явно задали значение одного поля из десятков так ведь важно.
Если кому-то «итуитивно понятнее», что один-единственный нолик там что-то меняет, то я вообще не уверен, что ему нужно на C++ программировать.
Потому что вот:
$ cat test.cc #include <iostream> int main() { int a[10] = {1}; for (int i=0;i<10;++i) { std::cout << a[i] << "\n"; } } $ g++ test.cc -o test $ ./test 1 0 0 0 0 0 0 0 0 0
Что там ваша интуитивность про ={1} говорит?olegator99
26.01.2018 17:41-3Ага, конечно. То, что вы явно задали значение одного поля из десятков так ведь важно.
Не вводите людей в заблуждение. Рекомендую прочитать reference перед тем, как громко что-то утверждать.
Конструкция
a[10000] = {0}гарантированно инициализирует нулями весь массив на c++.splav_asv
26.01.2018 17:55+5Дело не в этом. А в том, что изменение константы с одного(почему-то специального) значения, на другое (раньше вычисляли суммы — нейтральный элемент был 0, а теперь надо произведения и элемент 1) меняет поведение в корне.
На мой личный взгляд интуитивнее писать = {} для инициализации нулями, чтобы не возникло у кого-нибудь желание «просто поменять константу».
P.S. и да, остаётся открытым вопрос — а как другими( не 0) значениями вы бы инициализировали большой массив? Кроме fill особо ничего на ум не приходит.khim
26.01.2018 18:24+1Дело не в этом. А в том, что изменение константы с одного(почему-то специального) значения, на другое (раньше вычисляли суммы — нейтральный элемент был 0, а теперь надо произведения и элемент 1) меняет поведение в корне.
Спасибо за то, что обьяснили что именно меня в этом выражении раздражает. Да, именно так. Тот факт, что 0 заменить на 1 нельзя. Для людей, которые об этом знают — 0 там не нужен, они знают, что двух фигурных скобочек достаточно, для тех, кто этого не знает — он просто опасен, так как возникает соблазн «в случае чего» поставить там что-то другое… и потом долго ловить глюки. Так для кого он там?
khim
26.01.2018 18:32P.S. и да, остаётся открытым вопрос — а как другими( не 0) значениями вы бы инициализировали большой массив? Кроме fill особо ничего на ум не приходит.
Увы. Clang позволяет писать вот так, но, увы, GCC это поддерживает только в C, а многие другие компиляторы — не поддерживают такой синтаксис вовсе.
Но собственно уже наличие подобных (пусть и нестандартных) расширений указывает на то, что в рамках стандарта хорошего решения нету…datacompboy
26.01.2018 18:48+1Этот синтаксис тоже плох, так как размер указывается дважды и оба раза явно.
khim
27.01.2018 05:08Размер можно опустить (в описании массива) — тогда он будет вычислен так, чтобы все индексы «влезли». Или можно в правой части можно использовать arraysize. Конечно нельзя сделать и то и другое одновременно: где-то же ведь размер должен быть указан!
По большому счёту единственная проблема — то, что этого нет в стандарте… даже в проекте C++20…
firk
26.01.2018 22:26Некоторые старые компиляторы не поддерживают пустые скобки (не знаю что там со стандартами но речь не про них а про практику). Я например и не знал что так можно, пока не прочёл эту ветку комментариев. Пробовал когда-то и выдало syntax error, ну нельзя так нельзя, поставим первый ноль для успокоения компилятора, а так как это вещь не особо важная то разбирательства не последовало.
khim
26.01.2018 22:33Вот это — уже другая история. Возможно и в Chromium из-за совместимости со старыми компиляторами такое делали изначально, а потом вступило в силу правило if you are editing code, take a few minutes to look at the code around you and determine its style.
Но говорить, что это — что-то, что вот прямо лучше читается… жуть.
khim
26.01.2018 18:21+3Конструкция
А почему, собственно?a[10000] = {0}гарантированно инициализирует нулями весь массив на c++.
Рекомендую прочитать reference перед тем, как громко что-то утверждать.
Читаю: All array elements that are not initialized explicitly are initialized implicitly the same way as objects that have static storage duration.
В вашем примере происходит следуюшие:
1. Один первый элемент вы инициализируете нулём явно.
2. 9999 элементов получают нулевое значение вследствие неявной инициализации.
Если вы уж доверили компилятору инициализировать почти весь обьект в «значение по умолчанию» — то нафига вам отдельно указывать что один специально выделенный, первый, элемент вы тоже хотите в ноль превратить???
Единственный вариант — чтобы привлечь внимание. В смысле надёжности и понятности= {}и= {0}идентичны, только второй вариант поднимает вопрос: а автор точно знает как C++ вообще работает? Зачем он один-единственный элемент (из десятков, а, возможно, и тысяч) инициализировал явно-то? Какой в этом тайный смысл?olegator99
26.01.2018 19:00Кажется, это тупиковый спор. Технически обе конструкции инициализируют массив значениями 0.
Лично для меня важнее заметить факт непредвиденной инициализации большого массива 0, для вас, что бы автор кода понимал, детали подкапотного действа в компиляторе.
Ради интереса погрепал исходники хромиума, который разбирается в статье. Авторы в 10 раз чаще используют подход
arr[]={0}, чемarr[]={}
bash-3.2$ find chromium/ -name "*.h" -or -name "*.cc" | xargs -n1 grep "] = {}" | wc -l 5 bash-3.2$ find chromium/ -name "*.h" -or -name "*.cc" | xargs -n1 grep "] = {0}" | wc -l 59khim
26.01.2018 22:41+1Лично для меня важнее заметить факт непредвиденной инициализации большого массива 0, для вас, что бы автор кода понимал, детали подкапотного действа в компиляторе.
Проблема в том, запись вида{0}даёт очень много поводов неправильно воспринять код. Можно посчитать, что заменив на0на1вы получите правильную инициализацию, можно посчитать, что убрав этот несчастный ноль вы позволите компилятору не тратить время на обнуление массива… Много способов воспринять код неправильно.
Хуже того: не зная «деталей подкапотного действа» очень легко «увидеть», что инициализован тут только один элемент и «поправить» код всунув сюда memset — а это явно не то, чего мы хотим, ведь правда?
И да — это вопрос вкуса. Для меня самое главное — чтобы код был не был понят неправильно. А для вас — чтобы у читателя глаз «не зацепался», а если он чего-то неправильно понял и из-за этого машина вьедет в столб… ну чего такого — мы покойнику бесплатно patch вышлем!
Cryvage
29.01.2018 12:10Конструкция a[10000] = {0} гарантированно инициализирует нулями весь массив на c++.
Конструкция a[10000] = {} вызовет конструктор по умолчанию для всей 10000 элементов. Конструкция a[10000] = {0} вызовет конструктор с одним аргументом для первого элемента (если он определён), и конструктор по умолчанию для оставшихся 9999 элементов. Лично мне, первый вариант кажется более правильным, т.к. он обрабатывает все элементы одинаково. Во втором же случае, мы неявно подразумеваем, что конструктор по умолчанию эквивалентен конструктору с одним параметром. А это может быть совсем не так. Да и в принципе это нарушает семантику: мы хотим чтобы элементы обработались одинаково, но почему-то делаем первый элемент особенным. Это не правильно.
firk
26.01.2018 13:37Здесь вообще обнуляется только первый элемент массива и один байт во втором элементе.
Не отрицаю, что у memset есть и другие проблемы, но конкретно эта (на мой взгляд самая частая) решается запретом на её использование без sizeof() в третьем аргументе либо просто параметром, либо в качестве множителя. Причём, по возможности, не sizeof(тип) а sizeof(переменная) или sizeof(*переменная) — подсмотрел такой приём в исходниках FreeBSD и с тех пор везде стараюсь так делать. После чего про неё можно забыть. То же самое касается и memcpy.
olegator99
26.01.2018 13:41Вот не согласен, что это системное решение.
До компилятора такой запрет не донесешь. А ревьювер — человек, может пропустить.
не sizeof(тип) а sizeof(переменная)
согласен, если уж дошло до memset, то это безопаснее, но опять же с кучей оговорок
sizeof(*переменная)
это плохо, потенциально можно словить nullptr dereference (что, конечно, маловероятно на практике, но тем не менее без проверки переменная != nullptr — эта конструкция является UB)
firk
26.01.2018 13:50Если там null то он и в первом аргументе memset окажется (имелось ввиду что надо либо её размер либо размер её элемента (для массива) проверять). Так что UB будет и так и так.
Но с другой стороны мне сложно представить, какими мотивами могли руководствоваться авторы компилятора, который будет делать частное dereference внутри такого sizeof.khim
26.01.2018 14:42Но с другой стороны мне сложно представить, какими мотивами могли руководствоваться авторы компилятора, который будет делать частное dereference внутри такого sizeof.
Никакими. sizeof не вычисляет выражение.
Хотя я предпочитаю писатьsizeof переменная(без скобочек) — сразу ясно, что мы не с типом работаем.
Andrey2008 Автор
26.01.2018 14:37+2Примечание. sizeof не вычисляет выражение. Вычисляется только тип. sizeof(*переменная) не может привести к nullptr dereference.
olegator99
26.01.2018 15:39Согласен, прямо сейчас ни одна реализация не вычисляет, но стандарт c++17 допускает Temporary materialization prvalue в sizeof.
Да, *ptr это конечно не prvalue, но тем не менее — в какой то момент код очень похожий на sizeof *ptr может сломаться от nullptr в ptr.Andrey2008 Автор
26.01.2018 15:45+1Отлично! Меня радуют такие новшества в C++. С ними, анализатор PVS-Studio всегда будет нужен. :)
mayorovp
27.01.2018 21:27Temporary materialization occurs in the following situations:
- when sizeof is applied to a prvalue (this is part of an unevaluated expression)
Bobko
26.01.2018 14:49это плохо, потенциально можно словить nullptr dereference
Неа, nullptr dereference вы не словите, даже если если там null. sizeof() даже не пытается вычислять значение аргумента, а просто по честному подставляет его размер на эта преобразования в ассемблер. Именно по этому я пишу обращения в функции malloc и ей подобным в духе:
int *ptr = malloc(100 * sizeof(*ptr));
devalone
26.01.2018 18:45+2А у вас были статьи с рейтингом наиболее качественных проектов по мнению PVS? Было бы интересно почитать.
Andrey2008 Автор
26.01.2018 21:30Нет. Часто проекты большие и сложно внимательно изучить все предупреждения. Соответственно, для них невозможно указать плотность ошибок. Для небольших проектов, таких как Notepad++, мы указываем плотность ошибок. И возможно при случае напишем статью сравнение. Но жаль, что в ней будут только такие небольшие проекты.
mantiscorp
Даже в 23-м веке проблемы с памятью и указателями в C/C++ будут актуальны :)