GCC 4.9.2 -O3 gives a seg fault / GCC 4.8.2 -O3 works
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.):
0x00400a51 <+17>:mov(%rdi),%r14
which puts this->data (null) into %r14. Later on, with no intervening
reset of r14, we see this:
0x00400ac5 <+133>: cmp%r12,%r14
0x00400ac8 <+136>: je 0x400b18
0x00400aca <+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
rip0x400aca 0x400aca
(gdb) i r r14
r140x0 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
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
#include
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;
}
Re: GCC 4.9.2 -O3 gives a seg fault / GCC 4.8.2 -O3 works
On 2015.01.06 at 03:18 -0500, Paul Smith wrote: > 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.): > >0x00400a51 <+17>:mov(%rdi),%r14 > > which puts this->data (null) into %r14. Later on, with no intervening > reset of r14, we see this: > >0x00400ac5 <+133>: cmp%r12,%r14 >0x00400ac8 <+136>: je 0x400b18 const*)+216> >0x00400aca <+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 > rip0x400aca 0x400aca > (gdb) i r r14 > r140x0 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. gcc-help is more appropriate for this kind of question. If you compile with gcc-5 and -fsanitize=undefined you'll get: mystring.cpp:104:26: runtime error: null pointer passed as argument 2, which is declared to never be null So you should guard the memcpy() call. -- Markus
Re: GCC 4.9.2 -O3 gives a seg fault / GCC 4.8.2 -O3 works
On Tue, Jan 06, 2015 at 03:18:48AM -0500, Paul Smith wrote: > 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). Your testcase is invalid. GCC trunk -fsanitize=undefined (in particular -fsanitize=nonnull-attribute) diagnoses it: /tmp/mystring.cpp:103:26: runtime error: null pointer passed as argument 2, which is declared to never be null LD_PRELOAD=libmemstomp.so detects it too. Calling memcpy (p, NULL, 0); is invalid according to C and C++ standards, you need to guard it, e.g. with if (data) memcpy (p, data, len1); or if (len1) memcpy (p, data, len1); Jakub
Bad link on gcc.gnu.org front page
Hello! There's a link to "GCC5" right on top of the News section on the home page of gcc.gnu.org that takes me to a 403 forbidden access page: https://gcc.gnu.org/gcc-5/ . I think it's a bug. Bye! Tomas
Re: Bad link on gcc.gnu.org front page
On 6 January 2015 at 09:34, wrote: > > Hello! > There's a link to "GCC5" right on top of the News section on the home page of > gcc.gnu.org that takes me to a 403 forbidden access page: > https://gcc.gnu.org/gcc-5/ . > I think it's a bug. Yes https://gcc.gnu.org/bugzilla/show_bug.cgi?id=64469 https://gcc.gnu.org/ml/gcc/2014-12/msg00160.html
Re: Bad link on gcc.gnu.org front page
On Tue, Jan 06, 2015 at 10:27:29AM +, Jonathan Wakely wrote: > On 6 January 2015 at 09:34, wrote: > > > > Hello! > > There's a link to "GCC5" right on top of the News section on the home page > > of gcc.gnu.org that takes me to a 403 forbidden access page: > > https://gcc.gnu.org/gcc-5/ . > > I think it's a bug. > > Yes > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=64469 > https://gcc.gnu.org/ml/gcc/2014-12/msg00160.html I've already changed it to gcc-5/changes.html earlier today. gcc-5/ has not been released and won't for 2-3 months, so it is correct that gcc-5/ is inaccessible. Jakub
Re: GCC 4.9.2 -O3 gives a seg fault / GCC 4.8.2 -O3 works
On Tue, 2015-01-06 at 09:43 +0100, Jakub Jelinek wrote: > On Tue, Jan 06, 2015 at 03:18:48AM -0500, Paul Smith wrote: > > 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). > > Your testcase is invalid. > GCC trunk -fsanitize=undefined (in particular -fsanitize=nonnull-attribute) > diagnoses it: > /tmp/mystring.cpp:103:26: runtime error: null pointer passed as argument 2, > which is declared to never be null > LD_PRELOAD=libmemstomp.so detects it too. > > Calling memcpy (p, NULL, 0); is invalid according to C and C++ > standards, you need to guard it, e.g. with if (data) memcpy (p, data, len1); > or if (len1) memcpy (p, data, len1); Ah interesting. You're right, this is definitely not correct. But since len1 is 0 in this case, no implementation of memcpy() actually tried to dereference the data pointer and so there was no failure (we build and test with clang on OSX and MSVC on Windows, and run with valgrind and ASAN (clang)). I'll have to look at other possible failure situations.
Re: GCC 4.9.2 -O3 gives a seg fault / GCC 4.8.2 -O3 works
On Tue, Jan 06, 2015 at 08:50:58AM -0500, Paul Smith wrote: > On Tue, 2015-01-06 at 09:43 +0100, Jakub Jelinek wrote: > > On Tue, Jan 06, 2015 at 03:18:48AM -0500, Paul Smith wrote: > > > 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). > > > > Your testcase is invalid. > > GCC trunk -fsanitize=undefined (in particular -fsanitize=nonnull-attribute) > > diagnoses it: > > /tmp/mystring.cpp:103:26: runtime error: null pointer passed as argument 2, > > which is declared to never be null > > LD_PRELOAD=libmemstomp.so detects it too. > > > > Calling memcpy (p, NULL, 0); is invalid according to C and C++ > > standards, you need to guard it, e.g. with if (data) memcpy (p, data, len1); > > or if (len1) memcpy (p, data, len1); > > Ah interesting. You're right, this is definitely not correct. But > since len1 is 0 in this case, no implementation of memcpy() actually > tried to dereference the data pointer and so there was no failure (we > build and test with clang on OSX and MSVC on Windows, and run with > valgrind and ASAN (clang)). > > I'll have to look at other possible failure situations. Note, it is even mentioned in GCC 4.9 porting to documentation: https://gcc.gnu.org/gcc-4.9/porting_to.html Jakub
Re: GCC 4.9.2 -O3 gives a seg fault / GCC 4.8.2 -O3 works
On Tue, 2015-01-06 at 09:43 +0100, Jakub Jelinek wrote: > GCC trunk -fsanitize=undefined (in particular > -fsanitize=nonnull-attribute) > diagnoses it: > /tmp/mystring.cpp:103:26: runtime error: null pointer passed as > argument 2, which is declared to never be null Unfortunately adding -fsanitize=undefined in GCC 4.9.2 doesn't notice this (in fact it actually causes the segfault to go away). I can try to build a trunk version for this test, I suppose. > LD_PRELOAD=libmemstomp.so detects it too. > > Calling memcpy (p, NULL, 0); is invalid according to C and C++ > standards, you need to guard it, e.g. with if (data) memcpy (p, data, > len1); > or if (len1) memcpy (p, data, len1); I'm on a Debian-based system and can't find a memstomp package so I grabbed git://fedorapeople.org/home/fedora/wcohen/public_git/memstomp and built it myself, but for some reason it doesn't fire in my environment: $ LD_PRELOAD=/home/psmith/src/memstomp/.libs/libmemstomp.so ./tst memstomp: 0.1.4 sucessfully initialized for process tst (pid 26438). Segmentation fault (core dumped) Even if I rebuild without -O3 it passes with no warnings. My GCC installation uses --sysroot to build against an older glibc, etc. so maybe that's causing some sort of issue... Seems like I have some work to do here to come up with a way to detect other failure situations like this.
Re: GCC 4.9.2 -O3 gives a seg fault / GCC 4.8.2 -O3 works
On Tue, Jan 06, 2015 at 11:28:52AM -0500, Paul Smith wrote: > On Tue, 2015-01-06 at 09:43 +0100, Jakub Jelinek wrote: > > GCC trunk -fsanitize=undefined (in particular > > -fsanitize=nonnull-attribute) > > diagnoses it: > > /tmp/mystring.cpp:103:26: runtime error: null pointer passed as > > argument 2, which is declared to never be null > > Unfortunately adding -fsanitize=undefined in GCC 4.9.2 doesn't notice > this (in fact it actually causes the segfault to go away). > > I can try to build a trunk version for this test, I suppose. Right: -fsanitize=nonnull-attribute is a GCC 5 only thing. Marek
