Mark swapcontext returns_twice without indirect_return [BZ #23130]
Checks
| Context |
Check |
Description |
| redhat-pt-bot/TryBot-apply_patch |
success
|
Patch applied to master at the time it was sent
|
| linaro-tcwg-bot/tcwg_glibc_build--master-aarch64 |
success
|
Build passed
|
| redhat-pt-bot/TryBot-32bit |
success
|
Build for i686
|
| linaro-tcwg-bot/tcwg_glibc_check--master-aarch64 |
success
|
Test passed
|
| linaro-tcwg-bot/tcwg_glibc_build--master-arm |
success
|
Build passed
|
| linaro-tcwg-bot/tcwg_glibc_check--master-arm |
success
|
Test passed
|
Commit Message
swapcontext does not restore every callee-saved register on all targets.
On sparc the %l and %i registers are not in the saved context at all:
they live in the register window save area of the calling frame, which
another pass through that frame can overwrite. stdlib/tst-setcontext2
fails on sparc64 for this reason at -O2 with PIC, where gcc keeps
&global in %i5 across swapcontext and reuses %i5 as the GOT base on
another path through the same frame.
Make the generic bits/indirect-return.h fall back to returns_twice where
indirect_return is unavailable, covering every affected target. x86 and
aarch64 keep their own headers, which gate on CET and on GCS, so those
targets are unchanged.
string/tst-xbzero-opt.c calls swapcontext from an always_inline
function, which a returns_twice callee forbids; key its guard on the
availability of indirect_return rather than on __CET__. It is the only
such site in the tree.
Tested on sparc64: full make check is clean, with no test that passed
before failing after.
Signed-off-by: Stian Halseth <stian@itx.no>
---
bits/indirect-return.h | 15 +++++++++++++--
string/tst-xbzero-opt.c | 7 +++----
2 files changed, 16 insertions(+), 6 deletions(-)
Comments
On 02/09/26 09:43, Stian Halseth wrote:
> swapcontext does not restore every callee-saved register on all targets.
> On sparc the %l and %i registers are not in the saved context at all:
> they live in the register window save area of the calling frame, which
> another pass through that frame can overwrite. stdlib/tst-setcontext2
> fails on sparc64 for this reason at -O2 with PIC, where gcc keeps
> &global in %i5 across swapcontext and reuses %i5 as the GOT base on
> another path through the same frame.
>
> Make the generic bits/indirect-return.h fall back to returns_twice where
> indirect_return is unavailable, covering every affected target. x86 and
> aarch64 keep their own headers, which gate on CET and on GCS, so those
> targets are unchanged.
>
> string/tst-xbzero-opt.c calls swapcontext from an always_inline
> function, which a returns_twice callee forbids; key its guard on the
> availability of indirect_return rather than on __CET__. It is the only
> such site in the tree.
>
> Tested on sparc64: full make check is clean, with no test that passed
> before failing after.
>
> Signed-off-by: Stian Halseth <stian@itx.no>
> ---
> bits/indirect-return.h | 15 +++++++++++++--
> string/tst-xbzero-opt.c | 7 +++----
> 2 files changed, 16 insertions(+), 6 deletions(-)
>
> diff --git a/bits/indirect-return.h b/bits/indirect-return.h
> index 819bd25b..4bfe3d35 100644
> --- a/bits/indirect-return.h
> +++ b/bits/indirect-return.h
> @@ -21,5 +21,16 @@
> #endif
>
> /* __INDIRECT_RETURN is used on swapcontext to indicate if it requires
> - special compiler treatment. */
> -#define __INDIRECT_RETURN
> + special compiler treatment. swapcontext does not restore every
> + callee-saved register on all targets -- on sparc the %l and %i
> + registers are not in the saved context at all -- so where
> + indirect_return is unavailable, returns_twice stops the compiler
> + assuming such a value survives the call. Targets that need this only
> + under an optional feature override this file. */
I don't think it make much sense to add sparc-specific rationale on an
installed generic header. I think something like would suffice.
/* swapcontext does not restore every callee-saved register on all targets. */
> +#if __glibc_has_attribute (__indirect_return__)
> +# define __INDIRECT_RETURN __attribute__ ((__indirect_return__))
I think we should not use __indirect_return__ as the preferable attribute,
It was added as a way to mark a function may return via an indirect branch,
and as a way to optimize the swapcontext on x86_64/aarch64.
The optimization rationale is moot because the call will issue a syscall
anyway (sigprocmask).
The rest looks ok.
> +#elif __glibc_has_attribute (__returns_twice__)
> +# define __INDIRECT_RETURN __attribute__ ((__returns_twice__))
> +#else
> +# define __INDIRECT_RETURN
> +#endif
> diff --git a/string/tst-xbzero-opt.c b/string/tst-xbzero-opt.c
> index 3aa13d03..26e2ec1c 100644
> --- a/string/tst-xbzero-opt.c
> +++ b/string/tst-xbzero-opt.c
> @@ -111,10 +111,9 @@ use_test_buffer (unsigned char *buf)
> /* Always check the test buffer immediately after filling it; this
> makes externally visible side effects depend on the buffer existing
> and having been filled in. */
> -#if defined __CET__ && !__glibc_has_attribute (__indirect_return__)
> -/* Note: swapcontext returns via indirect branch when SHSTK is enabled.
> - Without indirect_return attribute, swapcontext is marked with
> - returns_twice attribute, which prevents always_inline to work. */
> +#if !__glibc_has_attribute (__indirect_return__)
> +/* Without indirect_return, swapcontext is marked returns_twice, which
> + prevents always_inline from working. */
> # define ALWAYS_INLINE
> #else
> # define ALWAYS_INLINE __attribute__ ((always_inline))
@@ -21,5 +21,16 @@
#endif
/* __INDIRECT_RETURN is used on swapcontext to indicate if it requires
- special compiler treatment. */
-#define __INDIRECT_RETURN
+ special compiler treatment. swapcontext does not restore every
+ callee-saved register on all targets -- on sparc the %l and %i
+ registers are not in the saved context at all -- so where
+ indirect_return is unavailable, returns_twice stops the compiler
+ assuming such a value survives the call. Targets that need this only
+ under an optional feature override this file. */
+#if __glibc_has_attribute (__indirect_return__)
+# define __INDIRECT_RETURN __attribute__ ((__indirect_return__))
+#elif __glibc_has_attribute (__returns_twice__)
+# define __INDIRECT_RETURN __attribute__ ((__returns_twice__))
+#else
+# define __INDIRECT_RETURN
+#endif
@@ -111,10 +111,9 @@ use_test_buffer (unsigned char *buf)
/* Always check the test buffer immediately after filling it; this
makes externally visible side effects depend on the buffer existing
and having been filled in. */
-#if defined __CET__ && !__glibc_has_attribute (__indirect_return__)
-/* Note: swapcontext returns via indirect branch when SHSTK is enabled.
- Without indirect_return attribute, swapcontext is marked with
- returns_twice attribute, which prevents always_inline to work. */
+#if !__glibc_has_attribute (__indirect_return__)
+/* Without indirect_return, swapcontext is marked returns_twice, which
+ prevents always_inline from working. */
# define ALWAYS_INLINE
#else
# define ALWAYS_INLINE __attribute__ ((always_inline))