public inbox for java-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* [patch] Do not call output_constant from the front end
@ 2012-03-27 20:36 Steven Bosscher
  2012-03-27 21:07 ` Eric Botcazou
  0 siblings, 1 reply; 9+ messages in thread
From: Steven Bosscher @ 2012-03-27 20:36 UTC (permalink / raw)
  To: GCC Patches, java-patches, Richard Guenther

[-- Attachment #1: Type: text/plain, Size: 947 bytes --]

Hello,

The Java front end emits assembly from class.c:emit_register_classes()
to fill the .jcr section. This is not something a front end ought to
be doing. Things to write out to the assembler output file should go
through the varpool/varasm mechanism. The attached patch makes
emit_register_classes build a constructor for the .jcr section, and
stops it calling output_constant.

The patch passes build+testing on powerpc64-unknown-linux-gnu.

With this patch a variable named "_Jv_CLS" is written out. The
assembly before and after the patch is the same, except for the
variable name. I want to write out a nameless variable but I don't
know how. Help welcome.

Ciao!
Steven


java/
        PR java/52730
        * class.c (emit_register_classes_in_jcr_section): New function.
        (emit_Jv_RegisterClass_calls): New function, split out from ...
        (emit_register_classes): ... here.  Reorganize.  Do not call
        output_constant.

[-- Attachment #2: java_no_output_constant.diff --]
[-- Type: text/x-patch, Size: 4810 bytes --]

java/
	* class.c (emit_register_classes_in_jcr_section): New function.
	(emit_Jv_RegisterClass_calls): New function, split out from ...
	(emit_register_classes): ... here.  Reorganize.  Do not call
	output_constant.

Index: class.c
===================================================================
--- class.c	(revision 185813)
+++ class.c	(working copy)
@@ -2786,10 +2786,75 @@ emit_indirect_register_classes (tree *list_p)
   append_to_statement_list (t, list_p);
 }
 
+/* Emit a list of pointers to all classes we have emitted to JCR_SECTION.  */
 
+static void
+emit_register_classes_in_jcr_section (void)
+{
+  tree klass, cdecl, class_array_type;
+  int i;
+  int size = VEC_length (tree, registered_class);
+  VEC(constructor_elt,gc) *init = VEC_alloc (constructor_elt, gc, size);
+
+#ifndef JCR_SECTION_NAME
+  /* A target has defined TARGET_USE_JCR_SECTION,
+     but doesn't have a JCR_SECTION_NAME.  */
+  gcc_unreachable ();
+#endif
+
+  FOR_EACH_VEC_ELT (tree, registered_class, i, klass)
+    CONSTRUCTOR_APPEND_ELT (init, NULL_TREE, build_fold_addr_expr (klass));
+
+  class_array_type = build_prim_array_type (ptr_type_node, size);
+  cdecl = build_decl (UNKNOWN_LOCATION,
+		      VAR_DECL, get_identifier ("_Jv_CLS"),
+		      class_array_type);
+  DECL_SECTION_NAME (cdecl) = build_string (strlen (JCR_SECTION_NAME),
+					    JCR_SECTION_NAME);
+  DECL_ALIGN (cdecl) = POINTER_SIZE;
+  DECL_INITIAL (cdecl) = build_constructor (class_array_type, init);
+  TREE_CONSTANT (DECL_INITIAL (cdecl)) = 1;
+  TREE_STATIC (cdecl) = 1;
+  DECL_ARTIFICIAL (cdecl) = 1;
+  DECL_IGNORED_P (cdecl) = 1;
+  TREE_READONLY (cdecl) = 1;
+  TREE_CONSTANT (cdecl) = 1;
+  pushdecl_top_level (cdecl);
+  relayout_decl (cdecl);
+  rest_of_decl_compilation (cdecl, 1, 0);
+  mark_decl_referenced (cdecl);
+}
+
+
+/* Emit a series of calls to _Jv_RegisterClass for every class we emitted.
+   A series of calls is added to LIST_P.  */
+
+static void
+emit_Jv_RegisterClass_calls (tree *list_p)
+{
+  tree klass, t, register_class_fn;
+  int i;
+
+  t = build_function_type_list (void_type_node, class_ptr_type, NULL);
+  t = build_decl (input_location,
+		  FUNCTION_DECL, get_identifier ("_Jv_RegisterClass"), t);
+  TREE_PUBLIC (t) = 1;
+  DECL_EXTERNAL (t) = 1;
+  register_class_fn = t;
+
+  FOR_EACH_VEC_ELT (tree, registered_class, i, klass)
+    {
+      t = build_fold_addr_expr (klass);
+      t = build_call_expr (register_class_fn, 1, t);
+      append_to_statement_list (t, list_p);
+    }
+}
+
 /* Emit something to register classes at start-up time.
 
-   The preferred mechanism is through the .jcr section, which contain
+   The default mechanism is to generate instances at run-time.
+
+   An alternative mechanism is through the .jcr section, which contain
    a list of pointers to classes which get registered during constructor
    invocation time.
 
@@ -2803,55 +2868,18 @@ emit_register_classes (tree *list_p)
   if (registered_class == NULL)
     return;
 
+  /* By default, generate instances of Class at runtime.  */
   if (flag_indirect_classes)
-    {
-      emit_indirect_register_classes (list_p);
-      return;
-    }
-
+    emit_indirect_register_classes (list_p);
   /* TARGET_USE_JCR_SECTION defaults to 1 if SUPPORTS_WEAK and
      TARGET_ASM_NAMED_SECTION, else 0.  Some targets meet those conditions
      but lack suitable crtbegin/end objects or linker support.  These
      targets can override the default in tm.h to use the fallback mechanism.  */
-  if (TARGET_USE_JCR_SECTION)
-    {
-      tree klass, t;
-      int i;
-
-#ifdef JCR_SECTION_NAME
-      switch_to_section (get_section (JCR_SECTION_NAME, SECTION_WRITE, NULL));
-#else
-      /* A target has defined TARGET_USE_JCR_SECTION,
-	 but doesn't have a JCR_SECTION_NAME.  */
-      gcc_unreachable ();
-#endif
-      assemble_align (POINTER_SIZE);
-
-      FOR_EACH_VEC_ELT (tree, registered_class, i, klass)
-	{
-	  t = build_fold_addr_expr (klass);
-	  output_constant (t, POINTER_SIZE / BITS_PER_UNIT, POINTER_SIZE);
-	}
-    }
+  else if (TARGET_USE_JCR_SECTION)
+    emit_register_classes_in_jcr_section ();
+  /* Use the fallback mechanism.  */
   else
-    {
-      tree klass, t, register_class_fn;
-      int i;
-
-      t = build_function_type_list (void_type_node, class_ptr_type, NULL);
-      t = build_decl (input_location,
-		      FUNCTION_DECL, get_identifier ("_Jv_RegisterClass"), t);
-      TREE_PUBLIC (t) = 1;
-      DECL_EXTERNAL (t) = 1;
-      register_class_fn = t;
-
-      FOR_EACH_VEC_ELT (tree, registered_class, i, klass)
-	{
-	  t = build_fold_addr_expr (klass);
-	  t = build_call_expr (register_class_fn, 1, t);
-	  append_to_statement_list (t, list_p);
-	}
-    }
+    emit_Jv_RegisterClass_calls (list_p);
 }
 
 /* Build a constructor for an entry in the symbol table.  */

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [patch] Do not call output_constant from the front end
  2012-03-27 20:36 [patch] Do not call output_constant from the front end Steven Bosscher
@ 2012-03-27 21:07 ` Eric Botcazou
  2012-03-27 21:52   ` Steven Bosscher
  0 siblings, 1 reply; 9+ messages in thread
From: Eric Botcazou @ 2012-03-27 21:07 UTC (permalink / raw)
  To: Steven Bosscher; +Cc: gcc-patches, java-patches, Richard Guenther

> With this patch a variable named "_Jv_CLS" is written out. The
> assembly before and after the patch is the same, except for the
> variable name. I want to write out a nameless variable but I don't
> know how.

Try tree_output_constant_def.

-- 
Eric Botcazou

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [patch] Do not call output_constant from the front end
  2012-03-27 21:07 ` Eric Botcazou
