Patchwork HPPA constructor merge patch, PR middle-end/45388

login
register
mail settings
Submitter Jan Hubicka
Date Dec. 13, 2010, 1:15 p.m.
Message ID <20101213131500.GA20394@kam.mff.cuni.cz>
Download mbox | patch
Permalink /patch/75332/
State New
Headers show

Comments

Jan Hubicka - Dec. 13, 2010, 1:15 p.m.
> > 
> > > I am testing the patch on x86_64-linux, can you, please, test it at hppa?
> > 
> > Will do.
> 
> Works.  No regressions seen in a build and check with hppa2.0w-hp-hpux11.11.
Great, thanks!
Mark, does the C++ change look OK? It changes GLOBAL__I function that will never
end up being the externally visible static constructor into GLOBAL__sub_I.
We can chose any other name, but having those static constructor recognizable
name seems like not bad idea.

Bootatrapped/regtested x86_64-linux.

Honza

	PR middle-end/45388
	* decl2.c (start_objects): Do not generate collect2 recognicable name
	for static ctor.
	* ipa.c (cgraph_build_static_cdtor_1): Break out from ... ; add FINAL parameter.
	(cgraph_build_static_cdtor): ... here.
	(build_cdtor): Use cgraph_build_static_cdtor_1.
Mark Mitchell - Dec. 13, 2010, 3:10 p.m.
On 12/13/2010 5:15 AM, Jan Hubicka wrote:

> Mark, does the C++ change look OK? It changes GLOBAL__I function that will never
> end up being the externally visible static constructor into GLOBAL__sub_I.

This might break targets (if any) that still use "munch".  This is a
utility that runs nm to find static constructors and generates a C
function called __main to call them all; that allows you to implement
C++ static constructors without any linker support for things like .ctors.

I've CC'd Nathan Sidwell and Nathan Froyd here; both of them are
familiar with VxWorks, which (at least the last time I used it) still
depended on munch.  Nathans, does this still apply?

Oh, wait, Jan, is that the whole point of your patch?  That we don't
want utilities like this to recognize these things as static constructors?

Thank you,
Jan Hubicka - Dec. 13, 2010, 5:23 p.m.
> On 12/13/2010 5:15 AM, Jan Hubicka wrote:
> 
> > Mark, does the C++ change look OK? It changes GLOBAL__I function that will never
> > end up being the externally visible static constructor into GLOBAL__sub_I.
> 
> This might break targets (if any) that still use "munch".  This is a
> utility that runs nm to find static constructors and generates a C
> function called __main to call them all; that allows you to implement
> C++ static constructors without any linker support for things like .ctors.
> 
> I've CC'd Nathan Sidwell and Nathan Froyd here; both of them are
> familiar with VxWorks, which (at least the last time I used it) still
> depended on munch.  Nathans, does this still apply?
> 
> Oh, wait, Jan, is that the whole point of your patch?  That we don't
> want utilities like this to recognize these things as static constructors?

Yes, that is the problem.  We now produce static constructors that are later
collected into single externally visible function.  We don't want those static
ctor functions to be externally visible to collect2 or munch.
The ctor collecting for this reason was introduced couple years ago, I've now
extended it to work on ELF targets too.  While doing so, I noticed that the code
adds always_inline flag to the function that is not always safe thing to do.
Removing always inlining makes those static functions to appear in output file
and be referenced from __main.

Honza
> 
> Thank you,
> 
> -- 
> Mark Mitchell
> CodeSourcery
> mark@codesourcery.com
> (650) 331-3385 x713
Mark Mitchell - Dec. 13, 2010, 5:25 p.m.
On 12/13/2010 9:23 AM, Jan Hubicka wrote:

>> Oh, wait, Jan, is that the whole point of your patch?  That we don't
>> want utilities like this to recognize these things as static constructors?
> 
> Yes, that is the problem.  We now produce static constructors that are later
> collected into single externally visible function.  We don't want those static
> ctor functions to be externally visible to collect2 or munch.

OK, in that case I have no concern about changing the names.

Thank you,
Jan Hubicka - Dec. 13, 2010, 5:31 p.m.
> On 12/13/2010 9:23 AM, Jan Hubicka wrote:
> 
> >> Oh, wait, Jan, is that the whole point of your patch?  That we don't
> >> want utilities like this to recognize these things as static constructors?
> > 
> > Yes, that is the problem.  We now produce static constructors that are later
> > collected into single externally visible function.  We don't want those static
> > ctor functions to be externally visible to collect2 or munch.
> 
> OK, in that case I have no concern about changing the names.
Thanks,
I've commited the patch.

Honza
> 
> Thank you,
> 
> -- 
> Mark Mitchell
> CodeSourcery
> mark@codesourcery.com
> (650) 331-3385 x713
John David Anglin - Dec. 13, 2010, 7:43 p.m.
On Mon, 13 Dec 2010, Jan Hubicka wrote:

> I've commited the patch.

Only the ChangeLog updates were committed.

