is stdlib/tst-setcontext7 a bad test?

[H.J. Lu]

On Fri, Mar 27, 2020 at 5:49 AM H.J. Lu <hjl.tools@gmail.com> wrote:
>
> On Fri, Mar 27, 2020 at 5:46 AM H.J. Lu <hjl.tools@gmail.com> wrote:
> >
> > On Fri, Mar 27, 2020 at 4:04 AM Szabolcs Nagy <szabolcs.nagy@arm.com> wrote:
> > >
> > > The 03/27/2020 11:10, Florian Weimer wrote:
> > > > * Szabolcs Nagy:
> > > > > i'm planing to change swapcontext on aarch64 and i see
> > > > >
> > > > > FAIL: stdlib/tst-setcontext7
> > > > >
> > > > > that test case seems wrong to me, a simplified version is
> > > > >
> > > > > #include <ucontext.h>
> > > > > #include <stdio.h>
> > > > >
> > > > > ucontext_t uc[2];
> > > > > volatile int count = 0;
> > > > >
> > > > > int main ()
> > > > > {
> > > > >     getcontext (uc+0);
> > > > >     printf ("%d\n", count);
> > > > >     if (count)
> > > > >             setcontext (uc+1);
> > > > >     count++;
> > > > >     swapcontext (uc+1, uc+0);
> > > > >     printf ("done\n");
> > > > >     return 0;
> > > > > }
> > > > >
> > > > > this seems to work today and prints
> > > > >
> > > > > 0
> > > > > 1
> > > > > done
> > > > >
> > > > > but i don't think we can guarantee that after a
> > > > > swapcontext to a getcontext on the same stack frame
> > > > > resuming the context works: if swapcontext uses the
> > > > > stack (which is what i plan to do) then that stack
> > > > > will be corrupted.
> > > > >
> > > > > does glibc plan to support this usage?
> > > >
> > > > I think it's expected to work.  This seems to be one of the more
> > > > harmless cases.
> > > >
> > > > > i.e. should i ensure swapcontext does not use the stack? (and should
> > > > > gcc ensure no stack is used in the example between getcontext and
> > > > > swapcontext?)
> > > >
> > > > Does the test case pass if you add the returns_twice attribute (or
> > > > __INDIRECT_RETURN) to getcontext as well (swapcontext already has it)?
> > >
> > > yes, the returns_twice attribute on swapcontext
> > > makes bti work, but that seems ugly (swapcontext
> > > only returns once).
> > >
> > > (getcontext has the return_twice attribute because
> > > it's a gcc builtin.)
> > >
> > > the way i planed to handle swapcontext is to do
> > >
> > > swapcontext:
> > >         bti c
> > >         stp x29, x30, [sp, #-16]! // save return address
> > >         bl internal_swapcontext
> > >         bti j
> > >         ldp x29, x30, [sp], #16 // restore return address
> > >         ret
> > >
> > > then bti j (landing pad for indirect jump) is
> > > in libc code, otherwise the compiler has to
> > > special case swapcontext and emit a bti j after
> > > every call site (getcontext is already special),
> > > with my solution swapcontext is not special.
> > >
> > > but if glibc plans to support jumping around
> > > within the same stack frame then my solution
> > > does not work so i will have to rely on the
> > > attribute.
> > >
> >
> > Can't you use indirect_return function attribute like the fix for
> >
> > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=85620
> >
>
> You should also check if c-c++-common/asan/swapcontext-test-1.c
> is OK:
>
> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=86560
>

You also need to provide bits/indirect-return.h for

/* __INDIRECT_RETURN is used on swapcontext to indicate if it requires
   special compiler treatment.  */
#define __INDIRECT_RETURN

See sysdeps/x86/bits/indirect-return.h for example.

-- 
H.J.