@ 2012-03-27 21:52   ` Steven Bosscher
  2012-03-27 22:23     ` [patch][rfa] " Steven Bosscher
  0 siblings, 1 reply; 9+ messages in thread
From: Steven Bosscher @ 2012-03-27 21:52 UTC (permalink / raw)
  To: Eric Botcazou; +Cc: gcc-patches, java-patches, Richard Guenther

On Tue, Mar 27, 2012 at 11:07 PM, Eric Botcazou <ebotcazou@adacore.com> wrote:
>> With this patch a variable named "_Jv_CLS" is written out. The
>> assembly before and after the patch is the same, except for the
>> variable name. I want to write out a nameless variable but I don't
>> know how.
>
> Try tree_output_constant_def.

I didn't know about this function. But as far as I can tell, there is
no way to instruct that function to put the data in a specific
section. In the Java case, the data must be put in the .jcr section. I
suppose that can be achieved with a bit more re-working of varasm, but
not with the code as-is. Thanks for the pointer, though!

Ciao!
Steven

^ permalink raw reply	[flat|nested] 9+ messages in thread

* [patch][rfa] Do not call output_constant from the front end
  2012-03-27 21:52   ` Steven Bosscher
@ 2012-03-27 22:23     ` Steven Bosscher
  2012-03-28  7:46       ` Andrew Haley
                         ` (2 more replies)
  0 siblings, 3 replies; 9+ messages in thread