Dave
H.J. Lu - Dec. 14, 2010, 6:04 a.m.
On Mon, Dec 13, 2010 at 9:31 AM, Jan Hubicka <hubicka@ucw.cz> wrote:
>> On 12/13/2010 9:23 AM, Jan Hubicka wrote:
>>
>> >> Oh, wait, Jan, is that the whole point of your patch?  That we don't
>> >> want utilities like this to recognize these things as static constructors?
>> >
>> > Yes, that is the problem.  We now produce static constructors that are later
>> > collected into single externally visible function.  We don't want those static
>> > ctor functions to be externally visible to collect2 or munch.
>>
>> OK, in that case I have no concern about changing the names.
> Thanks,
> I've commited the patch.
>
> Honza

This caused:

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=46933

Patch

Index: cp/decl2.c
===================================================================
--- cp/decl2.c	(revision 167726)
+++ cp/decl2.c	(working copy)
@@ -2691,7 +2691,7 @@  start_objects (int method_type, int init
 {
   tree body;
   tree fndecl;
-  char type[10];
+  char type[14];
 
   /* Make ctor or dtor function.  METHOD_TYPE may be 'I' or 'D'.  */
 
@@ -2705,10 +2705,10 @@  start_objects (int method_type, int init
       joiner = '_';
 #endif
 
-      sprintf (type, "%c%c%.5u", method_type, joiner, initp);
+      sprintf (type, "sub_%c%c%.5u", method_type, joiner, initp);
     }
   else
-    sprintf (type, "%c", method_type);
+    sprintf (type, "sub_%c", method_type);
 
   fndecl = build_lang_decl (FUNCTION_DECL,
 			    get_file_function_name (type),
Index: ipa.c
===================================================================
--- ipa.c	(revision 167726)
+++ ipa.c	(working copy)
@@ -1496,10 +1496,13 @@  struct ipa_opt_pass_d pass_ipa_profile =
 /* Generate and emit a static constructor or destructor.  WHICH must
    be one of 'I' (for a constructor) or 'D' (for a destructor).  BODY
    is a STATEMENT_LIST containing GENERIC statements.  PRIORITY is the
-   initialization priority for this constructor or destructor.  */
+   initialization priority for this constructor or destructor. 
 
-void
-cgraph_build_static_cdtor (char which, tree body, int priority)
+   FINAL specify whether the externally visible name for collect2 should
+   be produced. */
+
+static void
+cgraph_build_static_cdtor_1 (char which, tree body, int priority, bool final)
 {
   static int counter = 0;
   char which_buf[16];
@@ -1508,7 +1511,12 @@  cgraph_build_static_cdtor (char which, t
   /* The priority is encoded in the constructor or destructor name.
      collect2 will sort the names and arrange that they are called at
      program startup.  */
-  sprintf (which_buf, "%c_%.5d_%d", which, priority, counter++);
+  if (final)
+    sprintf (which_buf, "%c_%.5d_%d", which, priority, counter++);
+  else
+  /* Proudce sane name but one not recognizable by collect2, just for the
+     case we fail to inline the function.  */
+    sprintf (which_buf, "sub_%c_%.5d_%d", which, priority, counter++);
   name = get_file_function_name (which_buf);
 
   decl = build_decl (input_location, FUNCTION_DECL, name,
@@ -1528,7 +1536,7 @@  cgraph_build_static_cdtor (char which, t
   DECL_ARTIFICIAL (decl) = 1;
   DECL_NO_INSTRUMENT_FUNCTION_ENTRY_EXIT (decl) = 1;
   DECL_SAVED_TREE (decl) = body;
-  if (!targetm.have_ctors_dtors)
+  if (!targetm.have_ctors_dtors && final)
     {
       TREE_PUBLIC (decl) = 1;
       DECL_PRESERVE_P (decl) = 1;
@@ -1563,6 +1571,16 @@  cgraph_build_static_cdtor (char which, t
   current_function_decl = NULL;
 }
 
+/* Generate and emit a static constructor or destructor.  WHICH must
+   be one of 'I' (for a constructor) or 'D' (for a destructor).  BODY
+   is a STATEMENT_LIST containing GENERIC statements.  PRIORITY is the
+   initialization priority for this constructor or destructor.  */
+
+void
+cgraph_build_static_cdtor (char which, tree body, int priority)
+{
+  cgraph_build_static_cdtor_1 (which, body, priority, false);
+}
 
 /* A vector of FUNCTION_DECLs declared as static constructors.  */
 static VEC(tree, heap) *static_ctors;
@@ -1648,7 +1666,7 @@  build_cdtor (bool ctor_p, VEC (tree, hea
       gcc_assert (body != NULL_TREE);
       /* Generate a function to call all the function of like
 	 priority.  */
-      cgraph_build_static_cdtor (ctor_p ? 'I' : 'D', body, priority);
+      cgraph_build_static_cdtor_1 (ctor_p ? 'I' : 'D', body, priority, true);
     }
 }