This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [patch libgcc]: Adjust cygming-crtbegin code to use weak


On 22/03/2013 08:44, Kai Tietz wrote:

> 2013-03-22  Kai Tietz  <ktietz@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


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]