Potential fix: gdb/strace swallows stderr

Ryan Johnson ryan.johnson@cs.utoronto.ca
Thu Aug 18 15:49:00 GMT 2011


On 18/08/2011 5:14 AM, Corinna Vinschen wrote:
> On Aug 17 20:57, Ryan Johnson wrote:
>> 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. */
>>>
>> 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. :}
I know that feeling all too well... the wording of the question was no 
accident ;)

>> 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.
Hmm. Looking at webcvs, it seems you went with this route:
if (out == err && (!being_debugged () || not_open (2)))

Maybe I'm just paranoid, but can it ever arise that out==err when 
!being_debugged()? If so, is it correct to create a duplicate handle in 
that case? I assume it is, given the code comment about needing 
different handles. But then, if out==err because the debugger 
initialized them that way, wouldn't we again need to duplicate the 
handles to avoid whatever trouble is presumably caused by not 
duplicating them?

Ryan



More information about the Cygwin-developers mailing list