过去一年，我们如同巡游在开源代码的广袤平原上，调查"案件"，清除缺陷，收集"战利品"。今天，我们决定踏入那间尘土最厚的酒馆：一位经验丰富的警长斜倚在吧台旁，回忆着狂野西部十个最大胆、最危险的代码缺陷。
来听个有趣的故事？整整一年，我们都在与来自C和C++开源项目的各种Bug作斗争。我们排查了每一个缺陷，分析其根源，并将其劣迹记录在案。现在是时候回顾那些最引人注目的案例了。
今天，我将讲述我们在狂野西部不同角落遇到的10个最有趣的代码缺陷。对于每一个案例，我们都准备了详细的技术分析文章。而对于最专注的读者，我们精选了今年发布的关于C和C++项目的5篇最受欢迎的文章：
- 多态分配器的轻率使用如何让你的生活苦不堪言
- C++程序员未定义行为指南
- C和C++的历史。第二部分：C和C++的标准化、Qt、Clang和Unreal Engine
- C++中的std::array有时比C中的数组更快
- 安全的数组处理？没听说过，没听说过
第10名
有一次，我们接手了一宗关于马匹失踪的案子。那匹母马明明一直在马厩里，但每当我们想骑马进城时，它就好像凭空消失了。
PVS-Studio警告：V674 在调用'SetRenderColor'函数时，'float'类型的字面量'0.5f'正被隐式转换为'unsigned char'类型。请检查第二个参数。grenade_bugbait.cpp 第168行
typedef unsigned char byte;
inline void CBaseEntity::SetRenderColor( byte r, byte g, byte b, byte a )
{
m_clrRender.Init( r, g, b, a );
}
void CGrenadeBugBait::BugBaitTouch( CBaseEntity *pOther )
{
....
if ( pSporeExplosion )
{
....
pSporeExplosion->SetRenderColor( 0.0f, 0.5f, 0.25f, 0.15f ); // <=
....
}
....
}
SetRenderColor 函数设置 RGBA 颜色值，其中每个参数都是 unsigned char 类型，可能的值范围是 [0 .. 255]。当尝试传递 float 类型的参数时，小数部分将被截断。因此，r、g、b、a 函数参数的值将等于 0。
不幸的是，代码仓库缺少追溯信息，所以关于这个错误是如何出现在代码中的，我有两种推测：
- 该函数曾经处理浮点数表示的颜色。后来处理改为整数，但并非所有调用点都得到了更新。
- 开发人员错误地认为
SetRenderColor处理浮点数，并相应地设置了它们。
以下是类似的警告：
- V674 在调用'SetRenderColor'函数时，'float'类型的字面量'0.5f'正被隐式转换为'unsigned char'类型。请检查第二个参数。weapon_bugbait.cpp 第171行
- V674 在调用'SetScrollRate'函数时，'double'类型的字面量'25.6'正被隐式转换为'int'类型。请检查第一个参数。grenade_tripmine.cpp 第179行
第9名
我们曾经不得不在草原上骑行一百条相同的小径。碰巧的是，我们拐上了一条已经骑过的路。只是这一次，它通向了一个完全不同的地方。
PVS-Studio警告：V517 检测到使用了'if (A) {...} else if (A) {...}'模式。可能存在逻辑错误。检查行：2903, 3053。erl_bif_info.c 2903
BIF_RETTYPE system_info_1(BIF_ALIST_1)
{
....
if (is_tuple(BIF_ARG_1)) { // L2778
....
} else if (BIF_ARG_1 == am_scheduler_id) { // L2782
....
}
....
else if (BIF_ARG_1 == am_garbage_collection) { // L2903
....
} else if (BIF_ARG_1 == am_fullsweep_after) { // L2921
....
}
else if (BIF_ARG_1 == am_garbage_collection) { // L3053
....
} else if (BIF_ARG_1 == am_instruction_counts) { // L3056
....
}
....
else if (ERTS_IS_ATOM_STR("halt_flush_timeout", BIF_ARG_1)) { // L3552
....
}
}
分析器在一个包含大量 if-else if 语句的函数中检测到了几个具有相同检查条件的分支——大约800行代码。然而，每个分支都有不同的逻辑：第一次检查和第二次检查。考虑到分支的数量以及重复项之间150行的间隔，发生这种情况也就不足为奇了。静态分析有助于防止此类情况。
第8名
我认识一位警长，他对所有罪行的判决只有一个："无罪"。他是个怪人。
PVS-Studio警告：V523 'then'分支的语句与后续代码片段等效。cmComputeLinkInformation.cxx 1748
bool cmComputeLinkInformation::CheckImplicitDirItem(LinkEntry const& entry)
{
BT<std::string> const& item = entry.Item;
// We only switch to a pathless item if the link type may be
// enforced. Fortunately only platforms that support link types
// seem to have magic per-architecture implicit link directories.
if (!this->LinkTypeEnabled) {
return false;
}
// Check if this item is in an implicit link directory.
std::string dir = cmSystemTools::GetFilenamePath(item.Value);
if (!cm::contains(this->ImplicitLinkDirs, dir)) {
// Only libraries in implicit link directories are converted to
// pathless items.
return false;
}
// Only apply the policy below if the library file is one that can
// be found by the linker.
std::string file = cmSystemTools::GetFilenameName(item.Value);
if (!this->ExtractAnyLibraryName.find(file)) {
return false;
}
return false;
}
分析器提示 CheckImplicitDirItem 函数肯定有问题：
- 最后一个
if语句的then分支重复了下面的代码 (return false;)； - 函数的每个执行分支都以返回
false结束； - 当从
AddFullItem调用时，它永远不会触发提前返回； - 整个函数体可以用
return false;替换，因为这不会改变程序的行为。
请注意，该函数使用"提前返回"模式编写，这有助于减少代码嵌套：最"积极"的结果放在函数末尾，而其余代码——如果偏离函数目的——应尽早终止函数。
在我们的例子中，我们可以假设函数最"积极"的结果是 LinkEntry 类型的对象通过了所有必要的检查，返回 true 值。
以下是一个修复代码的选项：
....
std::string file = cmSystemTools::GetFilenameName(item.Value);
if (!this->ExtractAnyLibraryName.find(file)) {
return false;
}
return true;
第7名
有一次在一个村庄里，我看到一个绝望的赌徒，把他所有的金子都押在了一张甚至不在牌堆里的牌上。法官只是耸了耸肩，金子就留在了桌子上。
PVS-Studio警告：
V783 可能会解引用无效的迭代器'shades.end()'。ColorHelper.cpp 194
winrt::Windows::UI::Color ColorHelper::GetAccentColor(
const winrt::Windows::UI::Color& color
)
{
....
auto shades = std::map<float, HSL>();
....
// 3f is quite nice if the whole non-client area is painted
constexpr auto readability = 1.75f;
for (auto shade : shades)
{
if (shade.first >= readability)
{
return HslToRgb(shade.second);
}
}
return HslToRgb(shades.end()->second); // <=
}
是否可能没有任何色调符合可读性标准？我们不能肯定，但很有可能。这是一个_教科书式的_未定义行为案例——无需出示任何通灵纸来证明，因为解引用 std::map::end() 会导致这种情况，因为这个迭代器指向 std::map 中最后一个元素之后的位置。
第6名
我曾经和一个同行的旅人达成协议，平分我们找到的宝藏。他点了点头，转过身，然后像海市蜃楼一样消失了。从那以后就没人见过他。
PVS-Studio警告：V758 当函数返回的智能指针被销毁时，'graph'引用将变为无效。utils.cpp 391
template<typename T>
struct Ptr : public std::shared_ptr<T>;
// ....
Ptr<FlannNeighborhoodGraph> FlannNeighborhoodGraph::create(
const Mat &points, int points_size,
int k_nearest_neighbors_, bool get_distances,
int flann_search_params_, int num_kd_trees)
{
return makePtr<FlannNeighborhoodGraphImpl>(points, points_size,
k_nearest_neighbors_, get_distances,
flann_search_params_, num_kd_trees);
}
void Utils::densitySort (const Mat &points, int knn,
Mat &sorted_points, std::vector<int> &sorted_mask)
{
// ....
// get neighbors
FlannNeighborhoodGraph &graph = // <=
*FlannNeighborhoodGraph::create(points, points_size, knn,
true /*get distances */, 6, 1);
std::vector<double> sum_knn_distances (points_size, 0);
for (int p = 0; p < points_size; p++) {
const std::vector<double> &dists = graph.getNeighborsDistances(p);
for (int k = 0; k < knn; k++)
sum_knn_distances[p] += dists[k];
}
// ....
}
关于 Ptr 的更多上下文
template<typename T>
struct Ptr : public std::shared_ptr<T>
{
inline Ptr(const std::shared_ptr<T>& o)
CV_NOEXCEPT : std::shared_ptr<T>(o) {}
inline Ptr(std::shared_ptr<T>&& o)
CV_NOEXCEPT : std::shared_ptr<T>(std::move(o)) {}
typename std::add_lvalue_reference<T>::type operator*() const
CV_NOEXCEPT { return *std::shared_ptr<T>::get(); }
// ....
}
template<typename _Tp, typename ... A1> static inline
Ptr<_Tp> makePtr(const A1&... a1)
{
static_assert( !has_custom_delete<_Tp>::value,
"Can't use this makePtr with custom DefaultDeleter");
return (Ptr<_Tp>)std::make_shared<_Tp>(a1...);
}
在这里使用智能指针并不能解决悬空引用和内存访问的问题。让我们深入了解一下。代码的工作原理如下：
create函数创建并返回一个指向FlannNeighborhoodGraphImpl类型的智能指针，其对象引用计数为一。- 为该智能指针的值创建了
graph引用，而对象引用计数保持不变。 - 由于该指针是一个临时对象，初始化完成后引用计数将变为零，释放被管理的对象。现在，引用指向一个已销毁的对象。
for循环引用了一个无效的引用。
结果，看似正确的代码导致了未定义行为。此外，PVS-Studio 并不是唯一能检测到这个问题工具；消毒剂也能做到。
要修复此问题，我们需要保存智能指针，以便 FlannNeighborhoodGraph 对象在代码块结束前一直存在。例如，我们可以这样做：
std::vector<double> sum_knn_distances (points_size, 0);
{
// get neighbors
auto graph = FlannNeighborhoodGraph::create(points, points_size, knn,
true /*get distances */, 6, 1);
for (int p = 0; p < points_size; p++) {
const std::vector<double> &dists = graph->getNeighborsDistances(p);
for (int k = 0; k < knn; k++)
sum_knn_distances[p] += dists[k];
}
}
我们还限制了 graph 的作用域，以便在循环执行后释放资源。
第5名
有一次，一位当地专家画了一张渡河的地图，但他的木炭用完了。所以，用最后一点木炭画的最危险的部分，随着第一场雨被冲走了。所有人都在那里失踪了。
PVS-Studio警告：
V629 请检查表达式 '1 << (brake->type + 1)'。对32位值进行位移位，随后扩展到64位类型。phpdbg_bp.c 1209
V784 位掩码的大小小于第一个操作数的大小。这将导致高位比特丢失。phpdbg_bp.c 1209
uint64_t flags
....
PHPDBG_API void phpdbg_delete_breakpoint(zend_ulong num)
{
....
if ((brake = phpdbg_find_breakbase_ex(num, &table, &numkey, &strkey))) {
int type = brake->type;
char *name = NULL;
size_t name_len = 0L;
switch (type) {
....
default: {
if (zend_hash_num_elements(table) == 1) {
PHPDBG_G(flags) &= ~(1<<(brake->type+1)); // <=
}
}
}
....
}
}
数学爱好者们，别放松。flags 变量是 unsigned long int 类型，而 brake->type 是 int 类型。代码设计用于从 flags 中移除特定的比特位。现在，让我们仔细看看到底发生了什么：
int类型的常量1向左移动了一定数量的比特位。大多数情况下，int类型是32位的。我们希望移位不是32位或更多，否则我们会得到未定义行为。- 移位的结果被按位取反。取反的结果仍然是
int类型。 - 由于左操作数，取反的结果被扩展为64位无符号类型。因为原始类型是有符号的，所以会发生符号扩展。这意味着对于正数，高32位将包含零比特，而对于负数，将包含一比特。
- 按位"与"操作将转换结果应用于
flags。当右操作数为正数时，flags中的有效比特位将丢失。只有当向左移位31位时才会如此——当需要清除flags中的第31位时。
注意，对于这样一个看似无害的表达式，我们需要记住多少东西？问题在于操作数的大小不同以及一些子表达式的符号。要修复它，开发人员只需要将常量 1 的类型从 int 改为 unsigned long long，代码就会按预期执行：
PHPDBG_G(flags) &= ~( 1uLL <<(brake->type+1));
第4名
我曾经看到一个年轻的牛仔追捕一个亡命徒。他在当地一家酒吧的死胡同里把罪犯逼到了绝境，但他没有向罪犯开枪，而是向一面布满灰尘的镜子里自己的倒影开枪，镜子被打成了碎片。
PVS-Studio警告：V794 赋值运算符应防止'this == &other'的情况。fs_path.cpp 36
FsPath& FsPath::operator=(FsPath&& other)
{
m_path = std::move(other.m_path);
other.m_path.clear();
return *this;
}
在这个代码片段中，我们有 FsPath 类的移动赋值运算符，它将数据从另一个对象转移到当前实例。但是，没有检查自赋值 (this == &other)，这可能导致意外的后果。
如果尝试将对象赋值给它自己，m_path = std::move(other.m_path); 操作会将 other.m_path 的内容移动到 m_path 中，随后调用 other.m_path.clear(); 会清除数据。结果，m_path 最终处于意外状态，只能祝开发人员调试愉快了 :)
为了消除风险，我们建议在运算符的开头添加以下检查：
if (this == std::addressof(other))
{
return *this;
}
使用 std::addressof 而不是 & 运算符可以确保即使在类中重载了 & 运算符时也能正确进行地址比较。
第3名
我曾经看到一个萨满试图在没有到达圣地的情况下召唤灵魂。一个灵魂来了，但完全不是预期的那个——而是来自附近峡谷的一只郊狼。
PVS-Studio警告：V1099 在初始化基类'modal_dialog'时使用未初始化的派生类的'window_id'函数将导致未定义行为。install_dependencies.hpp 第29行
class install_dependencies : public modal_dialog
{
public:
explicit install_dependencies(const addons_list& addons)
: modal_dialog(window_id()), addons_(addons) // <=
{}
....
private:
virtual const std::string& window_id() const override;
....
}
多亏了这个代码片段，我可以告诉你更多关于未定义行为的信息。
如上所示，install_dependencies 类派生自 modal_dialog 类。在 install_dependencies 构造函数中，基类使用非静态的 window_id 函数返回的值进行初始化。所以，会发生以下情况：
- 初始化列表的执行：
-
调用
install_dependencies::window_id方法； -
调用
modal_dialog类的构造函数； - 初始化
addons_数据成员；install_dependencies类构造函数体的执行。
这导致调用了尚未初始化的类对象的函数！这违反了标准规则：
对于正在构造的对象，可以调用成员函数（包括虚成员函数）。
类似地，正在构造的对象可以是 typeid 运算符或 dynamic_cast 的操作数。
但是，如果在所有基类的 mem-initializer 完成之前，在 ctor-initializer 中（或从 ctor-initializer 直接或间接调用的函数中）执行这些操作，则程序具有未定义行为。
但是，等等，还有更多！你可能已经注意到，window_id 成员函数是虚函数，并在 install_dependencies 类中被重写。以后当程序员编写一个重写 window_id 的派生类时，可能会出现一些问题。
当创建这个派生类的对象并执行 installed_dependencies 构造函数时，还没有关于新重写存在的信息。因此，在初始化列表中总是会调用 installed_dependencies::window_id 函数。这可能与开发人员的初衷不同。
第2名
我认识一个牛仔，他朝墙上的影子开枪，误以为那是潜伏的敌人。枪声轰鸣，灰泥碎裂，只在墙上留下了一个洞。目标根本不存在，而修补工作却费了很大功夫。
PVS-Studio警告：V575 空指针被传递给'fseek'函数。检查第一个参数。vid_ati_eeprom.c 61
void
ati_eeprom_load_mach8(ati_eeprom_t *eeprom, char *fn, int mca)
{
FILE *fp;
....
fp = nvr_fopen(eeprom->fn, "rb");
size = 128;
if (!fp) {
if (mca) {
(void) fseek(fp, 2L, SEEK_SET); // <=
memset(eeprom->data + 2, 0xff, size - 2);
fp = nvr_fopen(eeprom->fn, "wb");
fwrite(eeprom->data, 1, size, fp);
....
}
我们需要加载存储在视频适配器 NVRAM 中的数据，这些数据保存在一个二进制文件中。如果文件不存在，我们需要用"默认"数据创建它。让我们看看文件缺失的情况。我们移动了文件指针，但它是空指针。结果，我们得到了一个 fp 空指针解引用。
让我们仔细看看 fseek。C11 标准没有定义函数第一个参数的要求，也不保证对 NULL 的检查。这意味着这取决于标准库开发人员来正确处理它。
拨动电源开关
我们从想象的架子上拿出一台 IBM PS/2 model 55SX，并"插入"由 ATI 制造的 IBM 8514/A 2D 加速器。
1330_top_cpp_2025/image2.png第一个测试对象是使用 MinGW 构建的 Windows 实例。我们在启动前确保 NVRAM 文件不存在——我们检查 %userprofile%\86Box VMs\<virtual machine name>\nvr 目录下的 ati8514_mca.nvr 文件。如果存在，我们就删除它。
1330_top_cpp_2025/image3.png打开电源，然后...
1330_top_cpp_2025/image4.png没有爆炸！一切都好：NVRAM 文件已写入，计算机正在运行，glibc 的烟雾测试完成。未检测到缺陷。
1330_top_cpp_2025/image5.png接下来是 FreeBSD。libc 库在这个操作系统中实现了标准 C 库。这对于所有 BSD 系列的操作系统来说通常是正确的。
我们使用相同的配置。我们检查 ~/.local/share/86Box/Virtual Machines/<virtual machine name>/nvr 路径下是否缺少 ati8514_mca.nvr NVRAM 文件。三、二、一、通电...
1330_top_cpp_2025/image6.png嗯，只有 Ben Grubbs' 最近的过去中的一个事件能更好地描述这种情况 :)
1330_top_cpp_2025/image7.png在爆炸后紧闭的双眼睁开后，我们看向控制台：我们确认了异常退出！
void VMManagerSystem::launchMainProcess() Full Command:
"/root/86Box/build_freebsd/src/86Box"
("--vmpath", "/root/.local/share/86Box/Virtual Machines/somevm",
"--vmname",
"somevm")
Connection received on 86Box.socket.5876c5
Connection disconnected
Abnormal program termination while launching main process:
exit code 11, exit status QProcess::CrashExit
模拟器可执行文件旁边出现了一个核心转储文件。让我们欢迎 LLDB：
root@freebsd:~/86Box/build_freebsd/src # lldb 86Box -c 86Box.core
(lldb) target create "86Box" --core "86Box.core"
Core file '/root/86Box/build_freebsd/src/86Box.core' (x86_64) was loaded.
(lldb) bt
* thread #1, name = '86Box', stop reason = signal SIGSEGV
* frame #0: 0x0000000832f880bf
libc.so.7`_flockfile(fp=0x0000000000000000)
at _flock_stub.c:65:20
frame #1: 0x0000000832f8b675
libc.so.7`fseek(fp=0x0000000000000000, offset=2, whence=0)
at fseek.c:62:2
frame #2: 0x00000000018cd964
86Box`ati_eeprom_load_mach8(eeprom=...., fn=<unavailable>, mca=1)
at vid_ati_eeprom.c:61:20
fp 空指针制造了一场壮观的烟火秀——无法锁定文件，因为其描述符无效。不幸的是，LLDB 并不想在实时模式下工作，要么安静地显示 lost connection，要么伴随着巨响和特效崩溃。因此，我无法像在 Windows 中那样向您展示代码是如何执行的。
第1名
我认识邻镇的一位警长，他曾在审讯记录中写道，一名证人证实了他自己的证词。法庭最终也没弄清这究竟是失误还是巧妙的辩护策略。
PVS-Studio警告：V501 '==' 运算符的左右两侧有相同的子表达式：PeekArg.getValNo() == PeekArg.getValNo() PPCISelLowering.cpp 7865
SDValue PPCTargetLowering::LowerCall_AIX(....) const {
....
for (unsigned I = 0, E = ArgLocs.size(); I != E;) {
....
CCValAssign &GPR1 = VA;
....
if (I != E) {
// If only 1 GPR was available, there will only be one custom GPR and
// the argument will also pass in memory.
CCValAssign &PeekArg = ArgLocs[I];
if (PeekArg.isRegLoc() && PeekArg.getValNo() == PeekArg.getValNo()) // <=
{
assert(PeekArg.needsCustom() && "A second custom GPR is expected.");
CCValAssign &GPR2 = ArgLocs[I++];
RegsToPass.push_back(std::make_pair(GPR2.getLocReg(),
DAG.getZExtOrTrunc(ArgAsInt, dl, MVT::i32)));
}
}
....
}
我们暂时假设这是又一个复制粘贴的受害者。让我们检查一下 getValNo 是否有任何副作用：
class CCValAssign{
....
unsigned ValNo;
unsigned getValNo() const { return ValNo; }
}
这里没什么奇怪的。看看最近的提交：
CCValAssign &GPR1 = VA;
....
assert(I != E && "A second custom GPR is expected!");
CCValAssign &GPR2 = ArgLocs[I++];
assert(GPR2.isRegLoc() && GPR2.getValNo() == GPR1.getValNo() &&
GPR2.needsCustom() && "A second custom GPR is expected!");
RegsToPass.push_back(
std::make_pair(GPR2.getLocReg(),
DAG.getZExtOrTrunc(ArgAsInt, dl, MVT::i32)));
意图很明确：一个先前由断言保护的异常情况被重新设计成了一个常规分支。提交文本也指出了这一点。
此补丁实现了将函数调用参数放入堆栈内存的调用方部分。这消除了当前限制，即当参数无法包含在寄存器中时，AIX 上的 LLVM 将报告致命错误。
请注意，除了发现的错误之外，还有一个奇怪的赋值：
CCValAssign &PeekArg = ArgLocs[I];
....
CCValAssign &GPR2 = ArgLocs[I++]; // 这里 PeekArg == GPR2
开发人员可能想写这样的东西：
if (I != E) {
CCValAssign &GPR2 = ArgLocs[I];
if (GPR2.isRegLoc() && PeekArg.getValNo() == GPR1.getValNo())
{
assert(PeekArg.needsCustom() && "A second custom GPR is expected.");
I++;
RegsToPass.push_back(std::make_pair(
GPR2.getLocReg(), DAG.getZExtOrTrunc(ArgAsInt, dl, MVT::i32)));
}
}
但为了清晰起见，开发人员将 PeekArg 从 GPR2 中分离出来，以表明与之前无条件的代码不同，现在参数需要先被"窥视"。而在复制粘贴过程中，GPR1 不小心从条件中丢失了。
修正后的 if 可能应该是：
if (PeekArg.isRegLoc() && PeekArg.getValNo() == GPR1.getValNo())
有趣的是，在迁移到 GitHub 之前，LLVM 有一个代码审查平台，该提交包含一个指向该网站的链接。在那里，我们可以看到人工审查并不总能挽救局面：
酒馆里一片寂静，只有入口门的吱呀声和壁炉里木柴的噼啪声打破宁静。2025年十个最大胆的代码缺陷现在只是代代相传的故事。
这些案例展示了即使在经验丰富的开发者手中，微小的疏忽也可能导致严重的后果。从类型转换错误到资源管理问题，从逻辑缺陷到未定义行为，每个Bug都提醒我们代码质量的重要性。
在软件开发这个"狂野西部"中，严谨的编码习惯、彻底的代码审查以及合适的工具辅助，都是确保代码可靠性的重要手段。希望这些案例能帮助开发者在日常工作中避免类似的陷阱。
共同学习，写下你的评论
评论加载中...
作者其他优质文章