From: Steven Bosscher @ 2012-03-27 22:23 UTC (permalink / raw)
  To: gcc-patches, java-patches; +Cc: Richard Guenther, Eric Botcazou

[-- Attachment #1: Type: text/plain, Size: 1030 bytes --]

On Tue, Mar 27, 2012 at 11:51 PM, Steven Bosscher <stevenb.gcc@gmail.com> wrote:
> On Tue, Mar 27, 2012 at 11:07 PM, Eric Botcazou <ebotcazou@adacore.com> wrote:
>>> With this patch a variable named "_Jv_CLS" is written out. The
>>> assembly before and after the patch is the same, except for the
>>> variable name. I want to write out a nameless variable but I don't
>>> know how.
>>
>> Try tree_output_constant_def.
>
> I didn't know about this function. But as far as I can tell, there is
> no way to instruct that function to put the data in a specific
> section. In the Java case, the data must be put in the .jcr section. I
> suppose that can be achieved with a bit more re-working of varasm, but
> not with the code as-is. Thanks for the pointer, though!

It also doesn't appear to be possible to adjust the alignment via
tree_output_constant_def(), so going down that path seems more trouble
than it's worth.

Therefore, an RFA for the attached patch. Bootstrapped&tested on
powerpc64-unknown-linux-gnu. OK?

Ciao!
Steven

[-- Attachment #2: java_no_output_constant.diff --]
[-- Type: text/x-patch, Size: 5043 bytes --]

java/
	PR java/52730
	* class.c (emit_register_classes_in_jcr_section): New function.
	(emit_Jv_RegisterClass_calls): New function, split out from ...
	(emit_register_classes): ... here.  Reorganize.  Do not call
	output_constant.

Index: class.c
===================================================================
--- class.c	(revision 185813)
+++ class.c	(working copy)
@@ -2786,10 +2786,78 @@ emit_indirect_register_classes (tree *list_p)
   append_to_statement_list (t, list_p);
 }
 
+/* Emit a list of pointers to all classes we have emitted to JCR_SECTION.  */
 
+static void
+emit_register_classes_in_jcr_section (void)
+{
+  tree klass, cdecl, class_array_type;
+  int i;
+  int size = VEC_length (tree, registered_class);
+  VEC(constructor_elt,gc) *init = VEC_alloc (constructor_elt, gc, size);
+
+#ifndef JCR_SECTION_NAME
+  /* A target has defined TARGET_USE_JCR_SECTION,
+     but doesn't have a JCR_SECTION_NAME.  */
+  gcc_unreachable ();
+#endif
+
+  FOR_EACH_VEC_ELT (tree, registered_class, i, klass)
+    CONSTRUCTOR_APPEND_ELT (init, NULL_TREE, build_fold_addr_expr (klass));
+
+  /* ??? I would like to use tree_output_constant_def() but there is no way
+	 to put the data in a named section name, or to set the alignment,
+	 via that function.  So do everything manually here.  */
+  class_array_type = build_prim_array_type (ptr_type_node, size);
+  cdecl = build_decl (UNKNOWN_LOCATION,
+		      VAR_DECL, get_identifier ("_Jv_JCR_SECTION_data"),
+		      class_array_type);
+  DECL_SECTION_NAME (cdecl) = build_string (strlen (JCR_SECTION_NAME),
+					    JCR_SECTION_NAME);
+  DECL_ALIGN (cdecl) = POINTER_SIZE;
+  DECL_INITIAL (cdecl) = build_constructor (class_array_type, init);
+  TREE_CONSTANT (DECL_INITIAL (cdecl)) = 1;
+  TREE_STATIC (cdecl) = 1;
+  TREE_READONLY (cdecl) = 1;
+  TREE_CONSTANT (cdecl) = 1;
+  DECL_ARTIFICIAL (cdecl) = 1;
+  DECL_IGNORED_P (cdecl) = 1;
+  pushdecl_top_level (cdecl);
+  relayout_decl (cdecl);
+  rest_of_decl_compilation (cdecl, 1, 0);
+  mark_decl_referenced (cdecl);
+}
+
+
+/* Emit a series of calls to _Jv_RegisterClass for every class we emitted.
+   A series of calls is added to LIST_P.  */
+
+static void
+emit_Jv_RegisterClass_calls (tree *list_p)
+{
+  tree klass, t, register_class_fn;
+  int i;
+
+  t = build_function_type_list (void_type_node, class_ptr_type, NULL);
+  t = build_decl (input_location,
+		  FUNCTION_DECL, get_identifier ("_Jv_RegisterClass"), t);
+  TREE_PUBLIC (t) = 1;
+  DECL_EXTERNAL (t) = 1;
+  register_class_fn = t;
+
+  FOR_EACH_VEC_ELT (tree, registered_class, i, klass)
+    {
+      t = build_fold_addr_expr (klass);
+      t = build_call_expr (register_class_fn, 1, t);
+      append_to_statement_list (t, list_p);
+    }
+}
+
 /* Emit something to register classes at start-up time.
 
-   The preferred mechanism is through the .jcr section, which contain
+   The default mechanism is to generate instances at run-time.
+
+   An alternative mechanism is through the .jcr section, which contain
    a list of pointers to classes which get registered during constructor
    invocation time.
 
@@ -2803,55 +2871,18 @@ emit_register_classes (tree *list_p)
   if (registered_class == NULL)
     return;
 
+  /* By default, generate instances of Class at runtime.  */
   if (flag_indirect_classes)
-    {
-      emit_indirect_register_classes (list_p);
-      return;
-    }
-
+    emit_indirect_register_classes (list_p);
   /* TARGET_USE_JCR_SECTION defaults to 1 if SUPPORTS_WEAK and
      TARGET_ASM_NAMED_SECTION, else 0.  Some targets meet those conditions
      but lack suitable crtbegin/end objects or linker support.  These
      targets can override the default in tm.h to use the fallback mechanism.  */
-  if (TARGET_USE_JCR_SECTION)
-    {
-      tree klass, t;
-      int i;
-
-#ifdef JCR_SECTION_NAME
-      switch_to_section (get_section (JCR_SECTION_NAME, SECTION_WRITE, NULL));
-#else
-      /* A target has defined TARGET_USE_JCR_SECTION,
-	 but doesn't have a JCR_SECTION_NAME.  */
-      gcc_unreachable ();
-#endif
-      assemble_align (POINTER_SIZE);
-
-      FOR_EACH_VEC_ELT (tree, registered_class, i, klass)
-	{
-	  t = build_fold_addr_expr (klass);
-	  output_constant (t, POINTER_SIZE / BITS_PER_UNIT, POINTER_SIZE);
-	}
-    }
+  else if (TARGET_USE_JCR_SECTION)
+    emit_register_classes_in_jcr_section ();
+  /* Use the fallback mechanism.  */
   else
-    {
-      tree klass, t, register_class_fn;
-      int i;
-
-      t = build_function_type_list (void_type_node, class_ptr_type, NULL);
-      t = build_decl (input_location,
-		      FUNCTION_DECL, get_identifier ("_Jv_RegisterClass"), t);
-      TREE_PUBLIC (t) = 1;
-      DECL_EXTERNAL (t) = 1;
-      register_class_fn = t;
-
-      FOR_EACH_VEC_ELT (tree, registered_class, i, klass)
-	{
-	  t = build_fold_addr_expr (klass);
-	  t = build_call_expr (register_class_fn, 1, t);
-	  append_to_statement_list (t, list_p);
-	}
-    }
+    emit_Jv_RegisterClass_calls (list_p);
 }
 
 /* Build a constructor for an entry in the symbol table.  */

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [patch][rfa] Do not call output_constant from the front end
  2012-03-27 22:23     ` [patch][rfa] " Steven Bosscher
@ 2012-03-28  7:46       ` Andrew Haley
  2012-03-28 10:02       ` Richard Guenther
  2012-04-02 12:24       ` Rainer Orth
  2 siblings, 0 replies; 9+ messages in thread
