VM管理C++程序执行replace方法时触发段错误问题排查

问题背景 #

开发了一款跨主机虚拟机（VM）管理C++程序，可获取VM所需CPU、RAM资源及主机的CPU、RAM信息，支持VM添加至主机、替换已有VM或拒绝部署。测试add方法运行正常，但执行replace方法时触发Segmentation Fault（段错误）。排查发现，replace方法开头获取vMachines.size()时返回极大数值，其他函数中该数值正常。

程序代码 #

#include <iostream>
#include <string>
#include <vector>

enum class Params{error, success, move};

class Logs
{
public:
    Logs() = default;
    ~Logs() = default;
    void add(std::string msg, int param)
    {
        switch (param){
            case static_cast<int>(Params::error):
                addParam = "\x1b[31;1m";
                break;
            case static_cast<int>(Params::success):
                addParam = "\x1b[32;1m";
                break;
            case static_cast<int>(Params::move):
                addParam = "\x1b[33;1m";
                break;
            default:
                addParam = "";
            }

        logList.push_back(addParam + msg + addParam + "\n");
    }

    void check()
    {
        if(logList.empty())
        {
            std::cout << "\n\nLOGLIST IS EMPTY!!!\n\n";
        }
        else
        {
            for(auto i : logList)
            {
                std::cout << i;
            }
            logList.clear();
        }
        
    }
private:
    std::vector<std::string> logList;
    std::string addParam;
};

class VM
{
public:
    VM(int inp_cpu_v, int inp_ram_v, std::string inp_name_v)
    {
        RAM_V = inp_ram_v;
        CPU_V = inp_cpu_v;
        name = inp_name_v;

    }
    ~VM() = default;
    int RAM_V {};
    int CPU_V {};
    std::string name {""};
};

class Hosts
{
public:
    Hosts(int inp_cpu_h, int inp_ram_h, std::string name)
    {
        RAM_H = inp_ram_h;
        CPU_H = inp_cpu_h;
        RAM_curr = inp_ram_h;
        CPU_curr = inp_cpu_h;
        hostName = name;
    }
    ~Hosts() = default;
    bool add(VM vrM)
    {
        std::cout << "\n\nvMachines size: " << vMachines.size() << "\n\n";
        if(checkFree())
        {
            log1.add("Here's some space! Check if it's enough...", 1);
            if(checkEnough(vrM.RAM_V, vrM.CPU_V))
            {
                log1.add("Enough space! Adding...", 1);
                vMachines.push_back(vrM);

                RAM_curr -= vrM.RAM_V;
                CPU_curr -= vrM.CPU_V;
                return true;
            }
            else 
            {
                log1.add("Not enough space! Check if some VM can be replaced", 0);
                return false;
            }
        }
        else
        {
            log1.add("Here isn't any space!", 0);
            return false;
        }
    }

    bool checkFree()
    {
        if(RAM_curr > 0 && CPU_curr > 0) return true;
        else return false;
    }

    bool checkEnough(int ram, int cpu)
    {
        if(RAM_curr >= ram && CPU_curr >= cpu) return true;
        else return false;
    }

    void log_check()
    {
        log1.check();
    }

    bool replace(VM vm_to_replace, std::vector<Hosts> avaibleHosts)
    {
        //PROBLEM HERE. vMachines.size() is too big
        std::cout << "\n\nvMachines size: " << vMachines.size() << "\n\n";
        // log1.add("Tring to replace some VM with " + vm_to_replace.name, 2);

        if(vMachines.empty() && vMachines.size() == 1)
        {
            log1.add("Failure! There're no VM to swap with... ", 0);
            return false;
        }
        else
        {
            std::vector<VM> max {};
            for(int i = 0; i < (vMachines.size()-1); i++)
            {
                if(vMachines[i].CPU_V < vMachines[i+1].CPU_V)
                {
                    max.push_back(vMachines[i]);
                    vMachines[i] = vMachines[i+1];
                    vMachines[i+1] = max.back();
                    max.clear();
                }
            }
            std::cout << "\n\nTEST SORT OF vMACHINES ON " << hostName << ": \n";
            for(int j = 0; j < vMachines.size(); j++)
            {
                std::cout << vMachines[j].CPU_V << " ";
            }
            return true;
        }
            return true;
    }


// private:
    int RAM_H {};
    int CPU_H {};
    int RAM_curr {};
    int CPU_curr {};
    std::vector<VM> vMachines {};
    Logs log1;
    std::string hostName {};
};


