Re: backtrace(3) in Cygwin

[David Stacey <drstacey at tiscali dot co dot uk>]

On 16/03/15 10:07, Corinna Vinschen wrote:
> Unfortunately there are some preliminaries before I can take a closer
> look, namely the copyright assignment we need from everybody who's
> providing more than just trivial patches to Cygwin, see
> https://cygwin.com/contrib.html, the "Before you get started" section.
> Fill out https://cygwin.com/assign.txt and send it to the address given
> in the pamphlet.  If you're writing code for Cygwin only in your spare
> time, forget the part about your employer.

I'm hoping that this won't be a problem. I'm more than happy to assign 
my own copyright to Red Hat. My employer has no claim to work I do in my 
own time on my own equipment. However, Cygwin might be a grey area, as 
we use Cygwin extensively in our development work. So I've already asked 
my employer to sign the copyright and patent waiver. Assuming this is 
signed, I'll send both parts together.

So let's proceed assuming that this is going to happen.

> But  submissions almost always have to jump a couple of hurdles
> (i.e., see how many iterations some patches need to make it into the
> official Linux kernel) , so I'm asking for your patience in return, ok?

Submitting code to the Linux kernel also involves one of us ranting and 
shouting obscenities, but hopefully we don't have to emulate Linux that 
closely ;-)

> * While you're already providing code, I'm missing a bit of discussion
>    first.  You didn't respond to
>    https://cygwin.com/ml/cygwin/2015-03/msg00206.html at all.
>
>    Of course, it's ideally discussed on cygwin-developers anyway, but a
>    discussion should ideally preceed a code submission :}

Sorry - that's my fault for not understanding. The code I posted was in 
no way a finished item; it was just to get the discussion started. And I 
wasn't trying to avoid a discussion - in fact quite the opposite. 
Talking about code is *so* much more interesting when there's actual 
code to talk about! I simply presented one way in which it /might/ work, 
but...

>    My main point: Only *one* implementation of the stack walk code,
>    not one for generating stackdumps and one for implementing backtrace(3).
>
>    That doesn't mean we have to stick to the current stack_info class in
>    exceptions.cc, but it's certainly still a good idea to implement the
>    entire stack walking stuff in a class which allows usage from both
>    places.

... it sounds like you have an idea of how you'd like this implemented - 
and that's fine. This is your project, so I'll keep reworking the code 
until we're both happy. I haven't had chance to look at the code you 
mentioned yet, but I will. I'm rather busy this week, so it might be a 
while before I come back to you.

> * Your coding style doesn't match the GNU coding style,

That's OK. I've been long enough in the coding game that I can write 
code in anybody's style. Now I know what you want, future code will 
adhere to the GNU standard (hopefully). A couple of questions not 
covered in the standard:

  - I assume from the examples that indentation is two spaces (I didn't 
see this stated explicitly). And I assume that you always use spaces and 
never tab characters;

  - in comments, do you prefer English spellings or their American 
equivalents?

> Don't use assert here.  Generate an error instead.

Please could you point me at an example of error reporting elsewhere in 
the Cygwin code so I can see how you'd like that doing.

> > /* backtrace_symbols_fd() takes the same buffer and size arguments as
> >   * backtrace_symbols(), but instead of returning an array of strings to the
> >   * caller, it writes the strings, one per line, to the file descriptor fd.
> >   * backtrace_symbols_fd() does not call malloc(3), and so can be employed in
> >   * situations where the latter function might fail.
> >   */
> > void backtrace_symbols_fd(void *const *buffer, int size, int fd)
> > {
> >      /* A Cygwin implementation of backtrace_symbols_fd(3) is going to be
> >       * somewhat problematic and will demand a compromise at some point along
> >       * the way. The problem is how to allocate memory for the SYMBOL_INFO
> >       * structure, given that we're not supposed to use heap memory. Windows
> >       * defines MAX_SYM_NAME as 2000 characters, and clearly we shouldn't go
> >       * trying to allocate that much memory on the stack.
> This is Windows.  What you can do to make sure not to use any existing
> heap is this:  Create your own Windows heap and use this exclusively
> from the C++ class implementing the stack walk.  If creating the heap
> fails, we're screwed, but otherwise you can allocate memory from a
> pristine heap not related to other stuff in the application.

I've done a little investigation, and it seems that the stack size on 
Cygwin is much larger than I thought. So I should be able to allocate a 
couple of 2k buffers on the stack without any problem. Coverity (for 
instance) will only start grumbling about excessive stack use once a 
function has used 10k. However, if I use your existing stack walker 
class then backtrace_symbols_fd(3) will be implemented differently anyway.

Thank you for your feedback, and apologies once again for any 
misunderstanding.

Dave.