Привет! Несмотря на то, что сезон конференций 2019 года ещё в самом разгаре, мы бы хотели обсудить задачи, которые ранее предлагали посетителям нашего стенда. Осень 2019 года мы начали с новым набором задач, поэтому уже можно обнародовать решение старых задачек за 2018 год, а также первую половину 2019. Тем более, многие из них были взяты из ранее опубликованных статей, а листовки с задачами содержали ссылку или QR-код с информацией о статье.
Если вы бывали на конференциях, где мы стояли со стендом, то наверняка видели или даже решали какие-то из наших задач. Это всегда фрагменты кода из реальных открытых проектов на языках программирования C, C++, C# или Java. В коде содержатся ошибки, которые мы предлагаем поискать посетителям. За решение (или просто обсуждение ошибки) мы выдаем призы — статусы на рабочий стол, брелоки и т.п.:
Хотите такие же? Приходите на наш стенд на следующих конференциях.
Кстати, в статьях "Время конференций! Подводим итоги 2018 года" и "Конференции. Промежуточные итоги по первому полугодию 2019" содержится описание нашей активности на конференциях в этом и прошлом году.
Итак, начнём игру «Найди ошибку в коде». Сначала рассмотрим более старые задачки за 2018 год, будем использовать группировку по языкам программирования.
2018
С++
Chromium bug
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];
}
}
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 февраля". Отличный пример ошибки, которую непросто обнаружить на code review. Также это хорошая иллюстрация использования технологии анализа потока данных.
Unreal Engine bug
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)
{
....
}
if (!foo(....) && !foo(....) && !foo(....) & arg)
Теперь видно, что допущена ошибка. Из-за опечатки третий вызов функции VertInfluencedByActiveBone() производится с тремя аргументами вместо четырех, а к результату этого вызова применяется оператор & (побитовое И, слева будет результат функции VertInfluencedByActiveBone() типа bool, справа – целочисленная переменная BoneIndex3). Код при этом компилируется. Исправленный вариант кода (добавлена запятая, закрывающая скобка перемещена в другое место):
if(!VertInfluencedByActiveBone(
Owner, SourceComponent, VertIndex[0], &BoneIndex1) &&
!VertInfluencedByActiveBone(
Owner, SourceComponent, VertIndex[1], &BoneIndex2) &&
!VertInfluencedByActiveBone(
Owner, SourceComponent, VertIndex[2], &BoneIndex3))
Ошибка, которая первоначально была описана в статье "Долгожданная проверка Unreal Engine 4". В статье ошибка озаглавлена как «Самая красивая из найденных ошибок». Я согласен с данным утверждением.
Android bugs
void TagMonitor::parseTagsToMonitor(String8 tagNames) {
std::lock_guard<std::mutex> 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] = '';
....
}
....
}
if (ssize_t idx = (tagNames.find("3a") != -1))
Переменная idx будет получать значения 0 или 1, а выполнение условия будет зависеть от этого значения, что является ошибкой. Исправленный вариант кода:
ssize_t idx = tagNames.find("3a");
if (idx != -1)
Ошибка из статьи "Проверили с помощью PVS-Studio исходные коды Android, или никто не идеален".
И еще одна задачка на поиск нетривиальной ошибки в Android:
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));
}
Программист хотел проверить, что 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, или никто не идеален".
2019
С++
«Во всём виноват GCC»
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. Так ли это?
Ошибка из статьи "Релиз PVS-Studio 6.26".
QT bug
static inline const QMetaObjectPrivate *priv(const uint* data)
{ return reinterpret_cast<const QMetaObjectPrivate*>(data); }
bool QMetaEnum::isFlag() const
{
const int offset = priv(mobj->d.data)->revision >= 8 ? 2 : 1;
return mobj && mobj->d.data[handle + offset] & EnumIsFlag;
}
Ошибка была описана в статье "Третья проверка Qt 5 с помощью PVS-Studio".
C#
Infer.NET bug
public static void
WriteAttribute(TextWriter writer,
string name,
object defaultValue,
object value,
Func<object, string> converter = null)
{
if ( defaultValue == null && value == null
|| value.Equals(defaultValue))
{
return;
}
string stringValue = converter == null ? value.ToString() :
converter(value);
writer.Write($"{name}="{stringValue}" ");
}
Ошибка из статьи "Какие ошибки прячутся в коде Infer.NET?"
FastReport bug
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?
public FastString(int iniCapacity){ Init(initCapacity); }
Параметр конструктора iniCapacity не будет использован. Вместо этого в метод Init передают константу initCapacity.
Ошибка была описана в статье "Самые быстрые отчёты на диком западе. И горстка багов в придачу..."
Roslyn bug
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;
}
Ошибка из статьи "Проверяем исходный код Roslyn".
Unity bug
....
staticFields = packedSnapshot.typeDescriptions
.Where(t =>
t.staticFieldBytes != null &
t.staticFieldBytes.Length > 0)
.Select(t => UnpackStaticFields(t))
.ToArray()
....
Впервые эта ошибка была показана в статье "Анализируем ошибки в открытых компонентах Unity3D".
Java
IntelliJ IDEA bug
private static boolean checkSentenceCapitalization(@NotNull String value) {
List<String> words = StringUtil.split(value, " ");
....
int capitalized = 1;
....
return capitalized / words.size() < 0.2; // allow reasonable amount of
// capitalized words
}
Предлагается определить, почему будет неправильно подсчитано число слов с заглавными буквами.
Ошибка из статьи "PVS-Studio для Java".
SpotBugs bug
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-тега.
Эта ошибка, как и предыдущая, была описана в статье "PVS-Studio для Java".
На этом – всё. Ждём вас на следующих конференциях. Ищите стенд с единорогом. Выдадим новые интересные задачки и, конечно же, задарим призы. До встречи!
Если хотите поделиться этой статьей с англоязычной аудиторией, то прошу использовать ссылку на перевод: Sergey Khrenov. Solutions to Bug-Finding Challenges Offered by the PVS-Studio Team at Conferences in 2018-2019.
Автор: Сергей Хренов