Re: Prevent inlining of weak hidden symbols.

[Richard Guenther <richard.guenther@gmail.com>]

On Fri, Jul 9, 2010 at 6:26 PM, Richard Sandiford
<rdsandiford@googlemail.com> wrote:
> This patch fixes a wrong-code bug in the compilation of gPXE.  gPXE has
> a weak symbol called pxe_menu_boot whose default implementation is
> defined in the same file as (one of?) the callers.  However, this
> implementation can be overridden by earlier objects on the link line.
>
> gPXE has only a single module -- there's no symbol preemption --
> so as an optimisation, it also marks every symbol as hidden.
> This causes GCC to think it can inline pxe_menu_boot, something
> it wouldn't do for default-visiblity weak symbols.
>
> The problem is that our notion of "replaceability" is based on whether
> the definition binds locally to the current _module_, not to the current
> TU.  That's fine for most cases, but not for weak symbols.
>
> Bootstrapped & regression-tested on x86_64-linux-gnu.  OK to install?
>
> Richard
>
>
> This patch fixes a wrong-code bug in the compilation of gPXE.  gPXE has
> a weak symbol called pxe_menu_boot whose default implementation is
> defined in the same file as (one of?) the callers.  However, this
> implementation can be overridden by earlier objects on the link line.
>
> gPXE has only a single module -- there's no symbol preemption --
> so as an optimisation, it also marks every symbol as hidden.
> This causes GCC to think it can inline pxe_menu_boot, something
> it wouldn't do for default-visiblity weak symbols.
>
> The problem is that our notion of "replaceability" is based on whether
> the definition binds locally to the current _module_, not to the current
> TU.  That's fine for most cases, but not for weak symbols.
>
> Bootstrapped & regression-tested on x86_64-linux-gnu.  OK to install?
>
> Richard

Ok.

Thanks,
Richard.

Ok.

Thanks,
Richard.

;)

>
> gcc/
>        * tree.h (DECL_REPLACEABLE_P): Strengthen check for weak symbols.
>
> gcc/testsuite/
>        * gcc.dg/attr-weak-hidden-1.c, gcc.dg/attr-weak-hidden-1a.c: New test.
>
> Index: gcc/tree.h
> ===================================================================
> --- gcc/tree.h  2010-07-09 09:44:27.000000000 +0100
> +++ gcc/tree.h  2010-07-09 12:59:26.000000000 +0100
> @@ -3012,6 +3012,11 @@ #define DECL_ONE_ONLY(NODE) (DECL_COMDAT
>    not be treated as replaceable so that use of C++ template
>    instantiations is not penalized.
>
> +   In other respects, the condition is usually equivalent to whether
> +   the function binds to the current module (shared library or executable).
> +   However, weak functions can always be overridden by earlier TUs
> +   in the same module, even if they bind locally to that module.
> +
>    For example, DECL_REPLACEABLE is used to determine whether or not a
>    function (including a template instantiation) which is not
>    explicitly declared "inline" can be inlined.  If the function is
> @@ -3020,7 +3025,7 @@ #define DECL_ONE_ONLY(NODE) (DECL_COMDAT
>    function that is not DECL_REPLACEABLE can be inlined, since all
>    versions of the function will be functionally identical.  */
>  #define DECL_REPLACEABLE_P(NODE) \
> -  (!DECL_COMDAT (NODE) && !targetm.binds_local_p (NODE))
> +  (!DECL_COMDAT (NODE) && (DECL_WEAK (NODE) || !targetm.binds_local_p (NODE)))
>
>  /* The name of the object as the assembler will see it (but before any
>    translations made by ASM_OUTPUT_LABELREF).  Often this is the same
> Index: gcc/testsuite/gcc.dg/attr-weak-hidden-1.c
> ===================================================================
> --- /dev/null   2010-06-01 09:29:56.413951209 +0100
> +++ gcc/testsuite/gcc.dg/attr-weak-hidden-1.c   2010-07-09
> 12:58:03.000000000 +0100
> @@ -0,0 +1,6 @@
> +/* { dg-do run } */
> +/* { dg-require-weak "" } */
> +/* { dg-require-visibility "" } */
> +/* { dg-options "-O2" } */
> +/* { dg-additional-sources "attr-weak-hidden-1a.c" } */
> +int __attribute__((weak, visibility("hidden"))) foo (void) { return 0; }
> Index: gcc/testsuite/gcc.dg/attr-weak-hidden-1a.c
> ===================================================================
> --- /dev/null   2010-06-01 09:29:56.413951209 +0100
> +++ gcc/testsuite/gcc.dg/attr-weak-hidden-1a.c  2010-07-09
> 13:00:53.000000000 +0100
> @@ -0,0 +1,9 @@
> +void abort (void);
> +int __attribute__((weak, visibility("hidden"))) foo (void) { return 1; }
> +int
> +main (void)
> +{
> +  if (foo ())
> +    abort ();
> +  return 0;
> +}
>