diff mbox series

strub: avoid lto inlining

Message ID orsf4436ki.fsf@lxoliva.fsfla.org
State Unresolved
Headers
Series strub: avoid lto inlining |

Checks

Context Check Description
snail/gcc-patch-check warning Git am fail log

Commit Message

Alexandre Oliva Dec. 14, 2023, 7:52 p.m. UTC 
  The strub builtins are not suited for cross-unit inlining, they should
only be inlined by the builtin expanders, if at all.  While testing on
sparc64, it occurred to me that, if libgcc was built with LTO enabled,
lto1 might inline them, and that would likely break things.  So, make
sure they're clearly marked as not inlinable.

Regstrapped on x86_64-linux-gnu, also testing on sparc-solaris2.11.3.
Ok to install?


for  libgcc/ChangeLog

	* strub.c (ATTRIBUTE_NOINLINE): New.
	(ATTRIBUTE_STRUB_CALLABLE): Add it.
	(__strub_dummy_force_no_leaf): Drop it.
---
 libgcc/strub.c |    8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

Comments

Richard Biener Dec. 15, 2023, 7 a.m. UTC | #1 
On Thu, Dec 14, 2023 at 8:53 PM Alexandre Oliva <oliva@adacore.com> wrote:
>
>
> The strub builtins are not suited for cross-unit inlining, they should
> only be inlined by the builtin expanders, if at all.  While testing on
> sparc64, it occurred to me that, if libgcc was built with LTO enabled,
> lto1 might inline them, and that would likely break things.  So, make
> sure they're clearly marked as not inlinable.
>
> Regstrapped on x86_64-linux-gnu, also testing on sparc-solaris2.11.3.
> Ok to install?
>
>
> for  libgcc/ChangeLog
>
>         * strub.c (ATTRIBUTE_NOINLINE): New.
>         (ATTRIBUTE_STRUB_CALLABLE): Add it.
>         (__strub_dummy_force_no_leaf): Drop it.
> ---
>  libgcc/strub.c |    8 ++++++--
>  1 file changed, 6 insertions(+), 2 deletions(-)
>
> diff --git a/libgcc/strub.c b/libgcc/strub.c
> index b0f990d9deebb..5062554d0e1e6 100644
> --- a/libgcc/strub.c
> +++ b/libgcc/strub.c
> @@ -36,7 +36,12 @@ see the files COPYING3 and COPYING.RUNTIME respectively.  If not, see
>  # define TOPS <
>  #endif
>
> -#define ATTRIBUTE_STRUB_CALLABLE __attribute__ ((__strub__ ("callable")))
> +/* Make sure these builtins won't be inlined, even with LTO.  */
> +#define ATTRIBUTE_NOINLINE \
> +  __attribute__ ((__noinline__, __noclone__))

I think __noipa__ is more complete and will make the libgcc functions appear
as black boxes to callers.

OK your or this way.

Richard.

> +
> +#define ATTRIBUTE_STRUB_CALLABLE \
> +  __attribute__ ((__strub__ ("callable"))) ATTRIBUTE_NOINLINE
>
>  /* Enter a stack scrubbing context, initializing the watermark to the caller's
>     stack address.  */
> @@ -72,7 +77,6 @@ __strub_update (void **watermark)
>  /* Dummy function, called to force the caller to not be a leaf function, so
>     that it can't use the red zone.  */
>  static void ATTRIBUTE_STRUB_CALLABLE
> -__attribute__ ((__noinline__, __noipa__))
>  __strub_dummy_force_no_leaf (void)>  {
>  }
>
> --
> Alexandre Oliva, happy hacker            https://FSFLA.org/blogs/lxo/
>    Free Software Activist                   GNU Toolchain Engineer
> More tolerance and less prejudice are key for inclusion and diversity
> Excluding neuro-others for not behaving ""normal"" is *not* inclusive
Alexandre Oliva Dec. 19, 2023, 11:53 p.m. UTC | #2 
On Dec 15, 2023, Richard Biener <richard.guenther@gmail.com> wrote:

> I think __noipa__ is more complete and will make the libgcc functions appear
> as black boxes to callers.

I was hesitant to use __noipa__ because I thought it might disable
relevant optimizations even when not using LTO, but it is likely safer
in the long run to use it, so I've added it, thanks.

> OK your or this way.

Here's what I'm checking in shortly.  Regstrapped on x86_64-linux-gnu.


strub: avoid lto inlining

The strub builtins are not suited for cross-unit inlining, they should
only be inlined by the builtin expanders, if at all.  While testing on
sparc64, it occurred to me that, if libgcc was built with LTO enabled,
lto1 might inline them, and that would likely break things.  So, make
sure they're clearly marked as not inlinable.


for  libgcc/ChangeLog

	* strub.c (ATTRIBUTE_NOINLINE): New.
	(ATTRIBUTE_STRUB_CALLABLE): Add it.
	(__strub_dummy_force_no_leaf): Drop it.
---
 libgcc/strub.c |    8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/libgcc/strub.c b/libgcc/strub.c
index b0f990d9deebb..3b7cc26b3d8f0 100644
--- a/libgcc/strub.c
+++ b/libgcc/strub.c
@@ -36,7 +36,12 @@ see the files COPYING3 and COPYING.RUNTIME respectively.  If not, see
 # define TOPS <
 #endif
 
-#define ATTRIBUTE_STRUB_CALLABLE __attribute__ ((__strub__ ("callable")))
+/* Make sure these builtins won't be inlined, even with LTO.  */
+#define ATTRIBUTE_NOINLINE \
+  __attribute__ ((__noinline__, __noclone__, __noipa__))
+
+#define ATTRIBUTE_STRUB_CALLABLE \
+  __attribute__ ((__strub__ ("callable"))) ATTRIBUTE_NOINLINE
 
 /* Enter a stack scrubbing context, initializing the watermark to the caller's
    stack address.  */
@@ -72,7 +77,6 @@ __strub_update (void **watermark)
 /* Dummy function, called to force the caller to not be a leaf function, so
    that it can't use the red zone.  */
 static void ATTRIBUTE_STRUB_CALLABLE
-__attribute__ ((__noinline__, __noipa__))
 __strub_dummy_force_no_leaf (void)
 {
 }
diff mbox series

Patch

diff --git a/libgcc/strub.c b/libgcc/strub.c
index b0f990d9deebb..5062554d0e1e6 100644
--- a/libgcc/strub.c
+++ b/libgcc/strub.c
@@ -36,7 +36,12 @@  see the files COPYING3 and COPYING.RUNTIME respectively.  If not, see
 # define TOPS <
 #endif
 
-#define ATTRIBUTE_STRUB_CALLABLE __attribute__ ((__strub__ ("callable")))
+/* Make sure these builtins won't be inlined, even with LTO.  */
+#define ATTRIBUTE_NOINLINE \
+  __attribute__ ((__noinline__, __noclone__))
+
+#define ATTRIBUTE_STRUB_CALLABLE \
+  __attribute__ ((__strub__ ("callable"))) ATTRIBUTE_NOINLINE
 
 /* Enter a stack scrubbing context, initializing the watermark to the caller's
    stack address.  */
@@ -72,7 +77,6 @@  __strub_update (void **watermark)
 /* Dummy function, called to force the caller to not be a leaf function, so
    that it can't use the red zone.  */
 static void ATTRIBUTE_STRUB_CALLABLE
-__attribute__ ((__noinline__, __noipa__))
 __strub_dummy_force_no_leaf (void)
 {
 }