Hi all.  It's possible my code is doing something illegal, but it's also
possible I've found a problem with -O3 optimization in GCC 4.9.2.  I've
built this same code with GCC 4.8.2 -O3 on GNU/Linux and it works fine.
It also works with GCC 4.9.2 with lower -O (-O2 for example).

When I try a build with GCC 4.9.2 -O3 I'm seeing a segfault, because we
get confused and invoke code that we should be skipping.

I've compressed the test down to a self-contained sample file that I've
attached here.  Save it and run:

  g++-4.9.2 -g -O3 -o mystring mystring.cpp

Then:

  ./mystring
  Segmentation fault

You can also add -fno-strict-aliasing etc. and it doesn't make any
difference.

The seg fault happens in the implementation of operator +=() where we're
appending to an empty string, so this->data is NULL.  That method starts
like this (after the standard pushes etc.):

   0x0000000000400a51 <+17>:    mov    (%rdi),%r14

which puts this->data (null) into %r14.  Later on, with no intervening
reset of r14, we see this:

   0x0000000000400ac5 <+133>:   cmp    %r12,%r14
   0x0000000000400ac8 <+136>:   je     0x400b18 <String::operator+=(char 
const*)+216>
   0x0000000000400aca <+138>:   subl   $0x1,-0x8(%r14)

We don't take the jump, and this (+138) is where we get the segfault
because r14 is still 0.  This is in the if-statement in the release()
method where it subtracts 1 from count... but it should never get here
because this->data (r14) is NULL!

  (gdb) i r rip
  rip            0x400aca 0x400aca <String::operator+=(char const*)+138>
  (gdb) i r r14
  r14            0x0      0

Anyone have any thoughts about this?  I know the inline new/delete is
weird but it's required to repro the bug, and we need our own new/delete
because we're using our own allocator.
#include <stdlib.h>

inline __attribute__((always_inline)) void* operator new[](size_t size) throw()
{
    void* p = malloc(size);
    if (p == 0) {
        exit(1);
    }
    return p;
}

inline __attribute__((always_inline)) void operator delete[](void* ptr) throw()
{
    free(ptr);
}

class String
{
public:
    String() : data(NULL) {}
    String(const char* string);

    ~String() { release(); }

    String& operator =(const String& string);
    String& operator +=(const char* str);

    size_t getLength() const { return data ? getData()->length : 0; }
    const char* getString() const { return data ? data : ""; }

    operator const char*() const { return getString(); }

private:
    char* data;

    struct Data
    {
        size_t length;
        unsigned int count;
    };

    char* allocate(size_t length);

    Data* getData() const { return (Data*)(data - sizeof(Data)); }

    void release()
    {
        if (data) {
            Data* sd = getData();
            if (--sd->count == 0) {
                delete[] (char*)sd;
            }

            data = NULL;
        }
    }
};

#include <stdio.h>
#include <string.h>

String::String(const char* string)
{
    data = NULL;

    if (string) {
        size_t length = strlen(string);
        allocate(length);
        memcpy(data, string, length);
    }
}

char* String::allocate(size_t length)
{
    release();

    data = new char[sizeof(Data) + length + 1] + sizeof(Data);
    getData()->count = 1;
    getData()->length = length;
    data[length] = '\0';

    return data;
}

String& String::operator =(const String& string)
{
    if (data != string.data) {
        release();

        if ((data = string.data)) {
            ++getData()->count;
        }
    }
    return *this;
}

String& String::operator +=(const char* str)
{
    size_t len1 = getLength();
    size_t len2 = strlen(str);
    String string;
    char* p = string.allocate(len1 + len2);

    memcpy(p, data, len1);
    memcpy(p + len1, str, len2);

    *this = string;

    return *this;
}

int main()
{
    String init;
    String value("foo");
    init += value;

    printf("init = %s\n", init.getString());
    return 0;
}

Reply via email to