# Ответы на задачи со стенда PVS\-Studio на конференциях 2018\-2019 Привет\! Несмотря на то, что сезон конференций 2019 года ещё в самом разгаре, мы бы хотели обсудить задачи, которые ранее предлагали посетителям нашего стенда\. Осень 2019 года мы начали с новым набором задач, поэтому уже можно обнародовать решение старых задачек за 2018 год, а также первую половину 2019\. Тем более, многие из них были взяты из ранее опубликованных статей, а листовки с задачами содержали ссылку или QR\-код с информацией о статье\. ![0685_conferences_answers_2018-2019_ru/image1.png](https://import.viva64.com/docx/blog/0685_conferences_answers_2018-2019_ru/image1.png) Если вы бывали на конференциях, где мы стояли со стендом, то наверняка видели или даже решали какие\-то из наших задач\. Это всегда фрагменты кода из реальных открытых проектов на языках программирования C, C\+\+, C\# или Java\. В коде содержатся ошибки, которые мы предлагаем поискать посетителям\. За решение \(или просто обсуждение ошибки\) мы выдаем призы \- статусы на рабочий стол, брелоки и т\.п\.: ![0685_conferences_answers_2018-2019_ru/image3.jpg](https://import.viva64.com/docx/blog/0685_conferences_answers_2018-2019_ru/image3.jpg) Хотите такие же? Приходите на наш стенд на следующих конференциях\. Кстати, в статьях "[Время конференций\! Подводим итоги 2018 года](https://pvs-studio.ru/ru/blog/posts/0608/)" и "[Конференции\. Промежуточные итоги по первому полугодию 2019](https://pvs-studio.ru/ru/blog/posts/0648/)" содержится описание нашей активности на конференциях в этом и прошлом году\. Итак, начнём игру "Найди ошибку в коде"\. Сначала рассмотрим более старые задачки за 2018 год, будем использовать группировку по языкам программирования\. ## 2018 ### С\+\+ **Chromium bug** ```cpp static const int kDaysInMonth[13] = { 0, 31, 28, 31, 30, 31, 30, 31, 31, 30, 31, 30, 31 }; bool ValidateDateTime(const DateTime& time) { if (time.year < 1 || time.year > 9999 || time.month < 1 || time.month > 12 || time.day < 1 || time.day > 31 || time.hour < 0 || time.hour > 23 || time.minute < 0 || time.minute > 59 || time.second < 0 || time.second > 59) { return false; } if (time.month == 2 && IsLeapYear(time.year)) { return time.month <= kDaysInMonth[time.month] + 1; } else { return time.month <= kDaysInMonth[time.month]; } } ``` \[Ответ\] Пожалуй, самая "долгоиграющая" задачка из нашего набора\. Эту ошибку в проекте Chromium мы предлагали найти посетителям нашего стенда в течение всего 2018 года\. Также она была показана в ходе нескольких докладов\. ```cpp if (time.month == 2 && IsLeapYear(time.year)) { return time.month <= kDaysInMonth[time.month] + 1; // <= day } else { return time.month <= kDaysInMonth[time.month]; // <= day } ``` В теле последнего блока _If\-else_ содержатся опечатки в возвращаемом значении\. Вместо _time\.day_ программист дважды по ошибке указал _time\.month_\. Это привело к тому, что всегда будет возвращено значение _true_\. Ошибка подробно разобрана в статье "[31 февраля](https://pvs-studio.ru/ru/blog/posts/cpp/0550/)"\. Отличный пример ошибки, которую непросто обнаружить на code review\. Также это хорошая иллюстрация использования технологии анализа потока данных\. **Unreal Engine bug** ```cpp bool VertInfluencedByActiveBone( FParticleEmitterInstance* Owner, USkeletalMeshComponent* InSkelMeshComponent, int32 InVertexIndex, int32* OutBoneIndex = NULL); void UParticleModuleLocationSkelVertSurface::Spawn(....) { .... int32 BoneIndex1, BoneIndex2, BoneIndex3; BoneIndex1 = BoneIndex2 = BoneIndex3 = INDEX_NONE; if(!VertInfluencedByActiveBone( Owner, SourceComponent, VertIndex[0], &BoneIndex1) && !VertInfluencedByActiveBone( Owner, SourceComponent, VertIndex[1], &BoneIndex2) && !VertInfluencedByActiveBone( Owner, SourceComponent, VertIndex[2]) &BoneIndex3) { .... } ``` \[Ответ\] Первым делом следует обратить внимание на то, что последний аргумент функции _VertInfluencedByActiveBone\(\)_ имеет значение по умолчанию и может быть не указан\. Теперь взгляните на блок _if_\. Упрощенно его можно переписать так: ```cpp if (!foo(....) && !foo(....) && !foo(....) & arg) ``` Теперь видно, что допущена ошибка\. Из\-за опечатки третий вызов функции _VertInfluencedByActiveBone\(\)_ производится с тремя аргументами вместо четырех, а к результату этого вызова применяется оператор _&_ \(побитовое И, слева будет результат функции _VertInfluencedByActiveBone\(\)_ типа _bool_, справа – целочисленная переменная _BoneIndex3_\)\. Код при этом компилируется\. Исправленный вариант кода \(добавлена запятая, закрывающая скобка перемещена в другое место\): ```cpp if(!VertInfluencedByActiveBone( Owner, SourceComponent, VertIndex[0], &BoneIndex1) && !VertInfluencedByActiveBone( Owner, SourceComponent, VertIndex[1], &BoneIndex2) && !VertInfluencedByActiveBone( Owner, SourceComponent, VertIndex[2], &BoneIndex3)) ``` Ошибка, которая первоначально была описана в статье "[Долгожданная проверка Unreal Engine 4](https://pvs-studio.ru/ru/blog/posts/cpp/0249/)"\. В статье ошибка озаглавлена как "Самая красивая из найденных ошибок"\. Я согласен с данным утверждением\. **Android bugs** ```cpp void TagMonitor::parseTagsToMonitor(String8 tagNames) { std::lock_guard lock(mMonitorMutex); // Expand shorthands if (ssize_t idx = tagNames.find("3a") != -1) { ssize_t end = tagNames.find(",", idx); char* start = tagNames.lockBuffer(tagNames.size()); start[idx] = '\0'; .... } .... } ``` \[Ответ\] В условии блока _if_ перепутан приоритет операций\. Код работает не так, как задумал программист: ```cpp if (ssize_t idx = (tagNames.find("3a") != -1)) ``` Переменная _idx_ будет получать значения 0 или 1, а выполнение условия будет зависеть от этого значения, что является ошибкой\. Исправленный вариант кода: ```cpp ssize_t idx = tagNames.find("3a"); if (idx != -1) ``` Ошибка из статьи "[Проверили с помощью PVS\-Studio исходные коды Android, или никто не идеален](https://pvs-studio.ru/ru/blog/posts/cpp/0579/)"\. И еще одна задачка на поиск нетривиальной ошибки в Android: ```cpp typedef int32_t GGLfixed; GGLfixed gglFastDivx(GGLfixed n, GGLfixed d) { if ((d>>24) && ((d>>24)+1)) { n >>= 8; d >>= 8; } return gglMulx(n, gglRecip(d)); } ``` \[Ответ\] Проблема в выражении _\(d \>\> 24\) \+ 1_\. Программист хотел проверить, что 8 старших бит переменной _d_ содержат единицы, но при этом не все биты сразу\. Другими словами, программист хотел проверить, что в старшем байте находится любое значение, отличное от 0x00 и 0xFF\. Сначала он проверил, что старшие биты ненулевые, написав выражение \(d\>\>24\)\. Далее он сдвигает старшие восемь бит в младший байт\. При этом он рассчитывает, что самый старший знаковый бит дублируется во всех остальных битах\. То есть если переменная d равна 0b11111111'00000000'00000000'00000000, то после сдвига получится значение 0b11111111'11111111'11111111'11111111\. Прибавив 1 к значению 0xFFFFFFFF типа _int_, программист планирует получить 0 \(\-1\+1\=0\)\. Таким образом, выражением _\(\(d\>\>24\)\+1\)_ он проверяет, что не все старшие восемь бит равны 1\. Однако при сдвиге старший знаковый бит вовсе не обязательно "размазывается"\. Стандарт говорит: "The value of E1 \>\> E2 is E1 right\-shifted E2 bit positions\. If E1 has an unsigned type or if E1 has a signed type and a non\-negative value, the value of the result is the integral part of the quotient of E1/2^E2\. _If E1 has a signed type and a negative value, the resulting value is implementation\-defined_"\. Таким образом, это пример поведения, зависящего от реализации \(implementation\-defined behavior\)\. Как будет работать этот код \- зависит от архитектуры микропроцессора и реализации компилятора\. После сдвига в старших битах вполне могут оказаться нули, и тогда результат выражения _\(\(d\>\>24\)\+1\)_ всегда будет отличен от 0, то есть будет всегда истинным значением\. Действительно, непростая задачка\. Эта ошибка, как и предыдущая, была описана в статье "[Проверили с помощью PVS\-Studio исходные коды Android, или никто не идеален](https://pvs-studio.ru/ru/blog/posts/cpp/0579/)"\. ## 2019 ### С\+\+ **"Во всём виноват GCC"** ```cpp int foo(const unsigned char *s) { int r = 0; while(*s) { r += ((r * 20891 + *s *200) | *s ^ 4 | *s ^ 3) ^ (r >> 1); s++; } return r & 0x7fffffff; } ``` Программист утверждает, что данный код работает с ошибкой по вине компилятора GCC 8\. Так ли это? \[Ответ\] Функция возвращает отрицательные значения\. Причина в том, что компилятор не генерирует код для оператора побитового И \(&\)\. Ошибка связана с неопределённым поведением\. Компилятор видит, что в переменной _r_ считается некоторая сумма\. При этом прибавляются только положительные числа\. Переполнения переменной _r_ произойти не должно, иначе это неопределённое поведение, которое компилятор никак не должен рассматривать и учитывать\. Итак, компилятор считает, что раз значение в переменной _r_ после окончания цикла не может быть отрицательным, то операция _r & 0x7fffffff_ для сброса знакового бита является лишней и компилятор просто возвращает из функции значение переменной _r_\. Ошибка из статьи "[Релиз PVS\-Studio 6\.26](https://pvs-studio.ru/ru/blog/posts/0585/)"\. **QT bug** ```cpp static inline const QMetaObjectPrivate *priv(const uint* data) { return reinterpret_cast(data); } bool QMetaEnum::isFlag() const { const int offset = priv(mobj->d.data)->revision >= 8 ? 2 : 1; return mobj && mobj->d.data[handle + offset] & EnumIsFlag; } ``` \[Ответ\] С указателем _mobj_ работают небезопасно\. Сначала он разыменовывается, а далее проверяется\. Классика\. Ошибка была описана в статье "[Третья проверка Qt 5 с помощью PVS\-Studio](https://pvs-studio.ru/ru/blog/posts/cpp/0584/)"\. ### C\# **Infer\.NET bug** ```cpp public static void WriteAttribute(TextWriter writer, string name, object defaultValue, object value, Func converter = null) { if ( defaultValue == null && value == null || value.Equals(defaultValue)) { return; } string stringValue = converter == null ? value.ToString() : converter(value); writer.Write($"{name}=\"{stringValue}\" "); } ``` \[Ответ\] В выражении _value\.Equals\(defaultValue\)_ возможен доступ по нулевой ссылке _value_\. Это произойдет при таких значениях переменных, когда _defaultValue \!\= null_, а _value \=\= null_\. Ошибка из статьи "[Какие ошибки прячутся в коде Infer\.NET?](https://pvs-studio.ru/ru/blog/posts/csharp/0590/)" **FastReport bug** ```cpp public class FastString { private const int initCapacity = 32; private void Init(int iniCapacity) { sb = new StringBuilder(iniCapacity); .... } public FastString() { Init(initCapacity); } public FastString(int iniCapacity) { Init(initCapacity); } public StringBuilder StringBuilder => sb; } .... Console.WriteLine(new FastString(256).StringBuilder.Capacity); ``` Что будет выведено на консоль? Что не так с классом _FastString_? \[Ответ\] На консоль будет выведено 32\. Причина \- опечатка в имени переменной, передаваемой в метод _Init_ в конструкторе: ```cpp public FastString(int iniCapacity){ Init(initCapacity); } ``` Параметр конструктора _iniCapacity_ не будет использован\. Вместо этого в метод _Init_ передают константу _initCapacity_\. Ошибка была описана в статье "[Самые быстрые отчёты на диком западе\. И горстка багов в придачу\.\.\.](https://pvs-studio.ru/ru/blog/posts/csharp/0595/)" **Roslyn bug** ```cpp private SyntaxNode GetNode(SyntaxNode root) { var current = root; .... while (current.FullSpan.Contains(....)) { .... var nodeOrToken = current.ChildThatContainsPosition(....); .... current = nodeOrToken.AsNode(); } .... } public SyntaxNode AsNode() { if (_token != null) { return null; } return _nodeOrParent; } ``` \[Ответ\] Возможен доступ по нулевой ссылке _current_ в выражении _current\.FullSpan\.Contains\(\.\.\.\.\)_\. Переменная _current_ может получить нулевое значение как результат выполнения метода _nodeOrToken\.AsNode\(\)_\. Ошибка из статьи "[Проверяем исходный код Roslyn](https://pvs-studio.ru/ru/blog/posts/csharp/0622/)"\. **Unity bug** ```cpp .... staticFields = packedSnapshot.typeDescriptions .Where(t => t.staticFieldBytes != null & t.staticFieldBytes.Length > 0) .Select(t => UnpackStaticFields(t)) .ToArray() .... ``` \[Ответ\] Допущена опечатка: вместо оператора _&&_ использован оператор _&_\. Это приводит к тому, что проверка _t\.staticFieldBytes\.Length \> 0_ выполняется всегда, даже в случае равенства _null_ переменной _t\.staticFieldBytes_, что, в свою очередь, приведет к доступу по нулевой ссылке\. Впервые эта ошибка была показана в статье "[Анализируем ошибки в открытых компонентах Unity3D](https://pvs-studio.ru/ru/blog/posts/csharp/0423/)"\. ### Java **IntelliJ IDEA bug** ```cpp private static boolean checkSentenceCapitalization(@NotNull String value) { List words = StringUtil.split(value, " "); .... int capitalized = 1; .... return capitalized / words.size() < 0.2; // allow reasonable amount of // capitalized words } ``` Предлагается определить, почему будет неправильно подсчитано число слов с заглавными буквами\. \[Ответ\] Функция должна возвращать истину, если менее 20% слов начинаются с заглавной буквы\. Но проверка не работает, так как происходит целочисленное деление, результатом которого будут только значения 0 или 1\. Функция вернёт ложное значение, только если все слова будут начинаться с заглавной буквы\. В остальных случаях при делении будет получаться 0, а функция будет возвращать истину\. Ошибка из статьи "[PVS\-Studio для Java](https://pvs-studio.ru/ru/blog/posts/java/0603/)"\. **SpotBugs bug** ```cpp public static String getXMLType(@WillNotClose InputStream in) throws IOException { .... String s; int count = 0; while (count < 4) { s = r.readLine(); if (s == null) { break; } Matcher m = tag.matcher(s); if (m.find()) { return m.group(1); } } throw new IOException("Didn't find xml tag"); .... } ``` Предлагается определить, в чём заключается ошибка поиска xml\-тега\. \[Ответ\] Условие _count < 4_ будет выполнено всегда, так как переменную _count_ не инкрементируют внутри цикла\. Предполагалось, что поиск xml\-тега должен осуществляться только в первых четырёх строках файла, но из\-за ошибки будет прочитан весь файл\. Эта ошибка, как и предыдущая, была описана в статье "[PVS\-Studio для Java](https://pvs-studio.ru/ru/blog/posts/java/0603/)"\. На этом – всё\. Ждём вас на следующих конференциях\. Ищите стенд с единорогом\. Выдадим новые интересные задачки и, конечно же, задарим призы\. До встречи\!