Some bugs in Intel code
bearophile
bearophileHUGS at lycos.com
Mon Feb 7 13:09:27 PST 2011
This article shows some common bugs found by a static analysis tool in about 1.6 millions of code lines of the Intel IPP Samples for Windows, a high-quality C++ project.
"Intel IPP Samples for Windows - error correction", by Andrey Karpov, January 2011:
http://software.intel.com/en-us/articles/intel-ipp-samples-for-windows-error-correction/
Here I show a reduced version of some of the bugs discussed.
-------------------
"Identical code branches" and "Copy-Paste and missing +1", caused by copy & paste with missed modify of one of the two branches:
void main(string[] args) {
int x;
if (args.length > 1) {
x++;
} else {
x++;
}
}
-------------------
"Loss of error flag", "Incorrect processing of buffer overflow", "Overrun in the wake of incorrect check":
import core.stdc.stdio: printf;
int foo(bool cant_be_true) {
int error = cant_be_true ? -1 : 1;
return error;
}
void main() {
uint x = foo(true);
if (x < 0)
printf("error!");
}
This is a serious, common and easy to catch bug, so I have added this:
http://d.puremagic.com/issues/show_bug.cgi?id=5539
-------------------
"Remove that does not remove anything":
Time ago I have added an enhancement request about that:
http://d.puremagic.com/issues/show_bug.cgi?id=5464
-------------------
"Comparing structures' fields to themselves":
bool H264_AU_Stream::IsPictureSame(H264SliceHeaderParse & p_newHeader)
{
if ((p_newHeader.frame_num != m_lastSlice.frame_num) ||
(p_newHeader.pic_parameter_set_id !=
p_newHeader.pic_parameter_set_id) ||
(p_newHeader.field_pic_flag != p_newHeader.field_pic_flag) ||
(p_newHeader.bottom_field_flag != m_lastSlice.bottom_field_flag)
){
return false;
}
...
}
The bug is:
if (x != x) {...
Generally similar redundancies in the code are a sign of a probable bug, so I have added this:
http://d.puremagic.com/issues/show_bug.cgi?id=5540
-------------------
"Two loops for one variable":
void main() {
int c;
for (c = 0; c < 10; c++) {
for (c = 0; c < 20; c++) {
// ...
}
}
}
-------------------
"Double assignment for additional safety", "Code making you alert", "Strange handling of array" and "Paranormal assignments":
void main() {
int x;
x = 10;
x = 20;
}
-------------------
"Minimum value which is not quite minimum", "Maximum value which is not quite maximum":
import std.algorithm: min, max;
void main() {
int x = 10;
int y = 20;
int z = min(x, x);
int w = max(y, y);
}
-------------------
"Subtraction result always amounts to 0":
void main() {
int x = 10;
return x - x;
}
Another example of redundancy that often hides a bug. Covered by 5540.
-------------------
"Error of using '?:' ternary operator", it's a problem caused by ternary operator precedence
enum { F0, F1, F2 }
void main() {
int x = 10;
int y = F1 | (x == 0) ? 0 : F2;
assert(y == 0);
}
-------------------
>From the Summary:
void main() {
int x = 10;
assert(x == x);
}
Covered by 5540.
===========================
The following cases are from this related article:
http://www.viva64.com/en/a/0068/
-------------------
inline_ bool Contains(const LSS& lss)
{
// We check the LSS contains the two
// spheres at the start and end of the sweep
return
Contains(Sphere(lss.mP0, lss.mRadius)) &&
Contains(Sphere(lss.mP0, lss.mRadius));
}
void COX3DTabViewContainer::OnNcPaint()
{
...
if(rectClient.top<rectClient.bottom &&
rectClient.top<rectClient.bottom)
{
dc.ExcludeClipRect(rectClient);
}
...
}
That are of this kind:
x && x
-------------------
The last case can't be reached:
string TimePeriod::toString() const
{
...
if (_relativeTime <= 143)
os << ((int)_relativeTime + 1) * 5 << _(" minutes");
else if (_relativeTime <= 167)
os << 12 * 60 + ((int)_relativeTime - 143) * 30 << _(" minutes");
else if (_relativeTime <= 196)
os << (int)_relativeTime - 166 << _(" days");
else if (_relativeTime <= 143)
os << (int)_relativeTime - 192 << _(" weeks");
...
}
While here one case is duplicated:
void DXUTUpdateD3D10DeviceStats(...)
{
...
else if( DeviceType == D3D10_DRIVER_TYPE_SOFTWARE )
wcscpy_s( pstrDeviceStats, 256, L"WARP" );
else if( DeviceType == D3D10_DRIVER_TYPE_HARDWARE )
wcscpy_s( pstrDeviceStats, 256, L"HARDWARE" );
else if( DeviceType == D3D10_DRIVER_TYPE_SOFTWARE )
wcscpy_s( pstrDeviceStats, 256, L"SOFTWARE" );
...
}
-------------------
One nice advice to avoid some bugs caused by code copy & paste:
>The second method to make errors fewer in some cases is to discipline oneself and edit the code being copied in a special way. [...] You should edit the code so that fragments which must differ from each other are visually arranged in a column. It is much more difficult to make an error if you use this method.<
int ztrend = sgn(
buffer[samplesleft - WindowSizeInt-2]-buffer[samplesleft
- WindowSizeInt-2]);
==>
int ztrend = sgn(
buffer[samplesleft - WindowSizeInt-2] -
buffer[samplesleft - WindowSizeInt-2]);
Bye,
bearophile
More information about the Digitalmars-d
mailing list