public inbox for libc-hacker@sourceware.org
 help / color / mirror / Atom feed
* Re: patch to make init_array work (2nd version; resend)
@ 2002-11-08 11:34 Roland McGrath
  2002-11-08 11:38 ` David Mosberger
  2002-11-27 14:35 ` patch to make init_array work (3nd version) David Mosberger
  0 siblings, 2 replies; 18+ messages in thread
From: Roland McGrath @ 2002-11-08 11:34 UTC (permalink / raw)
  To: davidm; +Cc: libc-hacker

When this approach came up before, Ulrich also objected to the implementation,
see http://sources.redhat.com/ml/libc-alpha/2002-03/msg00063.html.
In future repostings of patches previously discussed, it would be helpful
if you could cite the prior posting and discussion in the mailing list
archives, for the benefit of people like me who were not involved in the
discussion the first time around.

For dynamic executables, the dynamic linker or libc could in fact examine
the DT_INIT_ARRAY slots directly, rather than having the crt1.o code supply
the values.  I think this is what Ulrich wanted to see from what he said in
the message cited above.

The same is true of DT_INIT/DT_FINI, but for whatever reason the convention
inherited from SVR4 is to have the startup code call its own _init and
_fini entry points rather than the dynamic linker doing it.  

That suggested approach is not available for static linking, however.  We
need to keep the same crt1.o code for both static and dynamic linking (or
else all go raving mad).  So it is not entirely clear to me what exact
criteria the ideal solution should meet.

I am inclined to think the plan we discussed yesterday is the best one.
But if Ulrich prefers to have ld.so or libc look up DT_INIT_ARRAY et al,
then I would like to hear his thoughts on the plan for static linking.



Thanks,
Roland

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

* Re: patch to make init_array work (2nd version; resend)
  2002-11-08 11:34 patch to make init_array work (2nd version; resend) Roland McGrath
@ 2002-11-08 11:38 ` David Mosberger
  2002-11-27 14:35 ` patch to make init_array work (3nd version) David Mosberger
  1 sibling, 0 replies; 18+ messages in thread
From: David Mosberger @ 2002-11-08 11:38 UTC (permalink / raw)
  To: Roland McGrath; +Cc: davidm, libc-hacker

>>>>> On Fri, 8 Nov 2002 11:34:31 -0800, Roland McGrath <roland@frob.com> said:

  Roland> When this approach came up before, Ulrich also objected to
  Roland> the implementation, see
  Roland> http://sources.redhat.com/ml/libc-alpha/2002-03/msg00063.html.
  Roland> In future repostings of patches previously discussed, it
  Roland> would be helpful if you could cite the prior posting and
  Roland> discussion in the mailing list archives, for the benefit of
  Roland> people like me who were not involved in the discussion the
  Roland> first time around.

