0

I'm writing a little shell for class. I call execvp for non-built-in commands. For a while in developing this, it returned normally with -1 and all was right. Now, however, I can type anything in to the shell, it starts a process which immediately terminates with status 11 (SIGSEGV, a segmentation fault).

Meta: I have tried cutting this down into a SSCCE but was unsuccessful: when I remove everything that surrounds the execvp, it functions normally. I'm happy to provide the full source, but at over 750 lines that seems excessive at this point.

Here is the function in which execvp is called.

void eval(char *cmdline)
{
char **argv = malloc(MAXLINE);
int bg = parseline(cmdline,argv);

if (argv[0] == 0)
{
    free(argv);
    return;
}
if(builtin_cmd(argv) == 0)
{
    pid_t pid;
    sigset_t set, oset;

    /* set mask */
    sigemptyset(&set);
    sigaddset(&set, SIGCHLD);
    sigprocmask(SIG_SETMASK, &set, &oset);

    /* proceed to fork */
    if ((pid = Fork()) == 0)
    {
        app_debug("execvp <%s>", argv[0]);
        setpgid(0,0);
        sigprocmask(SIG_UNBLOCK, &set, &oset);
        execvp(argv[0], argv);
        app_notify("%s command not found.", errno, argv[0]);
        exit(0);
    } else if (pid == -1) {
        app_notify("fork failed");
        return;
    } else {
        addjob(jobs, pid, bg + 1, cmdline);
        sigprocmask(SIG_UNBLOCK, &set, &oset);
        if (bg == 0) {
            app_debug("[%d] (%d) Waiting for %s", pid2jid(pid), pid, cmdline);
            waitfg(pid);
        } else {
            app_notify("[%d] (%d) %s", pid2jid(pid), pid, cmdline);
        }
    }
}
free(argv);
return;
}

Here is parseline.

int parseline(const char *cmdline, char **argv)
{
static char array[MAXLINE]; /* holds local copy of command line */
char *buf = array;          /* ptr that traverses command line */
char *delim;                /* points to first space delimiter */
int argc;                   /* number of args */
int bg;                     /* background job? */

strcpy(buf, cmdline);
buf[strlen(buf)-1] = ' ';  /* replace trailing '\n' with space */
while (*buf && (*buf == ' ')) /* ignore leading spaces */
buf++;

/* Build the argv list */
argc = 0;
if (*buf == '\'') {
buf++;
delim = strchr(buf, '\'');
}
else {
delim = strchr(buf, ' ');
}

while (delim) {
argv[argc++] = buf;
*delim = '\0';
buf = delim + 1;
while (*buf && (*buf == ' ')) /* ignore spaces */
       buf++;

if (*buf == '\'') {
    buf++;
    delim = strchr(buf, '\'');
}
else {
    delim = strchr(buf, ' ');
}
}
argv[argc] = NULL;

if (argc == 0)  /* ignore blank line */
return 1;

/* should the job run in the background? */
if ((bg = (*argv[argc-1] == '&')) != 0) {
argv[--argc] = NULL;
}
return bg;
}
TravisThomas
  • 611
  • 1
  • 8
  • 21
  • You need to at least give us a sequence of what you do before/after your execvp. Also, you should try to see if you can find the minimal amount of code that you can remove that will get rid of the problem. Doing so will probably tell you where the problem is (or at least give you a clue). – CrazyCasta Oct 08 '12 at 20:35
  • If your alleged minimal example of your error doesn't actually have the error, then the error isn't caused by the code you're talking about! It lies elsewhere. Do further debugging until you find it, and *then* ask a pertinent question. Most likely it will have nothing to do with `execve`. – Kerrek SB Oct 08 '12 at 20:35
  • I have added the function in which `execvp` is called. I will start trimming away at the code to see if I can remove the problem. – TravisThomas Oct 08 '12 at 20:42
  • So if we use say `eval("ls");` it will fail? – CrazyCasta Oct 08 '12 at 20:50
  • We also need to see the code for parseline. – CrazyCasta Oct 08 '12 at 20:52
  • No, Crazy, the problem would be if we say eval("thisdoesntexist"), then execvp would create a process instead of returning -1 and control returning to the eval function. – TravisThomas Oct 08 '12 at 21:05
  • 'execvp would create a process' -- a) You're assuming that. b) execvp doesn't create processes, fork does. – Jim Balter Oct 08 '12 at 21:19
  • Where is the code for `Fork`? That's your own routine which presumably calls `fork`. – Jim Balter Oct 08 '12 at 21:22
  • (b) You're right, of course. (a) OK, more precisely: execvp did something other than return. Subsequently, my SIGCHLD handler picked up the process I just forked being terminated by signal 11. – TravisThomas Oct 08 '12 at 21:24
  • I would guess that you have stomped on argv -- perhaps a buffer overflow of some other malloced memory. Put a breakpoint on your execvp and verify that argv is exactly as it should be. – Jim Balter Oct 08 '12 at 21:27
  • Ack sorry for formatting fail. Fork() is a small wrapper for fork() to handle errors. pid_t Fork() { pid_t pid; if ((pid = fork()) < 0) unix_error("fork error"); return pid; } I will check argv. – TravisThomas Oct 08 '12 at 21:28
  • Check that `char **argv = malloc(MAXLINE);` allocates enough memory. Keep in mind that this array holds pointers each 4-8 bytes long. If you pass something with too many arguments you will run into trouble here. – Sergey L. Oct 08 '12 at 22:27

1 Answers1

0

Immediately after the execvp, app_notify("%s command not found.", errno, argv[0]); seg faulted. It essentially wraps a printf and should have only one argument following the quoted string, as there is only one substitution.

The key to debugging this was discovering set follow-fork-mode child for gdb.

Thanks for your help, Jim, et al.

TravisThomas
  • 611
  • 1
  • 8
  • 21