Re: [PATCH][gnulib] Add the Sframe package

[Bruno Haible]

Weimin Pan wrote:
> We came to design the SFrame format (The S stands for `simple') due to some
> concrete requirements of a very big program that ships its own "online"
> stack tracer and unwinder to handle error conditions:
> 
> 1) They wanted something simple to decode and simple to compute.  This
>     is in contrast with DWARF/eh_frame that, generally, requires
>     executing DWARF expressions and therefore the maintenance of a small
>     stack machine.
> 
> 2) They wanted the unwind info to be fast to decode and to compute.
> 
> 3) They wanted the unwind info to be as compact as possible, of course
>     maintaining a good trade-off vs. simplicity.
> 
> 4) They wanted something that wouldn't be stripped out of the
>     executables or shared objects, even maybe as a mistake.

Thanks for the explanations.

> Now, both libbacktrace and libunwind use eh_frame or DWARF as an
> underlying format from which to extract the unwind info.  We could (and
> have considered) adding support for SFrame as well to these libraries,
> but that is orthogonal to adding the modules (definition of the stored
> format data structures, and convenience encoders/decoders) to gnulib:
> the applications using the modules will most likely implement its own
> stack tracer and debug infrastructure.

I agree that it is orthogonal. Since you wrote that one of the goals was
"to minimize external dependencies", it makes sense to make the code
available outside of libbacktrace and libunwind, and Gnulib seems an
appropriate place.

Anyone has an objection to including this in Gnulib?

> Yes, I believe this issue has been addressed and resolved.

Yes, the copyright issue has been addressed and resolved.

> 5) They wanted something to the point: just unwind information, i.e. the
>     ability of jumping back the stack frames.  This doesn't include type
>     information, the contents of every callee-saved register, etc.

I read the same thing in <https://www.phoronix.com/news/GNU-Binutils-SFrame>:
"callee-saved registers other than FP are not needed for stack unwinding,
 and hence are not included in the .sframe section."
but I don't understand it: If you don't have the contents of the
callee-saved registers, you can perfectly well produce a textual stack trace
but how would you "jump back", in the sense of transferring control,
like longjmp() does?

> >    - The *.c and *.h files should be untabified (run 'expand -t 8' on each).
> 
> OK, will do.

Two other points:

  * sframe.h should have a pointer to the specification document of Sframe.
    AFAIU, it is
    
https://sourceware.org/git/?p=binutils-gdb.git;a=blob;f=libsframe/doc/sframe-spec.texi
    , right?

  * sframe-api.h and sframe.c contains both a decoder and an encoder.
    You said that the "main use-case for this format are "online" debugging
    tools like stack tracers". Does an average stack trace require the
    ability to encode Sframe?
    If not, then we should have two Gnulib modules: an sframe-decode and
    an sframe-encode module. The former would be sufficient for stack tracers,
    the latter would only be required for other uses (in gas, for example).

You also wrote:
  "the plan is to switch the later to use this gnulib module instead
   in order to minimize external dependencies."
So, you plan to avoid code duplication between
  https://sourceware.org/git/?p=binutils-gdb.git;a=tree;f=include
  https://sourceware.org/git/?p=binutils-gdb.git;a=tree;f=libsframe
and Gnulib by having binutils import the code from Gnulib?

Finally, can you propose a reviewer for this code? Someone who will
try to find mistakes or bugs and who is not a co-author. I am reviewing
most of the Gnulib contributions, but this large contribution is more
than I can easily take on.

Bruno