[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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

From: Weimin Pan
Subject: Re: [PATCH][gnulib] Add the Sframe package
Date: Thu, 17 Nov 2022 12:12:10 -0800
User-agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:102.0) Gecko/20100101 Thunderbird/102.4.2

Hi Bruno,

Thanks for your comments.

On 11/15/2022 4:52 PM, Bruno Haible wrote:
Hello Weimin,

The main use-case for this format are
"online" debugging tools like stack tracers
I'll appreciate anything that can help producing a universally working
backtrace for C, since experience (e.g. from Lisp, Java, Python) has
shown that such a backtrace function can tremendously improve the
developer productivity and the usefulness of bug reports from anonymous

My experiments on this topic, two years ago, were not satisfactory:
   * The backtrace function from glibc
       - produces no file names, no line numbers,
       - produces function names only when the binary was linked with
       - is not portable.
   * The libbacktrace library produced hardly useful information as well, even
     in simple cases. See [1].

Can you describe how the SFrame format fits in, w.r.t. to
   - backtrace from glibc (based on libunwind AFAIK),
   - libbacktrace ?

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.

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.

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.

This patch adds two new modules to gnulib, related to the Simple Frame
format (SFrame) recently introduced in binutils [1].
I'm pleased to see that it is extensively commented, includes sanity checks,
and is already in GNU coding style.

Since this is a significant contribution (regardless whether it finally
goes into Gnulib [2] or into Binutils [3]), we will need a copyright assignment
to the FSF for this code. Are you already aware how this works, in your context
as an Oracle employee? Maybe José Marchesi can share his experience on this
topic with you?

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

Once the big picture is clear and the copyright issue is handled, we'll
go into the detailed code review. For now, what I can see are only nits,
due to Gnulib conventions:
   - The header file module should better be named 'sframe-h' instead of

OK, done.

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

OK, will do.

   - tests/sframe-api.h should not be included since it's a copy of
     lib/sframe-api.h, right?

Yes, it is and will be deleted.

   - tests/DATA* should better be moved to a subdirectory:
     tests/sframe-data/DATA*, to make it clear to which part of Gnulib it



[1] https://github.com/ianlancetaylor/libbacktrace/issues
[2] https://www.gnu.org/software/gnulib/manual/html_node/Copyright.html

reply via email to

[Prev in Thread] Current Thread [Next in Thread]