is stdlib/tst-setcontext7 a bad test?

[Szabolcs Nagy]

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)
      | ^~~~~~~~~~~~~~~~~~~

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.

> 
>     int res = REAL(swapcontext)(oucp, ucp);
>                   ^^^^^^^^^^^^^^^^^^^^^^^^^^^^  This function may be
>     returned via an indirect branch.
> 
>     Here compiler must insert ENDBR after call, like
> 
>     call *bar(%rip)
>     endbr64
> 
>     I opened an LLVM bug:
> 
>     https://bugs.llvm.org/show_bug.cgi?id=38207
> 
>     to add the indirect_return attribute so that it can be used to inform
>     compiler to insert ENDBR after REAL(swapcontext) call.  We mark
>     REAL(swapcontext) with the indirect_return attribute if it is available.
> 
>     This fixed:
> 
>     https://bugs.llvm.org/show_bug.cgi?id=38249
> 
>     Reviewed By: eugenis
> 
>     Differential Revision: https://reviews.llvm.org/D49608
> 
>             PR target/86560
>             * asan/asan_interceptors.cc (swapcontext) Cherry-pick
>             compiler-rt revision 337603.
>             * sanitizer_common/sanitizer_internal_defs.h (__has_attribute):
>             Likewise.
> 
> > the only problem is that currently swapcontext is
> > not allowed to use the stack which to me seems like
> > an unusual restriction since the compiler can make
> > transformations at the call site that adds stack
> > usage around the call so it does not seem to be
> > worth supporting.
> 
> 
> 
> -- 
> H.J.

--