toybox: Rough edges in pending

toybox: Rough edges in pending

[Isaac Dunham]

Hello,
I don't expect these to be very high priority, but I ran into a few rough edges when enabling almost all the toys in pending:
-sh:
toys/pending/sh.c: In function 'run_pipeline':
toys/pending/sh.c:303: warning: assignment from incompatible pointer type
Apparently, gcc doesn't recognize both rebound and toys.rebound

Also, when toybox is built with musl, and toybox sh executes ls, I get a hang; strace indicates that something funny is going on:

execve("./toybox-musl", ["./toybox-musl", "sh", "-c", "ls"], [/* 22 vars */]) = 0
getuid32()                              = 1000
geteuid32()                             = 1000
umask(0)                                = 022
umask(022)                              = 0
getuid32()                              = 1000
geteuid32()                             = 1000
brk(0)                                  = 0x8081000
brk(0x8082000)                          = 0x8082000
umask(0)                                = 022
umask(022)                              = 0
ioctl(0, SNDCTL_TMR_TIMEBASE or TCGETS, {B9600 opost isig icanon echo ...}) = 0
vfork(Config.in README    kconfig scripts     toybox_unstripped toys.h 
LICENSE   configure lib     toybox      toynet.h          www    
Makefile  generated main.c  toybox-musl toys              
)                                 = 27832
--- SIGCHLD (Child exited) @ 0 (0) ---
wait4(27832, [{WIFEXITED(s) && WEXITSTATUS(s) == 0}], 0, NULL) = 27832
pause(^C <unfinished ...>

With glibc, that last is:
ioctl(0, SNDCTL_TMR_TIMEBASE or TCGETS, {B9600 opost isig icanon echo ...}) = 0
vfork(Config.in README    kconfig scripts     toybox_unstripped toys.h 
LICENSE   configure lib     toybox      toynet.h          www    
Makefile  generated main.c  toybox-musl toys              
)                                 = 27838
waitpid(27838, [{WIFEXITED(s) && WEXITSTATUS(s) == 0}], 0) = 27838
--- SIGCHLD (Child exited) @ 0 (0) ---
exit_group(1)                           = ?

I anticipate this is a bug in musl, so I'll cross-post.



Thanks,
Isaac Dunham <idunham-N9AOi2cAC9ZBDgjK7y7TUQ@public.gmane.org>

Re: toybox: Rough edges in pending

[Szabolcs Nagy]

* Isaac Dunham <idunham@lavabit.com> [2013-03-18 23:50:43 -0700]:
> execve("./toybox-musl", ["./toybox-musl", "sh", "-c", "ls"], [/* 22 vars */]) = 0
...
> ioctl(0, SNDCTL_TMR_TIMEBASE or TCGETS, {B9600 opost isig icanon echo ...}) = 0
> vfork(Config.in README    kconfig scripts     toybox_unstripped toys.h 
> LICENSE   configure lib     toybox      toynet.h          www    
> Makefile  generated main.c  toybox-musl toys              
> )                                 = 27832
> --- SIGCHLD (Child exited) @ 0 (0) ---
> wait4(27832, [{WIFEXITED(s) && WEXITSTATUS(s) == 0}], 0, NULL) = 27832
> pause(^C <unfinished ...>

exit tries to flush some stdio buffers but it is locked

maybe an issue around the way vfork is used by the shell

Re: toybox: Rough edges in pending

[Szabolcs Nagy]

* Szabolcs Nagy <nsz@port70.net> [2013-03-19 10:42:25 +0100]:
> * Isaac Dunham <idunham@lavabit.com> [2013-03-18 23:50:43 -0700]:
> > execve("./toybox-musl", ["./toybox-musl", "sh", "-c", "ls"], [/* 22 vars */]) = 0
> ...
> > ioctl(0, SNDCTL_TMR_TIMEBASE or TCGETS, {B9600 opost isig icanon echo ...}) = 0
> > vfork(Config.in README    kconfig scripts     toybox_unstripped toys.h 
> > LICENSE   configure lib     toybox      toynet.h          www    
> > Makefile  generated main.c  toybox-musl toys              
> > )                                 = 27832
> > --- SIGCHLD (Child exited) @ 0 (0) ---
> > wait4(27832, [{WIFEXITED(s) && WEXITSTATUS(s) == 0}], 0, NULL) = 27832
> > pause(^C <unfinished ...>
> 
> exit tries to flush some stdio buffers but it is locked
> 
> maybe an issue around the way vfork is used by the shell


this is what toybox sh does:

    cmd->pid = vfork();
    if (!cmd->pid) xexec(cmd->argv);
    else waitpid(cmd->pid, &status, 0);


it's an incorrect use of vfork

either exec or _exit should be called after vfork but
xexec calls toy_exec that runs ls in-place then calls
exit (so the exit lock gets locked hence the
deadlock when the parent tries to exit)


void xexec(char **argv)
{
  toy_exec(argv);
  execvp(argv[0], argv);

  perror_exit("exec %s", argv[0]);
}

void toy_exec(char *argv[])
{
  struct toy_list *which;

  which = toy_find(argv[0]);
  if (!which) return;
  toy_init(which, argv);
  toys.which->toy_main();
  if (fflush(NULL) || ferror(stdout)) perror_exit("write");
  exit(toys.exitval);
}

Re: [musl] toybox: Rough edges in pending

[Rob Landley]

On 03/19/2013 06:09:39 AM, Szabolcs Nagy wrote:
> * Szabolcs Nagy <nsz-4P1ElwuDYu6sTnJN9+BGXg@public.gmane.org> [2013-03-19 10:42:25 +0100]:
> > * Isaac Dunham <idunham-N9AOi2cAC9ZBDgjK7y7TUQ@public.gmane.org> [2013-03-18 23:50:43 -0700]:
> > > execve("./toybox-musl", ["./toybox-musl", "sh", "-c", "ls"], [/*  
> 22 vars */]) = 0
> > ...
> > > ioctl(0, SNDCTL_TMR_TIMEBASE or TCGETS, {B9600 opost isig icanon  
> echo ...}) = 0
> > > vfork(Config.in README    kconfig scripts     toybox_unstripped  
> toys.h
> > > LICENSE   configure lib     toybox      toynet.h          www
> > > Makefile  generated main.c  toybox-musl toys
> > > )                                 = 27832
> > > --- SIGCHLD (Child exited) @ 0 (0) ---
> > > wait4(27832, [{WIFEXITED(s) && WEXITSTATUS(s) == 0}], 0, NULL) =  
> 27832
> > > pause(^C <unfinished ...>
> >
> > exit tries to flush some stdio buffers but it is locked
> >
> > maybe an issue around the way vfork is used by the shell
> 
> 
> this is what toybox sh does:
> 
>     cmd->pid = vfork();
>     if (!cmd->pid) xexec(cmd->argv);
>     else waitpid(cmd->pid, &status, 0);
> 
> 
> it's an incorrect use of vfork

It is indeed. I was going to add some plumbing to xexec() to exec  
/proc/self/exe in the internal toy called from toysh case, but toysh is  
unfinished and I'm not sure it's the best approach and potentially  
implementing the nofork logic with longjmp() for error handling works  
into that, although I have to prototype bits of that to figure out  
whether it's worth the effort.

(It's complicated. In bash you can't ctrl-z to suspend "read" because  
it's a shell builtin and doesn't have a separate process context to  
suspend, even though it can block. I feel it _should_ be possible to  
suspend this but that involves potentially calling fork() from the  
ctrl-z handler and is the shell not letting you do something obvious  
better than complicating the implementation? There's _details_ to  
getting this all right, and I haven't reopened the toysh can of worms  
because I'd need more than a month to reach the next good stopping  
point. The first week of which is a close re-reading of the SUSv4 shell  
stuff and the bash man page and digging up my old test cases. I have  
lots of lower hanging fruit to clear first.)

Rob

Re: [Toybox] toybox: Rough edges in pending

[Rob Landley]

On 03/19/2013 01:50:43 AM, Isaac Dunham wrote:
> Hello,
> I don't expect these to be very high priority, but I ran into a few  
> rough edges
> when enabling almost all the toys in pending:

Um, yes. Until recently half the stuff in pending didn't even  
_compile_. There's a _reason_ the default to 'n'. It's a directory full  
of things that people are unhappy if I keep out of tree (usually for  
months), but which aren't ready to be used either.

On balance, if I can't find a warning sign large enough, I'm just going  
to delete the whole directory and start keeping it out of tree again.

> -sh:
> toys/pending/sh.c: In function 'run_pipeline':
> toys/pending/sh.c:303: warning: assignment from incompatible pointer  
> type
> Apparently, gcc doesn't recognize both rebound and toys.rebound

Oh wow, toysh. No, it was never more than a stub and the rest of the  
code has changed drastically around it in the ~4 years since the last  
time it was touched. It needed fixups just to compile again, and I only  
did that because after my talk at CELF people kept telling me that  
"allyesconfig" didn't build. (And I went "I know, it's not supposed  
to", and this confused them...)

I should have called the directory "slushpile".

> Also, when toybox is built with musl, and toybox sh executes ls,
> I get a hang; strace indicates that something funny is going on:

I am honestly amazed it got _that_ far.

> I anticipate this is a bug in musl, so I'll cross-post.

If toysh _isn't_ corrupting the heap or something similar, I'd be  
stunned. It's not a real command yet.

Rob

Re: [musl] Re: toybox: Rough edges in pending

[Isaac Dunham]

On Tue, 19 Mar 2013 12:20:27 -0500
Rob Landley <rob-VoJi6FS/r0vR7s880joybQ@public.gmane.org> wrote:

> On 03/19/2013 01:50:43 AM, Isaac Dunham wrote:
> > Hello,
> > I don't expect these to be very high priority, but I ran into a few  
> > rough edges
> > when enabling almost all the toys in pending:
> 
> Um, yes. Until recently half the stuff in pending didn't even  
> _compile_. There's a _reason_ the default to 'n'. It's a directory full  
> of things that people are unhappy if I keep out of tree (usually for  
> months), but which aren't ready to be used either.

I knew it was "unsupported" but didn't realize it was expected to be quite that bad.  I assumed (yes, I know the saying...) that if find, 
uu{en,de}code, xzcat, and stat seemed to be functional in my testing, the other parts might also.

A couple things that would have cleared this up for me--
Either a note in toys/pending/README that said:
"Code in this directory may or may not work." (somehow, "...await review and/or cleanup" doesn't seem to communicate this)
or a "CONFIG_WORKING" that prevents enabling toys that are nonfunctional without realizing it (iirc, the kernel has a trick along these lines, so make allyesconfig doesn't turn _everything_ on).
But please leave pending in.

> > Also, when toybox is built with musl, and toybox sh executes ls,
> > I get a hang; strace indicates that something funny is going on:
> 
> I am honestly amazed it got _that_ far.
> 
> > I anticipate this is a bug in musl, so I'll cross-post.
> 
> If toysh _isn't_ corrupting the heap or something similar, I'd be  
> stunned. It's not a real command yet.

Surprisingly enough, it makes it far enough to give the illusion it might be usable.

-- 
Isaac Dunham <idunham-N9AOi2cAC9ZBDgjK7y7TUQ@public.gmane.org>

Re: [Toybox] [musl] Re: toybox: Rough edges in pending

[Rob Landley]

On 03/19/2013 02:32:21 PM, Isaac Dunham wrote:
> A couple things that would have cleared this up for me--
> Either a note in toys/pending/README that said:
> "Code in this directory may or may not work." (somehow, "...await  
> review
> and/or cleanup" doesn't seem to communicate this)

Good point. I'll add a note.

> or a "CONFIG_WORKING" that prevents enabling toys that are  
> nonfunctional without
> realizing it (iirc, the kernel has a trick along these lines, so make  
> allyesconfig
> doesn't turn _everything_ on).

The rule has been that "default n" means it's not ready.

(My current corner case here is "who", which works as advertised but  
posix says it should support a bunch of options that it doesnt' yet,  
and I don't want to make it default y and forget to add them. But when  
I say "a bunch" I mean "-abdHlmpqrstTu" which I haven't had time to  
just sit down and implement yet, and possibly we want to document a  
variance and go "really, this is minicomputer stuff, modern systems  
don't usually have lots of multiple simultaneous users without  
containers anymore...")

> > > Also, when toybox is built with musl, and toybox sh executes ls,
> > > I get a hang; strace indicates that something funny is going on:
> >
> > I am honestly amazed it got _that_ far.
> >
> > > I anticipate this is a bug in musl, so I'll cross-post.
> >
> > If toysh _isn't_ corrupting the heap or something similar, I'd be
> > stunned. It's not a real command yet.
> 
> Surprisingly enough, it makes it far enough to give the illusion it  
> might be usable.

It's an illusion. It's using toybuf for parsing so the command line  
can't be longer than 4k (kernel limit's 128k and I've seen patches  
raise it), it has no environment variable support, can't parse quotes  
(which turns out to be quite funky because "$(echo "hello world")" has  
nested quote contexts), no flow control implemented yet (if, while,  
shell functions...), no &&, ||, or pipeline support...

And so on, and so forth... One of my current half-finished pending  
things I haven't even bothered to check in yet is test.c (which  
implements [ and [[ the way "bc" shares code with $(())), and once I've  
got that I can dink at toysh a bit and at least redo the parsing.

Anyway, not a musl thing. Feel free to poke me just on the toybox list  
if you think i should take a weekend and make toysh suck less, but the  
_scope_ of the shell promblem is...

   http://landley.net/notes-2006.html#31-08-2006
   http://landley.net/notes-2006.html#10-09-2006
   http://landley.net/notes-2007.html#20-01-2007
   http://landley.net/notes-2007.html#19-10-2007

Stuff.

Rob