Re: Prevent inlining of weak hidden symbols.

[Richard Guenther <richard dot guenther at gmail dot 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;
> +}
>