On 22/03/2013 08:44, Kai Tietz wrote:
> 2013-03-22 Kai Tietz <kti...@redhat.com>
>
> * config/i386/cygming-crtbegin.c (__register_frame_info): Make weak.
> (__deregister_frame_info): Likewise.
Hi Kai,
I read your explanation of the problem relating to x86-64 memory models over
on the Cygwin dev list, and that explained your motivation for making this
change; I see why it's not easy to get an *ABS* 0 reference there. So,
providing dummy versions of the functions makes perfect sense to me, and
certainly won't cause problems for i686. (I did a lot of testing, and the
only problem I found is that a weak definition has to be provided on the
linker command line *after* the file that contains the weak-with-zero-default
definition if it is to override that; in the case here however we're going to
be overriding the weak-with-default by a strong function declaration, so that
issue does not arise.)
I still have a comment or two about the patch itself:
> Index: libgcc/config/i386/cygming-crtbegin.c
> ===================================================================
> --- libgcc/config/i386/cygming-crtbegin.c (Revision 196898)
> +++ libgcc/config/i386/cygming-crtbegin.c (Arbeitskopie)
> @@ -46,15 +46,33 @@ see the files COPYING3 and COPYING.RUNTIME respect
> #define LIBGCJ_SONAME "libgcj_s.dll"
> #endif
>
> -
> +#if DWARF2_UNWIND_INFO
> /* Make the declarations weak. This is critical for
> _Jv_RegisterClasses because it lives in libgcj.a */
> -extern void __register_frame_info (const void *, struct object *)
> +extern void __register_frame_info (__attribute__((unused)) const void *,
> + __attribute__((unused)) struct object *)
> TARGET_ATTRIBUTE_WEAK;
> -extern void *__deregister_frame_info (const void *)
> +extern void *__deregister_frame_info (__attribute__((unused)) const void *)
> TARGET_ATTRIBUTE_WEAK;
> -extern void _Jv_RegisterClasses (const void *) TARGET_ATTRIBUTE_WEAK;
> +TARGET_ATTRIBUTE_WEAK void
> +__register_frame_info (__attribute__((unused)) const void *p,
> + __attribute__((unused)) struct object *o)
> +{}
Braces should go on separate lines I think.
> +TARGET_ATTRIBUTE_WEAK void *
> +__deregister_frame_info (__attribute__((unused)) const void *p)
> +{ return (void*) 0; }
Certainly here.
> +#endif /* DWARF2_UNWIND_INFO */
> +
> +#if TARGET_USE_JCR_SECTION
> +extern void _Jv_RegisterClasses (__attribute__((unused)) const void *)
> + TARGET_ATTRIBUTE_WEAK;
> +
> +TARGET_ATTRIBUTE_WEAK void
> +_Jv_RegisterClasses (__attribute__((unused)) const void *p)
> +{}
> +#endif /* TARGET_USE_JCR_SECTION */
> +
> #if defined(HAVE_LD_RO_RW_SECTION_MIXING)
> # define EH_FRAME_SECTION_CONST const
> #else
Also, now that you've provided a default weak definition of the functions in
the file itself, it's no longer possible for the function pointer variables
(register_frame_fn, register_class_fn, deregister_frame_fn) to be zero, so you
should remove the if () tests on them and just call them unconditionally.
cheers,
DaveK
