Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

make little overflows its stack #797

Closed
bugaevc opened this issue Nov 18, 2019 · 3 comments
Closed

make little overflows its stack #797

bugaevc opened this issue Nov 18, 2019 · 3 comments

Comments

@bugaevc
Copy link
Member

bugaevc commented Nov 18, 2019

Some recent changes must have broken GNU Make. Now, when I run cd little && make (make sure to either comment out the mkdir() call or #include <sys/stat.h> to make it compile), I get a bunch of make: write error: stdout: Bad fd number errors, then it overflows its stack and the new stack pointer validation code kills it.

Here's a stacktrace for the first time it calls close() on stdout:

0x0806a48b  fclose +24
0x0805ba0d  close_stdout +29
0x080590ec  die +236
0x08049ece  _start +94

and here's the second time:

0x0806a48b  fclose +24
0x0805ba0d  close_stdout +29
0x0805ba3b  close_stdout +75
0x080590ec  die +236
0x08049ece  _start +94

The rest are presumably similar. Here's the close_stdout() function:

static void
close_stdout (void)
{
  int prev_fail = ferror (stdout);
  int fclose_fail = fclose (stdout);

  if (prev_fail || fclose_fail)
    {
      if (fclose_fail)
        perror_with_name (_("write error: stdout"), "");
      else
        O (error, NILF, _("write error: stdout"));
      exit (MAKE_TROUBLE);
    }
}

it's set as an atexit handler.

Now, POSIX states that

If exit() is called more than once, the behavior is undefined.

and the Linux man page says

The behavior is undefined if one of the functions registered using atexit(3) and on_exit(3) calls either exit() or longjmp(3).

This is clearly the case here — close_stdout() calls exit() on errors despite being an exit handler itself, so I don't think this needs fixing on our side. But why does it get an error (prev_fail || fclose_fail) in the first place? Other times, such as when doing make clean, or when running make again (with it reporting that there's nothing to do), this does not happen.

@awesomekling
Copy link
Member

The breakage is probably from removing the isatty() syscall (because it's now implemented in userspace.)
Rebuilding the make port will probably work, but maybe there's more trouble here we can fix :)

@bugaevc
Copy link
Member Author

bugaevc commented Nov 18, 2019

The breakage is probably from removing the isatty() syscall (because it's now implemented in userspace.)

No, it seems unrelated to that. I'm seeing this from other programs in the log:

collect2(25): Null syscall 36 requested: "isatty (removed)", you probably need to rebuild this program.

But not for make, which I have rebuilt.

@bugaevc
Copy link
Member Author

bugaevc commented Nov 18, 2019

prev_fail is EINTR

bugaevc added a commit to bugaevc/serenity that referenced this issue Nov 19, 2019
Remove explicit checking for pending signals from writing code paths,
since this is handled automatically when blocking, and should not
happen if the write() call is "short", i.e. doesn't block. All the
other syscalls already work like this.

Fixes SerenityOS#797
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants