Re: C startup code: make crtbegin code safe for clang

2016-08-07 Thread Mark Kettenis
> Date: Sun, 7 Aug 2016 20:43:23 +0200
> From: Stefan Kempf 
> 
> Philip Guenther wrote:
> > On Mon, Aug 1, 2016 at 11:45 AM, Mark Kettenis  
> > wrote:
> > >> From: j...@wxcvbn.org (Jeremie Courreges-Anglas)
> > >> Date: Mon, 01 Aug 2016 20:30:33 +0200
> > >>
> > >> Stefan Kempf  writes:
> > >>
> > >> > The constructor and destructor tables are declared as arrays with one
> > >> > non-NULL element. Walking those until a NULL element is reached looks
> > >> > like out-of-bound accesses to newer compilers, and they turn the code
> > >> > into infinite loops (e.g. clang 3.8), because it is undefined behavior.
> > 
> > So it needs to reference the pointers via an extern incomplete array?
> > Can we move to setting up the leading count via the linker script
> > magic documented in the ld info page, and then just use __CTOR_LIST__
> > as the extern array?
>  
> Here's an attempt at this. The startup code itself is unchanged except for
> using extern instead static arrays now.
> 
> This needs building and installing binutils first. Afterwards lib/csu
> can be recompiled.
> 
> Does that diff look better?

Not to me.  I really don't like depending on linker magic for this.

Besides, this conflicts heavily with the relro work going on.  Even if
other people think this is the way to go, this will have to be shelved
for now.

If your are emitting inline asm in crtbegin.c anyway, perhaps you can
just do this for __CTOR_LIST__ and __DTOR_LIST__ as well?

