bug诞生记——临时变量栈变量导致的双杀

Posted 2022-07-29 breaksoftware

        这是《bug诞生记》的第一篇文章。本来想起个文艺点的名字，比如《Satan（撒旦）来了》，但是最后还是想让这系列的重心放在“bug的产生过程”和“缺失的知识点”上，于是就有了本系列这个稍微中性的名称。（转载请指明出于breaksoftware的csdn博客）

        本系列博文的案例将都秉承一个原则——“因知识缺失，而非粗心大意导致”。在实际工作中，粗心大意产生的bug太多了。但这是工作态度或者工作状态导致的，要去解决这个问题，可能会从机制、管理等角度来讲述。但是这不是本系列想讨论的。本系列更想讲述的是一种“无心之过”，即因为相应知识点的缺乏，程序员想当然的做法导致的bug。

        这篇我将讲述一个真实发生在工作中的例子。当然实际的代码和逻辑远比下文例子要复杂很多，我只是抽出比较核心的点来分析。至于为什么要这么做？为什么要这么设计？为什么要这种风格？为什么代码不严谨？……等与问题核心无关的疑问，我都将不做辩论。这个“免责声明”也将贯穿本系列所有例子。

       下面这段代码用于拼装出一个绝对路径

std::string get_name(const std::string& index) 
	char name[16] = "Movie";
	std::string full_name = std::string(name) + index;
	return full_name;


int main() 
	char full_path[32] = "/home/work/";
	std::string name = get_name("1");
	const char* ptr_name = name.c_str();
	strcat(full_path + strlen(full_path), ptr_name);
	printf(full_path);
	return 0;

        它将输出/home/work/Movie1。

        目前它还没什么问题，后来发生的逻辑变更：只要返回固定的/home/work/Movie路径就行了。于是程序员的改法是

std::string get_name(/*const std::string& index*/) 
	char name[16] = "Movie";
	std::string full_name = std::string(name);// + index;
	return full_name;


int main() 
	char full_path[32] = "/home/work/";
	std::string name = get_name(/*"1"*/);
	const char* ptr_name = name.c_str();
	strcat(full_path + strlen(full_path), ptr_name);
	printf(full_path);
	return 0;

        程序员将之前的代码注释掉了，当然这个行为是非常不好的。

        此时一个具有“良知”但是“缺乏知识点”的同事将走上“犯罪”的道路，因为他将抱着“优化代码”的目的去产生了第一个bug。他是这么改的

std::string get_name() 
	char name[16] = "Movie";
	return std::string(name);


int main() 
	char full_path[32] = "/home/work/";
	const char* ptr_name = get_name().c_str();
	strcat(full_path + strlen(full_path), ptr_name);
	printf(full_path);
	return 0;

1. 把注释掉的废弃代码给删掉了。这是值得肯定的。
2. 精简了get_name函数，不再以full_name作为返回值，减少了一次std::string类型的构造和释放。这个也是值得肯定的。
3. 精简了main函数，删除了std::string name局部变量，试图直接从get_name()获取const char*指针。他的想法是好的，但是这步将导致bug。

        这段代码没有经过测试就上线了。因为他和QA都觉得这点小的改动不需要测试。很可惜，自以为是往往要付出代价。因为这段代码的最终执行结果是/home/work，而不是约定的/home/work/Movie。那Movie去哪里了？

        问题就出在第8行代码。一般情况下我们认为它可以翻译为下面两行

std::string temp = get_name();
const char* ptr_name = temp.c_str();

        如果的确是上面这么翻译的，那也没问题。但是实际上，temp是个行内的临时变量，它脱离了该行就被释放了。我们看下反汇编

	const char* ptr_name = get_name().c_str();
 lea         eax,[ebp-148h]  
 push        eax  
 call        get_name (0851672h)  
 add         esp,4  
 mov         dword ptr [ebp-15Ch],eax  
 mov         ecx,dword ptr [ebp-15Ch]  
 call        std::basic_string<char,std::char_traits<char>,std::allocator<char> >::c_str (085166Dh)  
 mov         dword ptr [ptr_name],eax  
 lea         ecx,[ebp-148h]  
 call        std::basic_string<char,std::char_traits<char>,std::allocator<char> >::~basic_string<char,std::char_traits<char>,std::allocator<char> > (08513D9h)  

        第4行调用了get_name方法，返回的std::string对象指针放在eax中。

        第6行将该对象指针放到当前函数栈帧内——即一个临时对象。

        第7行又将临时对象地址放到ecx中。ecx在C++编译中，一般用于传递this指针。

        第8行对ecx中保存的std::string临时对象的this指针调用了c_str成员方法，得到的const char*地址保存在eax中。

        第9行将上一指令返回的const char*地址保存到ptr_name局部变量中，此时ptr_name指向的是std::string临时对象的字符空间地址。它也就在这一刻是该程序员所期望的样子。

        第10行和第11行，又通过ecx调用std::string的析构函数。这样保存在[ebp-148h]中的std::string对象指针指向的临时对象被析构，也就意味着第9步得到的指针数据被删除了。

        所以const char* ptr_name = get_name().c_str();正确的翻译是

