RFR [XS] 8041658: Use of -fdevirtualize on macroAssembler_x86.o (via -O2) with gcc 4.9.0 creates broken VM

[Christian Thalinger]

On Apr 24, 2014, at 4:51 AM, Andrew Haley <aph at redhat.com> wrote:

> On 04/24/2014 01:12 PM, Andrew Haley wrote:
>> On 04/24/2014 10:42 AM, Volker Simonis wrote:
>> 
>>> could you pleas hold on a little bit.
>>> 
>>> I just found out that for x86_64 we additionally need -fno-devirtualize
>>> when compiling 'assembler_x86.cpp'. Without the option, the compilation of
>>> "Assembler::reachable(AddressLiteral adr)" is totally broken:
>>> 
>>>   0x7ffff6046910 <_ZN9Assembler9reachableE14AddressLiteral>:    push   %rbp
>>>   0x7ffff6046911 <_ZN9Assembler9reachableE14AddressLiteral+1>:    mov
>>> %rsp,%rbp
>>>   0x7ffff6046914:    data32 data32 nopw %cs:0x0(%rax,%rax,1)
>>>   0x7ffff6046920 <_ZN9Assembler19is_polling_page_farEv>:    mov
>>> 0x12d7e69(%rip),%rax
>>> 
>>> As you can see it only contains two instructions before it unconditionally
>>> falls into 'Assembler::is_polling_page_far()'
>>> 
>>> Maybe we should do some more thorough tests on both, x86 and x86_64 with
>>> these settings to avoid follow-up changes.
>>> 
>>> What bothers me however is the fact that we now get this sever error at
>>> several places in the OpenJDK while it doesn't seem to affect others and I
>>> can not see what's special in the coding that triggers the misbehavior?
>> 
>> I think I might be able to.  I'm debugging GCC now.
> 
> I've found it, and I don't think this is a bug.  GCC's interprocedural
> analysis looks at this type:
> 
> class RelocationHolder VALUE_OBJ_CLASS_SPEC {
>  friend class Relocation;
>  friend class CodeSection;
> 
> private:
>  // this preallocated memory must accommodate all subclasses of Relocation
>  // (this number is assertion-checked in Relocation::operator new)
>  enum { _relocbuf_size = 5 };
>  void* _relocbuf[ _relocbuf_size ];

Why is it void* after all?  Can’t it just be a Relocation*?  Not sure if that would fix the problem, though.

> 
> public:
>  Relocation* reloc() const { return (Relocation*) &_relocbuf[0]; }
>  ...
> 
> and decides that no object of type Relocation is reachable from it.
> 
> This is correct: strictly speaking, you can't portably copy a
> Relocation into an array of void* and then cast the void* to a
> Relocation* and expect to use it.
> 
> It could be argued that -fno-strict-aliasing should mean that this
> analysis is incorrect, and all types are reachable.  The attached
> patch fixes GCC to do that.  However, I think it would be better to
> fix HotSpot so that it's correct C++.
> 
> Andrew.
> 
> 
> Index: gcc/ipa-devirt.c
> ===================================================================
> --- gcc/ipa-devirt.c	(revision 209656)
> +++ gcc/ipa-devirt.c	(working copy)
> @@ -1362,8 +1362,9 @@
> 
> 		  /* Only type inconsistent programs can have otr_type that is
> 		     not part of outer type.  */
> -		  if (!contains_type_p (TREE_TYPE (base),
> -					context->offset + offset2, *otr_type))
> +		  if (flag_strict_aliasing
> +		      && !contains_type_p (TREE_TYPE (base),
> +					   context->offset + offset2, *otr_type))
> 		    {
> 		      /* Use OTR_TOKEN = INT_MAX as a marker of probably type inconsistent
> 			 code sequences; we arrange the calls to be builtin_unreachable
> @@ -1441,7 +1442,8 @@
> 	  gcc_assert (!POINTER_TYPE_P (context->outer_type));
> 	  /* Only type inconsistent programs can have otr_type that is
> 	     not part of outer type.  */
> -	  if (!contains_type_p (context->outer_type, context->offset,
> +	  if (flag_strict_aliasing
> +	      && !contains_type_p (context->outer_type, context->offset,
> 			        *otr_type))
> 	    {
> 	      /* Use OTR_TOKEN = INT_MAX as a marker of probably type inconsistent