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

[Volker Simonis]

And I'm not quite sure how to fix this in HotSpot.

I first thought I could solve this with an anonymous union like:

union {
  void* _relocbuf[ _relocbuf_size ];
  Relocation _reloc;
}
Relocation* reloc() const { return &_reloc; }

but unfortunately I can't put a Relocation into a union because it is
not a POD (at least not with C++98).

Any other ideas how we could fool GCC 4.9?

I more and more think this should be fixed in GCC because I can
imagine this will also break other code.

Regards,
Volker


On Thu, Apr 24, 2014 at 5:10 PM, Volker Simonis
<volker.simonis at gmail.com> wrote:
> Wow, that was fast!
>
> I had just distilled a preprocessed version of 'assembler_x86.cpp'
> down to about 1800 lines of code and guess what - I just discovered
> and analyzed the strange way how RelocationHolder is allocated and
> casted to a Relocation.
>
> Thanks for the patch and I agree with you that we should fix the HotSpot.
>
> Nevertheless I think gcc should be fixed as well. I shouldn't generate
> incomplete functions like for "Assembler::reachable(AddressLiteral
> adr)" in the example from my previous mail.
>
> Regards,
> Volker
>
>
> On Thu, Apr 24, 2014 at 4:51 PM, 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 ];
>>
>>  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
>>
>>
>>
>>
>>