Yes, though I did point to the thread earlier on
(see http://sources.redhat.com/ml/libc-hacker/2002-10/msg00085.html).

  Roland> That suggested approach is not available for static linking,
  Roland> however.

Right.

  Roland> I am inclined to think the plan we discussed yesterday is
  Roland> the best one.

I like it, too.

	--david

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

* patch to make init_array work (3nd version)
  2002-11-08 11:34 patch to make init_array work (2nd version; resend) Roland McGrath
  2002-11-08 11:38 ` David Mosberger
@ 2002-11-27 14:35 ` David Mosberger
  2002-11-27 15:10   ` H. J. Lu
  2002-12-09 10:52   ` Roland McGrath
  1 sibling, 2 replies; 18+ messages in thread
From: David Mosberger @ 2002-11-27 14:35 UTC (permalink / raw)
  To: Roland McGrath; +Cc: davidm, libc-hacker

Below is a revised patch to get init_array working.  This is basically
what Richard suggested.  Roland, you suggested that the new routines
in csu/init.c could be "static", but I don't see how this would work
given that the routines need to be accessed from start.S.

A problem with this new approach is that the preinit_array functions
need to be executed by csu/init.c only for statically linked programs.
I don't know of a good way to detect this inside init.c, so I changed
the init callback to take an extra argument (which will be ignored on
platforms that don't support init_array).

To be honest, HJ's original solution seems nicer to me (e.g., doesn't
require changes to platform-specific code), though I see the point
about additional relocs.

Note: the patch below makes init_array working on ia64 only.  Other
platforms would have to make the analogous (trivial) change to
start.S.

In terms of testing, "make check" passed, so the basics seem to be
right.

	--david

ChangeLog

2002-11-27  David Mosberger  <davidm@hpl.hp.com>

	* sysdeps/generic/libc-start.c (__libc_start_main): Declare
	INIT callback as taking an integer argument indicating whether
	program is a statically-linked executable.  Update call-site
	accordingly.

	* elf/Makefile (tests): Re-enable init_array tests.

	* csu/init.c (__preinit_array_start): Declare.
	(__preinit_array_end): Ditto.
	(__init_array_start): Ditto.
	(__init_array_end): Ditto.
	(__fini_array_start): Ditto.
	(__fini_array_end): Ditto.
	(_init): Declare as an external function.
	(_fini): Ditto.
	(__libc_do_init_calls): New function.
	(__libc_do_fini_calls): Ditto.

Index: csu/init.c
===================================================================
RCS file: /cvs/glibc/libc/csu/init.c,v
retrieving revision 1.4
diff -u -r1.4 init.c
--- csu/init.c	6 Jul 2001 04:54:45 -0000	1.4
+++ csu/init.c	27 Nov 2002 22:28:11 -0000
@@ -25,3 +25,54 @@
 const int _IO_stdin_used = _G_IO_IO_FILE_VERSION;
 
 #endif
+
+#ifdef HAVE_INITFINI_ARRAY
+extern void (*__preinit_array_start []) (void);
+extern void (*__preinit_array_end []) (void);
+extern void (*__init_array_start []) (void);
+extern void (*__init_array_end []) (void);
+extern void (*__fini_array_start []) (void);
+extern void (*__fini_array_end []) (void);
+#endif
+
+extern void _init (void);
+extern void _fini (void);
+
+void
+__libc_do_init_calls (int static_executable)
+{
+  size_t size, i;
+
+#ifdef HAVE_INITFINI_ARRAY
+  /* For dynamically linked executables the preinit array must be
+     executed by the loader (before initializing any shared
+     object.  */
+  if (static_executable)
+    {
+      size = __preinit_array_end - __preinit_array_start;
+      for (i = 0; i < size; i++)
+	(*__preinit_array_start [i]) ();
+    }
+#endif
+
+  _init ();
+
+#ifdef HAVE_INITFINI_ARRAY
+  size =  __init_array_end - __init_array_start;
+  for (i = 0; i < size; i++)
+    (*__init_array_start [i]) ();
+#endif
+}
+
+void
+__libc_do_fini_calls (void)
+{
+  size_t i;
+
+#ifdef HAVE_INITFINI_ARRAY
+  for (i = __fini_array_end - __fini_array_start; i-- > 0; )
+    (*__fini_array_start [i]) ();
+#endif
+
+  _fini ();
+}
Index: elf/Makefile
===================================================================
RCS file: /cvs/glibc/libc/elf/Makefile,v
retrieving revision 1.245
diff -u -r1.245 Makefile
--- elf/Makefile	23 Nov 2002 21:34:16 -0000	1.245
+++ elf/Makefile	27 Nov 2002 22:28:11 -0000
@@ -118,7 +118,7 @@
 
 tests = tst-tls1 tst-tls2 tst-tls9
 ifeq (yes,$(have-initfini-array))
-#tests += tst-array1 tst-array2 tst-array3
+tests += tst-array1 tst-array2 tst-array3
 endif
 ifeq (yes,$(build-static))
 tests-static = tst-tls1-static tst-tls2-static
Index: sysdeps/generic/libc-start.c
===================================================================
RCS file: /cvs/glibc/libc/sysdeps/generic/libc-start.c,v
retrieving revision 1.33
diff -u -r1.33 libc-start.c
--- sysdeps/generic/libc-start.c	25 Oct 2002 19:41:24 -0000	1.33
+++ sysdeps/generic/libc-start.c	27 Nov 2002 22:28:11 -0000
@@ -40,7 +40,7 @@
 extern int BP_SYM (__libc_start_main) (int (*main) (int, char **, char **),
 				       int argc,
 				       char *__unbounded *__unbounded ubp_av,
-				       void (*init) (void),
+				       void (*init) (int),
 				       void (*fini) (void),
 				       void (*rtld_fini) (void),
 				       void *__unbounded stack_end)
@@ -51,7 +51,7 @@
    BPs in the arglist of startup_info.main and startup_info.init. */
 BP_SYM (__libc_start_main) (int (*main) (int, char **, char **),
 		   int argc, char *__unbounded *__unbounded ubp_av,
-		   void (*init) (void), void (*fini) (void),
+		   void (*init) (int), void (*fini) (void),
 		   void (*rtld_fini) (void), void *__unbounded stack_end)
 {
   char *__unbounded *__unbounded ubp_ev = &ubp_av[argc + 1];
@@ -121,7 +121,11 @@
     _dl_debug_printf ("\ninitialize program: %s\n\n", argv[0]);
 #endif
   if (init)
-    (*init) ();
+# ifdef SHARED
+    (*init) (0);
+# else
+    (*init) (1);
+# endif
 
 #ifdef SHARED
   if (__builtin_expect (GL(dl_debug_mask) & DL_DEBUG_IMPCALLS, 0))
Index: sysdeps/ia64/elf/start.S
===================================================================
RCS file: /cvs/glibc/libc/sysdeps/ia64/elf/start.S,v
retrieving revision 1.10
diff -u -r1.10 start.S
--- sysdeps/ia64/elf/start.S	6 Jul 2001 04:55:54 -0000	1.10
+++ sysdeps/ia64/elf/start.S	27 Nov 2002 22:28:11 -0000
@@ -1,4 +1,4 @@
-/* Copyright (C) 1999, 2000, 2001 Free Software Foundation, Inc.
+/* Copyright (C) 1999, 2000, 2001, 2002 Free Software Foundation, Inc.
    This file is part of the GNU C Library.
    Written by Jes Sorensen, <Jes.Sorensen@cern.ch>, April 1999.
 
@@ -64,13 +64,13 @@
 	{
 	  addl r11 = @ltoff(__libc_ia64_register_backing_store_base), gp
 	  addl out0 = @ltoff(@fptr(main)), gp
-	  addl out3 = @ltoff(@fptr(_init)), gp
+	  addl out3 = @ltoff(@fptr(__libc_do_init_calls)), gp
 	  ;;
 	}
 	{ .mmi
 	  ld8 r3 = [r11]	/* pointer to __libc_ia64_register_backing_store_base */
 	  ld8 out0 = [out0]	/* pointer to `main' function descriptor */
-	  addl out4 = @ltoff(@fptr(_fini)), gp
+	  addl out4 = @ltoff(@fptr(__libc_do_fini_calls)), gp
 	  ;;
 	}
 	{ .mmi

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

* Re: patch to make init_array work (3nd version)
  2002-11-27 14:35 ` patch to make init_array work (3nd version) David Mosberger
@ 2002-11-27 15:10   ` H. J. Lu
  2002-11-27 15:34     ` David Mosberger
  2002-12-09 10:52   ` Roland McGrath
  1 sibling, 1 reply; 18+ messages in thread
From: H. J. Lu @ 2002-11-27 15:10 UTC (permalink / raw)
  To: davidm; +Cc: Roland McGrath, libc-hacker

On Wed, Nov 27, 2002 at 02:35:33PM -0800, David Mosberger wrote:
> Below is a revised patch to get init_array working.  This is basically
> what Richard suggested.  Roland, you suggested that the new routines
> in csu/init.c could be "static", but I don't see how this would work
> given that the routines need to be accessed from start.S.
> 
> A problem with this new approach is that the preinit_array functions
> need to be executed by csu/init.c only for statically linked programs.
> I don't know of a good way to detect this inside init.c, so I changed
> the init callback to take an extra argument (which will be ignored on
> platforms that don't support init_array).
> 
> To be honest, HJ's original solution seems nicer to me (e.g., doesn't
> require changes to platform-specific code), though I see the point
> about additional relocs.

I am not sure how this approach will work. If I get it right,
__libc_do_init_calls and __libc_do_fini_calls are defined in
executables. But they are only available when executables are
linked against with the new glibc. For the existing executables,
there are no __libc_do_init_calls nor __libc_do_fini_calls. That
is why my original solution has

#ifdef HAVE_INITFINI_ARRAY
# ifdef SHARED
extern void __libc_init_array (void) __attribute__ ((weak));
extern void __libc_fini_array (void) __attribute__ ((weak));
# else
extern void __libc_preinit_array (void); 
extern void __libc_init_array (void); 
extern void __libc_fini_array (void); 
# endif
#endif

...

#ifdef HAVE_INITFINI_ARRAY
# ifdef SHARED
  if (__libc_fini_array)
# endif
    __cxa_atexit ((void (*) (void *)) __libc_fini_array, NULL, NULL);

# ifndef SHARED
  __libc_preinit_array ();
# endif
#endif

which will work with all exutables.

> 
> Note: the patch below makes init_array working on ia64 only.  Other
> platforms would have to make the analogous (trivial) change to
> start.S.
> 
> In terms of testing, "make check" passed, so the basics seem to be
> right.
> 
> 	--david
> 
> ChangeLog
> 
> 2002-11-27  David Mosberger  <davidm@hpl.hp.com>
> 
> 	* sysdeps/generic/libc-start.c (__libc_start_main): Declare
> 	INIT callback as taking an integer argument indicating whether
> 	program is a statically-linked executable.  Update call-site
> 	accordingly.
> 
> 	* elf/Makefile (tests): Re-enable init_array tests.
> 
> 	* csu/init.c (__preinit_array_start): Declare.
> 	(__preinit_array_end): Ditto.
> 	(__init_array_start): Ditto.
> 	(__init_array_end): Ditto.
> 	(__fini_array_start): Ditto.
> 	(__fini_array_end): Ditto.
> 	(_init): Declare as an external function.
> 	(_fini): Ditto.
> 	(__libc_do_init_calls): New function.
> 	(__libc_do_fini_calls): Ditto.
> 

You missed the ChangeLog entry for sysdeps/ia64/elf/start.S.


H.J.

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

* Re: patch to make init_array work (3nd version)
  2002-11-27 15:10   ` H. J. Lu
@ 2002-11-27 15:34     ` David Mosberger
  0 siblings, 0 replies; 18+ messages in thread
From: David Mosberger @ 2002-11-27 15:34 UTC (permalink / raw)
  To: H. J. Lu; +Cc: davidm, Roland McGrath, libc-hacker

>>>>> On Wed, 27 Nov 2002 15:09:58 -0800, "H. J. Lu" <hjl@lucon.org> said:

  HJ> I am not sure how this approach will work. If I get it right,
  HJ> __libc_do_init_calls and __libc_do_fini_calls are defined in
  HJ> executables. But they are only available when executables are
  HJ> linked against with the new glibc. For the existing executables,
  HJ> there are no __libc_do_init_calls nor __libc_do_fini_calls. That
  HJ> is why my original solution has

Old executables directly pass _init() and _fini() to
__libc_start_main, so they'll work normally.  Of course, the
preinit_array, init_array, and fini_array in such executables won't be
executed either, but since this support is new, those binaries
presumably don't have such sections.  Also, ld.so should take care of
executing init_array and fini_array for the shared objects that the
executable depends on, so I think everything should be fine.

	--david

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

* Re: patch to make init_array work (3nd version)
  2002-11-27 14:35 ` patch to make init_array work (3nd version) David Mosberger
  2002-11-27 15:10   ` H. J. Lu
@ 2002-12-09 10:52   ` Roland McGrath
  2002-12-09 13:36     ` David Mosberger
  1 sibling, 1 reply; 18+ messages in thread
From: Roland McGrath @ 2002-12-09 10:52 UTC (permalink / raw)
  To: davidm; +Cc: libc-hacker

I had not been aware of the difference in behavior for static vs dynamic
executables when making my previous suggestion.  Your change to the calling
convention of the function passed to __libc_start_main would require symbol
versioning of __libc_start_main to avoid creating new executables with the
false appearance of compatibility with existing shared libraries.  It's
also an ugly kludge that there is no need for.

I reimplemented it myself.  I put the functions called from start.S into
libc.a and libc_nonshared.a with different versions for the two cases, so
the correct behavior (calling the executable's preinit array or not) is
selected at link time instead of using a kludge with run-time overhead.  
I also performed the query replace on all the start.S files.  

No patch to the generic code in libc is complete without updating all the
existing sysdeps code, and it is not hard to do a query replace.  For
future contributions that change generic code in ways that necessitate
updating sysdeps code, please include those changes if you expect your
patches to go in as they are or in a timely fashion.


Thanks,
Roland

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

* Re: patch to make init_array work (3nd version)
  2002-12-09 10:52   ` Roland McGrath
@ 2002-12-09 13:36     ` David Mosberger
  0 siblings, 0 replies; 18+ messages in thread
From: David Mosberger @ 2002-12-09 13:36 UTC (permalink / raw)
  To: Roland McGrath; +Cc: libc-hacker

>>>>> On Mon, 9 Dec 2002 01:13:13 -0800, Roland McGrath <roland@redhat.com> said:

  Roland> I reimplemented it myself.  I put the functions called from
  Roland> start.S into libc.a and libc_nonshared.a with different
  Roland> versions for the two cases, so the correct behavior (calling
  Roland> the executable's preinit array or not) is selected at link
  Roland> time instead of using a kludge with run-time overhead.

Cool, that is a much nicer solution.

I'm glad that init_array will finally be working.

Thanks,

	--david

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

* Re: patch to make init_array work (2nd version; resend)
  2002-11-07 18:30           ` David Mosberger
@ 2002-11-07 18:45             ` Roland McGrath
  0 siblings, 0 replies; 18+ messages in thread
From: Roland McGrath @ 2002-11-07 18:45 UTC (permalink / raw)
  To: davidm; +Cc: Richard Henderson, libc-hacker, hjl

> I suppose it makes sense to put do_all_init() and do_all_fini() in a
> shared source file.  Do you want to do that or should I work on it?
> If the latter, just tell me where you want the functions.

I suppose csu/init.c makes sense.  I think you can make them static if you
use attribute_used so they are definitely there when the ld -r with start.o
is done.

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

* Re: patch to make init_array work (2nd version; resend)
  2002-11-07 18:20         ` Roland McGrath
@ 2002-11-07 18:30           ` David Mosberger
  2002-11-07 18:45             ` Roland McGrath
  0 siblings, 1 reply; 18+ messages in thread
From: David Mosberger @ 2002-11-07 18:30 UTC (permalink / raw)
  To: Roland McGrath; +Cc: Richard Henderson, davidm, libc-hacker, hjl

>>>>> On Thu, 7 Nov 2002 18:20:09 -0800, Roland McGrath <roland@redhat.com> said:

  >> On Thu, Nov 07, 2002 at 01:58:52PM -0800, Roland McGrath wrote: >
  >> That seems like a reasonable thing to do, though it's unfortunate
  >> it means > changing all the platforms' start.S files.  For
  >> preinit_array+init_array, > start.S could just contain the code
  >> in the .init section, no?  But probably > it is better to write
  >> generic _init/_fini replacements in C.
  >>
  >> I think the best solution is to write
  >>
  >> static void do_all_init(void) { // Loop over .preinit_array //
  >> Loop over .init_array _init (); }
  >>
  >> or whatever the proper ordering is, and then pass this new
  >> function to __libc_start_main.

  Roland> Yup, that's the C I was talking about.

I suppose it makes sense to put do_all_init() and do_all_fini() in a
shared source file.  Do you want to do that or should I work on it?
If the latter, just tell me where you want the functions.

	--david

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

* Re: patch to make init_array work (2nd version; resend)
  2002-11-07 18:10       ` Richard Henderson
@ 2002-11-07 18:20         ` Roland McGrath
  2002-11-07 18:30           ` David Mosberger
  0 siblings, 1 reply; 18+ messages in thread
From: Roland McGrath @ 2002-11-07 18:20 UTC (permalink / raw)
  To: Richard Henderson; +Cc: davidm, libc-hacker, hjl

> On Thu, Nov 07, 2002 at 01:58:52PM -0800, Roland McGrath wrote:
> > That seems like a reasonable thing to do, though it's unfortunate it means
> > changing all the platforms' start.S files.  For preinit_array+init_array,
> > start.S could just contain the code in the .init section, no?  But probably
> > it is better to write generic _init/_fini replacements in C.
> 
> I think the best solution is to write
> 
> 	static void
> 	do_all_init(void)
> 	{
> 	  // Loop over .preinit_array
> 	  // Loop over .init_array
> 	  _init ();
> 	}
> 
> or whatever the proper ordering is, and then pass this
> new function to __libc_start_main.

Yup, that's the C I was talking about.

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

* Re: patch to make init_array work (2nd version; resend)
  2002-11-07 13:58     ` Roland McGrath
  2002-11-07 14:10       ` David Mosberger
@ 2002-11-07 18:10       ` Richard Henderson
  2002-11-07 18:20         ` Roland McGrath
  1 sibling, 1 reply; 18+ messages in thread
From: Richard Henderson @ 2002-11-07 18:10 UTC (permalink / raw)
  To: Roland McGrath; +Cc: davidm, libc-hacker, hjl

On Thu, Nov 07, 2002 at 01:58:52PM -0800, Roland McGrath wrote:
> That seems like a reasonable thing to do, though it's unfortunate it means
> changing all the platforms' start.S files.  For preinit_array+init_array,
> start.S could just contain the code in the .init section, no?  But probably
> it is better to write generic _init/_fini replacements in C.

I think the best solution is to write

	static void
	do_all_init(void)
	{
	  // Loop over .preinit_array
	  // Loop over .init_array
	  _init ();
	}

or whatever the proper ordering is, and then pass this
new function to __libc_start_main.


r~

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

* Re: patch to make init_array work (2nd version; resend)
  2002-11-07 14:29         ` Roland McGrath
@ 2002-11-07 14:38           ` David Mosberger
  0 siblings, 0 replies; 18+ messages in thread
From: David Mosberger @ 2002-11-07 14:38 UTC (permalink / raw)
  To: Roland McGrath; +Cc: davidm, libc-hacker, hjl

>>>>> On Thu, 7 Nov 2002 14:29:24 -0800, Roland McGrath <roland@redhat.com> said:

  >> Perhaps, but it would defeat the whole purpose of switching to
  >> init_array (see my previous mail about why any call in
  >> .init/.fini causes incorrect unwind info).

  Roland> Since this part of the section is generated normally along
  Roland> with the prologue, I would think that its unwind info would
  Roland> come out normally too.

For .init, wouldn't the new code have to go into the epilogue to
ensure that the .init_array entries are called after .init?

	--david

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

* Re: patch to make init_array work (2nd version; resend)
  2002-11-07 14:10       ` David Mosberger
@ 2002-11-07 14:29         ` Roland McGrath
  2002-11-07 14:38           ` David Mosberger
  0 siblings, 1 reply; 18+ messages in thread
From: Roland McGrath @ 2002-11-07 14:29 UTC (permalink / raw)
  To: davidm; +Cc: libc-hacker, hjl

> Perhaps, but it would defeat the whole purpose of switching to
> init_array (see my previous mail about why any call in .init/.fini
> causes incorrect unwind info).

Since this part of the section is generated normally along with the
prologue, I would think that its unwind info would come out normally too.

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

* Re: patch to make init_array work (2nd version; resend)
  2002-11-07 13:58     ` Roland McGrath
@ 2002-11-07 14:10       ` David Mosberger
  2002-11-07 14:29         ` Roland McGrath
  2002-11-07 18:10       ` Richard Henderson
  1 sibling, 1 reply; 18+ messages in thread
From: David Mosberger @ 2002-11-07 14:10 UTC (permalink / raw)
  To: Roland McGrath; +Cc: davidm, libc-hacker, hjl

>>>>> On Thu, 7 Nov 2002 13:58:52 -0800, Roland McGrath <roland@redhat.com> said:

  Roland> That seems like a reasonable thing to do, though it's
  Roland> unfortunate it means changing all the platforms' start.S
  Roland> files.

Yes.

  Roland> For preinit_array+init_array, start.S could just contain the
  Roland> code in the .init section, no?

Perhaps, but it would defeat the whole purpose of switching to
init_array (see my previous mail about why any call in .init/.fini
causes incorrect unwind info).

	--david

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

* Re: patch to make init_array work (2nd version; resend)
  2002-11-07 13:51   ` David Mosberger
@ 2002-11-07 13:58     ` Roland McGrath
  2002-11-07 14:10       ` David Mosberger
  2002-11-07 18:10       ` Richard Henderson
  0 siblings, 2 replies; 18+ messages in thread
From: Roland McGrath @ 2002-11-07 13:58 UTC (permalink / raw)
  To: davidm; +Cc: libc-hacker, hjl

> OK, I _think_ I understand your concern.  Perhaps we could just change
> the platform-specific start.S to pass a different _init/_fini function
> which takes care of calling init_array/fini_array?  If you think that
> would work, I can take a stab at doing that for sysdeps/ia64/elf/start.S.

That seems like a reasonable thing to do, though it's unfortunate it means
changing all the platforms' start.S files.  For preinit_array+init_array,
start.S could just contain the code in the .init section, no?  But probably
it is better to write generic _init/_fini replacements in C.

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

* Re: patch to make init_array work (2nd version; resend)
  2002-11-07 13:31 ` Roland McGrath
@ 2002-11-07 13:51   ` David Mosberger
  2002-11-07 13:58     ` Roland McGrath
  0 siblings, 1 reply; 18+ messages in thread
From: David Mosberger @ 2002-11-07 13:51 UTC (permalink / raw)
  To: Roland McGrath; +Cc: davidm, libc-hacker, hjl

>>>>> On Thu, 7 Nov 2002 13:31:39 -0800, Roland McGrath <roland@redhat.com> said:

  Roland> Your dl-fini.c fix is obviously correct and I put that in.

Great!  Thanks a lot!

  Roland> None of the messages I have seen from you or HJ have
  Roland> mentioned what the problem being fixed by HJ's changes was,
  Roland> so I had to look at the whole thing for a while to figure
  Roland> out what the point was.  (The arrays are already processed
  Roland> properly in loaded objects, but not in the executable
  Roland> itself.)  Now that I see the problem, I can at least put in
  Roland> those test cases.

Sorry, I think neither HJ nor I were trying to be cryptic.  Yes, the
main problem is indeed that .init_array/.fini_array don't get called
for the main program (though HJ's patch obviously address other things
as well, such as correcting the order of .fini_array calls and adding
test case).

  Roland> I don't like HJ's implementation, which introduces three new
  Roland> relocs in libc.so with weak references to symbols defined in
  Roland> crt1.o.  That's just nasty.  The simple thing would be to
  Roland> pass more arguments to __libc_start_main, which would
  Roland> require versioning.

  Roland> But I think that, in keeping with the convention that its
  Roland> own .init/.fini are the executable's problem, it would be
  Roland> cleanest for the arrays just to be handled by the existing
  Roland> init/fini functions in the executable.  That is, have the
  Roland> single function pair it passes (now _init/_fini) do all the
  Roland> work.  My first thought was to just put the code into
  Roland> crti.o/crtn.o, but this code should not go into shared
  Roland> libraries.

OK, I _think_ I understand your concern.  Perhaps we could just change
the platform-specific start.S to pass a different _init/_fini function
which takes care of calling init_array/fini_array?  If you think that
would work, I can take a stab at doing that for
sysdeps/ia64/elf/start.S.

Thanks,

	--david

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

* Re: patch to make init_array work (2nd version; resend)
  2002-11-07 11:43 patch to make init_array work (2nd version; resend) David Mosberger
@ 2002-11-07 13:31 ` Roland McGrath
  2002-11-07 13:51   ` David Mosberger
  0 siblings, 1 reply; 18+ messages in thread
From: Roland McGrath @ 2002-11-07 13:31 UTC (permalink / raw)
  To: davidm; +Cc: libc-hacker, hjl

You'll have to forgive me if there is some context I don't have about this
new init-array stuff, which was invented while I was not actively involved. 
Unless Uli answers with specific comments, I will just be figuring out the
issues from scratch myself starting now.

Your dl-fini.c fix is obviously correct and I put that in.

None of the messages I have seen from you or HJ have mentioned what the
problem being fixed by HJ's changes was, so I had to look at the whole
thing for a while to figure out what the point was.  (The arrays are already
processed properly in loaded objects, but not in the executable itself.)
Now that I see the problem, I can at least put in those test cases.

I don't like HJ's implementation, which introduces three new relocs in
libc.so with weak references to symbols defined in crt1.o.  That's just
nasty.  The simple thing would be to pass more arguments to
__libc_start_main, which would require versioning.

But I think that, in keeping with the convention that its own .init/.fini
are the executable's problem, it would be cleanest for the arrays just to
be handled by the existing init/fini functions in the executable.  That is,
have the single function pair it passes (now _init/_fini) do all the work.
My first thought was to just put the code into crti.o/crtn.o, but this code
should not go into shared libraries.

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

* patch to make init_array work (2nd version; resend)
@ 2002-11-07 11:43 David Mosberger
  2002-11-07 13:31 ` Roland McGrath
  0 siblings, 1 reply; 18+ messages in thread
From: David Mosberger @ 2002-11-07 11:43 UTC (permalink / raw)
  To: libc-hacker; +Cc: hjl, davidm

[I'm resending this patch since there has been no response to the original
 mail on October 28th.  I'd appreciate it if someone could let me know if
 something is wrong with the patch.  If not, please apply.  The patch will
 be helpful for any platform that needs/wants correct unwind info for
 constructors/destructors.  Thanks.]

My previous patch (adopted from HJ's work) was missing a change to
dl-fini.c which broke the handling of .fini_array.  The patch below is
updated to fix that and "make check" now passes as expected (I must
have made a mistake previously as I thought it worked with the
previous patch already).

If it looks OK, please apply.  (Yes, I know both Uli and Roland are
out this week, but I'll likely be out next week...).

Thanks,

	--david

2002-10-28  David Mosberger  <davidm@hpl.hp.com>

	* elf/dl-fini.c (_dl_fini): Invoke fini_array in _reverse_ order.
	Don't add l->l_addr to array entry values.

2002-03-12  H.J. Lu  <hjl@gnu.org>

	* csu/init.c (__libc_preinit_array): New. Define if
	HAVE_INITFINI_ARRAY is defined.
	(__libc_init_array): Likewise.
	(__libc_fini_array): Likewise.

	* elf/Makefile (tests): Add tst-array1, tst-array2 and
	tst-array3 if $(have-initfini-array) is yes.
	(tests-static): Add tst-array3 if $(have-initfini-array) is
	yes.
	(modules-names): Add tst-array2dep if $(have-initfini-array)
	is yes.
	($(objpfx)tst-array1.out): New target.
	($(objpfx)tst-array2): Likewise.
	($(objpfx)tst-array2.out): Likewise.
	($(objpfx)tst-array3.out): Likewise.

	* elf/tst-array1.c: New file.
	* elf/tst-array1.exp: Likewise.
	* elf/tst-array2.c: Likewise.
	* elf/tst-array2dep.c: Likewise.
	* elf/tst-array2.exp: Likewise.
	* elf/tst-array3.c: Likewise.

	* sysdeps/generic/libc-start.c (__libc_start_main): Process
	__libc_preinit_array, __libc_init_array and __libc_fini_array
	if HAVE_INITFINI_ARRAY is defined.

Index: csu/init.c
===================================================================
RCS file: /cvs/glibc/libc/csu/init.c,v
retrieving revision 1.4
diff -u -r1.4 init.c
--- csu/csu/init.c	6 Jul 2001 04:54:45 -0000	1.4
+++ csu/csu/init.c	29 Oct 2002 05:35:15 -0000
@@ -25,3 +25,43 @@
 const int _IO_stdin_used = _G_IO_IO_FILE_VERSION;
 
 #endif
+
+#ifdef HAVE_INITFINI_ARRAY
+extern void (*__preinit_array_start []) (void);
+extern void (*__preinit_array_end []) (void);
+
+void
+__libc_preinit_array (void)
+{
+  unsigned int size, i;
+
+  size = __preinit_array_end - __preinit_array_start;
+  for (i = 0; i < size; i++)
+    __preinit_array_start [i] ();
+}
+
+extern void (*__init_array_start []) (void);
+extern void (*__init_array_end []) (void);
+
+void
+__libc_init_array (void)
+{
+  unsigned int size, i;
+
+  size =  __init_array_end - __init_array_start;
+  for (i = 0; i < size; i++)
+    __init_array_start [i] ();
+}
+
+extern void (*__fini_array_start []) (void);
+extern void (*__fini_array_end []) (void);
+
+void
+__libc_fini_array (void)
+{
+  int i;
+
+  for (i = __fini_array_end - __fini_array_start - 1; i >= 0; i--)
+    __fini_array_start [i] ();
+}
+#endif
Index: elf/Makefile
===================================================================
RCS file: /cvs/glibc/libc/elf/Makefile,v
retrieving revision 1.242
diff -u -r1.242 Makefile
--- elf/elf/Makefile	24 Oct 2002 00:20:38 -0000	1.242
+++ elf/elf/Makefile	29 Oct 2002 05:35:15 -0000
@@ -117,6 +117,9 @@
 endif
 
 tests = tst-tls1 tst-tls2 tst-tls9
+ifeq (yes,$(have-initfini-array))
+tests += tst-array1 tst-array2 tst-array3
+endif
 ifeq (yes,$(build-static))
 tests-static = tst-tls1-static tst-tls2-static
 ifeq (yesyesyes,$(build-static)$(build-shared)$(elf))
@@ -156,6 +159,9 @@
 		tst-tlsmod5 tst-tlsmod6 \
 		circlemod1 circlemod1a circlemod2 circlemod2a \
 		circlemod3 circlemod3a
+ifeq (yes,$(have-initfini-array))
+modules-names += tst-array2dep
+endif
 modules-vis-yes = vismod1 vismod2 vismod3
 modules-nodelete-yes = nodelmod1 nodelmod2 nodelmod3 nodelmod4
 modules-nodlopen-yes = nodlopenmod nodlopenmod2
@@ -543,3 +549,22 @@
 $(objpfx)tst-tls9-static: $(common-objpfx)dlfcn/libdl.a
 $(objpfx)tst-tls9-static.out: $(objpfx)tst-tlsmod5.so $(objpfx)tst-tlsmod6.so
 endif
+
+$(objpfx)tst-array1.out: $(objpfx)tst-array1
+	$(elf-objpfx)$(rtld-installed-name) \
+	  --library-path $(rpath-link)$(patsubst %,:%,$(sysdep-library-path)) \
+	  $(objpfx)tst-array1 > $@
+	cmp $@ tst-array1.exp > /dev/null
+
+$(objpfx)tst-array2: $(objpfx)tst-array2dep.so
+$(objpfx)tst-array2.out: $(objpfx)tst-array2
+	$(elf-objpfx)$(rtld-installed-name) \
+	  --library-path $(rpath-link)$(patsubst %,:%,$(sysdep-library-path)) \
+	  $(objpfx)tst-array2 > $@
+	cmp $@ tst-array2.exp > /dev/null
+
+$(objpfx)tst-array3.out: $(objpfx)tst-array3
+	$(elf-objpfx)$(rtld-installed-name) \
+	  --library-path $(rpath-link)$(patsubst %,:%,$(sysdep-library-path)) \
+	  $(objpfx)tst-array3 > $@
+	cmp $@ tst-array1.exp > /dev/null
Index: elf/dl-fini.c
===================================================================
RCS file: /cvs/glibc/libc/elf/dl-fini.c,v
retrieving revision 1.27
diff -u -r1.27 dl-fini.c
--- elf/elf/dl-fini.c	1 Mar 2002 09:11:58 -0000	1.27
+++ elf/elf/dl-fini.c	29 Oct 2002 05:35:15 -0000
@@ -157,12 +157,11 @@
 	      ElfW(Addr) *array =
 		(ElfW(Addr) *) (l->l_addr
 				+ l->l_info[DT_FINI_ARRAY]->d_un.d_ptr);
-	      unsigned int sz = (l->l_info[DT_FINI_ARRAYSZ]->d_un.d_val
-				 / sizeof (ElfW(Addr)));
-	      unsigned int cnt;
+	      int i = (l->l_info[DT_FINI_ARRAYSZ]->d_un.d_val
+		       / sizeof (ElfW(Addr))) - 1;
 
-	      for (cnt = 0; cnt < sz; ++cnt)
-		((fini_t) (l->l_addr + array[cnt])) ();
+	      for (; i >= 0; --i)
+		((fini_t) array[i]) ();
 	    }
 
 	  /* Next try the old-style destructor.  */
Index: elf/tst-array1.c
===================================================================
RCS file: elf/tst-array1.c
diff -N elf/tst-array1.c
--- /dev/null	1 Jan 1970 00:00:00 -0000
+++ elf/elf/tst-array1.c	29 Oct 2002 05:35:15 -0000
@@ -0,0 +1,101 @@
+#include <unistd.h>
+
+static void init (void) __attribute__ ((constructor));
+
+static void
+init (void)
+{
+  write (STDOUT_FILENO, "init\n", 5);
+}
+
+static void fini (void) __attribute__ ((destructor));
+
+static void
+fini (void)
+{
+  write (STDOUT_FILENO, "fini\n", 5);
+}
+
+static void
+preinit_0 (void)
+{
+  write (STDOUT_FILENO, "preinit array 0\n", 16);
+}
+
+static void
+preinit_1 (void)
+{
+  write (STDOUT_FILENO, "preinit array 1\n", 16);
+}
+
+static void
+preinit_2 (void)
+{
+  write (STDOUT_FILENO, "preinit array 2\n", 16);
+}
+
+void
+(*preinit_array []) (void) __attribute__ ((section (".preinit_array"))) =
+{
+  &preinit_0,
+  &preinit_1,
+  &preinit_2
+};
+
+static void
+init_0 (void)
+{
+  write (STDOUT_FILENO, "init array 0\n", 13);
+}
+
+static void
+init_1 (void)
+{
+  write (STDOUT_FILENO, "init array 1\n", 13);
+}
+
+static void
+init_2 (void)
+{
+  write (STDOUT_FILENO, "init array 2\n", 13);
+}
+
+void
+(*init_array []) (void) __attribute__ ((section (".init_array"))) =
+{
+  &init_0,
+  &init_1,
+  &init_2
+};
+
+static void
+fini_0 (void)
+{
+  write (STDOUT_FILENO, "fini array 0\n", 13);
+}
+
+static void
+fini_1 (void)
+{
+  write (STDOUT_FILENO, "fini array 1\n", 13);
+}
+
+static void
+fini_2 (void)
+{
+  write (STDOUT_FILENO, "fini array 2\n", 13);
+}
+
+void
+(*fini_array []) (void) __attribute__ ((section (".fini_array"))) =
+{
+  &fini_0,
+  &fini_1,
+  &fini_2
+};
+
+int
+main (void)
+{
+  return 0;
+}
Index: elf/tst-array1.exp
===================================================================
RCS file: elf/tst-array1.exp
diff -N elf/tst-array1.exp
--- /dev/null	1 Jan 1970 00:00:00 -0000
+++ elf/elf/tst-array1.exp	29 Oct 2002 05:35:15 -0000
@@ -0,0 +1,11 @@
+preinit array 0
+preinit array 1
+preinit array 2
+init
+init array 0
+init array 1
+init array 2
+fini array 2
+fini array 1
+fini array 0
+fini
Index: elf/tst-array2.c
===================================================================
RCS file: elf/tst-array2.c
diff -N elf/tst-array2.c
--- /dev/null	1 Jan 1970 00:00:00 -0000
+++ elf/elf/tst-array2.c	29 Oct 2002 05:35:15 -0000
@@ -0,0 +1 @@
+#include "tst-array1.c"
Index: elf/tst-array2.exp
===================================================================
RCS file: elf/tst-array2.exp
diff -N elf/tst-array2.exp
--- /dev/null	1 Jan 1970 00:00:00 -0000
+++ elf/elf/tst-array2.exp	29 Oct 2002 05:35:15 -0000
@@ -0,0 +1,19 @@
+preinit array 0
+preinit array 1
+preinit array 2
+DSO init
+DSO init array 0
+DSO init array 1
+DSO init array 2
+init
+init array 0
+init array 1
+init array 2
+fini array 2
+fini array 1
+fini array 0
+fini
+DSO fini array 2
+DSO fini array 1
+DSO fini array 0
+DSO fini
Index: elf/tst-array2dep.c
===================================================================
RCS file: elf/tst-array2dep.c
diff -N elf/tst-array2dep.c
--- /dev/null	1 Jan 1970 00:00:00 -0000
+++ elf/elf/tst-array2dep.c	29 Oct 2002 05:35:15 -0000
@@ -0,0 +1,69 @@
+#include <unistd.h>
+
+static void init (void) __attribute__ ((constructor));
+
+static void
+init (void)
+{
+  write (STDOUT_FILENO, "DSO init\n", 9);
+}
+
+static void fini (void) __attribute__ ((destructor));
+
+static void
+fini (void)
+{
+  write (STDOUT_FILENO, "DSO fini\n", 9);
+}
+
+static void
+init_0 (void)
+{
+  write (STDOUT_FILENO, "DSO init array 0\n", 17);
+}
+
+static void
+init_1 (void)
+{
+  write (STDOUT_FILENO, "DSO init array 1\n", 17);
+}
+
+static void
+init_2 (void)
+{
+  write (STDOUT_FILENO, "DSO init array 2\n", 17);
+}
+
+void
+(*init_array []) (void) __attribute__ ((section (".init_array"))) =
+{
+  &init_0,
+  &init_1,
+  &init_2
+};
+
+static void
+fini_0 (void)
+{
+  write (STDOUT_FILENO, "DSO fini array 0\n", 17);
+}
+
+static void
+fini_1 (void)
+{
+  write (STDOUT_FILENO, "DSO fini array 1\n", 17);
+}
+
+static void
+fini_2 (void)
+{
+  write (STDOUT_FILENO, "DSO fini array 2\n", 17);
+}
+
+void
+(*fini_array []) (void) __attribute__ ((section (".fini_array"))) =
+{
+  &fini_0,
+  &fini_1,
+  &fini_2
+};
Index: elf/tst-array3.c
===================================================================
RCS file: elf/tst-array3.c
diff -N elf/tst-array3.c
--- /dev/null	1 Jan 1970 00:00:00 -0000
+++ elf/elf/tst-array3.c	29 Oct 2002 05:35:15 -0000
@@ -0,0 +1 @@
+#include "tst-array1.c"
Index: sysdeps/generic/libc-start.c
===================================================================
RCS file: /cvs/glibc/libc/sysdeps/generic/libc-start.c,v
retrieving revision 1.33
diff -u -r1.33 libc-start.c
--- sysdeps/generic/sysdeps/generic/libc-start.c	25 Oct 2002 19:41:24 -0000	1.33
+++ sysdeps/generic/sysdeps/generic/libc-start.c	29 Oct 2002 05:35:16 -0000
@@ -36,6 +36,16 @@
      ;
 #endif
 
+#ifdef HAVE_INITFINI_ARRAY
+# ifdef SHARED
+extern void __libc_init_array (void) __attribute__ ((weak));
+extern void __libc_fini_array (void) __attribute__ ((weak));
+# else
+extern void __libc_preinit_array (void);
+extern void __libc_init_array (void);
+extern void __libc_fini_array (void);
+# endif
+#endif
 
 extern int BP_SYM (__libc_start_main) (int (*main) (int, char **, char **),
 				       int argc,
@@ -115,6 +125,17 @@
   if (fini)
     __cxa_atexit ((void (*) (void *)) fini, NULL, NULL);
 
+#ifdef HAVE_INITFINI_ARRAY
+# ifdef SHARED
+  if (__libc_fini_array)
+# endif
+    __cxa_atexit ((void (*) (void *)) __libc_fini_array, NULL, NULL);
+
+# ifndef SHARED
+  __libc_preinit_array ();
+# endif
+#endif
+
   /* Call the initializer of the program, if any.  */
 #ifdef SHARED
   if (__builtin_expect (GL(dl_debug_mask) & DL_DEBUG_IMPCALLS, 0))
@@ -122,6 +143,13 @@
 #endif
   if (init)
     (*init) ();
+
+#ifdef HAVE_INITFINI_ARRAY
+# ifdef SHARED
+  if (__libc_init_array)
+# endif
+    __libc_init_array ();
+#endif
 
 #ifdef SHARED
   if (__builtin_expect (GL(dl_debug_mask) & DL_DEBUG_IMPCALLS, 0))

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

end of thread, other threads:[~2002-12-09 21:36 UTC | newest]

Thread overview: 18+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2002-11-08 11:34 patch to make init_array work (2nd version; resend) Roland McGrath
2002-11-08 11:38 ` David Mosberger
2002-11-27 14:35 ` patch to make init_array work (3nd version) David Mosberger
2002-11-27 15:10   ` H. J. Lu
2002-11-27 15:34     ` David Mosberger
2002-12-09 10:52   ` Roland McGrath
2002-12-09 13:36     ` David Mosberger
  -- strict thread matches above, loose matches on Subject: below --
2002-11-07 11:43 patch to make init_array work (2nd version; resend) David Mosberger
2002-11-07 13:31 ` Roland McGrath
2002-11-07 13:51   ` David Mosberger
2002-11-07 13:58     ` Roland McGrath
2002-11-07 14:10       ` David Mosberger
2002-11-07 14:29         ` Roland McGrath
2002-11-07 14:38           ` David Mosberger
2002-11-07 18:10       ` Richard Henderson
2002-11-07 18:20         ` Roland McGrath
2002-11-07 18:30           ` David Mosberger
2002-11-07 18:45             ` Roland McGrath

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).