[java] : Intialize va_list_type_node to avoid segfault for x86_64 targets in respect for alignment

2011/1/7 H.J. Lu <hjl.tools@gmail.com>:
> On Fri, Jan 7, 2011 at 11:20 AM, Kai Tietz <ktietz70@googlemail.com> wrote:
>> 2011/1/7 H.J. Lu <hjl.tools@gmail.com>:
>>> On Fri, Jan 7, 2011 at 5:51 AM, Kai Tietz <ktietz70@googlemail.com> wrote:
>>>> 2011/1/7 Andrew Haley <aph@redhat.com>:
>>>>> On 01/07/2011 11:42 AM, Kai Tietz wrote:
>>>>>>
>>>>>> Hello,
>>>>>>
>>>>>> we noticed that java doesn't initialize in decl.c
>>>>>> (java_init_decl_processing) the va_list_type_node. This leads to
>>>>>> issues in some architectures for wrong checks and/or ICE (as for
>>>>>> x86_64 one in i386.c (ix86_local_alignment)).
>>>>>>
>>>>>> ChangeLog
>>>>>>
>>>>>> 2011-01-07  Kai Tietz
>>>>>>
>>>>>>         * decl.c (java_init_decl_processing): Setup va_list_type_node.
>>>>>>
>>>>>> Tested for i686-pc-cygwin and x86_64-w64-mingw32. Ok for apply?
>>>>>
>>>>> Sure.
>>>>>
>>>>> Andrew.
>>>>>
>>>>
>>>> Ok, applied to trunk at revision 168569. Is this patch ok for 4.5 branch, too?
>>>>
>>>>
>>>
>>> This caused:
>>>
>>> http://gcc.gnu.org/bugzilla/show_bug.cgi?id=47215
>>>
>>>
>>>
>>> --
>>> H.J.
>>>
>>
>> Hmm, I see that java doesn't initialize unsigned_type_node. HJ does it
>> solve the issue if you add 'unsigned_type_node = make_unsigned_type
>> (INT_TYPE_SIZE);' before the target hook in java/decl.c 't =
>> targetm.build_builtin_va_list ();' is called?
>>
>
> Please send me a proper patch.
>
> Thanks.
>
> --
> H.J.
>

Ok, please try this patch. I am just about to do a build for 4.6 on
gcc14 of gcc's cfarm.

Regards,

2011/1/7 Dave Korn <dave.korn.cygwin@gmail.com>:
> On 07/01/2011 19:36, Kai Tietz wrote:
>
>> Index: decl.c
>> ===================================================================
>> --- decl.c      (revision 168580)
>> +++ decl.c      (working copy)
>> @@ -1153,8 +1153,8 @@
>>    soft_lrem_node
>>      = add_builtin_function ("_Jv_remJ", t,
>>                             0, NOT_BUILT_IN, NULL, NULL_TREE);
>> -
>>    /* Initialize va_list_type_node.  */
>> +  unsigned_type_node = make_unsigned_type (INT_TYPE_SIZE);
>>    t = targetm.build_builtin_va_list ();
>>
>>    /* Many back-ends define record types without setting TYPE_NAME.
>
>
>  I think this should go somewhat further up, along with all the other
> xxxx_type_nodes around the start of java_init_decl_processing().
>
>    cheers,
>      DaveK
>

Well, not sure here. The other type_node language ones AFAICS, but
that one is just related to x86_64 ABI's record structure generation.
Btw the other unsigned_type_nodes (like short_... etc) aren't
necessary here.

My multilib bootstrap on linux x86_64 for this patch passed. I got no
segmentation fault on libjava.
So is patch ok for apply? Or should I move the unsigned_type_node
initialization more up?

Regards,
Kai

On Fri, Jan 7, 2011 at 12:39 PM, Kai Tietz <ktietz70@googlemail.com> wrote:
> 2011/1/7 Dave Korn <dave.korn.cygwin@gmail.com>:
>> On 07/01/2011 19:36, Kai Tietz wrote:
>>
>>> Index: decl.c
>>> ===================================================================
>>> --- decl.c      (revision 168580)
>>> +++ decl.c      (working copy)
>>> @@ -1153,8 +1153,8 @@
>>>    soft_lrem_node
>>>      = add_builtin_function ("_Jv_remJ", t,
>>>                             0, NOT_BUILT_IN, NULL, NULL_TREE);
>>> -
>>>    /* Initialize va_list_type_node.  */
>>> +  unsigned_type_node = make_unsigned_type (INT_TYPE_SIZE);
>>>    t = targetm.build_builtin_va_list ();
>>>
>>>    /* Many back-ends define record types without setting TYPE_NAME.
>>
>>
>>  I think this should go somewhat further up, along with all the other
>> xxxx_type_nodes around the start of java_init_decl_processing().
>>
>>    cheers,
>>      DaveK
>>
>
> Well, not sure here. The other type_node language ones AFAICS, but
> that one is just related to x86_64 ABI's record structure generation.
> Btw the other unsigned_type_nodes (like short_... etc) aren't
> necessary here.
>
> My multilib bootstrap on linux x86_64 for this patch passed. I got no
> segmentation fault on libjava.
> So is patch ok for apply? Or should I move the unsigned_type_node
> initialization more up?
>

Let's fix the bootstrap now.

Thanks.

On 07/01/2011 19:36, Kai Tietz wrote:

> Index: decl.c
> ===================================================================
> --- decl.c      (revision 168580)
> +++ decl.c      (working copy)
> @@ -1153,8 +1153,8 @@
>    soft_lrem_node
>      = add_builtin_function ("_Jv_remJ", t,
>                             0, NOT_BUILT_IN, NULL, NULL_TREE);
> -
>    /* Initialize va_list_type_node.  */
> +  unsigned_type_node = make_unsigned_type (INT_TYPE_SIZE);
>    t = targetm.build_builtin_va_list ();
> 
>    /* Many back-ends define record types without setting TYPE_NAME.


  I think this should go somewhat further up, along with all the other
xxxx_type_nodes around the start of java_init_decl_processing().

    cheers,
      DaveK

>>>>> "Kai" == Kai Tietz <ktietz70@googlemail.com> writes:

Kai> My multilib bootstrap on linux x86_64 for this patch passed. I got no
Kai> segmentation fault on libjava.
Kai> So is patch ok for apply? Or should I move the unsigned_type_node
Kai> initialization more up?

This is ok as-is.

thanks,
Tom

2011/1/7 Tom Tromey <tromey@redhat.com>:
>>>>>> "Kai" == Kai Tietz <ktietz70@googlemail.com> writes:
>
> Kai> My multilib bootstrap on linux x86_64 for this patch passed. I got no
> Kai> segmentation fault on libjava.
> Kai> So is patch ok for apply? Or should I move the unsigned_type_node
> Kai> initialization more up?
>
> This is ok as-is.
>
> thanks,
> Tom
>

Thanks, applied at revision 168585.

Regards,
Kai