is stdlib/tst-setcontext7 a bad test?

[H.J. Lu]

On Fri, Mar 27, 2020 at 9:11 AM Szabolcs Nagy <szabolcs.nagy@arm.com> wrote:
>
> The 03/27/2020 08:42, H.J. Lu wrote:
> > On Fri, Mar 27, 2020 at 8:36 AM Szabolcs Nagy <szabolcs.nagy@arm.com> wrote:
> > > i dont think that test would be a problem if the
> > > BTI J (or ENDBR) landing pad was inside libc in a
> > > wrapper around the internal swapcontext where
> > > context switching calls can return to.
> >
> > lcommit c83b4b824214039fea696083e6a888aa7c9063ce
> > Author: H.J. Lu <hongjiu.lu@intel.com>
> > Date:   Thu Jul 26 14:48:55 2018 +0000
> >
> >     libsanitizer: Mark REAL(swapcontext) with indirect_return attribute on x86
> >
> >     Cherry-pick compiler-rt revision 337603:
> >
> >     When shadow stack from Intel CET is enabled, the first instruction of all
> >     indirect branch targets must be a special instruction, ENDBR.
> >
> >     lib/asan/asan_interceptors.cc has
> >
> >     ...
> >       int res = REAL(swapcontext)(oucp, ucp);
> >     ...
> >
> >     REAL(swapcontext) is a function pointer to swapcontext in libc.  Since
> >     swapcontext may return via indirect branch on x86 when shadow stack is
> >     enabled, as in this case,
>
> With my solution the indirect branch does not go to
> the swapcontext call site, instead it goes to a stub
> in libc (which is under the libc control so it can
> have the ENDBR) and that stub just normally returns
> to the call site with RET (so there is nothing special
> about the call site).
>
> Now i tried the returns_twice attribute on swapcontext
> and it creates more problems, string/tst-xbzero-opt.o
> fails to build in glibc:
>
> tst-xbzero-opt.c:123:1: error: function ‘prepare_test_buffer’ can never be inlined because it uses setjmp
>   123 | prepare_test_buffer (unsigned char *buf)
>       | ^~~~~~~~~~~~~~~~~~~

commit e27f41ba2b6101be1a511905bff5502ffd23828b
Author: H.J. Lu <hjl.tools@gmail.com>
Date:   Tue Jul 24 07:55:35 2018 -0700

    Add <bits/indirect-return.h>

    Add <bits/indirect-return.h> and include it in <ucontext.h>.
    __INDIRECT_RETURN defined in <bits/indirect-return.h> indicates if
    swapcontext requires special compiler treatment.  The default
    __INDIRECT_RETURN is empty.

    On x86, when shadow stack is enabled, __INDIRECT_RETURN is defined
    with indirect_return attribute, which has been added to GCC 9, to
    indicate that swapcontext returns via indirect branch.  Otherwise
    __INDIRECT_RETURN is defined with returns_twice attribute.

    When shadow stack is enabled, remove always_inline attribute from
    prepare_test_buffer in string/tst-xbzero-opt.c to avoid:

    tst-xbzero-opt.c: In function ‘prepare_test_buffer’:
    tst-xbzero-opt.c:105:1: error: function ‘prepare_test_buffer’ can
never be inlined because it uses setjmp
     prepare_test_buffer (unsigned char *buf)

    when indirect_return attribute isn't available.

diff --git a/string/tst-xbzero-opt.c b/string/tst-xbzero-opt.c
index cf7041f37a..aab4a7f715 100644
--- a/string/tst-xbzero-opt.c
+++ b/string/tst-xbzero-opt.c
@@ -100,7 +100,15 @@ static ucontext_t uc_main, uc_co;
 /* 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.  */
-static inline __attribute__  ((always_inline)) void
+#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.  */
+# define ALWAYS_INLINE
+#else
+# define ALWAYS_INLINE __attribute__ ((always_inline))
+#endif
+static inline ALWAYS_INLINE void
 prepare_test_buffer (unsigned char *buf)
 {
   for (unsigned int i = 0; i < PATTERN_REPS; i++)

> i know a new attribute can avoid this, but there are
> existing compilers with bti support, so i'm still
> leaning toward my original approach.
>

My points are

1. stdlib/tst-setcontext7 is a valid test.  I can move it to
sysdeps/x86 if needed.
2. X86 solution works for CET GCC with and without indirect_return attribute.
3. We should keep x86 solution ASIS.

-- 
H.J.