From: Andrew Haley @ 2012-03-28  7:46 UTC (permalink / raw)
  To: java-patches

On 03/27/2012 11:23 PM, Steven Bosscher wrote:
> java/
> 	PR java/52730
> 	* class.c (emit_register_classes_in_jcr_section): New function.
> 	(emit_Jv_RegisterClass_calls): New function, split out from ...
> 	(emit_register_classes): ... here.  Reorganize.  Do not call
> 	output_constant.


Sure, this looks fine.

Thanks,
Andrew.

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [patch][rfa] Do not call output_constant from the front end
  2012-03-27 22:23     ` [patch][rfa] " Steven Bosscher
  2012-03-28  7:46       ` Andrew Haley
@ 2012-03-28 10:02       ` Richard Guenther
  2012-03-29 21:04         ` Steven Bosscher
  2012-04-02 12:24       ` Rainer Orth
  2 siblings, 1 reply; 9+ messages in thread
From: Richard Guenther @ 2012-03-28 10:02 UTC (permalink / raw)
  To: Steven Bosscher; +Cc: gcc-patches, java-patches, Eric Botcazou

On Wed, Mar 28, 2012 at 12:23 AM, Steven Bosscher <stevenb.gcc@gmail.com> wrote:
> On Tue, Mar 27, 2012 at 11:51 PM, Steven Bosscher <stevenb.gcc@gmail.com> wrote:
>> On Tue, Mar 27, 2012 at 11:07 PM, Eric Botcazou <ebotcazou@adacore.com> wrote:
>>>> With this patch a variable named "_Jv_CLS" is written out. The
>>>> assembly before and after the patch is the same, except for the
>>>> variable name. I want to write out a nameless variable but I don't
>>>> know how.
>>>
>>> Try tree_output_constant_def.
>>
>> I didn't know about this function. But as far as I can tell, there is
>> no way to instruct that function to put the data in a specific
>> section. In the Java case, the data must be put in the .jcr section. I
>> suppose that can be achieved with a bit more re-working of varasm, but
>> not with the code as-is. Thanks for the pointer, though!
>
> It also doesn't appear to be possible to adjust the alignment via
> tree_output_constant_def(), so going down that path seems more trouble
> than it's worth.
>
> Therefore, an RFA for the attached patch. Bootstrapped&tested on
> powerpc64-unknown-linux-gnu. OK?

Ok.

Thanks for applying TLC to Java.

Richard.

> Ciao!
> Steven

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [patch][rfa] Do not call output_constant from the front end
  2012-03-28 10:02       ` Richard Guenther
