Sign in to follow this  
Miss

Segfault on GCC 6 release builds

Recommended Posts

Judging by the disassembly, it seems like it completely excludes this part of the code from the binary and crashes when trying to access member variables. (Because optimization says this can never be nullptr, I suppose?)
 
	// Allow call on null pointer
	if (this == 0) return 0;
Crash is on the first mov instruction here:
 
(gdb) disas
Dump of assembler code for function asCTypeInfo::CastToObjectType():
=> 0x00005555557acc80 <+0>:     mov    eax,DWORD PTR [rdi+0x30]
   0x00005555557acc83 <+3>:     test   eax,0x2000003
   0x00005555557acc88 <+8>:     je     0x5555557acca0 <asCTypeInfo::CastToObjectType()+32>
   0x00005555557acc8a <+10>:    test   eax,0x1000000
   0x00005555557acc8f <+15>:    mov    eax,0x0
   0x00005555557acc94 <+20>:    cmove  rax,rdi
   0x00005555557acc98 <+24>:    ret    
   0x00005555557acc99 <+25>:    nop    DWORD PTR [rax+0x0]
   0x00005555557acca0 <+32>:    xor    eax,eax
   0x00005555557acca2 <+34>:    ret
GCC 6 changelist mentions this:

Value range propagation now assumes that the this pointer of C++ member functions is non-null. This eliminates common null pointer checks but also breaks some non-conforming code-bases (such as Qt-5, Chromium, KDevelop). As a temporary work-around -fno-delete-null-pointer-checks can be used. Wrong code can be identified by using -fsanitize=undefined.

Share this post


Link to post
Share on other sites

It would be easier to understand if you'd posted the full code instead of just that one line. There is definitely a test there which, if successful, jumps to the end and returns zero. (That's the xor eax,eax bit - anything xored with itself becomes zero.) No idea why the value there is 0x2000003 but maybe someone more competent in this area can explain.

 

Why do you need to do this anyway? Why not having it as a free-standing function which checks for null directly?

Share this post


Link to post
Share on other sites
This is not my code, it's inside of Angelscript smile.png

It's also "fixed" by passing the -fno-delete-null-pointer-checks compiler flag, confirming that the nullptr check gets optimized out.

0x20000003 are flags checked against in the asCTypeInfo::CastToObjectType() method.

Share this post


Link to post
Share on other sites

Afaik, standard C++ only defines existence of 'this' inside a method. The GNU compiler at least added 'this' also outside methods, where it is(was?) a null pointer.

I never understood how this is ever useful, but maybe it was normal in one of the very first implementations, before there was a proper C++ compiler.

 

It looks like they are now removing that extension.

Share this post


Link to post
Share on other sites

try it with clang, or without any optimizations like -o2 to see that works or not, maybe there is a bug in GCC (as said that kDevelop and qt and chromium have problem with it)
and, was this segfault exist in previous versions?
if yes, this is a bug in gcc

Share this post


Link to post
Share on other sites

maybe there is a bug in GCC (as said that kDevelop and qt and chromium have problem with it)
 

It's not a bug.

Mentioned sw was relying on compiler's specific behavior while walking through the minefield of Undefined Behavior.

It was a matter of time when that mine would explode.

Share this post


Link to post
Share on other sites

The changes you made now cause the following error when building on GCC 6.2.0:

../../source/as_callfunc.cpp: In function ‘int PrepareSystemFunction(asCScriptFunction*, asSSystemFunctionInterface*, asCScriptEngine*)’:
../../source/as_callfunc.cpp:489:47: error: ‘class asCTypeInfo’ has no member named ‘CastToObjectType’
    asSTypeBehaviour *beh = &dt.GetTypeInfo()->CastToObjectType()->beh;


Edit: Changing line 489 to the following fixes it

asSTypeBehaviour *beh = &CastToObjectType(dt.GetTypeInfo())->beh;
Edited by dkrusu

Share this post


Link to post
Share on other sites

Create an account or sign in to comment

You need to be a member in order to leave a comment

Create an account

Sign up for a new account in our community. It's easy!

Register a new account

Sign in

Already have an account? Sign in here.

Sign In Now

Sign in to follow this