Hi! Though the 2019 conference season is not over yet, we'd like to talk about the bug-finding challenges we offered to visitors at our booth during the past conferences. Starting with the fall of 2019, we've been bringing a new set of challenges, so we can now reveal the solutions to the previous tasks of 2018 and the first half of 2019 – after all, many of them came from previously posted articles, and we had a link or QR code with information about the respective articles printed on our challenge leaflets.
If you attended events where we participated with a booth, you probably saw or even tried to solve some of our challenges. These are snippets of code from real open-source projects written in C, C++, C#, or Java. Each snippet contains a bug, and the guests are challenged to try to find it. A successful solution (or simply participation in the discussion of the bug) is rewarded with a prize: a spiral-bound desktop status, a keychain, and the like:
Want some too? Then welcome to drop by our booth at the upcoming events.
By the way, in the articles "Conference Time! Summing up 2018" and "Conferences. Sub-totals for the first half of 2019", we share our experience of participating in the events held earlier this year and in 2018.
Okay, let's play our «Find the bug» game. First we'll take a look at the earlier challenges of 2018, grouped by language.
2018
C++
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
}
The body of the last If-else block contains typos in the return statements: time.month was accidentally written for a second time instead of time.day. This mistake makes the function return true all the time. The bug is discussed in detail in the article "February 31" and is a cool example of a bug that isn't easily spotted by code review. This case is also a good demonstration of how we use dataflow analysis.
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)
The bug is now clearly visible. Because of the typo, the third call of the VertInfluencedByActiveBone() function is performed with three arguments instead of four, with the return value then participating in a & operation (bitwise AND: the left operand is the value of type bool returned by VertInfluencedByActiveBone(), and the right operand is the integer variable BoneIndex3). The code is still compilable. This is the fixed version (a comma added, the closing parenthesis moved to the end of the expression):
if(!VertInfluencedByActiveBone(
Owner, SourceComponent, VertIndex[0], &BoneIndex1) &&
!VertInfluencedByActiveBone(
Owner, SourceComponent, VertIndex[1], &BoneIndex2) &&
!VertInfluencedByActiveBone(
Owner, SourceComponent, VertIndex[2], &BoneIndex3))
This error was originally mentioned in the article "A Long-Awaited Check of Unreal Engine 4", where it was titled «the nicest error», which I totally agree with.
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))
The idx variable will be assigned the value 0 or 1, and whether the condition is true or false will depend on this value, which is a mistake. This is the fixed version:
ssize_t idx = tagNames.find("3a");
if (idx != -1)
This bug was mentioned in the article "We Checked the Android Source Code by PVS-Studio, or Nothing is Perfect".
Here's another non-trivial challenge with an Android bug:
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));
}
The programmer wanted to check that the 8 most significant bits of the d variable are set to 1 but not all of them at a time. In other words, they wanted to check that the most significant byte stores any value except 0x00 and 0xFF. First the programmer checks the most significant bits for null using the (d>>24) expression. Then they shift the eight most significant bits to the least significant byte, expecting the most significant sign bit to get duplicated in all the other bits. That is, if the d variable has the value 0b11111111'00000000'00000000'00000000, it will turn into 0b11111111'11111111'11111111'11111111 after the shift. By adding 1 to the int value 0xFFFFFFFF, the programmer is expecting to get 0 (-1+1=0). Thus, the ((d>>24)+1) expression is used to check that not all of the eight most significant bits are set to 1.
However, the most significant sign bit does not necessarily get «spread» when shifted. This is what the standard says: «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».
So, this is an example of implementation-defined behavior. How exactly this code will work depends on the CPU architecture and compiler implementation. The most significant bits may well end up as zeroes after the shift, and the ((d>>24)+1) expression would then always return a value other than 0, i.e. an always true value.
That, indeed, is a non-trivial challenge. Like the previous bug, this one was originally discussed in the article "We Checked the Android Source Code by PVS-Studio, or Nothing is Perfect".
2019
C++
«It's all GCC's fault»
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;
}
The programmer blames the GCC 8 compiler for the bug. Is it really GCC's fault?
This error was described in the article "PVS-Studio 6.26 Released".
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;
}
The bug was mentioned in the article "A Third Check of Qt 5 with 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}" ");
}
This bug is from the article "What Errors Lurk in Infer.NET Code?"
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);
What will the program output in the console? What's wrong with the FastString class?
public FastString(int iniCapacity){ Init(initCapacity); }
The constructor parameter iniCapacity won't be used; what gets passed instead is the constant initCapacity.
The bug was discussed in the article "The Fastest Reports in the Wild West — and a Handful of Bugs..."
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;
}
This bug is from the article "Checking the Roslyn Source Code".
Unity bug
....
staticFields = packedSnapshot.typeDescriptions
.Where(t =>
t.staticFieldBytes != null &
t.staticFieldBytes.Length > 0)
.Select(t => UnpackStaticFields(t))
.ToArray()
....
This bug was originally shown in the article "Discussing Errors in Unity3D's Open-Source Components".
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
}
Why does the program incorrectly calculate the number of capitalized words?
This bug is from the article "PVS-Studio for 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");
....
}
What's wrong with the search of the xml tag?
Like the previous bug, this one was described in the article "PVS-Studio for Java".
That's all for today. Come see us at the upcoming events – look for the unicorn. We'll be offering new interesting challenges and, of course, giving prizes. See you!
Автор: n0mo