[Boost-users] Stacktrace library starts review today 14th Dec

[Niall Douglas]

The formal review of the Stacktrace library by Antony Polukhin starts

today 14th Dec and will conclude before Christmas. I appreciate we
are likely a bit tired out from the many library reviews recently and
of course it's Christmas, but given the lack of a portable way to
work with stack backtraces, which you inevitably need to do
eventually in any non-toy production application, Stacktrace needs
your review!

Stacktrace is an optionally header-only library providing four
implementation backends, libunwind (POSIX only), windbg (Windows
only), backtrace (from the C library on most POSIX implementations)
and a null backend. At its very simplest it lets you capture the
stack backtrace for the calling thread and to print it to a
std::ostream& of your choice. The basic_stacktrace<> class quacks
like a STL container of frame object instances. The rest of the API
pretty much follows STL design principles for the most part. Use is
therefore unsurprising.

You can find the documentation at
http://apolukhin.github.io/stacktrace/index.html and the github repo
at https://github.com/apolukhin/stacktrace.

Review guidelines
=================

Reviews should be submitted to the developer list
(***@lists.boost.org), preferably with '[stacktrace]' in the
subject. Or if you don't wish to for some reason or are not
subscribed to the developer list you can send them privately to me at
's_sourceforge at nedprod dot com'. If so, please let me know whether
or not you'd like your review to be forwarded to the list.

For your review you may wish to consider the following questions:

- What is your evaluation of the design?

- What is your evaluation of the implementation? Most of my
personal concerns with this library are with the implementation and I
would hugely appreciate feedback from others with substantial
experience of using stacktracing "in anger" in non-trivial use case
scenarios.

- What is your evaluation of the documentation?
- What is your evaluation of the potential usefulness of the
library?
- Did you try to use the library? With what compiler? Did you
have any problems?
- How much effort did you put into your evaluation? A glance? A
quick reading? In-depth study?
- Are you knowledgeable about the problem domain?

And finally, every review should attempt to answer this question:

- Do you think the library should be accepted as a Boost
library?

Be sure to say this explicitly so that your other comments don't
obscure your overall opinion.

Even if you do not wish to give a full review any technical comment
regarding the library is welcome as part of the review period and
will help me as the review manager decide whether the library should
be accepted as a Boost library. Any questions about the use of the
library are also welcome.

Finally, thanks to Edward whose announcement of the Synapse library
review I borrowed heavily from as I thought it very well structured.
Hopefully the above is just as clear.

Niall

[Edward Diener]

In the documentation for stacktrace I do not see anything about what
level of C++ is required for a compiler using the stacktrace library or
any information on what compilers have been tested when using the library.

In the section of "Build, Macros and Backends" I do not think there is
enough information about using a particular backend, other than saying
that you need to link with such-and-such. Does stacktrace
"automatically" find the header files it needs from these backends ?
Even so I think some information about where it looks for the backend
header files need to be given. I also think that some information of
what libraries it looks to link with needs to be given. I am assuming it
links with the static versions of these backend libraries unless I
define either BOOST_STACKTRACE_LINK and BOOST_ALL_DYN_LINK, or
BOOST_STACKTRACE_DYN_LINK. What if I want to use the header-only version
but I want to link with the shared library backend ? I also assume that
if I link with the static library backend I should probably be linking
with the RTL static library, while if I link with the shared library
backend I should also be linking with the RTL shared library.

What about 32-bit versus 64-bit use of stacktrace ? Do they both work as
advertised with the correct backends, whether static or shared ? I think
the "Build, Macros and Backends" need rto be expanded to cover alol
these topics.

[Antony Polukhin]

In the documentation for stacktrace I do not see anything about what level
of C++ is required for a compiler using the stacktrace library or any
information on what compilers have been tested when using the library.

It's C++98. Tested on latest clang(c++03, c++11), gcc(c++98, c++11,
c++1y), msvc.

In the section of "Build, Macros and Backends" I do not think there is
enough information about using a particular backend, other than saying that
you need to link with such-and-such. Does stacktrace "automatically" find
the header files it needs from these backends ? Even so I think some
information about where it looks for the backend header files need to be
given. I also think that some information of what libraries it looks to link
with needs to be given. I am assuming it links with the static versions of
these backend libraries unless I define either BOOST_STACKTRACE_LINK and
BOOST_ALL_DYN_LINK, or BOOST_STACKTRACE_DYN_LINK. What if I want to use the
header-only version but I want to link with the shared library backend ? I
also assume that if I link with the static library backend I should probably
be linking with the RTL static library, while if I link with the shared
library backend I should also be linking with the RTL shared library.
What about 32-bit versus 64-bit use of stacktrace ? Do they both work as
advertised with the correct backends, whether static or shared ? I think the
"Build, Macros and Backends" need rto be expanded to cover alol these
topics.

