Re: [boost] Stacktrace review: concern, probable solution and review news

Subject: Re: [boost] Stacktrace review: concern, probable solution and review news
From: Andrey Semashev (andrey.semashev_at_[hidden])
Date: 2016-12-21 16:28:15

On 12/21/16 22:43, Antony Polukhin wrote:
> Hi,
>
> * reviewers wish to have a smaller size of the `stacktrace` class,
> want to avoid template parameter and have a std::vector<frames> like
> interface (this is async unsafe because  vector does dynamic
> allocations).

Actually, that depends on the allocator. A stack-based allocator or an 
allocator backed by static storage could be async-safe, making `vector` 
async-safe as well.

> So, the solution:
> * Change `basic_stacktrace<size_t>` to `basic_stacktrace<class
> Allocator = std::allocator<void*>>` and make it's constructor
> async-unsafe and add additional runtime parameter for stack depth.

Why not:

   template<
     class Backend = default_backend,
     class Allocator = std::allocator<basic_frame<Backend>>
   >
   class basic_stacktrace;

? (As noted above, the constructor can still be signal-safe with an 
appropriate allocator.) I suggested in my review that backend as a 
template parameter could be beneficial and makes more sense.

Note also that the allocator is instantiated on `frame`, which is what 
is supposed to be the element type of the stacktrace container. If 
`frame` is a standard layout class with only `void*` as its data member, 
you could still use it with underlying APIs that take `void**` as the 
list of stack frames. It would require a `reinterpret_cast` inside the 
library to cast between `frame*` and `void**`, but in return the user's 
interface would be clean and operate on `frame`s instead of void pointers.

> * Provide async-safe functions that dump addresses separated by
> whitespace (format also acceptable by addr2line and atos):
>     * `void dump_stacktrace(int fd)`
>     * `void dump_stacktrace(HANDLE fd)`
>     * `void dump_stacktrace(const char* filename)`

Yes, if that's in addition to the non-async-safe function that decodes 
the stacktrace in-process.

Also, on Windows you might have to provide `wchar_t`-based function as 
well. I'm not sure if `char`-based one would be safe to use in a signal 
handler since narow character WinAPI perform character conversion 
internally.

> * Provide an example with dumping callstack to a file and printing it
> on program restart

I would suggest an example of printing the stacktrace by a separate 
program. Printing the stacktrace upon restart seems like a contrived case.

> My list of TODOs for the library:
> * Take a look at the llvm-symbolizer, atos and libdwraf; try hard to
> produce a non-forking solution

+many. Thanks for working on this.