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

[Andrew Haley]

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 ];

 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