> Index: gnu/usr.bin/binutils-2.17/ld/emulparams/elf_obsd.sh
> ===
> RCS file: /cvs/src/gnu/usr.bin/binutils-2.17/ld/emulparams/elf_obsd.sh,v
> retrieving revision 1.2
> diff -u -p -r1.2 elf_obsd.sh
> --- gnu/usr.bin/binutils-2.17/ld/emulparams/elf_obsd.sh   30 Apr 2015 
> 17:56:18 -  1.2
> +++ gnu/usr.bin/binutils-2.17/ld/emulparams/elf_obsd.sh   7 Aug 2016 
> 17:42:23 -
> @@ -7,4 +7,23 @@ PAD_GOT=
>  PAD_PLT=
>  DATA_START_SYMBOLS='__data_start = . ;'
>  
> +if [ "$ELFSIZE" = 64 ]
> +then
> + CTOR_START="PROVIDE_HIDDEN(__CTOR_LIST__ = .); \
> + QUAD((__CTOR_END__ - __CTOR_LIST__) / 8 - 2);"
> + CTOR_END="QUAD(0); PROVIDE_HIDDEN(__CTOR_END__ = .);"
> +
> + DTOR_START="PROVIDE_HIDDEN(__DTOR_LIST__ = .); \
> + QUAD((__DTOR_END__ - __DTOR_LIST__) / 8 - 2);"
> + DTOR_END="QUAD(0); PROVIDE_HIDDEN(__DTOR_END__ = .);"
> +else
> + CTOR_START="PROVIDE_HIDDEN(__CTOR_LIST__ = .); \
> + LONG((__CTOR_END__ - __CTOR_LIST__) / 4 - 2);"
> + CTOR_END="LONG(0); PROVIDE_HIDDEN(__CTOR_END__ = .);"
> +
> + DTOR_START="PROVIDE_HIDDEN(__DTOR_LIST__ = .); \
> + LONG((__DTOR_END__ - __DTOR_LIST__) / 4 - 2);"
> + DTOR_END="LONG(0); PROVIDE_HIDDEN(__DTOR_END__ = .);"
> +fi
> +
>  unset SEPARATE_GOTPLT
> Index: gnu/usr.bin/binutils-2.17/ld/scripttempl/elf.sc
> ===
> RCS file: /cvs/src/gnu/usr.bin/binutils-2.17/ld/scripttempl/elf.sc,v
> retrieving revision 1.8
> diff -u -p -r1.8 elf.sc
> --- gnu/usr.bin/binutils-2.17/ld/scripttempl/elf.sc   9 Aug 2014 04:49:47 
> -   1.8
> +++ gnu/usr.bin/binutils-2.17/ld/scripttempl/elf.sc   7 Aug 2016 17:42:23 
> -
> @@ -203,7 +203,8 @@ fi
>  CTOR=".ctors${CONSTRUCTING-0} : 
>{
>  ${CONSTRUCTING+${CTOR_START}}
> -/* gcc uses crtbegin.o to find the start of
> +/* on some systems,
> +   gcc uses crtbegin.o to find the start of
> the constructors, so we make sure it is
> first.  Because this is a wildcard, it
> doesn't matter if the user does not
> @@ -218,7 +219,8 @@ CTOR=".ctors${CONSTRUCTING-0} : 
>  /* We don't want to include the .ctor section from
> the crtend.o file until after the sorted ctors.
> The .ctor section from the crtend file contains the
> -   end of ctors marker and it must be last */
> +   end of ctors marker and it must be last
> +   on some systems */
>  
>  KEEP (*(EXCLUDE_FILE (*crtend*.o $OTHER_EXCLUDE_FILES) .ctors))
>  KEEP (*(SORT(.ctors.*)))
> @@ -371,7 +373,11 @@ cat <${CREATE_SHLIB-${SBSS2}}
>${OTHER_READONLY_SECTIONS}
>.eh_frame_hdr : { *(.eh_frame_hdr) }
> -  .eh_frame ${RELOCATING-0} : ONLY_IF_RO { KEEP (*(.eh_frame)) }
> +  .eh_frame ${RELOCATING-0} : ONLY_IF_RO
> +  {
> +${RELOCATING+PROVIDE_HIDDEN(__EH_FRAME_BEGIN__ = .);}
> +KEEP (*(.eh_frame))
> +  }
>.gcc_except_table ${RELOCATING-0} : ONLY_IF_RO { *(.gcc_except_table 
> .gcc_except_table.*) }
>  
>/* Adjust the address for the data segment.  We want to adjust up to
> @@ -381,7 +387,11 @@ cat <${CREATE_PIE+${RELOCATING+. = ${SHLIB_DATA_ADDR-${DATA_SEGMENT_ALIGN}};}}
>  
>/* Exception handling  */
> -  .eh_frame ${RELOCATING-0} : ONLY_IF_RW { KEEP (*(.eh_frame)) }
> +  .eh_frame ${RELOCATING-0} : ONLY_IF_RW
> +  {
> +

Re: C startup code: make crtbegin code safe for clang

2016-08-07 Thread Stefan Kempf
Philip Guenther wrote:
> On Mon, Aug 1, 2016 at 11:45 AM, Mark Kettenis  
> wrote:
> >> From: j...@wxcvbn.org (Jeremie Courreges-Anglas)
> >> Date: Mon, 01 Aug 2016 20:30:33 +0200
> >>
> >> Stefan Kempf  writes:
> >>
> >> > The constructor and destructor tables are declared as arrays with one
> >> > non-NULL element. Walking those until a NULL element is reached looks
> >> > like out-of-bound accesses to newer compilers, and they turn the code
> >> > into infinite loops (e.g. clang 3.8), because it is undefined behavior.
> 
> So it needs to reference the pointers via an extern incomplete array?
> Can we move to setting up the leading count via the linker script
> magic documented in the ld info page, and then just use __CTOR_LIST__
> as the extern array?
 
Here's an attempt at this. The startup code itself is unchanged except for
using extern instead static arrays now.

This needs building and installing binutils first. Afterwards lib/csu
can be recompiled.

Does that diff look better?

Index: gnu/usr.bin/binutils-2.17/ld/emulparams/elf_obsd.sh
===
RCS file: /cvs/src/gnu/usr.bin/binutils-2.17/ld/emulparams/elf_obsd.sh,v
retrieving revision 1.2
diff -u -p -r1.2 elf_obsd.sh
--- gnu/usr.bin/binutils-2.17/ld/emulparams/elf_obsd.sh 30 Apr 2015 17:56:18 
-  1.2
+++ gnu/usr.bin/binutils-2.17/ld/emulparams/elf_obsd.sh 7 Aug 2016 17:42:23 
-
@@ -7,4 +7,23 @@ PAD_GOT=
 PAD_PLT=
 DATA_START_SYMBOLS='__data_start = . ;'
 
+if [ "$ELFSIZE" = 64 ]
+then
+   CTOR_START="PROVIDE_HIDDEN(__CTOR_LIST__ = .); \
+   QUAD((__CTOR_END__ - __CTOR_LIST__) / 8 - 2);"
+   CTOR_END="QUAD(0); PROVIDE_HIDDEN(__CTOR_END__ = .);"
+
+   DTOR_START="PROVIDE_HIDDEN(__DTOR_LIST__ = .); \
+   QUAD((__DTOR_END__ - __DTOR_LIST__) / 8 - 2);"
+   DTOR_END="QUAD(0); PROVIDE_HIDDEN(__DTOR_END__ = .);"
+else
+   CTOR_START="PROVIDE_HIDDEN(__CTOR_LIST__ = .); \
+   LONG((__CTOR_END__ - __CTOR_LIST__) / 4 - 2);"
+   CTOR_END="LONG(0); PROVIDE_HIDDEN(__CTOR_END__ = .);"
+
+   DTOR_START="PROVIDE_HIDDEN(__DTOR_LIST__ = .); \
+   LONG((__DTOR_END__ - __DTOR_LIST__) / 4 - 2);"
+   DTOR_END="LONG(0); PROVIDE_HIDDEN(__DTOR_END__ = .);"
+fi
+
 unset SEPARATE_GOTPLT
Index: gnu/usr.bin/binutils-2.17/ld/scripttempl/elf.sc
===
RCS file: /cvs/src/gnu/usr.bin/binutils-2.17/ld/scripttempl/elf.sc,v
retrieving revision 1.8
diff -u -p -r1.8 elf.sc
--- gnu/usr.bin/binutils-2.17/ld/scripttempl/elf.sc 9 Aug 2014 04:49:47 
-   1.8
+++ gnu/usr.bin/binutils-2.17/ld/scripttempl/elf.sc 7 Aug 2016 17:42:23 
-
@@ -203,7 +203,8 @@ fi
 CTOR=".ctors${CONSTRUCTING-0} : 
   {
 ${CONSTRUCTING+${CTOR_START}}
-/* gcc uses crtbegin.o to find the start of
+/* on some systems,
+   gcc uses crtbegin.o to find the start of
the constructors, so we make sure it is
first.  Because this is a wildcard, it
doesn't matter if the user does not
@@ -218,7 +219,8 @@ CTOR=".ctors${CONSTRUCTING-0} : 
 /* We don't want to include the .ctor section from
the crtend.o file until after the sorted ctors.
The .ctor section from the crtend file contains the
-   end of ctors marker and it must be last */
+   end of ctors marker and it must be last
+   on some systems */
 
 KEEP (*(EXCLUDE_FILE (*crtend*.o $OTHER_EXCLUDE_FILES) .ctors))
 KEEP (*(SORT(.ctors.*)))
@@ -371,7 +373,11 @@ cat < >> > While there, clean up crtbegin.c and crtbegin.S a little and make them
> >> > more similar.
> 
> I'm fine with this being done...once we sure we actually have nailed
> down what are the actual changes necessary to get the code to compile
> correctly without changing behavior.
> 
> ...
> >> The existing code tries to retrieve the number of valid ctors entries
> >> from __CTOR_LIST__[0], only when that number is -1 it tries to find
> >> the actual value by walking the array.
> 
> Since we only have one version of ld doing linking now, can't we
> switch from the -1 to the real count version?
> 
> ...
> > Also, aren't ctor_list and dtor_list polluting the namespace?
> 
> Yep.
> 
> 
> Philip



Re: C startup code: make crtbegin code safe for clang

2016-08-04 Thread Joerg Sonnenberger
On Mon, Aug 01, 2016 at 07:28:34PM +0200, Stefan Kempf wrote:
> The constructor and destructor tables are declared as arrays with one
> non-NULL element. Walking those until a NULL element is reached looks
> like out-of-bound accesses to newer compilers, and they turn the code
> into infinite loops (e.g. clang 3.8), because it is undefined behavior.

I don't think your code is safe for the more aggressive behavior esp. of
newer GCC. Given how much fun I had recently on this, consider just
using the NetBSD version of this code.

Joerg



Re: C startup code: make crtbegin code safe for clang

2016-08-03 Thread Stefan Kempf
Philip Guenther wrote:
> On Mon, Aug 1, 2016 at 11:45 AM, Mark Kettenis  
> wrote:
> >> From: j...@wxcvbn.org (Jeremie Courreges-Anglas)
> >> Date: Mon, 01 Aug 2016 20:30:33 +0200
> >>
> >> Stefan Kempf  writes:
> >>
> >> > The constructor and destructor tables are declared as arrays with one
> >> > non-NULL element. Walking those until a NULL element is reached looks
> >> > like out-of-bound accesses to newer compilers, and they turn the code
> >> > into infinite loops (e.g. clang 3.8), because it is undefined behavior.
> 
> So it needs to reference the pointers via an extern incomplete array?
> Can we move to setting up the leading count via the linker script
> magic documented in the ld info page, and then just use __CTOR_LIST__
> as the extern array?
 
Right, we need the extern trick.
I'll take a look into doing it with counts and __CTOR_LIST__ and
__CTOR_END__. We want __CTOR_LIST__ and __CTOR_END__ to be .hidden
symbols, right?
 
> >> > While there, clean up crtbegin.c and crtbegin.S a little and make them
> >> > more similar.
> 
> I'm fine with this being done...once we sure we actually have nailed
> down what are the actual changes necessary to get the code to compile
> correctly without changing behavior.
> 
> ...
> >> The existing code tries to retrieve the number of valid ctors entries
> >> from __CTOR_LIST__[0], only when that number is -1 it tries to find
> >> the actual value by walking the array.
> 
> Since we only have one version of ld doing linking now, can't we
> switch from the -1 to the real count version?
> 
> ...
> > Also, aren't ctor_list and dtor_list polluting the namespace?
> 
> Yep.
 
> Philip



Re: C startup code: make crtbegin code safe for clang

2016-08-01 Thread Philip Guenther
On Mon, Aug 1, 2016 at 11:45 AM, Mark Kettenis  wrote:
>> From: j...@wxcvbn.org (Jeremie Courreges-Anglas)
>> Date: Mon, 01 Aug 2016 20:30:33 +0200
>>
>> Stefan Kempf  writes:
>>
>> > The constructor and destructor tables are declared as arrays with one
>> > non-NULL element. Walking those until a NULL element is reached looks
>> > like out-of-bound accesses to newer compilers, and they turn the code
>> > into infinite loops (e.g. clang 3.8), because it is undefined behavior.

So it needs to reference the pointers via an extern incomplete array?
Can we move to setting up the leading count via the linker script
magic documented in the ld info page, and then just use __CTOR_LIST__
as the extern array?


>> > While there, clean up crtbegin.c and crtbegin.S a little and make them
>> > more similar.

I'm fine with this being done...once we sure we actually have nailed
down what are the actual changes necessary to get the code to compile
correctly without changing behavior.

...
>> The existing code tries to retrieve the number of valid ctors entries
>> from __CTOR_LIST__[0], only when that number is -1 it tries to find
>> the actual value by walking the array.

Since we only have one version of ld doing linking now, can't we
switch from the -1 to the real count version?

...
> Also, aren't ctor_list and dtor_list polluting the namespace?

Yep.


Philip



Re: C startup code: make crtbegin code safe for clang

2016-08-01 Thread Mark Kettenis
> From: j...@wxcvbn.org (Jeremie Courreges-Anglas)
> Date: Mon, 01 Aug 2016 20:30:33 +0200
> 
> Stefan Kempf  writes:
> 
> > The constructor and destructor tables are declared as arrays with one
> > non-NULL element. Walking those until a NULL element is reached looks
> > like out-of-bound accesses to newer compilers, and they turn the code
> > into infinite loops (e.g. clang 3.8), because it is undefined behavior.
> >
> > Use constructor/destructor calling code that should be legal in both the
> > gcc and clang C dialect, to hopefully be immune from undefined behavior
> > optimizations in the future.
> >
> > While there, clean up crtbegin.c and crtbegin.S a little and make them
> > more similar.
> >
> > ok?
> >
> > Index: lib/csu/crtbegin.c
> > ===
> > RCS file: /cvs/src/lib/csu/crtbegin.c,v
> > retrieving revision 1.20
> > diff -u -p -r1.20 crtbegin.c
> > --- lib/csu/crtbegin.c  10 Nov 2015 04:14:03 -  1.20
> > +++ lib/csu/crtbegin.c  1 Aug 2016 16:56:31 -
> > @@ -85,36 +85,37 @@ long __guard_local __dso_hidden __attrib
> >  
> >  
> >  static const init_f __CTOR_LIST__[1]
> > -__attribute__((section(".ctors"))) = { (void *)-1 };   /* XXX */
> > +__used __attribute__((section(".ctors"))) = { (void *)-1 };/* XXX 
> > */
> >  static const init_f __DTOR_LIST__[1]
> > -__attribute__((section(".dtors"))) = { (void *)-1 };   /* XXX */
> > +__used __attribute__((section(".dtors"))) = { (void *)-1 };/* XXX 
> > */
> > +
> > +extern const init_f ctor_list[] asm(".ctors");
> > +extern const init_f dtor_list[] asm(".dtors");
> >  
> >  static void__dtors(void) __used;
> >  static void__ctors(void) __used;
> >  
> >  static void
> > -__ctors()
> > +__ctors(void)
> >  {
> > -   unsigned long i = (unsigned long) __CTOR_LIST__[0];
> > -   const init_f *p;
> > +   int i;
> > +
> > +   for (i = 0; ctor_list[i + 1] != NULL; i++)
> > +   continue;
> >  
> > -   if (i == -1)  {
> > -   for (i = 1; __CTOR_LIST__[i] != NULL; i++)
> > -   ;
> > +   while (i > 0) {
> > +   ctor_list[i]();
> 
> The existing code tries to retrieve the number of valid ctors entries
> from __CTOR_LIST__[0], only when that number is -1 it tries to find
> the actual value by walking the array.
> 
> The ld(1) info page states:
> 
>  The symbol `__CTOR_LIST__' marks the start of the global
>  constructors, and the symbol `__CTOR_END__' marks the end.
>  Similarly, `__DTOR_LIST__' and `__DTOR_END__' mark the start and
>  end of the global destructors.  The first word in the list is the
>  number of entries, followed by the address of each constructor or
>  destructor, followed by a zero word.  The compiler must arrange to
>  actually run the code.  For these object file formats GNU C++
>  normally calls constructors from a subroutine `__main'; a call to
>  `__main' is automatically inserted into the startup code for
>  `main'.  GNU C++ normally runs destructors either by using
>  `atexit', or directly from the function `exit'.
> 
> If that is correct your code should behave the same.  But what if...?

Right, the code is not equivalent.  We'd have to look carefully at gcc
and ld to see if that matters.

Also, aren't ctor_list and dtor_list polluting the namespace?



Re: C startup code: make crtbegin code safe for clang

2016-08-01 Thread Jeremie Courreges-Anglas
Stefan Kempf  writes:

> The constructor and destructor tables are declared as arrays with one
> non-NULL element. Walking those until a NULL element is reached looks
> like out-of-bound accesses to newer compilers, and they turn the code
> into infinite loops (e.g. clang 3.8), because it is undefined behavior.
>
> Use constructor/destructor calling code that should be legal in both the
> gcc and clang C dialect, to hopefully be immune from undefined behavior
> optimizations in the future.
>
> While there, clean up crtbegin.c and crtbegin.S a little and make them
> more similar.
>
> ok?
>
> Index: lib/csu/crtbegin.c
> ===
> RCS file: /cvs/src/lib/csu/crtbegin.c,v
> retrieving revision 1.20
> diff -u -p -r1.20 crtbegin.c
> --- lib/csu/crtbegin.c10 Nov 2015 04:14:03 -  1.20
> +++ lib/csu/crtbegin.c1 Aug 2016 16:56:31 -
> @@ -85,36 +85,37 @@ long __guard_local __dso_hidden __attrib
>  
>  
>  static const init_f __CTOR_LIST__[1]
> -__attribute__((section(".ctors"))) = { (void *)-1 }; /* XXX */
> +__used __attribute__((section(".ctors"))) = { (void *)-1 };  /* XXX 
> */
>  static const init_f __DTOR_LIST__[1]
> -__attribute__((section(".dtors"))) = { (void *)-1 }; /* XXX */
> +__used __attribute__((section(".dtors"))) = { (void *)-1 };  /* XXX 
> */
> +
> +extern const init_f ctor_list[] asm(".ctors");
> +extern const init_f dtor_list[] asm(".dtors");
>  
>  static void  __dtors(void) __used;
>  static void  __ctors(void) __used;
>  
>  static void
> -__ctors()
> +__ctors(void)
>  {
> - unsigned long i = (unsigned long) __CTOR_LIST__[0];
> - const init_f *p;
> + int i;
> +
> + for (i = 0; ctor_list[i + 1] != NULL; i++)
> + continue;
>  
> - if (i == -1)  {
> - for (i = 1; __CTOR_LIST__[i] != NULL; i++)
> - ;
> + while (i > 0) {
> + ctor_list[i]();

The existing code tries to retrieve the number of valid ctors entries
from __CTOR_LIST__[0], only when that number is -1 it tries to find
the actual value by walking the array.

The ld(1) info page states:

 The symbol `__CTOR_LIST__' marks the start of the global
 constructors, and the symbol `__CTOR_END__' marks the end.
 Similarly, `__DTOR_LIST__' and `__DTOR_END__' mark the start and
 end of the global destructors.  The first word in the list is the
 number of entries, followed by the address of each constructor or
 destructor, followed by a zero word.  The compiler must arrange to
 actually run the code.  For these object file formats GNU C++
 normally calls constructors from a subroutine `__main'; a call to
 `__main' is automatically inserted into the startup code for
 `main'.  GNU C++ normally runs destructors either by using
 `atexit', or directly from the function `exit'.

If that is correct your code should behave the same.  But what if...?

>   i--;
>   }
> - p = __CTOR_LIST__ + i;
> - while (i--)
> - (**p--)();
>  }
>  
>  static void
> -__dtors()
> +__dtors(void)
>  {
> - const init_f *p = __DTOR_LIST__ + 1;
> + int i;
>  
> - while (*p)
> - (**p++)();
> + for (i = 1; dtor_list[i] != NULL; i++)
> + dtor_list[i]();
>  }
>  
>  void __init(void);
> @@ -130,8 +131,8 @@ MD_SECT_CALL_FUNC(".init", __do_init);
>  MD_SECT_CALL_FUNC(".fini", __do_fini);
>  
>  
> -void
> -__do_init()
> +static void
> +__do_init(void)
>  {
>   static int initialized = 0;
>   static struct dwarf2_eh_object object;
> @@ -146,14 +147,14 @@ __do_init()
>   __register_frame_info(__EH_FRAME_BEGIN__, );
>   if (__JCR_LIST__[0] && _Jv_RegisterClasses)
>   _Jv_RegisterClasses(__JCR_LIST__);
> - (__ctors)();
> + __ctors();
>  
>   atexit(__fini);
>   }
>  }
>  
> -void
> -__do_fini()
> +static void
> +__do_fini(void)
>  {
>   static int finalized = 0;
>  
> @@ -162,7 +163,7 @@ __do_fini()
>   /*
>* Call global destructors.
>*/
> - (__dtors)();
> + __dtors();
>   }
>  }
>  
> Index: lib/csu/crtbeginS.c
> ===
> RCS file: /cvs/src/lib/csu/crtbeginS.c,v
> retrieving revision 1.16
> diff -u -p -r1.16 crtbeginS.c
> --- lib/csu/crtbeginS.c   7 Apr 2015 01:27:06 -   1.16
> +++ lib/csu/crtbeginS.c   1 Aug 2016 16:56:31 -
> @@ -38,7 +38,6 @@
>   * constructors and destructors. The first element contains the
>   * number of pointers in each.
>   * The tables are also null-terminated.
> -
>   */
>  #include 
>  
> @@ -96,39 +95,39 @@ asm(".hidden pthread_atfork\n .weak pthr
>  
>  
>  static init_f __CTOR_LIST__[1]
> -__attribute__((section(".ctors"))) = { (void *)-1 }; /* XXX */
> +__used 

C startup code: make crtbegin code safe for clang

2016-08-01 Thread Stefan Kempf
The constructor and destructor tables are declared as arrays with one
non-NULL element. Walking those until a NULL element is reached looks
like out-of-bound accesses to newer compilers, and they turn the code
into infinite loops (e.g. clang 3.8), because it is undefined behavior.

Use constructor/destructor calling code that should be legal in both the
gcc and clang C dialect, to hopefully be immune from undefined behavior
optimizations in the future.

While there, clean up crtbegin.c and crtbegin.S a little and make them
more similar.

ok?

Index: lib/csu/crtbegin.c
===
RCS file: /cvs/src/lib/csu/crtbegin.c,v
retrieving revision 1.20
diff -u -p -r1.20 crtbegin.c
--- lib/csu/crtbegin.c  10 Nov 2015 04:14:03 -  1.20
+++ lib/csu/crtbegin.c  1 Aug 2016 16:56:31 -
@@ -85,36 +85,37 @@ long __guard_local __dso_hidden __attrib
 
 
 static const init_f __CTOR_LIST__[1]
-__attribute__((section(".ctors"))) = { (void *)-1 };   /* XXX */
+__used __attribute__((section(".ctors"))) = { (void *)-1 };/* XXX 
*/
 static const init_f __DTOR_LIST__[1]
-__attribute__((section(".dtors"))) = { (void *)-1 };   /* XXX */
+__used __attribute__((section(".dtors"))) = { (void *)-1 };/* XXX 
*/
+
+extern const init_f ctor_list[] asm(".ctors");
+extern const init_f dtor_list[] asm(".dtors");
 
 static void__dtors(void) __used;
 static void__ctors(void) __used;
 
 static void
-__ctors()
+__ctors(void)
 {
-   unsigned long i = (unsigned long) __CTOR_LIST__[0];
-   const init_f *p;
+   int i;
+
+   for (i = 0; ctor_list[i + 1] != NULL; i++)
+   continue;
 
-   if (i == -1)  {
-   for (i = 1; __CTOR_LIST__[i] != NULL; i++)
-   ;
+   while (i > 0) {
+   ctor_list[i]();
i--;
}
-   p = __CTOR_LIST__ + i;
-   while (i--)
-   (**p--)();
 }
 
 static void
-__dtors()
+__dtors(void)
 {
-   const init_f *p = __DTOR_LIST__ + 1;
+   int i;
 
-   while (*p)
-   (**p++)();
+   for (i = 1; dtor_list[i] != NULL; i++)
+   dtor_list[i]();
 }
 
 void __init(void);
@@ -130,8 +131,8 @@ MD_SECT_CALL_FUNC(".init", __do_init);
 MD_SECT_CALL_FUNC(".fini", __do_fini);
 
 
-void
-__do_init()
+static void
+__do_init(void)
 {
static int initialized = 0;
static struct dwarf2_eh_object object;
@@ -146,14 +147,14 @@ __do_init()
__register_frame_info(__EH_FRAME_BEGIN__, );
if (__JCR_LIST__[0] && _Jv_RegisterClasses)
_Jv_RegisterClasses(__JCR_LIST__);
-   (__ctors)();
+   __ctors();
 
atexit(__fini);
}
 }
 
-void
-__do_fini()
+static void
+__do_fini(void)
 {
static int finalized = 0;
 
@@ -162,7 +163,7 @@ __do_fini()
/*
 * Call global destructors.
 */
-   (__dtors)();
+   __dtors();
}
 }
 
Index: lib/csu/crtbeginS.c
===
RCS file: /cvs/src/lib/csu/crtbeginS.c,v
retrieving revision 1.16
diff -u -p -r1.16 crtbeginS.c
--- lib/csu/crtbeginS.c 7 Apr 2015 01:27:06 -   1.16
+++ lib/csu/crtbeginS.c 1 Aug 2016 16:56:31 -
@@ -38,7 +38,6 @@
  * constructors and destructors. The first element contains the
  * number of pointers in each.
  * The tables are also null-terminated.
-
  */
 #include 
 
@@ -96,39 +95,39 @@ asm(".hidden pthread_atfork\n .weak pthr
 
 
 static init_f __CTOR_LIST__[1]
-__attribute__((section(".ctors"))) = { (void *)-1 };   /* XXX */
+__used __attribute__((section(".ctors"))) = { (void *)-1 };/* XXX 
*/
 static init_f __DTOR_LIST__[1]
-__attribute__((section(".dtors"))) = { (void *)-1 };   /* XXX */
+__used __attribute__((section(".dtors"))) = { (void *)-1 };/* XXX 
*/
+
+extern const init_f ctor_list[] asm(".ctors");
+extern const init_f dtor_list[] asm(".dtors");
 
 static void__dtors(void) __used;
 static void__ctors(void) __used;
 
-void
+static void
 __ctors(void)
 {
-   unsigned long i = (unsigned long) __CTOR_LIST__[0];
-   init_f *p;
+   int i;
 
-   if (i == -1)  {
-   for (i = 1; __CTOR_LIST__[i] != NULL; i++)
-   ;
+   for (i = 0; ctor_list[i + 1] != NULL; i++)
+   continue;
+
+   while (i > 0) {
+   ctor_list[i]();
i--;
}
-   p = __CTOR_LIST__ + i;
-   while (i--) {
-   (**p--)();
-   }
 }
 
 static void
 __dtors(void)
 {
-   init_f *p = __DTOR_LIST__ + 1;
+   int i;
 
-   while (*p) {
-   (**p++)();
-   }
+   for (i = 1; dtor_list[i] != NULL; i++)
+   dtor_list[i]();
 }
+
 void _init(void);
 void _fini(void);
 static void _do_init(void) __used;
@@ -141,7 +140,7 @@