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

login
register
mail settings
Submitter Kai Tietz
Date Jan. 11, 2011, 4:01 p.m.
Message ID <AANLkTinbaZiBFF+fCbJ6gbZfx4ZtaiVkuTfpG_+wF+W7@mail.gmail.com>
Download mbox | patch
Permalink /patch/78401/
State New
Headers show

Comments

Kai Tietz - Jan. 11, 2011, 4:01 p.m.
2011/1/11 Andreas Schwab <schwab@redhat.com>:
> Kai Tietz <ktietz70@googlemail.com> writes:
>
>> Following patch should solve your issue:
>
> It doesn't.
>
> Andreas.
>
> --
> Andreas Schwab, schwab@redhat.com
> GPG Key fingerprint = D4E8 DBE3 3813 BB5D FA84  5EC7 45C6 250E 6F00 984E
> "And now for something completely different."
>

Yes, it uses also the unsigned_short_type_node ...

Here a bunch of missing type-node initializers. Could you please test again?

Thanks in advance,
Kai
Andreas Schwab - Jan. 11, 2011, 4:09 p.m.
Kai Tietz <ktietz70@googlemail.com> writes:

> Here a bunch of missing type-node initializers. Could you please test again?

What about TYPE_STRING_FLAG?  What about char_type_node?  What about all
the other standard types created by build_common_tree_nodes?

Andreas.
Richard Guenther - Jan. 11, 2011, 4:11 p.m.
On Tue, Jan 11, 2011 at 5:09 PM, Andreas Schwab <schwab@redhat.com> wrote:
> Kai Tietz <ktietz70@googlemail.com> writes:
>
>> Here a bunch of missing type-node initializers. Could you please test again?
>
> What about TYPE_STRING_FLAG?  What about char_type_node?  What about all
> the other standard types created by build_common_tree_nodes?

Java should really be fixed to call build_common_tree_nodes instead
of replicating parts of it.

Richard.

> Andreas.
>
> --
> Andreas Schwab, schwab@redhat.com
> GPG Key fingerprint = D4E8 DBE3 3813 BB5D FA84  5EC7 45C6 250E 6F00 984E
> "And now for something completely different."
>
Kai Tietz - Jan. 11, 2011, 7:24 p.m.
2011/1/11 Richard Guenther <richard.guenther@gmail.com>:
> On Tue, Jan 11, 2011 at 5:09 PM, Andreas Schwab <schwab@redhat.com> wrote:
>> Kai Tietz <ktietz70@googlemail.com> writes:
>>
>>> Here a bunch of missing type-node initializers. Could you please test again?
>>
>> What about TYPE_STRING_FLAG?  What about char_type_node?  What about all
>> the other standard types created by build_common_tree_nodes?
>
> Java should really be fixed to call build_common_tree_nodes instead
> of replicating parts of it.

Yes, would be obviously the best. Nevertheless nothing for stage 4 (as
I talked to Tom on IRC already).

Andreas, did this patch fixed rs6000 java bootstrap for you?

Regards,
Kai
Andreas Schwab - Jan. 11, 2011, 7:55 p.m.
Kai Tietz <ktietz70@googlemail.com> writes:

> Andreas, did this patch fixed rs6000 java bootstrap for you?

The discrepancies with TYPE_STRING_FLAG and char_type_node remain.  I
don't know what kind of hidden effect that will have.

Andreas.
Kai Tietz - Jan. 11, 2011, 8:14 p.m.
2011/1/11 Andreas Schwab <schwab@linux-m68k.org>:
> Kai Tietz <ktietz70@googlemail.com> writes:
>
>> Andreas, did this patch fixed rs6000 java bootstrap for you?
>
> The discrepancies with TYPE_STRING_FLAG and char_type_node remain.  I
> don't know what kind of hidden effect that will have.
>
> Andreas.

I don't know if there are side-effects, or not. I just know that this
type wasn't set before, so it seems not to be used before. Otherwise
we would have seen a crash in java bootstrap before.
Maybe java maintainers could clarify this.

Regards,
Kai
Andreas Schwab - Jan. 11, 2011, 8:38 p.m.
Kai Tietz <ktietz70@googlemail.com> writes:

