[Toybox] 81207825c50fa9a4ec1475024543f830dee247fd broke bionic build

[enh]

On Tue, Jan 22, 2019 at 5:26 PM Rob Landley <rob at landley.net> wrote:
>
> On 1/22/19 2:54 PM, enh via Toybox wrote:
> > this hunk from 81207825c50fa9a4ec1475024543f830dee247fd:
> >
> > diff --git a/toys.h b/toys.h
> > index 4083725a..df37b55e 100644
> > --- a/toys.h
> > +++ b/toys.h
> > @@ -108,7 +108,7 @@ extern struct toy_context {
> >    char wasroot;            // dropped setuid
> >
> >    // This is at the end so toy_init() doesn't zero it.
> > -  jmp_buf *rebound;        // longjmp here instead of exit when do_rebound set
> > +  sigjmp_buf *rebound;     // longjmp here instead of exit when do_rebound set
> >    struct arg_list *xexit;  // atexit() functions for xexit(), set by sigatexit(
> > )
> >    void *stacktop;          // nested toy_exec() call count, or 0 if vforked
> >  } toys;
>
> Which was because of a warning the freebsd build threw...

yeah, not a great day for diagnostics. FreeBSD was the only setup that
noticed anything was wrong, but only noticed half the problem. but
then that let Android (but only Android) spot the other half. (but
only courtesy of the assignment: if you'd only used setjmp/longjmp
we'd have been none the wiser.)

i can't speak for other OSes, but on Android this would always have
worked anyway... it's only a historical accident (copy & paste from
some BSD?) that means our buffers are different sizes: we don't use
that extra element.

> > caused:
> >
> > external/toybox/toys/net/netcat.c:188:37: error: incompatible pointer
> > types assigning to 'sigjmp_buf *' (aka 'long (*)[33]') from 'jmp_buf
> > *' (aka 'long (*)[32]') [-Werror,-Wincompatible-pointer-types]
> >           if (toys.optflags&FLAG_L) NOEXIT(child = XVFORK());
> >                                     ^~~~~~~~~~~~~~~~~~~~~~~~
> > external/toybox/lib/lib.h:375:19: note: expanded from macro 'NOEXIT'
> > #define NOEXIT(x) WOULD_EXIT(_noexit_res, x)
> >                   ^~~~~~~~~~~~~~~~~~~~~~~~~~
> > external/toybox/lib/lib.h:367:16: note: expanded from macro 'WOULD_EXIT'
> >   toys.rebound = &_noexit; \
> >                ^ ~~~~~~~~
> > 1 error generated.
> >
> > hmm... looks like you always call setjmp (rather than sigsetjmp) but
> > also always call siglongjmp?
>
> That codepath is there for error_exit() and friends returning from NOFORK
> commands, which are primarily shell builtins. Until I do toysh, they haven't
> gotten a lot of testing. Sorry.
>
> > so the real fix is probably to s/setjmp/sigsetjmp/ and move _noexit
> > over to sigjmp_buf too:
> >
> > #define WOULD_EXIT(y, x) do { jmp_buf _noexit; \
> >
> > patch attached...
>
> Applied,

thanks. i'll get this in to AOSP and hopefully will have time this
week to re-test cp, hostname, and sed.

> Rob