diff mbox

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

Message ID AANLkTimPKX9m9+onmgC=Pey_NKP09=hBcB_uBnHnEn_-@mail.gmail.com
State New
Headers show

Commit Message

Kai Tietz Jan. 16, 2011, 9:21 a.m. UTC 
2011/1/12 Richard Guenther <richard.guenther@gmail.com>:
> 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.

Well, as described in bug-report this missing va_list_type_node in
java's decl.c is a regression from 1999. I attaced a patch, which is
reverting it, but it is  a paper-patch. That char_type_node is used
within va_list_type_node of java's decl.c, shouldn't lead here to any
issues, as java FE didn't used that types before (as otherwise there
would be ICEs without that patch already), and now those types are
used just in va_list_type_node construction. The va_list types are
just used in comparison by some backends. Java doesn't operate with
va_list_type_node and you can't specify them within this language
AFAICS.


2011-01-16  Kai Tietz

	PR bootstrap/47215
	* decl.c (java_init_decl_processing): Remove
	va_list_type_node related type initializations.

2011-01-16  Kai Tietz

	PR bootstrap/47215
	* config/i386/i386.c (ix86_local_alignment): Handle
	case for va_list_type_node is nil.
	(ix86_canonical_va_list_type): Likewise.

Tested for x86_64-pc-linux-gnu and x86_64-w64-mingw32. Ok for apply?

Regards,
Kai

Comments

Tom Tromey Jan. 17, 2011, 2:28 p.m. UTC | #1 
>>>>> "Kai" == Kai Tietz <ktietz70@googlemail.com> writes:

Kai> Java doesn't operate with va_list_type_node and you can't specify
Kai> them within this language AFAICS.

That is correct.

Tom
Jakub Jelinek Jan. 19, 2011, 4:06 p.m. UTC | #2 
Hi!

I'd prefer if we go with this patch, at least for 4.6.  For 4.7 somebody can
do something about the type initialization unification, but that is IMHO
stage1 material, not stage4.  This is a P1 bootstrap failure on several
targets AFAIK.

Uros/Richard, could you please review the i386.c changes?  I think Kai
can just revert his own recent changes without any kind of acking.

I believe other targets use va_list_type_node just in va_start expansion,
va_arg gimplification and vaarg setup in ... functions, but all those
should never happen in Java which doesn't have them.

On Sun, Jan 16, 2011 at 10:21:50AM +0100, Kai Tietz wrote:
> Well, as described in bug-report this missing va_list_type_node in
> java's decl.c is a regression from 1999. I attaced a patch, which is
> reverting it, but it is  a paper-patch. That char_type_node is used
> within va_list_type_node of java's decl.c, shouldn't lead here to any
> issues, as java FE didn't used that types before (as otherwise there
> would be ICEs without that patch already), and now those types are
> used just in va_list_type_node construction. The va_list types are
> just used in comparison by some backends. Java doesn't operate with
> va_list_type_node and you can't specify them within this language
> AFAICS.
> 
> 
> 2011-01-16  Kai Tietz
> 
> 	PR bootstrap/47215
> 	* decl.c (java_init_decl_processing): Remove
> 	va_list_type_node related type initializations.
> 
> 2011-01-16  Kai Tietz
> 
> 	PR bootstrap/47215
> 	* config/i386/i386.c (ix86_local_alignment): Handle
> 	case for va_list_type_node is nil.
> 	(ix86_canonical_va_list_type): Likewise.
> 
> Tested for x86_64-pc-linux-gnu and x86_64-w64-mingw32. Ok for apply?
> 
> Regards,
> Kai