All the headers are found automatically and the library is header only
if you do not define BOOST_STACKTRACE_LINK or
BOOST_STACKTRACE_DYN_LINK.

Backend is chosen automatically in header only mode. You need to do
nothing, unless you encounter some problems on default setup (like you
OS is POSIX but not LSB Core Specification 4.1) or unless you wish to
disable stacktraces.

If you define BOOST_STACKTRACE_LINK or BOOST_STACKTRACE_DYN_LINK you
have to manually link with one of the backends.


I'll try to improve the "Build, Macros and Backends" section to be more clear.

[Edward Diener]

Post by Antony Polukhin

In the documentation for stacktrace I do not see anything about what level
of C++ is required for a compiler using the stacktrace library or any
information on what compilers have been tested when using the library.

It's C++98. Tested on latest clang(c++03, c++11), gcc(c++98, c++11,
c++1y), msvc.

Some of your examples in the doc use c++11 features, so I think you need
to specifically say in the doc that the library itself is c++98 on up.

Post by Antony Polukhin

In the section of "Build, Macros and Backends" I do not think there is
enough information about using a particular backend, other than saying that
you need to link with such-and-such. Does stacktrace "automatically" find
the header files it needs from these backends ? Even so I think some
information about where it looks for the backend header files need to be
given. I also think that some information of what libraries it looks to link
with needs to be given. I am assuming it links with the static versions of
these backend libraries unless I define either BOOST_STACKTRACE_LINK and
BOOST_ALL_DYN_LINK, or BOOST_STACKTRACE_DYN_LINK. What if I want to use the
header-only version but I want to link with the shared library backend ? I
also assume that if I link with the static library backend I should probably
be linking with the RTL static library, while if I link with the shared
library backend I should also be linking with the RTL shared library.
What about 32-bit versus 64-bit use of stacktrace ? Do they both work as
advertised with the correct backends, whether static or shared ? I think the
"Build, Macros and Backends" need rto be expanded to cover alol these
topics.

All the headers are found automatically and the library is header only
if you do not define BOOST_STACKTRACE_LINK or
BOOST_STACKTRACE_DYN_LINK.

I think you need to explain where you are "automatically" looking for
header files in case header files are not found.

Post by Antony Polukhin
Backend is chosen automatically in header only mode. You need to do
nothing, unless you encounter some problems on default setup (like you
OS is POSIX but not LSB Core Specification 4.1) or unless you wish to
disable stacktraces.

Again you need to explain where you are "automatically" looking for
appropriate static libraries in case one is not being found.

Post by Antony Polukhin
If you define BOOST_STACKTRACE_LINK or BOOST_STACKTRACE_DYN_LINK you
have to manually link with one of the backends.
I'll try to improve the "Build, Macros and Backends" section to be more clear.

Please do, You don't want annoyed end-users in case all this "automatic"
finding does not work as you imagine it should. Furthermore explanations
of what RTL ( static or shared ) should/need be used should be explained
also. These various combinations of static or shared backends and/or
RTLs need to be explained. I gather the backends themselves are not
hooked to compiler RTLs, but I do not know if stacktrace itself uses
compiler RTLs or not, but I expect it might.

Don't annoy end-users of your library by assuming that all this stuff
must "just work", so you do not need to explain these things in detail,
because if it does not "just work" as you imagine the first thing they
will probably do is dismiss your library.

[Antony Polukhin]

2016-12-17 20:51 GMT+03:00 Andrey Semashev <***@gmail.com>:
<...>

That said, I'd like to thank Antony for the submission and encourage
him to continue his work on the library. The library does try to fill
an important gap.

Thank you for the review!

I'd like to discuss a proposed changes. Thing that I'm worried about:
* replacing class `stacktrace` with vector<void*> will definitely
break the most common use case std::cout << get_stacktrace(); as it's
not a good idea to overload ostream operator for vector of void
pointers
* addr2line and forking is not nice. Is linking with a GPL licensed
code an acceptable alternative?
* removing `basic_stacktrace` will definitely remove all the
possibilities for optimization of caching COM/parsed DWARF. This is a
quick win that later may turn into big troubles.