const char* ptr_name = NULL;

	std::string temp = get_name();
	ptr_name = temp.c_str();

        最终这个程序员老老实实的把main中那行“精简代码”变成了两行

std::string get_name() 
	char name[16] = "Movie";
	return std::string(name);


int main() 
	char full_path[32] = "/home/work/";	
	std::string name = get_name();
	const char* ptr_name = name.c_str();
	strcat(full_path + strlen(full_path), ptr_name);
	printf(full_path);
	return 0;

        我想经过这次惊魂实践，这个程序员应该再也不想碰这块代码了。

        但是肯定还有其他“有良知”的程序员，他会觉得这代码很不爽——搞什么std::string？转来转去，最终还是用了const char*。

        于是他修改如下

const char* get_name() 
	char name[16] = "Movie";
	return name;


int main() 
	char full_path[32] = "/home/work/";	
	const char* ptr_name = get_name();
	strcat(full_path + strlen(full_path), ptr_name);
	printf(full_path);
	return 0;

1. 将get_name方法的返回类型改成了const char*，省去了中间std::string的转换。
2. 将main中的std::string全干掉了。

        这段代码修改的足够简单了。有人可能会觉得get_name可能可以干掉，直接在main函数中写死路径就行了。我想当时这个程序员保留get_name的原因可能是他预测该函数可能还是存在被定制的可能性。

        抛开其他问题，这段程序的执行结果是正确的。相信这个版本的修改者此时内心是澎湃的，因为他觉得他不费吹灰之力，就干了一件好事。可是实际他却挖了一个坑。

        这段代码在线上稳定运行了很久，后来随着业务逻辑的增长。一个同学不小心在第8和第9行代码之间插入了一个函数调用，然后这个程序就崩掉了。相信这个同学一定很郁闷，因为他可能仅仅修改了一下函数调用顺序，或者写了一个足够简单到不太可能出错的代码。结果进程崩溃，他要背锅！

        为了把问题简单化，我让新插入的代码只干一件事——初始化一个栈上空间。

const char* get_name() 
	char name[16] = "Movie";
	return name;


void satan() 
	char name[16] = "I am Satan";


int main() 
	char full_path[32] = "/home/work/";	
	const char* ptr_name = get_name();
	satan();
	strcat(full_path + strlen(full_path), ptr_name);
	printf(full_path);
	return 0;

        这个程序的输出是/home/work/I am Satan。仅仅加了一个satan函数，就改变了结果？是的。这是由于之前那个做代码修改的同学对栈变量和栈帧不熟悉导致的。

        如果要介绍栈变量和栈帧，这个就需要从计算机基础知识讲起。本文当然不会讲的很细，大家可以通过其他渠道获取这些知识。

        下面的图只是表意，不是准确表达栈空间清空。但是核心的意思是一致的。

        进入main函数时，栈结构如下

        红色表示当前活动的栈空间；绿色表示“未知区域”，该区域的数据我们可以认为是“野”的，不可保障的。

        进入get_name函数后，栈结构变化如下

        get_name的name数组将保存“Movie”。

        get_name调用结束，发生退栈行为。于是程序又回到main函数中，其栈结构如下

        注意一下，第12行代码已经让ptr_name指向了“野”空间。此时“野”空间数据还没被污染，所以执行结果还是正确的。

        然后我们调用了satan函数。进入satan函数后栈结构如下

        注意一下，之前get_name的name空间已经被satan的name空间覆盖了。此时它的数据是“I am Satan”。

        satan函数结束后，又回到main函数空间，其栈结构如下

        我们看到，此时ptr_name还是指向“野”空间。只是此时空间不再是调用get_name时的情况，栈上数据已经被satan函数“污染”了。

        所以出现错误的结果是必然的。

        这段不到15行的代码经过多个程序员的修改后，仿佛成了一个江湖。其中发生了各种因为“知识点缺乏”而导致的bug。所以如果我们不知道代码最终精确的表达，可能就会写出各种“不经意”的问题。