segfault in basic_string destructor

Question

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.

I once had a similar problem, which was caused by compiling a DLL with one version of Visual Studio (and the runtime library), but compiling the executable with another version of Visual Studio (and the runtime library). They linked fine, but any calls to the DLL that involved passing std::vector, std::string, etc. would fail in ways that didn't make sense according to the language. I would recommend looking at that first: make sure that everything in the project is built by the same compiler, same version of that compiler, and with essentially the same compiler flags. — Max Lybbert
I don't see anywhere in the constructor that could directly corrupt memory. The only problem I see is that you should save errno before the call to close as close might set errno. Please show the request function instead. — Some programmer dude
Then the problem maybe isn't where you think it is? And at least you can show us the declaration of request? — Some programmer dude
@JoachimPileborg Thanks for that, when I went to copy the declaration I noticed a return type on that function of std::string, however I wasn't returning a std::string. See the update in my question. — developerbmw
@Brett, if you pass -Wall to g++, it should warn about missing return values from functions. No error will be generated by default, though, as the standard only deems it undefined behavior. — Frédéric Hamidi

developerbmw developerbmw · Accepted Answer · 2014-08-04T08:56:10

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)

segfault in basic_string destructor

Update

1 Answers