> 2011/1/11 Andreas Schwab <schwab@linux-m68k.org>:
>> Kai Tietz <ktietz70@googlemail.com> writes:
>>
>>> Andreas, did this patch fixed rs6000 java bootstrap for you?
>>
>> The discrepancies with TYPE_STRING_FLAG and char_type_node remain.  I
>> don't know what kind of hidden effect that will have.
>>
>> Andreas.
>
> I don't know if there are side-effects, or not. I just know that this
> type wasn't set before, so it seems not to be used before.

But it is used now, so care must be taken.  Especially the different
char_type_node is worrying.

Andreas.
Richard Guenther - Jan. 12, 2011, 10:03 a.m.
On Tue, Jan 11, 2011 at 9:38 PM, Andreas Schwab <schwab@linux-m68k.org> wrote:
> Kai Tietz <ktietz70@googlemail.com> writes:
>
>> 2011/1/11 Andreas Schwab <schwab@linux-m68k.org>:
>>> Kai Tietz <ktietz70@googlemail.com> writes:
>>>
>>>> Andreas, did this patch fixed rs6000 java bootstrap for you?
>>>
>>> The discrepancies with TYPE_STRING_FLAG and char_type_node remain.  I
>>> don't know what kind of hidden effect that will have.
>>>
>>> Andreas.
>>
>> I don't know if there are side-effects, or not. I just know that this
>> type wasn't set before, so it seems not to be used before.
>
> But it is used now, so care must be taken.  Especially the different
> char_type_node is worrying.

Indeed.  Please either revert the full series or work towards
using build_common_tree_nodes.

Richard.

> Andreas.
>
> --
> Andreas Schwab, schwab@linux-m68k.org
> GPG Key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
> "And now for something completely different."
>
Tom Tromey - Jan. 17, 2011, 2:28 p.m.
>>>>> "Richard" == Richard Guenther <richard.guenther@gmail.com> writes:

Richard> Indeed.  Please either revert the full series or work towards
Richard> using build_common_tree_nodes.

It seems to me that his current patches remove some crashes.
Presumably they can't affect correctness since those nodes are never
used or generated by the java front end in the first place.
So, I think reverting them is too strong.

I agree that java probably should use the common tree-building
infrastructure.  ISTR looking at this once though and deciding it was
hard.

Tom
Jakub Jelinek - Jan. 17, 2011, 2:56 p.m.
On Mon, Jan 17, 2011 at 07:28:13AM -0700, Tom Tromey wrote:
> >>>>> "Richard" == Richard Guenther <richard.guenther@gmail.com> writes:
> 
> Richard> Indeed.  Please either revert the full series or work towards
> Richard> using build_common_tree_nodes.
> 
> It seems to me that his current patches remove some crashes.
> Presumably they can't affect correctness since those nodes are never
> used or generated by the java front end in the first place.
> So, I think reverting them is too strong.

The crashes were just on i?86 AFAIK, and only because the backend (IMHO
incorrectly) assumed va_list_type_node is always non-NULL.  I think
languages that don't have va_list type nor support va_* macros shouldn't be
forced to create such type.

> I agree that java probably should use the common tree-building
> infrastructure.  ISTR looking at this once though and deciding it was
> hard.

I'd say that is clearly stage1 material though, not stage4.
The patch caused so many regressions that IMHO at least for stage4 reverting
it is the only sane approach.

	Jakub

Patch

Index: decl.c
===================================================================
--- decl.c      (revision 168662)
+++ decl.c      (working copy)
@@ -1154,8 +1154,14 @@ 
     = add_builtin_function ("_Jv_remJ", t,
                            0, NOT_BUILT_IN, NULL, NULL_TREE);
   /* Initialize va_list_type_node.  */
+  unsigned_char_type_node = make_unsigned_type (CHAR_TYPE_SIZE);
+  short_integer_type_node = make_signed_type (SHORT_TYPE_SIZE);
+  short_unsigned_type_node = make_unsigned_type (SHORT_TYPE_SIZE);
   unsigned_type_node = make_unsigned_type (INT_TYPE_SIZE);
   long_integer_type_node = make_signed_type (LONG_TYPE_SIZE);
+  long_unsigned_type_node = make_unsigned_type (LONG_TYPE_SIZE);
+  long_long_integer_type_node = make_signed_type (LONG_LONG_TYPE_SIZE);
+  long_long_unsigned_type_node = make_unsigned_type (LONG_LONG_TYPE_SIZE);

   t = targetm.build_builtin_va_list ();