Stefan Kempf <sisnk...@gmail.com> 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 -0000 1.20
> +++ lib/csu/crtbegin.c 1 Aug 2016 16:56:31 -0000
> @@ -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__, &object);
> 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 -0000 1.16
> +++ lib/csu/crtbeginS.c 1 Aug 2016 16:56:31 -0000
> @@ -38,7 +38,6 @@
> * constructors and destructors. The first element contains the
> * number of pointers in each.
> * The tables are also null-terminated.
> -
> */
> #include <stdlib.h>
>
> @@ -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 @@ MD_SECTION_PROLOGUE(".fini", _fini);
> MD_SECT_CALL_FUNC(".init", _do_init);
> MD_SECT_CALL_FUNC(".fini", _do_fini);
>
> -void
> +static void
> _do_init(void)
> {
> static int initialized = 0;
> @@ -159,7 +158,7 @@ _do_init(void)
> }
> }
>
> -void
> +static void
> _do_fini(void)
> {
> static int finalized = 0;
>
--
jca | PGP: 0x1524E7EE / 5135 92C1 AD36 5293 2BDF DDCC 0DFA 74AE 1524 E7EE
