俄罗斯OOO Program Verification Systems公司用自己的静态源码分析产品PVS-Studio对一些知名的C/C++开源项目,诸如Apache Http Server、Chromium、Clang、CMake、MySQL等的源码进行了分析,找出了100个典型的Bugs。个人觉得这份列表对C/C++ 程序员有一定参考意义。与其说事后用静态工具分析,倒不如在编码时就提高自知自觉,避免这份列表上的错误发生在你的代码中,因此这里将部分摘录一些Bugs(Bug编号这里不连续,为的是对应原文的编号)并做简要说明。原文将这份Bug列表分为了几类,这里也将沿用这个思路。
一、数组和字符串处理错误
数组和字符串处理错误是C/C++程序中最多的一类缺陷类型。这也可以看作是我们为拥有高效地底层内存操作能力而付出的代价。
[#1] Wolfenstein 3D项目 -"只有部分对象被clear了"
void CG_RegisterItemVisuals( int itemNum ) {
…
itemInfo_t *itemInfo;
…
memset( itemInfo, 0, sizeof( &itemInfo ) );
…
}
这里的Bug出现在memset那一行。代码的真实意图是clear iteminfo这块内存,但调用memset时,第三个参数传入的却是sizeof(&iteminfo),要知道 sizeof(&itemInfo) != sizeof(itemInfo_t),前者只是一个指针的大小罢了。正确的写法是:
memset(itemInfo, 0, sizeof(itemInfo_t)); 或memset(itemInfo, 0, sizeof(*itemInfo));
[#2] Wolfenstein 3D项目 -"只有部分Matrix被clear了"
ID_INLINE mat3_t::mat3_t( float src[ 3 ][ 3 ] ) {
memcpy( mat, src, sizeof( src ) );
}
这里的Bug出现在memcpy一行。程序的原意是将clear src[3][3]这个二维数组。但这里有个坑:那就是作为函数形式参数的数组名已经退化为指针了,对其sizeof只能得到一个指针的长度,因此这里的 memcpy只是copy了一个指针的长度,没有copy全。这里的代码是C++代码,原文中给出了正确的改正方法 – 传reference:
ID_INLINE mat3_t::mat3_t( float (&src)[3][3] )
{
memcpy( mat, src, sizeof( src ) );
}
[#4] ReactOS项目 – "错误地计算一个字符串的长度"
static const PCHAR Nv11Board = "NV11 (GeForce2) Board";
static const PCHAR Nv11Chip = "Chip Rev B2";
static const PCHAR Nv11Vendor = "NVidia Corporation";
BOOLEAN
IsVesaBiosOk(…)
{
…
if (!(strncmp(Vendor, Nv11Vendor, sizeof(Nv11Vendor))) &&
!(strncmp(Product, Nv11Board, sizeof(Nv11Board))) &&
!(strncmp(Revision, Nv11Chip, sizeof(Nv11Chip))) &&
(OemRevision == 0×311))
…
}
Bug处在IsVesaBiosOK中那一串strncmp调用中,代码将一个指针的size传入strncmp作为第三个参数,导致 strncmp实际只是比较了字符串的前4 or 8个字节,而不是字符串的全部内容。
[#6] CPU Identifying Tool项目 – 数组越界
#define FINDBUFFLEN 64 // Max buffer find/replace size
…
int WINAPI Sticky (…)
{
…
static char findWhat[FINDBUFFLEN] = {'\0'};
…
findWhat[FINDBUFFLEN] = '\0';
…
}
bug出在"findWhat[FINDBUFFLEN] = ‘\0′;”这一行。数组的最大长度为FINDBUFFLEN,但下标的最大值应该是FINDBUFFLEN-1,而不是FINDBUFFLEN。因此这 行代码显然应该改为findWhat[FINDBUFFLEN-1] = '\0';
[#7] Wolfenstein 3D项目 – 数组越界
typedef struct bot_state_s
{
…
char teamleader[32]; //netname of the team leader
…
} bot_state_t;
void BotTeamAI( bot_state_t *bs ) {
…
bs->teamleader[sizeof( bs->teamleader )] = '\0';
…
}
"sizeof( bs->teamleader )]"这行的结果值已经超出了数组的最大边界,正确的代码是:
bs->teamleader[
sizeof(bs->teamleader) / sizeof(bs->teamleader[0]) – 1
] = '\0';
[#8] Miranda IM项目 – 只Copy了部分字符串
struct _textrangew
{
CHARRANGE chrg;
LPWSTR lpstrText;
} TEXTRANGEW;
const wchar_t* Utils::extractURLFromRichEdit(…)
{
…
::CopyMemory(tr.lpstrText, L"mailto:", 7);
…
}
这里的bug在于L"mailto:"是宽字符串,宽字符串中的每个字符占2或4个字节(依Compiler使用的字符集编码而定),因此这里只 copy 7个字节显然是不够的,应该是7 * sizeof(wchar_t)。
[#9] CMake项目 – 循环內的数组越界
static const struct {
DWORD winerr;
int doserr;
} doserrors[] =
{
…
};
static void
la_dosmaperr(unsigned long e)
{
…
for (i = 0; i < sizeof(doserrors); i++)
{
if (doserrors[i].winerr == e)
{
errno = doserrors[i].doserr;
return;
}
}
…
}
作者原本意图la_dosmaperr中for循环的次数等于数组的元素个数,但sizeof(doserrors)返回的却是数组占用的字节个数,这远远大于数组元素个数,因此造成数组越界。正确的写法:
for (i = 0; i < sizeof(doserrors) / sizeof(*doserrors); i++)
[#10] CPU Identifying Tool项目 – 打印到自身的字符串
char * OSDetection ()
{
…
sprintf(szOperatingSystem,
"%sversion %d.%d %s (Build %d)",
szOperatingSystem,
osvi.dwMajorVersion,
osvi.dwMinorVersion,
osvi.szCSDVersion,
osvi.dwBuildNumber & 0xFFFF);
…
sprintf (szOperatingSystem, "%s%s(Build %d)",
szOperatingSystem, osvi.szCSDVersion,
osvi.dwBuildNumber & 0xFFFF);
…
补充:软件开发 , C++ ,