Boost logo

Boost :

Subject: Re: [boost] [Stacktrace] review, please stop discussing non-Stacktrace issues
From: Peter Dimov (lists_at_[hidden])
Date: 2016-12-17 10:31:59

Niall Douglas wrote:

> 3. Please could people actually review Stacktrace's implementation code
> instead of inferring implementation from what someone else said here about
> something they half read in the tutorial. Specifically, is its API design
> solid? Is the use of std::string in its public API acceptable?

There's nothing wrong with using std::string in principle, but it's
obviously not async safe. The operations however that return std::strings
are also not async safe at the moment, so it's not the std::string return
value that's the problem.

The documentation has an example of printing a stack trace from a SIGSEGV
handler, which is undefined behavior as it stands. So it's not clear to me
whether the library is supposed to be useful - in its decoding/printing
part, not just in its capture part - in signal handlers.

Changing/augmenting the interface to be async safe would be easy,

    size_t frame::get_name( char* buffer, size_t size ) const;

instead of, or in addition to,

    std::string frame::name() const;


    void frame::print_to_fd( int fd ) const;

in addition to

    ostream& operator<<( ostream&, frame const & );

but if the implementation can't be made async safe, this is not going to be
of much use.

A quick look of the implementation - and the list of POSIX async safe
functions - was not enough for me to be able to resolve this question one
way or the other. Most of what the Linux backend is doing is already async
safe, but not everything, addr2line has an option to demangle, so the
dependency on core::demangle could in principle be avoided... but I'm not
quite sure.

There is also an indication that the current frame interface is not the
right one. In

    std::string name =;
    if (!name.empty()) {
        os << name;
    } else {
        os << f.address();

    const std::size_t source_line = f.source_line();
    if (source_line) {
        os << " at " << f.source_file() << ':' << source_line;

which is perfectly reasonable code given the current interface, addr2line is
called three times instead of just one. And in operator<< for a stacktrace
of depth N, addr2line will be called 3*N times instead of one.

The operator<< implementations themselves can be fixed, but user code that
does the same will obviously not be able to benefit.

I understand why it's better for the value_type of
stacktrace::const_iterator to be 'frame' instead of just 'void*', but the
accessors each calling addr2line is a symptom of a serious problem that the
'stupid' void* design:

void get_frame_info( void* address, string& function, string& file, size_t&
line );


stacktrace itself could provide a function that calls a user-provided

void process_frame_info( size_t index, void* address, char const* function,
char const* file, size_t line );

for each frame. This would be able to call addr2line just once, and it's
also an async-safe interface, if the implementation can be made so.

This is not exactly a common C++ style, but async safe code obviously has
its own rules.

Boost list run by bdawes at, gregod at, cpdaniel at, john at