@ 2012-03-29 21:04         ` Steven Bosscher
  0 siblings, 0 replies; 9+ messages in thread
From: Steven Bosscher @ 2012-03-29 21:04 UTC (permalink / raw)
  To: Richard Guenther; +Cc: gcc-patches, java-patches, Eric Botcazou

On Wed, Mar 28, 2012 at 12:02 PM, Richard Guenther
<richard.guenther@gmail.com> wrote:
>> Therefore, an RFA for the attached patch. Bootstrapped&tested on
>> powerpc64-unknown-linux-gnu. OK?
>
> Ok.

Thanks. Committed as r185977.

Ciao!
Steven

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [patch][rfa] Do not call output_constant from the front end
  2012-03-27 22:23     ` [patch][rfa] " Steven Bosscher
  2012-03-28  7:46       ` Andrew Haley
  2012-03-28 10:02       ` Richard Guenther
@ 2012-04-02 12:24       ` Rainer Orth
  2012-04-02 12:29         ` Richard Guenther
  2 siblings, 1 reply; 9+ messages in thread
From: Rainer Orth @ 2012-04-02 12:24 UTC (permalink / raw)
  To: Steven Bosscher
  Cc: gcc-patches, java-patches, Richard Guenther, Eric Botcazou

[-- Attachment #1: Type: text/plain, Size: 3627 bytes --]

Steven Bosscher <stevenb.gcc@gmail.com> writes:

> Therefore, an RFA for the attached patch. Bootstrapped&tested on
> powerpc64-unknown-linux-gnu. OK?

Unfortunately, this patch completely broke libjava testing on
i386-pc-solaris2* and x86_64-unknown-linux-gnu: all execution tests fail
with a SEGV:

Program received signal SIGSEGV, Segmentation fault.
0xfd0ddf7d in _Jv_Linker::verify_class (klass=klass@entry=0xfe945d80)
    at /vol/gcc/src/hg/trunk/local/libjava/link.cc:1904
1904      klass->engine->verify(klass);

klass->engine is NULL at this point.  Checking the generated .jcr
sections, I found that a large number of them have sh_addralign 0x20
instead of 0x4 before your patch.  This caused a massive reduction in
the number of _Jv_RegisterClassHookDefault calls:

With the following DTrace one-liner

pid$target::_Jv_RegisterClassHookDefault:entry{ @[ustack()] = count(); }

to investigate ArrayStore.exe execution, I find for the unpatched jc1:

              libgcj.so.13.0.0`_Jv_RegisterClassHookDefault
              libgcj.so.13.0.0`_Jv_RegisterClasses+0x44
              libgcj.so.13.0.0`_ZN4java4lang9Cloneable6class$E
              0xe0ec
             7213

With your patch, I get

              libgcj.so.13.0.0`_Jv_RegisterClassHookDefault
              libgcj.so.13.0.0`_Jv_RegisterNewClasses+0xb7
              ArrayStore.exe`_Utf18
                1

              libgcj.so.13.0.0`_Jv_RegisterClassHookDefault
              libgcj.so.13.0.0`_Jv_RegisterClasses+0x44
              libgcj.so.13.0.0`_ZN4java4lang9Cloneable6class$E
              0xe0ec
               12

instead.

The alignment change happens because LOCAL_ALIGNMENT overrides
DECL_ALIGN.  This can be avoided by setting DECL_USER_ALIGN (which
unfortunately is undocumented).  The result looks better, but still
fails in a different way:

Exception in thread "main" java.lang.NoClassDefFoundError: ArrayStore
   at java.lang.Class.initializeClass(libgcj.so.13.0.0)
Caused by: java.lang.NullPointerException
   at java.lang.Class.initializeClass(libgcj.so.13.0.0)

But the _Jv_RegisterClassHookDefault are almost back to normal:

              libgcj.so.13.0.0`_Jv_RegisterClassHookDefault
              libgcj.so.13.0.0`_Jv_RegisterClasses+0x44
              libgcj.so.13.0.0`_ZN4java4lang9Cloneable6class$E
              0xe0ec
             7212

I could trace this to a change in .jcr section flags: in ArrayStore.exe,
I find

Section Header[18]:  sh_name: .jcr
    sh_addr:      0x8052058       sh_flags:   [ SHF_ALLOC ]
    sh_size:      0x4             sh_type:    [ SHT_PROGBITS ]
    sh_offset:    0x2058          sh_entsize: 0
    sh_link:      0               sh_info:    0
    sh_addralign: 0x4       

Section Header[26]:  sh_name: .jcr
    sh_addr:      0x80625e4       sh_flags:   [ SHF_WRITE SHF_ALLOC ]
    sh_size:      0x4             sh_type:    [ SHT_PROGBITS ]
    sh_offset:    0x25e4          sh_entsize: 0
    sh_link:      0               sh_info:    0
    sh_addralign: 0x4       

The first (without SHF_WRITE set) is from jc1, the second from
crtbegin.o.  The Solaris linker (and probably other ELF linkers which
don't special-case .jcr) merges sections not just by name, but only if
name and attributs (flags, type) match.  Not marking .jcr read-only
fixes that part of the failure and restores libjava testsuite results on
Solaris 11/x86 back to normal (no failures).

Bootstrapped without regressions on i386-pc-solaris2.11.

Ok for mainline?

	Rainer


2012-03-31  Rainer Orth  <ro@CeBiTec.Uni-Bielefeld.DE>

	* class.c (emit_register_classes_in_jcr_section): Set DECL_USER_ALIGN.
	Clear TREE_READONLY.


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: java-jcr-align.patch --]
[-- Type: text/x-patch, Size: 718 bytes --]

# HG changeset patch
# Parent ae1fe58e699b5ec26f0ad31675d3915eef63d963
Fix .jcr alignment

diff --git a/gcc/java/class.c b/gcc/java/class.c
--- a/gcc/java/class.c
+++ b/gcc/java/class.c
@@ -2815,10 +2815,11 @@ emit_register_classes_in_jcr_section (vo
   DECL_SECTION_NAME (cdecl) = build_string (strlen (JCR_SECTION_NAME),
 					    JCR_SECTION_NAME);
   DECL_ALIGN (cdecl) = POINTER_SIZE;
+  DECL_USER_ALIGN (cdecl) = 1;
   DECL_INITIAL (cdecl) = build_constructor (class_array_type, init);
   TREE_CONSTANT (DECL_INITIAL (cdecl)) = 1;
   TREE_STATIC (cdecl) = 1;
-  TREE_READONLY (cdecl) = 1;
+  TREE_READONLY (cdecl) = 0;
   TREE_CONSTANT (cdecl) = 1;
   DECL_ARTIFICIAL (cdecl) = 1;
   DECL_IGNORED_P (cdecl) = 1;

[-- Attachment #3: Type: text/plain, Size: 143 bytes --]


-- 
-----------------------------------------------------------------------------
Rainer Orth, Center for Biotechnology, Bielefeld University

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [patch][rfa] Do not call output_constant from the front end
  2012-04-02 12:24       ` Rainer Orth
@ 2012-04-02 12:29         ` Richard Guenther
  0 siblings, 0 replies; 9+ messages in thread
From: Richard Guenther @ 2012-04-02 12:29 UTC (permalink / raw)
  To: Rainer Orth; +Cc: Steven Bosscher, gcc-patches, java-patches, Eric Botcazou

On Mon, Apr 2, 2012 at 2:24 PM, Rainer Orth <ro@cebitec.uni-bielefeld.de> wrote:
> Steven Bosscher <stevenb.gcc@gmail.com> writes:
>
>> Therefore, an RFA for the attached patch. Bootstrapped&tested on
>> powerpc64-unknown-linux-gnu. OK?
>
> Unfortunately, this patch completely broke libjava testing on
> i386-pc-solaris2* and x86_64-unknown-linux-gnu: all execution tests fail
> with a SEGV:
>
> Program received signal SIGSEGV, Segmentation fault.
> 0xfd0ddf7d in _Jv_Linker::verify_class (klass=klass@entry=0xfe945d80)
>    at /vol/gcc/src/hg/trunk/local/libjava/link.cc:1904
> 1904      klass->engine->verify(klass);
>
> klass->engine is NULL at this point.  Checking the generated .jcr
> sections, I found that a large number of them have sh_addralign 0x20
> instead of 0x4 before your patch.  This caused a massive reduction in
> the number of _Jv_RegisterClassHookDefault calls:
>
> With the following DTrace one-liner
>
> pid$target::_Jv_RegisterClassHookDefault:entry{ @[ustack()] = count(); }
>
> to investigate ArrayStore.exe execution, I find for the unpatched jc1:
>
>              libgcj.so.13.0.0`_Jv_RegisterClassHookDefault
>              libgcj.so.13.0.0`_Jv_RegisterClasses+0x44
>              libgcj.so.13.0.0`_ZN4java4lang9Cloneable6class$E
>              0xe0ec
>             7213
>
> With your patch, I get
>
>              libgcj.so.13.0.0`_Jv_RegisterClassHookDefault
>              libgcj.so.13.0.0`_Jv_RegisterNewClasses+0xb7
>              ArrayStore.exe`_Utf18
>                1
>
>              libgcj.so.13.0.0`_Jv_RegisterClassHookDefault
>              libgcj.so.13.0.0`_Jv_RegisterClasses+0x44
>              libgcj.so.13.0.0`_ZN4java4lang9Cloneable6class$E
>              0xe0ec
>               12
>
> instead.
>
> The alignment change happens because LOCAL_ALIGNMENT overrides
> DECL_ALIGN.  This can be avoided by setting DECL_USER_ALIGN (which
> unfortunately is undocumented).  The result looks better, but still
> fails in a different way:
>
> Exception in thread "main" java.lang.NoClassDefFoundError: ArrayStore
>   at java.lang.Class.initializeClass(libgcj.so.13.0.0)
> Caused by: java.lang.NullPointerException
>   at java.lang.Class.initializeClass(libgcj.so.13.0.0)
>
> But the _Jv_RegisterClassHookDefault are almost back to normal:
>
>              libgcj.so.13.0.0`_Jv_RegisterClassHookDefault
>              libgcj.so.13.0.0`_Jv_RegisterClasses+0x44
>              libgcj.so.13.0.0`_ZN4java4lang9Cloneable6class$E
>              0xe0ec
>             7212
>
> I could trace this to a change in .jcr section flags: in ArrayStore.exe,
> I find
>
> Section Header[18]:  sh_name: .jcr
>    sh_addr:      0x8052058       sh_flags:   [ SHF_ALLOC ]
>    sh_size:      0x4             sh_type:    [ SHT_PROGBITS ]
>    sh_offset:    0x2058          sh_entsize: 0
>    sh_link:      0               sh_info:    0
>    sh_addralign: 0x4
>
> Section Header[26]:  sh_name: .jcr
>    sh_addr:      0x80625e4       sh_flags:   [ SHF_WRITE SHF_ALLOC ]
>    sh_size:      0x4             sh_type:    [ SHT_PROGBITS ]
>    sh_offset:    0x25e4          sh_entsize: 0
>    sh_link:      0               sh_info:    0
>    sh_addralign: 0x4
>
> The first (without SHF_WRITE set) is from jc1, the second from
> crtbegin.o.  The Solaris linker (and probably other ELF linkers which
> don't special-case .jcr) merges sections not just by name, but only if
> name and attributs (flags, type) match.  Not marking .jcr read-only
> fixes that part of the failure and restores libjava testsuite results on
> Solaris 11/x86 back to normal (no failures).
>
> Bootstrapped without regressions on i386-pc-solaris2.11.
>
> Ok for mainline?

Ok.  I wonder what happens if we mark __JCR_END__ in crtstuff.c
as const?

Thanks,
Richard.

>        Rainer
>
>
> 2012-03-31  Rainer Orth  <ro@CeBiTec.Uni-Bielefeld.DE>
>
>        * class.c (emit_register_classes_in_jcr_section): Set DECL_USER_ALIGN.
>        Clear TREE_READONLY.
>
>
>
> --
> -----------------------------------------------------------------------------
> Rainer Orth, Center for Biotechnology, Bielefeld University
>

^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2012-04-02 12:29 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-03-27 20:36 [patch] Do not call output_constant from the front end Steven Bosscher
2012-03-27 21:07 ` Eric Botcazou
2012-03-27 21:52   ` Steven Bosscher
2012-03-27 22:23     ` [patch][rfa] " Steven Bosscher
2012-03-28  7:46       ` Andrew Haley
2012-03-28 10:02       ` Richard Guenther
2012-03-29 21:04         ` Steven Bosscher
2012-04-02 12:24       ` Rainer Orth
2012-04-02 12:29         ` Richard Guenther

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).