> Index: gcc/gcc/config/i386/i386.c
> ===================================================================
> --- gcc.orig/gcc/config/i386/i386.c	2011-01-09 14:50:57.000000000 +0100
> +++ gcc/gcc/config/i386/i386.c	2011-01-16 09:53:42.379793400 +0100
> @@ -22931,8 +22931,9 @@ ix86_local_alignment (tree exp, enum mac
>        && TARGET_SSE)
>      {
>        if (AGGREGATE_TYPE_P (type)
> -	   && (TYPE_MAIN_VARIANT (type)
> -	       != TYPE_MAIN_VARIANT (va_list_type_node))
> +	   && (va_list_type_node == NULL_TREE
> +	       || (TYPE_MAIN_VARIANT (type)
> +		   != TYPE_MAIN_VARIANT (va_list_type_node)))
>  	   && TYPE_SIZE (type)
>  	   && TREE_CODE (TYPE_SIZE (type)) == INTEGER_CST
>  	   && (TREE_INT_CST_LOW (TYPE_SIZE (type)) >= 16
> @@ -33682,7 +33683,7 @@ ix86_canonical_va_list_type (tree type)
>    else if (POINTER_TYPE_P (type) && TREE_CODE (TREE_TYPE (type)) == ARRAY_TYPE)
>      type = TREE_TYPE (type);
>  
> -  if (TARGET_64BIT)
> +  if (TARGET_64BIT && va_list_type_node != NULL_TREE)
>      {
>        wtype = va_list_type_node;
>  	  gcc_assert (wtype != NULL_TREE);
> Index: gcc/gcc/java/decl.c
> ===================================================================
> --- gcc.orig/gcc/java/decl.c	2011-01-11 20:36:15.000000000 +0100
> +++ gcc/gcc/java/decl.c	2011-01-16 09:46:13.187101100 +0100
> @@ -1153,21 +1153,6 @@ java_init_decl_processing (void)
>    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);
> -  long_integer_type_node = make_signed_type (LONG_TYPE_SIZE);
> -
> -  t = targetm.build_builtin_va_list ();
> -
> -  /* Many back-ends define record types without setting TYPE_NAME.
> -     If we copied the record type here, we'd keep the original
> -     record type without a name.  This breaks name mangling.  So,
> -     don't copy record types and let c_common_nodes_and_builtins()
> -     declare the type to be __builtin_va_list.  */
> -  if (TREE_CODE (t) != RECORD_TYPE)
> -    t = build_variant_type_copy (t);
> -
> -  va_list_type_node = t;
>  
>    initialize_builtins ();
>  


	Jakub
Richard Henderson Jan. 21, 2011, 1:14 a.m. UTC | #3 
>> 2011-01-16  Kai Tietz
>>
>> 	PR bootstrap/47215
>> 	* config/i386/i386.c (ix86_local_alignment): Handle
>> 	case for va_list_type_node is nil.
>> 	(ix86_canonical_va_list_type): Likewise.

Ok.


r~
Kai Tietz Jan. 21, 2011, 9:06 a.m. UTC | #4 
2011/1/21 Richard Henderson <rth@redhat.com>:
>>> 2011-01-16  Kai Tietz
>>>
>>>      PR bootstrap/47215
>>>      * config/i386/i386.c (ix86_local_alignment): Handle
>>>      case for va_list_type_node is nil.
>>>      (ix86_canonical_va_list_type): Likewise.
>
> Ok.
>
>
> r~
>

Applied at revision 169080.

Kai
diff mbox

Patch

Index: gcc/gcc/config/i386/i386.c
===================================================================
--- gcc.orig/gcc/config/i386/i386.c	2011-01-09 14:50:57.000000000 +0100
+++ gcc/gcc/config/i386/i386.c	2011-01-16 09:53:42.379793400 +0100
@@ -22931,8 +22931,9 @@  ix86_local_alignment (tree exp, enum mac
       && TARGET_SSE)
     {
       if (AGGREGATE_TYPE_P (type)
-	   && (TYPE_MAIN_VARIANT (type)
-	       != TYPE_MAIN_VARIANT (va_list_type_node))
+	   && (va_list_type_node == NULL_TREE
+	       || (TYPE_MAIN_VARIANT (type)
+		   != TYPE_MAIN_VARIANT (va_list_type_node)))
 	   && TYPE_SIZE (type)
 	   && TREE_CODE (TYPE_SIZE (type)) == INTEGER_CST
 	   && (TREE_INT_CST_LOW (TYPE_SIZE (type)) >= 16
@@ -33682,7 +33683,7 @@  ix86_canonical_va_list_type (tree type)
   else if (POINTER_TYPE_P (type) && TREE_CODE (TREE_TYPE (type)) == ARRAY_TYPE)
     type = TREE_TYPE (type);
 
-  if (TARGET_64BIT)
+  if (TARGET_64BIT && va_list_type_node != NULL_TREE)
     {
       wtype = va_list_type_node;
 	  gcc_assert (wtype != NULL_TREE);
Index: gcc/gcc/java/decl.c
===================================================================
--- gcc.orig/gcc/java/decl.c	2011-01-11 20:36:15.000000000 +0100
+++ gcc/gcc/java/decl.c	2011-01-16 09:46:13.187101100 +0100
@@ -1153,21 +1153,6 @@  java_init_decl_processing (void)
   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);
-  long_integer_type_node = make_signed_type (LONG_TYPE_SIZE);
-
-  t = targetm.build_builtin_va_list ();
-
-  /* Many back-ends define record types without setting TYPE_NAME.
-     If we copied the record type here, we'd keep the original
-     record type without a name.  This breaks name mangling.  So,
-     don't copy record types and let c_common_nodes_and_builtins()
-     declare the type to be __builtin_va_list.  */
-  if (TREE_CODE (t) != RECORD_TYPE)
-    t = build_variant_type_copy (t);
-
-  va_list_type_node = t;
 
   initialize_builtins ();