brettwhiteman
brettwhiteman

Reputation: 4452

segfault in basic_string destructor

I have a function, ModuleManager::tick(), code is below:

void ModuleManager::tick()
{
auto now = std::chrono::steady_clock::now();
auto nowSys = std::chrono::system_clock::now().time_since_epoch();

for(auto& m : m_modules)
{
    if(std::chrono::duration_cast<std::chrono::seconds>(now - m.second) >=
        std::chrono::seconds(m.first.m_settings.m_interval))
    {
        std::string result = m.first.run(); //run() returns a std::string
        m.second = now;

        try
        {
            HTTPConn conn("127.0.0.1", 80);

            conn.request("POST", "/", std::vector<std::string>{"Host: localhost", "Connection: close"}, result);
        }
        catch(HTTPException& e)
        {
            Log::write(e.getErrorString());
        }
    }
}

The program segfaults upon returning from the HTTPConn::request() function, in the basic_string destructor (have used GDB to ascertain this). If I comment out all the code inside the request() function, the segfault still occurs, so the problem must be outside of that function.

I believe the problem is that, somewhere in my HTTPConn constructor I corrupt the heap. The code for this is below:

HTTPConn::HTTPConn(const std::string& host, int port)
{
addrinfo hints;
addrinfo* res;
memset(&hints, 0, sizeof(hints));

hints.ai_family = AF_UNSPEC;
hints.ai_socktype = SOCK_STREAM;

int result = getaddrinfo(host.c_str(), std::to_string(port).c_str(), &hints, &res);

if(result)
{
    throw HTTPException(HTTPE_GETADDRINFO_FAILED);
}

addrinfo* ptr = res;
bool validSocket = false;

while(ptr)
{
    m_socket = socket(ptr->ai_family, ptr->ai_socktype, ptr->ai_protocol);

    if(m_socket == -1)
    {
        ptr = ptr->ai_next;
    }
    else
    {
        validSocket = true;
        break;
    }
}

if(!validSocket)
{
    freeaddrinfo(res);
    throw HTTPException(HTTPE_SOCKET_FAILED);
}

result = connect(m_socket, ptr->ai_addr, ptr->ai_addrlen);

freeaddrinfo(res);

if(result == -1)
{
    close(m_socket);
    m_socket = -1;

    if(errno == ECONNREFUSED)
    {
        throw HTTPException(HTTPE_CONNECTION_REFUSED);
    }
    else if(errno == ENETUNREACH)
    {
        throw HTTPException(HTTPE_NETWORK_UNREACHABLE);
    }
    else if(errno == ETIMEDOUT)
    {
        throw HTTPException(HTTPE_TIMED_OUT);
    }
    else
    {
        throw HTTPException(HTTPE_CONNECT_FAILED);
    }
}
}

I apologize for the large amounts of code; I attempted to make a short, self-contained example but was unable to reproduce the problem.

Update

So the problem seems to be that I wasn't returning any std::string object in the HTTPConn::request function, but it was declared as having a std::string return type. My questions is now: why did this compile? This is the command line I used to compile it, using g++ 4.8.2:

g++ -Iinclude -std=c++11 -g -D__DEBUG -c src/HTTP.cpp -o obj/HTTP.o

No warnings or errors were issued.

Upvotes: 3

Views: 4057

Answers (1)

brettwhiteman
brettwhiteman

Reputation: 4452

The problem was that I had declared the HTTPConn::request() function with a return type of std::string, but wasn't returning anything. As Frédéric Hamidi states, this causes undefined behaviour.

In my opinion this should be a warning that is enabled by default in g++, seeing as it results in undefined behaviour. Or perhaps it should be an error. Adding the -Wall flag to the compilation command enables this warning (or -Wreturn-type to enable just that specific warning)

Upvotes: 5

Related Questions