Potential fix: gdb/strace swallows stderr

Corinna Vinschen corinna-cygwin@cygwin.com
Thu Aug 18 09:15:00 GMT 2011


On Aug 17 20:57, Ryan Johnson wrote:
> Hi all (attn: Corinna),
> 
> I've been looking into the way stderr disappears when a cygwin
> process is started by gdb or strace (64-bit win7 box), and it looks
> like cygheap->fdtab[2] somehow gets initialized to NULL. The EBADF
> which results kills off the rest of the write "syscall."
> 
> Looking deeper shows that, in dtable::stdio_init, GetStdHandle()
> returns the same value for stdout and stderr, but being_debugged()
> and not_open(2) both return 1, with the result that this code
> doesn't run:
> >  /* STD_ERROR_HANDLE has been observed to be the same as
> >     STD_OUTPUT_HANDLE.  We need separate handles (e.g. using pipes
> >     to pass data from child to parent).  */
> >  /* CV 2008-10-17: Under debugger control, std fd's have been potentially
> >     initialized in dtable::get_debugger_info ().  In this case
> >     init_std_file_from_handle is a no-op, so, even if out == err we don't
> >     want to duplicate the handle since it will be unused. */
> >  if (out == err && (!being_debugged () || !not_open (2)))
> >    {
> >      /* Since this code is not invoked for forked tasks, we don't have
> >         to worry about the close-on-exec flag here.  */
> >      if (!DuplicateHandle (GetCurrentProcess (), out,
> >                            GetCurrentProcess (), &err,
> >                            0, TRUE, DUPLICATE_SAME_ACCESS))
> >        {
> >          /* If that fails, do this as a fall back.  */
> >          err = out;
> >          system_printf ("couldn't make stderr distinct from stdout, %E");
> >        }
> >    }
> ... with the result that err is invalid when this runs immediately after:
> >  init_std_file_from_handle (1, out);
> >  init_std_file_from_handle (2, err);
> 
> Always duplicating the handle when out==err seems to fix the problem
> for both gdb and strace, without harming non-traced execution.
> However, I doubt that's the correct thing to do, since the other
> checks are clearly not accidental. Calls to not_open(1) and
> not_open(2) both return 1, so I wonder if an assumption has become
> invalid (e.g. did it used to be that stderr should have already been
> opened but may have been closed already as well, but now stderr has
> not even been opened yet?).
> 
> Corinna, can you dredge up any useful memories about the issue? The

Ha ha ha, huh huh, good joke.  After three years, the comment is all
what's left of the entire scenario. :}

> code in dtable::get_debugger_info definitely runs (gdb prints
> "warning: cYgstd 28cc69 d 3"), but std[][] remains empty, so none of
> the std handles was initialized in that way.
> 
> So, which of the following changes, if any, is a proper fix? The
> first assumes that the whole !not_open(2) thing has become
> completely bogus (or always was), while the second is a more
> conservative workaround. The third assumes that a reverse-sense
> boolean just slipped in unnoticed. All three changes seem to behave
> correctly under my limited testing...
> 
> -   if (out == err && (!being_debugged () || !not_open (2)))
> +  if (out == err)
> +  if (out == err && (!being_debugged () || (not_open (1) &&
> not_open (2)) || !not_open (2)))
> +  if (out == err && (!being_debugged () || not_open (2)))
> 
> Based on the code comments, I suspect #2 is the correct fix --
> stderr must be usable if there's no debugger, if the debugger
> explicitly initialized stderr (but to a duplicate handle that needs
> fixup), or -- this is the new case -- if the debugger didn't
> initialize any handles (so stderr needs initialized with a
> duplicated handle).

I'm wondering how I could ever apply this.  The !not_open(2) is just
plain wrong (looks like a copy-paste bug).  If not_open(2), then we
*want* to initialize fd 2, no matter what.  If !not_open(2), then fd 2
has been initialized and we don't want to create a useless handle.
So the condition is just upside down.  I'll apply a patch.

> P.S. this snippet from dtable::get_debugger_info looks suspicious:
> >            if (!fh->open ((i ? (i == 2 ? O_RDWR : O_WRONLY) : O_RDONLY)
> >                           | O_BINARY, 0777))
> >              release (i);
> >            else
> >              CloseHandle (h);
> >              /* Copy to Windows' idea of a standard handle, otherwise
> >                 we have invalid standard handles when calling Windows
> >                 functions (small_printf and strace might suffer, too). */
> >              SetStdHandle (std_consts[i], i ? fh->get_output_handle ()
> >                                             : fh->get_handle ());
> The indentation of the call to SetStdHandle (and the comment
> describing it) suggests that it was intended to be part of the else
> clause, but it's not. It's not causing problems in this particular
> case because it's unreached, but it does look confusing.

Right, braces are missing.  Looks like I had a really bad day on
2008-10-17.  I fix that as well.

Thanks for the heads up.


Corinna

-- 
Corinna Vinschen                  Please, send mails regarding Cygwin to
Cygwin Project Co-Leader          cygwin AT cygwin DOT com
Red Hat



More information about the Cygwin-developers mailing list