int main()
{
    std::vector <Hosts> hostList {};
    std::vector <VM> vmList {};
    Hosts host1(32, 16, "host1"); //CPU, RAM
    Hosts host2(32, 16, "host2");
    hostList.push_back(host1);
    hostList.push_back(host2);
    VM vm1(2, 2, "vm1"); //CPU, RAM
    VM vm2(2, 2, "vm2");
    VM vm3(30, 2, "vm3");
    VM vm4(2, 2, "vm4");
    vmList.push_back(vm1);
    vmList.push_back(vm2);
    vmList.push_back(vm3);
    vmList.push_back(vm4);

    for(int i = 0; i < vmList.size(); i++)
    {
        std::cout << "\x1b[37;0mTry to set " << vmList[i].name << " to... ";
        for(int j = 0; j < hostList.size(); j++)
        {
            std::cout << "\x1b[37;0mhost #\x1b[37;0m" << j + 1 << "...\n";
            if(hostList[j].add(vmList[i])) 
            {
                hostList[j].log_check();
                break;
            }
            else 
            {
                if(hostList[i].replace(vmList[i], hostList))
                {
                    hostList[j].log_check();
                    break;
                }
                else
                {
                    hostList[j].log_check();
                    continue;
                }
                
            }
        }
    }

    std::cout << "\nHost resources: \n";

    for(int k = 0; k < hostList.size(); k++)
    {
        std::cout << "Host #" << k + 1 << "\nCPU: " << hostList[k].CPU_curr << "\nRAM: " << hostList[k].RAM_curr;
        std::cout << "\nvMachines: \n";
        for(int e = 0; e < hostList[k].vMachines.size(); e++)
        {
            std::cout << e + 1 << ") " << hostList[k].vMachines[e].name << "\n";
        }

        std::cout << "\n\n";
    }


    return 0;
}

错误原因分析 #

1. 越界访问Hosts容器元素 #

main函数中，当hostList[j].add(vmList[i])返回false时，调用的是hostList[i].replace(...)，这里的i是vmList的索引，而hostList只有2个元素（索引0和1）。当处理第三个VM（i=2）时，hostList[i]访问了容器边界外的内存区域，此时获取的vMachines是未初始化的vector对象，其size()返回随机的极大值，后续对这个非法vector的操作直接触发段错误。

正确逻辑应该是调用当前循环的主机对象，即hostList[j].replace(...)。

2. Replace方法中的逻辑错误 #

replace方法中的条件判断：

if(vMachines.empty() && vMachines.size() == 1)

完全不合理——vMachines.empty()为true时，size()必然是0，这个条件永远无法成立，导致本该触发的错误提示逻辑失效。

3. 参数传递效率问题 #

replace方法的第二个参数std::vector<Hosts> avaibleHosts采用值传递，会拷贝整个主机列表，不仅效率低下，若后续需要修改主机列表，拷贝对象的修改也不会同步到原列表，应该改为引用传递：std::vector<Hosts>& avaibleHosts。

修复建议 #

1. 修正main函数中的replace调用：

// 原代码
if(hostList[i].replace(vmList[i], hostList))
// 修改为
if(hostList[j].replace(vmList[i], hostList))

2. 修正replace方法中的条件判断：

// 原代码
if(vMachines.empty() && vMachines.size() == 1)
// 修改为（根据需求，比如至少需要1个VM才能替换）
if(vMachines.empty() || vMachines.size() < 1)

3. 修改replace方法的参数为引用传递：

bool replace(VM vm_to_replace, std::vector<Hosts>& avaibleHosts)

内容的提问来源于stack exchange，提问作者Egor Egor