Re: [boost] [Boost] [Stacktrace] review

Nat Goodspeed

unread,

Dec 14, 2016, 2:42:33 PM12/14/16

to bo...@lists.boost.org

On Wed, Dec 14, 2016 at 3:43 AM, Niall Douglas <s_sour...@nedprod.com>
wrote:

For your review you may wish to consider the following questions:
>
> - What is your evaluation of the design?
>

I'm generally pleased with the design.

There's a documentation section "Exceptions with stacktrace" that discusses
techniques for incorporating stacktrace data into your own exceptions. I
see a real possibility for integration with Boost.Exception, perhaps even a
mechanism to make BOOST_THROW_EXCEPTION embed the stacktrace at the throw
point. This is not an objection to accepting the library, but rather a
feature request for when we do.

I do have a question about use of a template parameter to constrain the
number of entries, rather than a runtime parameter. Presumably that's
necessary to permit certain backend implementations to be
async-signal-safe? (Whatever the rationale, a note in the documentation
explaining it would be nice.)

My objection to that is that apparently, with the present design, if I want
to write a function that accepts any specialization of basic_stacktrace,
that must itself be a template function.

If I'm correct about the reason for using a template parameter, would it be
reasonable to split the class into a non-template base class supporting all
the methods, with a template subclass that provides the storage? The base
class could store a (pointer, length) passed by the subclass constructor.
Without having looked at the implementation, I expect that you wouldn't
even need virtual-function dispatch: the only indirection would be through
the data buffer pointer.

That would even permit a subclass that provides dynamic storage, for
async-signal-unsafe use.

Then a processing function could accept reference-to-base-class and handle
any stacktrace instance.

> - What is your evaluation of the implementation?

Sorry, I haven't looked at the implementation.

> - What is your evaluation of the documentation?
>

Generally good. It could stand a cleanup pass; there are a few typos here
and there.

I do like that each method explicitly states both complexity and async
signal safety.

The description of the nullary frame() constructor says that source_line()
will return both empty string and 0.

> - What is your evaluation of the potential usefulness of the
> library?
>

VERY! I want this.

> - Did you try to use the library? With what compiler? Did you
> have any problems?
>

No, I haven't tried to use it. (A deadline is coming up fast.)

> - How much effort did you put into your evaluation? A glance? A
> quick reading? In-depth study?
>

I read through the documentation.

> - Are you knowledgeable about the problem domain?
>

I have occasionally used platform-specific tactics to try to achieve the
same thing, on a couple different platforms. Each time it's been painful
enough to make me wish that this problem were solved, once and for all.

> And finally, every review should attempt to answer this question:
>
> - Do you think the library should be accepted as a Boost
> library?
>

Yes I do. I would like to see the non-template base class, but that's not a
showstopper: I would rather have this library without than not have it at
all.

As I mentioned, I would like to see integration with Boost.Exception. That
just seems like a natural next step.

I see further opportunities for integration with Boost.Context,
Boost.Fiber...

_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost

Peter Dimov

unread,

Dec 14, 2016, 6:27:38 PM12/14/16

to bo...@lists.boost.org

Nat Goodspeed wrote:
> There's a documentation section "Exceptions with stacktrace" that
> discusses techniques for incorporating stacktrace data into your own
> exceptions. I see a real possibility for integration with Boost.Exception,

> ...

Yes, this jumped out at me too. The whole point of Boost.Exception is to
carry arbitrary data along with the exception, so an example that uses
Boost.Exception instead of the custom 'traced' type should be given.

> ... perhaps even a mechanism to make BOOST_THROW_EXCEPTION embed the

> stacktrace at the throw point.

This sounds like a pretty good idea, although I'm not sure if we can enable
it by default as people have historically been very sensitive to changes to
BOOST_THROW_EXCEPTION. But an opt-in macro should be fine and very useful as
one would automatically get stack traces from every use of
BOOST_THROW_EXCEPTION. Hello Java.

Andrey Semashev

unread,

Dec 14, 2016, 6:45:19 PM12/14/16

to bo...@lists.boost.org

On 12/15/16 02:27, Peter Dimov wrote:

> Nat Goodspeed wrote:
>
>> ... perhaps even a mechanism to make BOOST_THROW_EXCEPTION embed the
>> stacktrace at the throw point.
>
> This sounds like a pretty good idea, although I'm not sure if we can
> enable it by default as people have historically been very sensitive to
> changes to BOOST_THROW_EXCEPTION. But an opt-in macro should be fine and
> very useful as one would automatically get stack traces from every use
> of BOOST_THROW_EXCEPTION. Hello Java.

I've not taken a look at the proposed Stacktrace library, but from what
I know obtaining a stacktrace is typically a rather expensive operation.
I expect it to be no less expensive than throwing the exception. That's
probably too much to pay for by default. Plus, it adds one more
dependency on another library via such a core component as
BOOST_THROW_EXCEPTION.

I think, a new macro in a separate library (i.e. not in throw_exception)
would be more preferable.

Emil Dotchevski

unread,

Dec 14, 2016, 7:13:07 PM12/14/16

to bo...@lists.boost.org

On Wed, Dec 14, 2016 at 3:44 PM, Andrey Semashev <andrey....@gmail.com>
wrote:

> On 12/15/16 02:27, Peter Dimov wrote:
>
>> Nat Goodspeed wrote:
>>
>> ... perhaps even a mechanism to make BOOST_THROW_EXCEPTION embed the
>>> stacktrace at the throw point.
>>>
>>
>> This sounds like a pretty good idea, although I'm not sure if we can
>> enable it by default as people have historically been very sensitive to
>> changes to BOOST_THROW_EXCEPTION. But an opt-in macro should be fine and
>> very useful as one would automatically get stack traces from every use
>> of BOOST_THROW_EXCEPTION. Hello Java.
>>
>
> I've not taken a look at the proposed Stacktrace library, but from what I
> know obtaining a stacktrace is typically a rather expensive operation. I
> expect it to be no less expensive than throwing the exception. That's
> probably too much to pay for by default. Plus, it adds one more dependency
> on another library via such a core component as BOOST_THROW_EXCEPTION.
>
> I think, a new macro in a separate library (i.e. not in throw_exception)
> would be more preferable.

I think that the stack trace should be embedded by default by
boost::throw_exception (not by BOOST_THROW_EXCEPTION), with a new
configuration macro that can be used to disable this new behavior. I think
that performance concerns are unwarranted in this case, since throwing
exceptions is expensive already; it is generally not a good idea to assume
anything about its performance.

And stack trace is very useful bit of information to have.

That said, I am much more concerned about added dependencies, because
boost/exception/exception.hpp (which boost/throw_exception.hpp includes) is
carefully designed to have NO dependencies. I would be open to providing
special hooks into the boost::exception class to support stack trace
(similarly to how there is special storage for __FILE__ and __LINE__), as
long as the dependencies make sense.

Emil

Emil

Robert Ramey

unread,

Dec 14, 2016, 7:53:54 PM12/14/16

to bo...@lists.boost.org

On 12/14/16 3:27 PM, Peter Dimov wrote:

> This sounds like a pretty good idea, although I'm not sure if we can
> enable it by default as people have historically been very sensitive to
> changes to BOOST_THROW_EXCEPTION. But an opt-in macro should be fine and
> very useful as one would automatically get stack traces from every use
> of BOOST_THROW_EXCEPTION. Hello Java.

I think that there has been a lot of confusion about what
BOOST_THROW_EXCEPTION is suppose to do.

My understanding was that it was a macro intended to support the writing
of portable code that could run on platforms which didn't support
exceptions or where the user / author didn't want to use the exception
mechanism so he could redefine the macro. Lot's of libraries used this
idiom to decouple their libraries from the the selection of exception
mechanism.

With the acceptance of Boost.Exception, this BOOST_THROW_EXCEPTION was
repurposed to call boost.exception. Obviously not what previous users
of BOOST_THROW_EXCEPTION intended or expected. This made many libraries
dependent on the new library boost.exception which had multiple
repercussions - one of which is that boost.exception has now become a
"core library" - not sure what that is supposed to mean.

Now it's being suggested that BOOST_THROW_EXCEPTION be
repurposed/enhanced yet again? Is this really a great idea? Wouldn't
it better just to encourage authors of library authors to use their own
code to throw exceptions and implement this code in the way most
suitable for their own libraries? Of course the stack trace library
might want to address this subject in the documentation and perhaps
provide it's own macros or wrappers to facilitate the usage of this
facility in other library code but the responsibility for a library
implementation would still remain with the library author/maintainer.

Robert Ramey

Peter Dimov

unread,

Dec 14, 2016, 8:47:43 PM12/14/16

to bo...@lists.boost.org

Robert Ramey wrote:

> This made many libraries dependent on the new library boost.exception
> which had multiple repercussions - one of which is that boost.exception
> has now become a "core library" - not sure what that is supposed to mean.

No, throw_exception is its own mini-library now. It depends on nothing but
Assert and Config.

C:\Projects\boost-git\boost>dist\bin\boostdep.exe throw_exception
Primary dependencies for throw_exception:

assert:
<boost/current_function.hpp>
from <boost/throw_exception.hpp>

config:
<boost/config.hpp>
from <boost/throw_exception.hpp>
<boost/detail/workaround.hpp>
from <boost/throw_exception.hpp>

Emil Dotchevski

to bo...@lists.boost.org

On 12/14/16 6:06 PM, Nat Goodspeed wrote:
> On Wed, Dec 14, 2016 at 7:53 PM, Robert Ramey <ra...@rrsd.com> wrote:
>
>> I think that there has been a lot of confusion about what
>> BOOST_THROW_EXCEPTION is suppose to do.
>>
>> My understanding was that it was a macro intended to support the writing of
>> portable code that could run on platforms which didn't support exceptions or
>> where the user / author didn't want to use the exception mechanism so he
>> could redefine the macro. Lot's of libraries used this idiom to decouple
>> their libraries from the the selection of exception mechanism.
>
> I admit that my usage of BOOST_THROW_EXCEPTION is fairly recent, but
> that use case is not even mentioned:
> http://www.boost.org/doc/libs/1_62_0/libs/exception/doc/BOOST_THROW_EXCEPTION.html

Right. Because the semantics of BOOST_THROW_EXCEPTION were changed and
the documentation updated to the new semantics. The history of the
current version only goes back to 2009 so it's not visible now.

Robert Ramey

Antony Polukhin

unread,

Dec 15, 2016, 1:28:21 AM12/15/16

to boost@lists.boost.org List

2016-12-15 5:15 GMT+03:00 Nat Goodspeed <n...@lindenlab.com>:
> On Wed, Dec 14, 2016 at 2:42 PM, Nat Goodspeed <n...@lindenlab.com> wrote:

<...>

> I forgot to mention another potential enhancement of interest. This is
> a feature request post-acceptance.
>
> I would like to be able to:
>
> * serialize a stacktrace from a running program without available symbol files
> * deserialize that stacktrace on a different system that has symbol
> files for the original program

Probably you already could do those two bulletins
http://apolukhin.github.io/stacktrace/boost_stacktrace/getting_started.html#boost_stacktrace.getting_started.saving_stacktraces_by_specified_

> * display symbolic output (function names, source files, line numbers)
> for the original stacktrace.

This is simple to do on POSIX and I will need further investigation on
how to do it on Windows. If it's possible - I'll add the functionality
after the review.

--
Best regards,
Antony Polukhin

Gavin Lambert

unread,

Dec 15, 2016, 1:37:10 AM12/15/16

to bo...@lists.boost.org

On 15/12/2016 13:53, Robert Ramey wrote:
> I think that there has been a lot of confusion about what
> BOOST_THROW_EXCEPTION is suppose to do.
>
> My understanding was that it was a macro intended to support the writing
> of portable code that could run on platforms which didn't support
> exceptions or where the user / author didn't want to use the exception
> mechanism so he could redefine the macro. Lot's of libraries used this
> idiom to decouple their libraries from the the selection of exception
> mechanism.

Shouldn't a macro with that intent be defined in
core/no_exceptions_support.hpp instead?

(There currently doesn't appear to be a throw macro there, presumably
because boost::throw_exception was sufficient?)

> With the acceptance of Boost.Exception, this BOOST_THROW_EXCEPTION
> was repurposed to call boost.exception. Obviously not what previous
> users of BOOST_THROW_EXCEPTION intended or expected. This made many
> libraries dependent on the new library boost.exception which had
> multiple repercussions - one of which is that boost.exception has now
> become a "core library" - not sure what that is supposed to mean.

It doesn't really look like (at least in current code) that
BOOST_THROW_EXCEPTION has any function unless you want to capture the
error_info automatically. Otherwise you can just always use
boost::throw_exception instead.

If you want to use BOOST_THROW_EXCEPTION without depending on
Boost.Exception, it looks like you can define BOOST_EXCEPTION_DISABLE,
although at the cost of losing the current_exception and error_info
wrappers.

This is independent of BOOST_NO_EXCEPTIONS, which controls whether
exceptions are thrown at all.

Antony Polukhin

unread,

Dec 15, 2016, 1:53:31 AM12/15/16

to boost@lists.boost.org List

2016-12-14 22:42 GMT+03:00 Nat Goodspeed <n...@lindenlab.com>:
<...>

> I do have a question about use of a template parameter to constrain the
> number of entries, rather than a runtime parameter. Presumably that's
> necessary to permit certain backend implementations to be
> async-signal-safe? (Whatever the rationale, a note in the documentation
> explaining it would be nice.)

Yes, this is for async-signal-safety + it gives an exception safety +
it removes all the allocator related stuff from the stacktrace. I'll
add a note.

> My objection to that is that apparently, with the present design, if I want
> to write a function that accepts any specialization of basic_stacktrace,
> that must itself be a template function.

Yes, you're right. But in my head - you'll be using the same default
stacktrace with the same template parameter all over the place. So
it's mostly for detecting problems (you'll get link time error while
attempting to link libraries that exchange stacktraces with different
default parameters) and for interoperability with libraries, that may
accept/produce stacktraces that differ from the current.

Robert Ramey

unread,

Dec 15, 2016, 2:34:46 PM12/15/16

to bo...@lists.boost.org

On 12/15/16 9:37 AM, Peter Dimov wrote:
> Robert Ramey wrote:
>
>> It wasn't BOOST_THROW_EXCEPTION. It was boost::throw_exception whose
>> semantics were changed in an unanticipated and surprising way. It
>> seems to me that it might be that this was being proposed again.
>> Maybe my concerns are overblown as those who got burned the first time
>> by this policy have already eliminated dependency on
>> boost::throw_exception.
>
> The whole point of using a central infrastructure for throwing
> exceptions (and asserts) is for us to be able to (carefully and after
> due deliberation, and in the interest of our users) apply changes to it
> which magically affect all Boost libraries without their maintainers
> having to do anything.

I understand the motivation for this. Still it's a very bad idea. It
changes the behavior of code in a silent, undocumented and sometimes
surprising way. It couples one library to another in a way which can
make the library inconvenient to use with other libraries.

> Last time the effect of the changes has been to make all Boost-emitted
> exceptions Boost.Exception enabled and, if the macro is used, to contain
> the file/line/function. This has a number of benefits and the downsides
> have been kept to a minimum.

Again - a matter of opinion. A typical program these days will use
libraries from several sources as well as internal code. It's going to
be difficult to make an exception design/policy for one's application
when it is set from within the library to a module whose semantics can
and have changed without notice.

> It's normal for changes to the default behavior of such a critical piece
> to be controversial and we won't make them without a consensus. Which is
> why I originally said
>
>> This sounds like a pretty good idea, although I'm not sure if we can
>> enable it by default as people have historically been very sensitive
>> to changes to BOOST_THROW_EXCEPTION. But an opt-in macro should be
>> fine and very useful as one would automatically get stack traces from
>> every use of BOOST_THROW_EXCEPTION.

LOL - well, I'm just illustrating your point here.

It's really a much deeper point here. What is a boost library? Is it
necessary part of something large - the "Boost Libraries" or does it
stand on it's own - the boost serialization library. The former view is
natural to developers who think in terms of "releasing" a humongous
product. Its natural to want to couple things like exception policy,
testing, build, etc. etc. as a more or less monolithic system. It
"seems" to make things easier. But it doesn't really - not in our
context. It engenders all the disputes like we constantly have about
testing, deployment, documentation, build systems etc. This is one
thing that the standard C++ library does better. There is less coupling
between individual components. (Of course the standard library has it's
own issues but we're not touching on them here.)

Getting back to the main point, the boost policy for exception handling
has been short sighted and the manner that it was imposed - by code
ambush - not appreciated by some of us. I don't want to dwell on it
since I lost that war a long time ago and I've tried to moved on.

Robert Ramey

Emil Dotchevski

to bo...@lists.boost.org

On 15/12/2016 19:52, Antony Polukhin wrote:
> 2016-12-14 22:42 GMT+03:00 Nat Goodspeed <n...@lindenlab.com>:
>> My objection to that is that apparently, with the present design, if I want
>> to write a function that accepts any specialization of basic_stacktrace,
>> that must itself be a template function.
>
> Yes, you're right. But in my head - you'll be using the same default
> stacktrace with the same template parameter all over the place. So
> it's mostly for detecting problems (you'll get link time error while
> attempting to link libraries that exchange stacktraces with different
> default parameters) and for interoperability with libraries, that may
> accept/produce stacktraces that differ from the current.

That makes me nervous, as I imagine that library code will want to
transport stack traces around (imagine exception_ptr with stack traces,
or future/thread/context/coroutine/fiber integration).

But the max required depth can't be known to a library as it depends on
the application code that uses the library (and indeed can get quite
deep with some of those constructs, eg. when making a future ready fires
a continuation that synchronously makes another future ready, and so
on). Application-configurable macros could solve some of that but it
still feels problematic to me.

Andrey Semashev

unread,

Dec 15, 2016, 6:11:34 PM12/15/16

to bo...@lists.boost.org

The problem is not the pointer per se. The problem is that your
proposal implies a dependency on the code that fills that pointer, the
Stacktrace library. Robert expands on the dependency injection and I'm
worried by performance degradation caused by this change.

I must say I'm not really buying the "automatic stacktrace from
exception" advantage. Really, the source file and line, combined with
sufficient logging, were quite enough for me for years; if there were
cases I wished to have a backtrace, they were rare enough I don't
remember one of them.

So please, just define a separate macro with this new feature and
leave BOOST_THROW_EXCEPTION as is.

The config macro to disable the stacktrace doesn't satisfy me. First,
because it is a way to opt out whereas I believe such a feature should
be an opt in as it is expensive. Second, because it doesn't work with
compiled binaries (e.g. distributed in package distros and Boost
installers). Third, I suspect this feature could be useful in a subset
of source code, e.g. a particular set of function calls or a scope,
while not so useful in other places of the application. For instance,
one might want to temporarily enable it to debug a particular place of
code without affecting the rest of the program. For that the feature
should be configurable in runtime.

What I'm thinking is that there could be a hook that would allow a
user-defined function to be called on the exception that is about to
be thrown. By default no hook is installed, so throwing an exception
works exactly as BOOST_THROW_EXCEPTION currently works.
Boost.Stacktrace could provide its hook, which the user could install
to enable automatic stacktraces. The important part is that this is
user's choice and that it decouples throwing the exception from
Boost.Stacktrace.

Emil Dotchevski

unread,

Dec 15, 2016, 6:34:11 PM12/15/16

to bo...@lists.boost.org

On Thu, Dec 15, 2016 at 1:24 PM, Robert Ramey <ra...@rrsd.com> wrote:

> It added a ton of included code to all my header files. I think the
> number was 5000 lines. I'm sure it did something - otherwise I wouldn't
> have noticed it.
>

False, it's 522 lines:
https://github.com/boostorg/throw_exception/blob/develop/include/boost/exception/exception.hpp

.

> There was no "coupling" to other libraries but to a single header with zero
>> dependencies, which is now included in the boost::throw_exception
>> submodule.
>>
>
> that's what coupling is.
>

Yes, boost/throw_exception.hpp is coupled with
boost/exception/exception.hpp (would you be happier if I copy the contents
of exception.hpp and paste it in throw_exception.hpp?)

No, the throw_exception module is not coupled with the exception module,
because boost/exception/exception.hpp resides in the throw_exception module
and not in the Boost Exception module:

throw_exception: https://github.com/boostorg/throw_exception.

Boost Exception: https://github.com/boostorg/exception

> Specifically, if you want to use boost::throw_exception, the
>
>> Boost Exception submodule is NOT necessary.
>>
>
> well, I was using boost::throw_exception and I was got a whole bunch of
> new stuff I didn't ask for.

Let's be specific: you got a base class for your exception types, for free,
which admittedly you didn't ask for.

> It seems that it's in there to support other users who do use boost
> exception. Which raises the question of why that is my problem. After not
> getting any traction I did post a patch which would permit support boost
> exception without having to make any changes in boost::throw_exception.

I'm extremely interested in this, I do want to implement Boost Exception
non-intrusively if possible. Can I see code?

> OK - I looked at boost/exception/exception.hpp and it only adds 500 lines
> of header code to every file which invokes boost::throw_exception. A big
> improvement over the original.
>

The original was ~400 lines.

> But my point is really that adding something like this in jamming into my
> library makes me responsible for something that I have to invest
> significant time to understand.

Your choice. Back then you overreacted, made changes to Serialization to
not call throw_exception, which broke BOOST_NO_EXCEPTIONS builds for users
of Serialization (for which you blamed me). And then, yes, to fix that mess
you had to invest time to understand how throw_exception works.

> As a practical matter, it's not really an issue any more.
>

If you don't want to talk about this issue any more, stop making false
claims.

Emil

Gavin Lambert

unread,

Dec 15, 2016, 6:37:16 PM12/15/16

to bo...@lists.boost.org

On 15/12/2016 20:46, Niall Douglas wrote:
>> Also, I'm not sure about other compilers, but with MSVC thrown
>> exceptions always carry a stack trace with them anyway; it's possible to
>> extract the stack trace from any exception with some platform-dependent
>> code (without any decoration on the throw -- and depending on build
>> options, it can even work for null references and other CPU exceptions,
>> although some misguided people dislike that).
>
> I was not aware that this is the case except when /EHa is used.
> Indeed, I had thought one of the big optimisations of /EHs was
> precisely the fact that backtraces are *not* captured, thus making
> C++ exception throws vastly faster than Win32 structured exception
> throws and the point of using /EHs.

I don't think so. The docs simply indicate that it reduces the
locations where it captures unwind info and does some checks. This does
increase performance by reducing code size (and thus the working set)
and removing some conditional logic, but it comes at the cost of failing
to execute some destructors if an exception does get thrown/raised.
(Which is probably ok if the process is going to be terminated by an
uncaught exception anyway, but less so if not.) I didn't see anything
about changing the actual implementation of exceptions.

The following code *does* print "caught exception" (in both Debug and
Release) even when compiled with /EHsc. Granted I suppose this doesn't
explicitly prove that a stack trace was captured, but I think it proves
that C++ exceptions are still internally implemented as SEH exceptions,
and so GetExceptionInformation (among other things) can probably still
be used to extract the stack context.

void badfn()
{
throw std::runtime_error("error");
}

template<typename F>
void seh_wrapper(F fn)
{
__try
{
fn();
}
__except (EXCEPTION_EXECUTE_HANDLER)
{
std::cout << "caught exception" << std::endl;
}
}

int main(int argc, char* argv[])
{
seh_wrapper(&badfn);
std::cout << "completed" << std::endl;
return 0;

Gavin Lambert

unread,

Dec 15, 2016, 6:45:42 PM12/15/16

to bo...@lists.boost.org

On 16/12/2016 12:11, Andrey Semashev wrote:
> The config macro to disable the stacktrace doesn't satisfy me. First,
> because it is a way to opt out whereas I believe such a feature should
> be an opt in as it is expensive. Second, because it doesn't work with
> compiled binaries (e.g. distributed in package distros and Boost
> installers). Third, I suspect this feature could be useful in a subset
> of source code, e.g. a particular set of function calls or a scope,
> while not so useful in other places of the application. For instance,
> one might want to temporarily enable it to debug a particular place of
> code without affecting the rest of the program. For that the feature
> should be configurable in runtime.

I'm curious whether your answer would change if your first two
objections were invalid, ie. if it were not expensive and if it could be
used for optimised and stripped binaries.

(Regarding that latter point, that's what both Nat and I [in the
pre-review] were talking about as a feature request, that the raw trace
could be serialised and decoded later on a separate machine with access
to the corresponding symbol files. This practice is not uncommon in
Windows with minidumps and pdb files.)

Emil Dotchevski

unread,

Dec 15, 2016, 7:01:34 PM12/15/16

to bo...@lists.boost.org

On Thu, Dec 15, 2016 at 3:11 PM, Andrey Semashev <andrey....@gmail.com>

wrote:

> On Thu, Dec 15, 2016 at 11:21 PM, Emil Dotchevski
> <emildot...@gmail.com> wrote:
> > On Thu, Dec 15, 2016 at 11:26 AM, Robert Ramey <ra...@rrsd.com> wrote:
> >
> > The boost::exception type (which, again, does not depend on anything,
> > boost/exception/exception.hpp does not include any headers) currently
> holds
> > an int and a char const * for BOOST_THROW_EXCEPTION to store __LINE__ and
> > __FILE__. The cost of adding the ability of boost::exception to carry a
> > stack trace would be one more pointer (note: for this purpose it's
> possible
> > to use Boost Exception's ability to transport arbitrary data in any
> > exception, but that would make boost::throw_exception dependent on Boost
> > Exception, which I've worked very hard to avoid so far.) That pointer can
> > remain unused if the user chooses to turn off the automatic capture of
> the
> > stack trace.
>
> The problem is not the pointer per se. The problem is that your
> proposal implies a dependency on the code that fills that pointer, the
> Stacktrace library.

I am not proposing a dependency of throw_exception on Starktrace, but
removing a minimal amount of code from Stacktrace (e.g. something that
packs everything into a memory buffer) and putting it into throw_exception.
I do think that logically, this code should be left in a separate header,
but it has to be written without any Boost dependencies. If that is not
possible, I agree that the integration is a bad idea.

> Robert expands on the dependency injection and I'm
> worried by performance degradation caused by this change.
>

I admit I worry about coupling more than I do about the performance. I am
speculating, but I suspect that in my own uses emitting exceptions has to
be something like 1000x slower than it is to maybe show in my profiling.
Perhaps I'm wrong.

I must say I'm not really buying the "automatic stacktrace from
> exception" advantage. Really, the source file and line, combined with
> sufficient logging, were quite enough for me for years; if there were
> cases I wished to have a backtrace, they were rare enough I don't
> remember one of them.
>
> So please, just define a separate macro with this new feature and
> leave BOOST_THROW_EXCEPTION as is.
>

First, let's again clarify: there is no reason to change the
BOOST_THROW_EXCEPTION macro. The stack trace can be captured by the
boost::throw_exception function template.

The problem is that users who want take advantage of the stack trace (I
understand you are not one of them) have no control over the library level
code that uses boost::throw_exception.

That said, if you agree that ultimately it is the user who should be able
to decide whether or not "good" Boost libraries embed stack trace in
exceptions they emit, the argument wouldn't be whether or not capturing the
stack trace is integrated into boost::throw_exception, but whether or not
it is enabled by default. Either way, the boost::exception class (defined
in boost/exception/exception.hpp) should provide a hook for that
integration in order to avoid coupling between boost::throw_exception and
Stacktrace.

> The config macro to disable the stacktrace doesn't satisfy me. First,
> because it is a way to opt out whereas I believe such a feature should
> be an opt in as it is expensive.

I think that how expensive it is needs to be investigated and discussed.
Either way it is a compromise.

> What I'm thinking is that there could be a hook that would allow a
> user-defined function to be called on the exception that is about to
> be thrown. By default no hook is installed, so throwing an exception
> works exactly as BOOST_THROW_EXCEPTION currently works.
> Boost.Stacktrace could provide its hook, which the user could install
> to enable automatic stacktraces. The important part is that this is
> user's choice and that it decouples throwing the exception from
> Boost.Stacktrace.
>

Do you mean a run-time hook or a compile-time hook?

I think this should be compile-time hook in order to avoid the perils of
dynamic initialization in complex multi-thread programs. But we already
have that compile-time hook, it's called boost::throw_exception.

Emil

Emil Dotchevski

unread,

BOOST_THROW_EXCEPTION calls boost::throw_exception, so by changing the
latter you also change the former.

> The problem is that users who want take advantage of the stack trace (I
> understand you are not one of them) have no control over the library level
> code that uses boost::throw_exception.

True.

> That said, if you agree that ultimately it is the user who should be able
> to decide whether or not "good" Boost libraries embed stack trace in
> exceptions they emit,

Yes, it should be primarilly the user's decision. Although I can see
library developers also wishing to avoid this feature, at least
selectively. That is why I propose a new macro for this.

> the argument wouldn't be whether or not capturing the
> stack trace is integrated into boost::throw_exception, but whether or not
> it is enabled by default. Either way, the boost::exception class (defined
> in boost/exception/exception.hpp) should provide a hook for that
> integration in order to avoid coupling between boost::throw_exception and
> Stacktrace.

The way I see it, boost::exception already provides everything needed,
which is the ability to attach arbitrary data to the exception. I don't
think adding more special case capability to it is a wise solution in
the long term.

>> The config macro to disable the stacktrace doesn't satisfy me. First,
>> because it is a way to opt out whereas I believe such a feature should
>> be an opt in as it is expensive.
>
> I think that how expensive it is needs to be investigated and discussed.
> Either way it is a compromise.

Yes, I think some performance numbers should be presented before making
the change to throw_exception.

Also, if we go with the run-time hook approach (with no hook by
default), the performance issue won't be so pressing.

>> What I'm thinking is that there could be a hook that would allow a
>> user-defined function to be called on the exception that is about to
>> be thrown. By default no hook is installed, so throwing an exception
>> works exactly as BOOST_THROW_EXCEPTION currently works.
>> Boost.Stacktrace could provide its hook, which the user could install
>> to enable automatic stacktraces. The important part is that this is
>> user's choice and that it decouples throwing the exception from
>> Boost.Stacktrace.
>
> Do you mean a run-time hook or a compile-time hook?

A run-time hook. I described why a compile-time hook doesn't work for me
(the config macro is equivalent to a compile-time hook with regard to
the points I made previously).

> I think this should be compile-time hook in order to avoid the perils of
> dynamic initialization in complex multi-thread programs. But we already
> have that compile-time hook, it's called boost::throw_exception.

The run-time hook can be tricky to implement, that is true, but in the
simplest form it should be doable. The implementation details are of
course discussable, but below is something more concrete to get things
started.

Introduce a process-wide singleton pointer, which is a pointer to the
function that will augment the exception about to be thrown:

typedef void (*exception_augmenter)(boost::exception&);

The augmenter will be able to use Boost.Exception API to add any data to
the exception.

The pointer can be updated atomically. If it deems necessary, a
thread-local and module-local copy can be maintained. The default value
of null means no augmenter and throwing the exception works as
BOOST_THROW_EXCEPTION currently does.

The tricky part is to maintain the singleton. On Windows one can use a
named semaphore to store the pointer as the semaphore state (see
https://github.com/boostorg/sync/blob/develop/include/boost/sync/detail/waitable_timer.hpp#L121
for an example, Boost.Interprocess also has something similar). On POSIX
systems it might be enough to just mark the pointer with default
visibility. A solution with shared memory (shmget/shmat/etc.) also seems
possible.

The user will be able to install his (or Boost.Stacktrace's) augmenter
through the API provided by throw_exception submodule and thus enable
stacktraces in exceptions.

Andrey Semashev

unread,

Dec 16, 2016, 4:38:03 AM12/16/16

to bo...@lists.boost.org

On 12/16/16 02:45, Gavin Lambert wrote:
> On 16/12/2016 12:11, Andrey Semashev wrote:
>> The config macro to disable the stacktrace doesn't satisfy me. First,
>> because it is a way to opt out whereas I believe such a feature should
>> be an opt in as it is expensive. Second, because it doesn't work with
>> compiled binaries (e.g. distributed in package distros and Boost
>> installers). Third, I suspect this feature could be useful in a subset
>> of source code, e.g. a particular set of function calls or a scope,
>> while not so useful in other places of the application. For instance,
>> one might want to temporarily enable it to debug a particular place of
>> code without affecting the rest of the program. For that the feature
>> should be configurable in runtime.
>
> I'm curious whether your answer would change if your first two
> objections were invalid, ie. if it were not expensive and if it could be
> used for optimised and stripped binaries.

If obtaining the backtrace was cheap and of constant complexity, that
would certainly be less of a problem. The source file location is a
great example of that: it is cheap to obtain and transport in the
exception, yet it provides one with a lot of information. But with
stacktraces somehow I don't believe this is the case (wrt. the "cheap"
part anyway).

Regarding the optimized and stripped binaries, I was actually talking
about a different thing, I think. For instance, if BOOST_THROW_EXCEPTION
attaches a stacktrace by default then Boost.Log library binaries
distributed in my favorite Linux packages also do that for all
exceptions the library throws. There is no way the user can disable that
without recompiling the library.

But you touched another interesing subject: does the library have the
capability of loading debug symbols to interpret the backtrace? Because
if it doesn't then the attached stacktraces will be nearly useless for
stripped binaries (which they all are when distributed in packages).

> (Regarding that latter point, that's what both Nat and I [in the
> pre-review] were talking about as a feature request, that the raw trace
> could be serialised and decoded later on a separate machine with access
> to the corresponding symbol files. This practice is not uncommon in
> Windows with minidumps and pdb files.)

I think, with load address randomization, the stacktrace alone is not
enough for that. You'd need a core dump of the process to interpret the
backtrace.

Robert Ramey

unread,

Dec 16, 2016, 11:47:08 AM12/16/16

to bo...@lists.boost.org

On 12/16/16 1:20 AM, Andrey Semashev wrote:

>> I think this should be compile-time hook in order to avoid the perils of
>> dynamic initialization in complex multi-thread programs. But we already
>> have that compile-time hook, it's called boost::throw_exception.
>
> The run-time hook can be tricky to implement, that is true, but in the
> simplest form it should be doable. The implementation details are of
> course discussable, but below is something more concrete to get things
> started.
>
> Introduce a process-wide singleton pointer,

<snip>

This illustrates what the issue is and where we need to be moving.

For many C++ programs and libraries, the exception is a convenient way
to handle errors and for this reason is widely used. Traditionally,
library authors have documented exceptions that they threw and let user
programs decide to catch and handle them. To deal with platforms which
didn't provide exceptions we threw through boost::throw_exception. So
far so good. But as we make bigger and more complex programs using
mulitple libraries from different sources, this becomes somewhat of a
problem to keep track of. And users would like to have systematic way
of "unifying" them. So I'm thinking the real solution is to develop
customs and idioms for usage by library authors which would permit users
to specify the way they would like to see exceptions handled.

For example, the boost serialization library uses the traditional method
of defining exceptions and calling boost::throw_exception. But this
doesn't let users control behavior. My more recent effort -
safe_numerics, (in the boost library incubator) specifies a template
parameter which refers to an error policy and includes several options -
invoke exception, invoke specified function, etc... Something along
these lines is what I'm now thinking is where we should be headed. When
I did it for safe_numerics, I wasn't thinking of any general solution or
approach to usage of exceptions inside of library code, it's just came
naturally from the fact that pretty much the whole safe_numerics is to
deal with errors with a library.

To summarize, what is need is to come up with a "best practices"
recommendation to library authors as to how to deal with exceptional
conditions detected from withing a library.

Were this to occur and be convincing to library authors, the whole
question of what boost::throw_exception should do wouldn't have to be
agreed upon as it would be in the hands of the library user to decide.

Robert Ramey

Paul A. Bristow

unread,

Dec 16, 2016, 1:21:23 PM12/16/16

to bo...@lists.boost.org

John Maddock gave serious thought to this when devising the Policies concept for Boost.Math (after some mutterings from me).

This allows people to be all C-ish and ignore just bad input thru to throwing C++ exceptions, and control other things like accuracy
and precision too.

It seems to work well (except that many people need reminding that if you don't catch exceptions, you don't get any info about when
went wrong - it just ends with a muted phut :-).

Downside is that it makes the code harder to write and much more confusing to read.

This too might be a model for other libraries.

Paul

---
Paul A. Bristow
Prizet Farmhouse
Kendal UK LA8 8AB
+44 (0) 1539 561830

Niall Douglas

unread,

Dec 16, 2016, 2:05:42 PM12/16/16

to bo...@lists.boost.org

On 16 Dec 2016 at 12:37, Andrey Semashev wrote:

> But you touched another interesing subject: does the library have the
> capability of loading debug symbols to interpret the backtrace? Because
> if it doesn't then the attached stacktraces will be nearly useless for
> stripped binaries (which they all are when distributed in packages).

Are you saying this information is not in Stacktrace's documentation?
Or it is insufficiently clear?

> > (Regarding that latter point, that's what both Nat and I [in the
> > pre-review] were talking about as a feature request, that the raw trace
> > could be serialised and decoded later on a separate machine with access
> > to the corresponding symbol files. This practice is not uncommon in
> > Windows with minidumps and pdb files.)
>
> I think, with load address randomization, the stacktrace alone is not
> enough for that. You'd need a core dump of the process to interpret the
> backtrace.

No, you just need the offset into each module. I did suggest to
Antony in my pre-manager review that he needed a serialisation
mechanism which dumps out module paths plus offsets in a format
consumable by tooling such as addr2line.

Niall

--
ned Productions Limited Consulting
http://www.nedproductions.biz/
http://ie.linkedin.com/in/nialldouglas/

Niall Douglas

unread,

Dec 16, 2016, 2:05:59 PM12/16/16

to bo...@lists.boost.org

On 16 Dec 2016 at 12:36, Gavin Lambert wrote:

> On 15/12/2016 20:46, Niall Douglas wrote:
> >> Also, I'm not sure about other compilers, but with MSVC thrown
> >> exceptions always carry a stack trace with them anyway; it's possible to
> >> extract the stack trace from any exception with some platform-dependent
> >> code (without any decoration on the throw -- and depending on build
> >> options, it can even work for null references and other CPU exceptions,
> >> although some misguided people dislike that).
> >
> > I was not aware that this is the case except when /EHa is used.
> > Indeed, I had thought one of the big optimisations of /EHs was
> > precisely the fact that backtraces are *not* captured, thus making
> > C++ exception throws vastly faster than Win32 structured exception
> > throws and the point of using /EHs.

BTW I did some research and SEH exceptions do NOT capture stack
backtraces. They do dump a CONTEXT structure, and from that you can
walk a stack backtrace. This was the only legal way of getting a
CONTEXT for the calling thread until Vista.

> The following code *does* print "caught exception" (in both Debug and
> Release) even when compiled with /EHsc. Granted I suppose this doesn't
> explicitly prove that a stack trace was captured, but I think it proves
> that C++ exceptions are still internally implemented as SEH exceptions,
> and so GetExceptionInformation (among other things) can probably still
> be used to extract the stack context.

We have to be careful here. There is the SEH unwind mechanism and
there is SEH itself. Absolutely everything unwinding a stack uses the
former for any language, even Visual Basic, and hence C++ too. The
latter is merely a client of the universal unwind mechanism, same as
C++ exceptions is also implemented as a client of that mechanism.
That's why throwing a C++ exception will get caught by a SEH
exception handler, though without knowledge of the internal SEH codes
MSVCRT uses you couldn't do much with that. Point is, it's universal
and interoperable for all programming languages which can invert
control flow (LLVM and GCC have a similar universal unwind
framework).

I also looked up some benchmarks. A C++ and SEH exception throw and
catch costs about 2 microseconds on x86 on MSVC back in 2011 on a
fairly low end CPU for the time. x86 SEH still uses the old
setjmp/longjmp mechanism i.e. each stack frame pushes its unwind
handler and pops it off the stack after i.e. it is not a "zero
runtime cost" exceptions implementation.

For x64, SEH unwind can now use table based unwinds i.e. zero runtime
overhead exception, which SEH exposes as "vectored exception handling
APIs". I didn't find any benchmarks online for what a throw and catch
is on MSVC under x64, but it is likely to still be under 2
microseconds. I'm just about to go back onto minding newborn duty so
I can't write the benchmark myself, but if anyone could quickly throw
together a benchmark comparing C++ to SEH exception throw and catch
on VS2015 it would be appreciated (just make sure the optimiser
hasn't elided the code entirely).

Just to remind everyone, a seven deep stack backtrace costs about 30
microseconds using the very optimised NT kernel backtrace routine. It
is therefore a significant order of magnitude overhead addition to
the cost of throwing and catching exceptions. Personally speaking, I
would not make backtracing on exception throw default on therefore.

Niall

--
ned Productions Limited Consulting
http://www.nedproductions.biz/
http://ie.linkedin.com/in/nialldouglas/

Niall Douglas

unread,

Dec 16, 2016, 2:06:21 PM12/16/16

to bo...@lists.boost.org

On 15 Dec 2016 at 16:00, Emil Dotchevski wrote:

> Do you mean a run-time hook or a compile-time hook?
>
> I think this should be compile-time hook in order to avoid the perils of
> dynamic initialization in complex multi-thread programs. But we already
> have that compile-time hook, it's called boost::throw_exception.

Firstly thanks to all for the detailed discussion. Bringing it back
to the review of Stacktrace which this is (and not of Exception,
another discussion thread would suit that), I am seeing the following
options proposed for Stacktrace:

Option 1: That Stacktrace implement a macro like
BOOST_STACKTRACE_THROW_EXCEPTION() which reimplements
BOOST_THROW_EXCEPTION() but with muxed in stacktrace.

Option 2: That Stacktrace optionally hook some compile time hook in
Boost.Exception to mux in stacktrace.

Option 3: That Stacktrace optionally hook some runtime hook in
Boost.Exception to mux in stacktrace.

Implied in all the above is that the exception type stacktrace mux in
framework as described in the current Stacktrace docs is considered
bad form and should be deleted?

Can people interested vote on one of the options above, or propose
their own option, and say whether they want the documented mux in
mechanism removed or not?

Thanks,

Niall

--
ned Productions Limited Consulting
http://www.nedproductions.biz/
http://ie.linkedin.com/in/nialldouglas/

Niall Douglas

unread,

Dec 16, 2016, 2:06:40 PM12/16/16

to bo...@lists.boost.org

On 16 Dec 2016 at 8:46, Robert Ramey wrote:

> This illustrates what the issue is and where we need to be moving.
>
> For many C++ programs and libraries, the exception is a convenient way
> to handle errors and for this reason is widely used. Traditionally,
> library authors have documented exceptions that they threw and let user
> programs decide to catch and handle them. To deal with platforms which
> didn't provide exceptions we threw through boost::throw_exception. So
> far so good. But as we make bigger and more complex programs using
> mulitple libraries from different sources, this becomes somewhat of a
> problem to keep track of. And users would like to have systematic way
> of "unifying" them. So I'm thinking the real solution is to develop
> customs and idioms for usage by library authors which would permit users
> to specify the way they would like to see exceptions handled.

Isn't that the whole point of proposed Boost.Outcome? I appreciate
the explanation was a very long and tedious document as posted here
some weeks ago, but once my paternity leave ends I'll be back onto
investing an hour per morning until the docs look like the feedback
from here requested.

(and just for reference, Outcome provides an optional method of
hooking when an errored state is created. One hook provided generates
a stack backtrace embedded with the errored state)

Niall

--
ned Productions Limited Consulting
http://www.nedproductions.biz/
http://ie.linkedin.com/in/nialldouglas/

Robert Ramey

unread,

Dec 16, 2016, 2:17:08 PM12/16/16

to bo...@lists.boost.org

On 12/16/16 11:05 AM, Niall Douglas wrote:
> Option 1: That Stacktrace implement a macro like
> BOOST_STACKTRACE_THROW_EXCEPTION() which reimplements
> BOOST_THROW_EXCEPTION() but with muxed in stacktrace.
>
> Option 2: That Stacktrace optionally hook some compile time hook in
> Boost.Exception to mux in stacktrace.
>
> Option 3: That Stacktrace optionally hook some runtime hook in
> Boost.Exception to mux in stacktrace.

Option 3:

My suggestion was to ignore the whole issue and assign it some other
library/component. A couple libraries use the idiom similar to

template MyTemplate<class T some_type, class E error_handler =
boost::throw_exception>

This decouples the error handling from the functionality of the library
itself and permits the user to unificate his error handling across
libraries. It also avoids the necessity of coordinating error handling
across libraries in time and space. In this particular case stacktrace
would provide the mechanism to invoke/serialization or whatever a stack
trace and it would be job of the library/application which uses it to
decide when to invoke it.

Robert Ramey

Andrey Semashev

unread,

Dec 16, 2016, 2:30:27 PM12/16/16

to bo...@lists.boost.org

On 12/16/16 22:05, Niall Douglas wrote:
> On 15 Dec 2016 at 16:00, Emil Dotchevski wrote:
>
>> Do you mean a run-time hook or a compile-time hook?
>>
>> I think this should be compile-time hook in order to avoid the perils of
>> dynamic initialization in complex multi-thread programs. But we already
>> have that compile-time hook, it's called boost::throw_exception.
>
> Firstly thanks to all for the detailed discussion. Bringing it back
> to the review of Stacktrace which this is (and not of Exception,
> another discussion thread would suit that), I am seeing the following
> options proposed for Stacktrace:
>
> Option 1: That Stacktrace implement a macro like
> BOOST_STACKTRACE_THROW_EXCEPTION() which reimplements
> BOOST_THROW_EXCEPTION() but with muxed in stacktrace.
>
> Option 2: That Stacktrace optionally hook some compile time hook in
> Boost.Exception to mux in stacktrace.
>
> Option 3: That Stacktrace optionally hook some runtime hook in
> Boost.Exception to mux in stacktrace.

Option 3 or 3+1 (i.e. the runtime hook is used by a new macro) look the
most preferable to me.

> Implied in all the above is that the exception type stacktrace mux in
> framework as described in the current Stacktrace docs is considered
> bad form and should be deleted?

Frankly, I haven't yet reviewed Stacktrace, although now that I entered
the discussion I probably should. I'll try to do that on the weekend.

For now it seems unnecessary to have two mechanisms for injecting
stacktraces. Given that we already have Boost.Exception, it seems
preferable to use its facilities.

Andrey Semashev

unread,

Dec 16, 2016, 2:31:13 PM12/16/16

to bo...@lists.boost.org

On 12/16/16 22:05, Niall Douglas wrote:

> On 16 Dec 2016 at 12:37, Andrey Semashev wrote:
>
>> But you touched another interesing subject: does the library have the
>> capability of loading debug symbols to interpret the backtrace? Because
>> if it doesn't then the attached stacktraces will be nearly useless for
>> stripped binaries (which they all are when distributed in packages).
>
> Are you saying this information is not in Stacktrace's documentation?
> Or it is insufficiently clear?

I have not read the docs yet.

Emil Dotchevski

unread,

Dec 16, 2016, 2:33:18 PM12/16/16

to bo...@lists.boost.org

On Fri, Dec 16, 2016 at 1:20 AM, Andrey Semashev <andrey....@gmail.com>
wrote:

> On 12/16/16 03:00, Emil Dotchevski wrote:
>
>>
>> I am not proposing a dependency of throw_exception on Starktrace, but
>> removing a minimal amount of code from Stacktrace (e.g. something that
>> packs everything into a memory buffer) and putting it into
>> throw_exception.
>> I do think that logically, this code should be left in a separate header,
>> but it has to be written without any Boost dependencies. If that is not
>> possible, I agree that the integration is a bad idea.
>>
>
> I'm not sure what else Boost.Stacktrace does; it looks like it means
> moving the bulk of Stacktrace into the throw_exception submodule. I don't
> think that is a reasonable solution in terms of code management.
>

Maybe not. What I'm saying is that 1) automatic capture of stack trace is
very useful and 2) any possible integration into boost::throw_exception
requires careful consideration. Let's see the actual code before we say it
is not acceptable.

> Also, I'm not sure what APIs Stacktrace uses behind the scene, does it
> depend on additional third party libraries to obtain the backtrace?
>

I am not sure either, I'm assuming it needs only depend on system
libraries. Obviously if that's not the case the integration into
throw_exception is a no-go.

> the argument wouldn't be whether or not capturing the
>> stack trace is integrated into boost::throw_exception, but whether or not
>> it is enabled by default. Either way, the boost::exception class (defined
>> in boost/exception/exception.hpp) should provide a hook for that
>> integration in order to avoid coupling between boost::throw_exception and
>> Stacktrace.
>>
>
> The way I see it, boost::exception already provides everything needed,
> which is the ability to attach arbitrary data to the exception. I don't
> think adding more special case capability to it is a wise solution in the
> long term.
>

This can be said about __FILE__ and __LINE__ which BOOST_THROW_EXCEPTION
captures automatically. The reason why it's done is that 1) the cost in
terms of time and space is considered negligible and 2) it is potentially
critical for debugging.

The reason why it Stacktrace would need special handling rather than go
through the usual Boost Exception machinery is also the same as why there
is special handling (members in boost::exception): to avoid coupling with
the rest of the Boost Exception machinery, which is considerably heavier
and bigger than boost/exception/exception.hpp.

> The run-time hook can be tricky to implement, that is true, but in the
>>> simplest form it should be doable. The implementation details are of course
>>> discussable, but below is something more concrete to get things started.
>>>
>>> Introduce a process-wide singleton pointer, which is a pointer to the
>>> function that will augment the exception about to be thrown:
>>>
>>> typedef void (*exception_augmenter)(boost::exception&);
>>>
>>

Consider dynamically loaded libraries. Also consider multiple libraries
wanting different hooks. A process-wide pointer is very difficult to do in
the general case, and I suspect it'd drag a lot more dependencies than
capturing the stacktrace would. Consider that boost::throw_exception must
support very very old compilers.

Besides, throwing exceptions and capturing stack trace are thread-local
operations, while global pointer would require synchronization with the
initialization code.

> The augmenter will be able to use Boost.Exception API to add any data to
>>> the exception.
>>>
>>> The pointer can be updated atomically. If it deems necessary, a
>>> thread-local and module-local copy can be maintained. The default value of
>>> null means no augmenter and throwing the exception works as
>>> BOOST_THROW_EXCEPTION currently does.
>>>
>>> The tricky part is to maintain the singleton. On Windows one can use a
>>> named semaphore to store the pointer as the semaphore state (see
>>> https://github.com/boostorg/sync/blob/develop/include/boost/
>>> sync/detail/waitable_timer.hpp#L121 for an example, Boost.Interprocess
>>> also has something similar). On POSIX systems it might be enough to just
>>> mark the pointer with default visibility. A solution with shared memory
>>> (shmget/shmat/etc.) also seems possible.
>>>
>>

Are you proposing to include <windows.h> in boost/throw_exception.hpp?
That's a no-go. Making it not header-only module? Also a no-go.

Emil

Andrey Semashev

unread,

Dec 16, 2016, 3:25:09 PM12/16/16

to bo...@lists.boost.org

On 12/16/16 22:32, Emil Dotchevski wrote:
> On Fri, Dec 16, 2016 at 1:20 AM, Andrey Semashev <andrey....@gmail.com>
> wrote:
>
>> the argument wouldn't be whether or not capturing the
>>> stack trace is integrated into boost::throw_exception, but whether or not
>>> it is enabled by default. Either way, the boost::exception class (defined
>>> in boost/exception/exception.hpp) should provide a hook for that
>>> integration in order to avoid coupling between boost::throw_exception and
>>> Stacktrace.
>>
>> The way I see it, boost::exception already provides everything needed,
>> which is the ability to attach arbitrary data to the exception. I don't
>> think adding more special case capability to it is a wise solution in the
>> long term.
>
> This can be said about __FILE__ and __LINE__ which BOOST_THROW_EXCEPTION
> captures automatically. The reason why it's done is that 1) the cost in
> terms of time and space is considered negligible and 2) it is potentially
> critical for debugging.
>
> The reason why it Stacktrace would need special handling rather than go
> through the usual Boost Exception machinery is also the same as why there
> is special handling (members in boost::exception): to avoid coupling with
> the rest of the Boost Exception machinery, which is considerably heavier
> and bigger than boost/exception/exception.hpp.

The backtrace itself is not a lightweght object, so the overhead of the
generic Boost.Exception machinery is probably not significant. And if
that overhead is significant then maybe it could be optimized instead.
After all, why have it if we don't want to use it.

Also, note that the source location is always injected into the
exception (well, as long as you use BOOST_THROW_EXCEPTION or a similar
macro), while the backtrace is optional.

>> The run-time hook can be tricky to implement, that is true, but in the
>>>> simplest form it should be doable. The implementation details are of course
>>>> discussable, but below is something more concrete to get things started.
>>>>
>>>> Introduce a process-wide singleton pointer, which is a pointer to the
>>>> function that will augment the exception about to be thrown:
>>>>
>>>> typedef void (*exception_augmenter)(boost::exception&);
>>>>
>>>
> Consider dynamically loaded libraries.

That is a separate can of worms, which can ruin your day even without
the global pointer. The global hook doesn't make it worse per se.

> Also consider multiple libraries wanting different hooks.

That would be problematic to support - both with run-time and
compile-time hooks (the latter is difficult because of ODR violations).
I'm not sure this level of flexibility would be useful either.

> A process-wide pointer is very difficult to do in
> the general case, and I suspect it'd drag a lot more dependencies than
> capturing the stacktrace would.

The most complicated part is Windows-specific implementation, and it
only requires a few functions from WinAPI. Marking the pointer visible
on POSIX is trivial.

> Consider that boost::throw_exception must
> support very very old compilers.

How old? I mean, it only has to support the oldest compiler Boost
libraries support.

> Besides, throwing exceptions and capturing stack trace are thread-local
> operations, while global pointer would require synchronization with the
> initialization code.

That's a valid concern, but I think it can be mitigated by caching the
pointer.

We could also accept the limitation that the augmentor has to be
installed once at the beginning of the program and simply not support
its multithreaded modification.

> Are you proposing to include <windows.h> in boost/throw_exception.hpp?
> That's a no-go.

No. Use approach similar to Boost.WinAPI on Windows (i.e. re-declare
WinAPI components).

> Making it not header-only module? Also a no-go.

No.

Emil Dotchevski

unread,

Dec 16, 2016, 3:41:42 PM12/16/16

to bo...@lists.boost.org

On Wed, Dec 14, 2016 at 11:46 PM, Niall Douglas <s_sour...@nedprod.com>
wrote:

> On 15 Dec 2016 at 15:01, Gavin Lambert wrote:
>
> > > I've not taken a look at the proposed Stacktrace library, but from what
> > > I know obtaining a stacktrace is typically a rather expensive
> operation.
> > > I expect it to be no less expensive than throwing the exception. That's
> > > probably too much to pay for by default. Plus, it adds one more
> > > dependency on another library via such a core component as
> > > BOOST_THROW_EXCEPTION.
>
> For a capture depth of seven frames, it costs about 30 microseconds
> on Microsoft Windows. Stack backtracing is inherently cache
> unfriendly. That is a lot more than throwing an exception.
>
> The people on SG14 have tried quite hard to demonstrate that throwing
> exceptions is slower than alternatives, but it has proven
> surprisingly hard to conclusively prove it in real world code. I am
> sure the compiler vendors would say "I told you so ..."
>

+1.

> > Capturing the stack to later perform a trace is relatively cheap and
> > fast (though perhaps memory wasteful, though you usually wouldn't care
> > about that unless the exception was due to running out of memory). It's
> > interpreting that data into a human-readable trace that's the slow part.
>
> Multiple orders of magnitude difference yes.
>

+1

With regards to integration into boost::throw_exception, obviously we only
need to capture the data not format it.

Emil

Emil Dotchevski

unread,

Dec 16, 2016, 3:49:47 PM12/16/16

to bo...@lists.boost.org

On Fri, Dec 16, 2016 at 8:46 AM, Robert Ramey <ra...@rrsd.com> wrote:

> For many C++ programs and libraries, the exception is a convenient way to
> handle errors and for this reason is widely used. Traditionally, library
> authors have documented exceptions that they threw and let user programs
> decide to catch and handle them. To deal with platforms which didn't
> provide exceptions we threw through boost::throw_exception. So far so
> good. But as we make bigger and more complex programs using mulitple
> libraries from different sources, this becomes somewhat of a problem to
> keep track of. And users would like to have systematic way of "unifying"
> them. So I'm thinking the real solution is to develop customs and idioms
> for usage by library authors which would permit users to specify the way
> they would like to see exceptions handled.
>

This is missing the point of using exceptions. Beyond a mechanism for
reporting errors, exceptions enforce postconditions: a function will either
succeed or it will not return. Specifically, note that even under
BOOST_NO_EXCEPTIONS, boost::throw_exception is NOT allowed to return, or
else it would be useless to library developers (because they couldn't use
it to enforce postconditions.)

There might be exceptions to this, but whether a function throws or not
should not be configurable by the user.

Emil

Emil Dotchevski

unread,

Dec 16, 2016, 3:54:50 PM12/16/16

to bo...@lists.boost.org

On Fri, Dec 16, 2016 at 11:05 AM, Niall Douglas <s_sour...@nedprod.com>
wrote:

> I also looked up some benchmarks. A C++ and SEH exception throw and
> catch costs about 2 microseconds on x86 on MSVC back in 2011 on a
> fairly low end CPU for the time. x86 SEH still uses the old
> setjmp/longjmp mechanism i.e. each stack frame pushes its unwind
> handler and pops it off the stack after i.e. it is not a "zero
> runtime cost" exceptions implementation.
>

The added run-time cost of SEH is not because it uses a different throw
mechanism but because under SEH the compiler must assume that absolutely
any CPU instruction may result in throwing a C++ exception. In MSVC this
adds overhead at every function entry and exit, even functions that do not
use the throw keyword (as a side note, all his overhead isn't critical
because it can be avoided by inline, which one presumably would do anyway
in critical functions.)

Emil

Emil Dotchevski

unread,

Dec 16, 2016, 4:11:45 PM12/16/16

to bo...@lists.boost.org

Yes it is. It's basically a list of pointers and we can cap it by default
at, say, 10.

Just to be clear, I'm talking about adding the equivalent of something like
this to the boost::exception class:

void (*stack_trace_[10])();

> so the overhead of the generic Boost.Exception machinery is probably not
> significant. And if that overhead is significant then maybe it could be
> optimized instead. After all, why have it if we don't want to use it.
>

The cost of using the generic Boost Exception machinery is not limited to
speed and space but also coupling. The boost::throw_exception function
template is not coupled with any Boost libraries, while Boost Exception
itself uses e.g. shared_ptr and a few others. That said, even the runtime
cost of the generic Boost Exception machinery is probably an order of
magnitude bigger than storing 10 pointers.

unread,

Dec 16, 2016, 5:06:19 PM12/16/16

to bo...@lists.boost.org

On Sat, Dec 17, 2016 at 12:54 AM, Robert Ramey <ra...@rrsd.com> wrote:
> On 12/16/16 12:48 PM, Emil Dotchevski wrote:
>>
>> a function will either succeed or it will not return.
>
> not necessarily
>
> bool f() {
> if ...
> invoke_error
> return failure or ignore error
> ...
> return success
> }
>
> if invoke error is mapped to throw exception then it will never return. If
> it's mapped to something else - like emitting an error message or invoking a
> user specified call back then it won't throw an exception.

That results in a really horrible API with dual error reporting mechanisms.

Emil Dotchevski

unread,

Dec 16, 2016, 5:50:09 PM12/16/16

to bo...@lists.boost.org

On Fri, Dec 16, 2016 at 1:59 PM, Andrey Semashev <andrey....@gmail.com>

wrote:

> On Sat, Dec 17, 2016 at 12:11 AM, Emil Dotchevski
> <emildot...@gmail.com> wrote:
> > On Fri, Dec 16, 2016 at 12:24 PM, Andrey Semashev <
> andrey....@gmail.com
> >> wrote:
> >>
> >> The backtrace itself is not a lightweght object
> >
> > Yes it is. It's basically a list of pointers and we can cap it by default
> > at, say, 10.
> >
> > Just to be clear, I'm talking about adding the equivalent of something
> like
> > this to the boost::exception class:
> >
> > void (*stack_trace_[10])();
>
> What happens when the backtrace exceeds this limit? Would it be
> possible to attach backtraces larger than that?
>

We capture the top 10, or maybe the top 32. This puts a limit to both the
space and speed overhead of capturing the stack trace.

> Adding an array of pointers to the base class of exceptions is also
> not free. Whether one uses stacktraces or not, all Boost exceptions
> become 80 bytes larger regardless. For comparison, std::runtime_error
> in gcc 6 takes 16 bytes (not counting the string, if it's dynamically
> allocated and not stored in-place). Not that I have thousands
> exceptions stored somewhere, but that doesn't look like a reasonable
> tradeoff.
>

In C++, it is unspecified how much memory is required to throw an
exception. By definition, throwing an exception may require a memory
allocation, which might fail with std::bad_alloc. That said, I've never
seen this in practice, I don't think we should be concerned.

> >> so the overhead of the generic Boost.Exception machinery is probably not
> >> significant. And if that overhead is significant then maybe it could be
> >> optimized instead. After all, why have it if we don't want to use it.
> >
> > The cost of using the generic Boost Exception machinery is not limited to
> > speed and space but also coupling. The boost::throw_exception function
> > template is not coupled with any Boost libraries, while Boost Exception
> > itself uses e.g. shared_ptr and a few others. That said, even the runtime
> > cost of the generic Boost Exception machinery is probably an order of
> > magnitude bigger than storing 10 pointers.
>
> Thing is, the 10 pointers affect everyone, including those not using
> the backtraces.

True, so do capturing __FILE__ and __LINE__. I'm not suggesting that
capturing the stack trace by default is free, only that it is the better
compromise (pending looking at the actual implementation).

> They also affect boost::exception interface making it
> more like a swiss knife with a bunch of case-specific APIs.

Not really, like __FILE__ and __LINE__, the data captured by the stack
trace would be available through the usual boost::get_error_info interface;
the fact that there is special handling of this data is an implementation
detail not exposed in the API.

> That means
> everyone using boost::exception are coupled with whatever is present
> with those APIs. You described the 10 pointers, but I don't think you
> would expose that array as is from boost::exception - if only for type
> safety, you would want to wrap it into a class or something.

No, it would be an array of pointers, or maybe just an array of bytes that
a boost::get_error_info specialization will know how to deal with. That
part of Boost Exception may be coupled with Stacktrace, that's fine.

> > Requiring to link the Windows kernel? That's a breaking change, too.
>
> But you do have to link something to obtain the backtrace. You're not
> planning to implement it inline in the header, for all platforms, do
> you? :)
>

I don't know, but I wouldn't exclude that as a possibility. If the goal is
to make this as lightweight as possible so that the stacktrace can be
reasonably captured by default, why not? As a general rule, I think that it
is permissible to use compiler-specific APIs that don't require explicit
linker parameters.

Emil

Peter Dimov

unread,

Dec 16, 2016, 5:50:33 PM12/16/16

to bo...@lists.boost.org

Niall Douglas wrote:

> Bringing it back to the review of Stacktrace which this is (and not of
> Exception, another discussion thread would suit that), I am seeing the
> following options proposed for Stacktrace:
>
> Option 1: That Stacktrace implement a macro like
> BOOST_STACKTRACE_THROW_EXCEPTION() which reimplements
> BOOST_THROW_EXCEPTION() but with muxed in stacktrace.
>
> Option 2: That Stacktrace optionally hook some compile time hook in
> Boost.Exception to mux in stacktrace.
>
> Option 3: That Stacktrace optionally hook some runtime hook in
> Boost.Exception to mux in stacktrace.

This is proving to be a distraction from the Stacktrace review and I think
that we need to postpone this discussion for when Stacktrace is accepted.

I, for one, don't back any of those options. Stacktrace itself shouldn't
hook anything. boost::throw_exception should provide some sort of a
mechanism via which the user should (easily) be able to include a stack
trace into the exceptions.

> Implied in all the above is that the exception type stacktrace mux in
> framework as described in the current Stacktrace docs is considered bad
> form and should be deleted?

That's not a problem as far as I'm concerned, but it would be nice if the
Boost.Exception way is documented as well.

Emil Dotchevski

unread,

Dec 16, 2016, 5:54:26 PM12/16/16

to bo...@lists.boost.org

On Fri, Dec 16, 2016 at 2:05 PM, Andrey Semashev <andrey....@gmail.com>
wrote:

> On Sat, Dec 17, 2016 at 12:54 AM, Robert Ramey <ra...@rrsd.com> wrote:
> > On 12/16/16 12:48 PM, Emil Dotchevski wrote:
> >>
> >> a function will either succeed or it will not return.
> >
> > not necessarily
> >
> > bool f() {
> > if ...
> > invoke_error
> > return failure or ignore error
> > ...
> > return success
> > }
> >
> > if invoke error is mapped to throw exception then it will never return.
> If
> > it's mapped to something else - like emitting an error message or
> invoking a
> > user specified call back then it won't throw an exception.
>
> That results in a really horrible API with dual error reporting mechanisms.
>

Indeed. Again, enforcing postconditions is the main benefit of throwing.
Specifically:

if( condition-that-code-below-cant-deal-with )
boost::throw_exception(my_error());
//safe to assume no error occurred because boost::throw_exception does nor
return.

The call to throw_exception is not merely reporting the error but also
protecting the scope that follows.

Emil

Robert Ramey

unread,

Dec 16, 2016, 6:51:59 PM12/16/16

to bo...@lists.boost.org

On 12/16/16 2:53 PM, Emil Dotchevski wrote:
> On Fri, Dec 16, 2016 at 2:05 PM, Andrey Semashev <andrey....@gmail.com>
> wrote:
>
>> On Sat, Dec 17, 2016 at 12:54 AM, Robert Ramey <ra...@rrsd.com> wrote:
>>> On 12/16/16 12:48 PM, Emil Dotchevski wrote:
>>>>
>>>> a function will either succeed or it will not return.
>>>
>>> not necessarily
>>>
>>> bool f() {
>>> if ...
>>> invoke_error
>>> return failure or ignore error
>>> ...
>>> return success
>>> }
>>>
>>> if invoke error is mapped to throw exception then it will never return.
>> If
>>> it's mapped to something else - like emitting an error message or
>> invoking a
>>> user specified call back then it won't throw an exception.
>>
>> That results in a really horrible API with dual error reporting mechanisms.

That's a matter of opinion. It's just an example. There are other ones
cited above which are better - but they're part of something a lot more
larger and not suitable for pasting here.

The real point is that we can't say apriori that that
boost::throw_exception should do a certain thing and no other thing.

> Indeed. Again, enforcing postconditions is the main benefit of throwing.
> Specifically:
>
> if( condition-that-code-below-cant-deal-with )
> boost::throw_exception(my_error());
> //safe to assume no error occurred because boost::throw_exception does nor
> return.
>
> The call to throw_exception is not merely reporting the error but also
> protecting the scope that follows.

There are cases when that's not the only choice. A floating point
calculation could result in a NaN. In some cases one might want to pass
it on and other cases one might want to trap it as an exception.

If you don't like this example, it's easy to conjure up other ones. One
user might want to invoke save and invoke a stack trace while another
doesn't want to do this for dependencies or other reasons. The real
point is that from where we work here, we can't know what the library
user wants/needs. It's better to factor this out and leave it to him.

Robert Ramey

unread,

Dec 16, 2016, 10:28:30 PM12/16/16

to bo...@lists.boost.org

On 17 Dec 2016 at 0:50, Peter Dimov wrote:

> > Bringing it back to the review of Stacktrace which this is (and not of
> > Exception, another discussion thread would suit that), I am seeing the
> > following options proposed for Stacktrace:

> [snip]

> This is proving to be a distraction from the Stacktrace review and I think
> that we need to postpone this discussion for when Stacktrace is accepted.

I agree.

1. Can those talking about Boost.Exception please move their
discussion to a new thread and stop distracting from the Stacktrace
review thread? I'm not saying it's not valuable discussion, just
please start a new thread.

2. I have seen repeated comments by more than one person of
"Stacktrace would need to have XXX functionality to do ...". Please
consider reviewing Stacktrace as presented before making more of
these comments. About 80% of those comments Stacktrace already has
that functionality and you are wasting time proving a rationale for
them being implemented because it's implemented already. For the 20%
of the time Stacktrace doesn't have something, I need to see a formal
review from you saying "Stacktrace needs YYY functionality to do ..."

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? Is its stacktrace capturing implementation malloc free?
That sort of thing.

These are the sorts of non-hand-waving review feedback we need.

Niall

--
ned Productions Limited Consulting
http://www.nedproductions.biz/
http://ie.linkedin.com/in/nialldouglas/

Robert Ramey

unread,

Dec 16, 2016, 11:37:35 PM12/16/16

to bo...@lists.boost.org

On 12/16/16 4:32 PM, Emil Dotchevski wrote:

> I am not saying that it is impossible to let the user decide if library
> functions throw or use some other error reporting mechanism, only that if
> the user is given that choice, it makes C++ exceptions useless to the
> library developer, because he must always define behavior for the error
> branch, like a C programmer must. This is error-prone and can be costly,
> both in terms of development effort and run-time overhead.
>
> Emil
>
> _______________________________________________
> Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
>

The long and short of this is:

a) boost::throw_exception can do whatever it's developer (you) wants it to.

b) stacktrace can do whatever it's developer want's it to.

c) Library X developers can either accept these choices or they can
permit their users to specify error handling through a parameter. We
can't and wouldn't want to inhibit them from making the best choice in
the context of the library they are writing.

There's no conflict here and nothing has to be agreed upon.

Robert Ramey

Andrey Semashev

unread,

Dec 17, 2016, 2:24:52 AM12/17/16

to bo...@lists.boost.org

On 12/17/16 02:53, Robert Ramey wrote:
> On 12/16/16 2:05 PM, Andrey Semashev wrote:
>> On Sat, Dec 17, 2016 at 12:54 AM, Robert Ramey <ra...@rrsd.com> wrote:
>>> On 12/16/16 12:48 PM, Emil Dotchevski wrote:
>>>>
>>>> a function will either succeed or it will not return.
>>>
>>> not necessarily
>>>
>>> bool f() {
>>> if ...
>>> invoke_error
>>> return failure or ignore error
>>> ...
>>> return success
>>> }
>>>
>>> if invoke error is mapped to throw exception then it will never
>>> return. If
>>> it's mapped to something else - like emitting an error message or
>>> invoking a
>>> user specified call back then it won't throw an exception.
>>
>> That results in a really horrible API with dual error reporting
>> mechanisms.
>
> FYI - it's not a dual reporting mechanism. It's leaving the choice of
> reporting/handling mechanism to the library user. Once he selects it,
> that one is it.

It is a dual reporting mechanism from the standpoint of interface
definition. If the function uses exceptions to report errors, I expect
it to _not_ use error codes and such. In particular, the return value
can be used to return generated data or void. I do not have to examine
the returned error code. If the function uses error codes, I expect it
to not throw. I can use that function in noexcept contexts and do not
have to wrap it in try/catch.

If the function definition enables both reporting mechanisms, I have to
assume the error can be communicating both ways. Saying it will
communicate error one way and not the other in the docs is not
sufficient because it leaves room for error (both on the user's side and
the function developer's side). The policy has to be expressed on the
language level, by the function interface definition, leaving no room
for interpretation.

Antony Polukhin

unread,

Dec 17, 2016, 6:29:24 AM12/17/16

to boost@lists.boost.org List

Quick update on the Stacktrace library:
* I've fixed a few issues with COM objects, that were highlighted in
private emails. Those fixed do not affect library interface or
functionality
* Docs were slightly updated (better English in Motivation section,
added missing name into the Acknowledgements section)

If you wish to see the library source codes without those fixes (just
as they were at the beginning of the Boost review), you can use the
link: https://github.com/apolukhin/stacktrace/tree/boost_review

TODOs so far:
* At github one of the users was complaining about the performance of
ostreaming operations. There is a way to significantly speedup
ostreaming without changing the interface of the library. I'll do the
optimization after the Boost Review.
* Improve docs for the "Build, Macros and Backends" sections (say that
library is using system headers, no 3rd party libraries must be
installed or manually linked)
* Add a note to the docs that library is header only by default and
usable in C++98/C++03.

--
Best regards,
Antony Polukhin

Peter Dimov

unread,

Dec 17, 2016, 10:32:41 AM12/17/16

to bo...@lists.boost.org

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;

and

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 = f.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 );

lacks.

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

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.

Antony Polukhin

unread,

Dec 17, 2016, 11:22:04 AM12/17/16

to boost@lists.boost.org List

2016-12-17 18:31 GMT+03:00 Peter Dimov <li...@pdimov.com>:
> 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.

Async safety of those functions won't change.

> 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;
>
> and
>
> 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.

Windows implementation of those functions is definitely impossible to
make async safe, as the Windows backend uses COM. And that can not be
changed :(

> 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.

I'd like to avoid functions that are async-safe on one platform and
async unsafe on other. Mixing safety is a perfect way for producing
issues in user code.

> There is also an indication that the current frame interface is not the
> right one. In
>
> std::string name = f.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.

Yes, I'm planing to provide optimized versions of ostream operators
after the review. Windows backend has exactly the same problem.

> 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 );
>
> lacks.
>
> stacktrace itself could provide a function that calls a user-provided
> callback
>
> 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.

Hm... I need to think about it, but I'm not sure that the feature is
widely useful. Most of the time people just want all the available
information and default ostream operators are OK for them. Those, who
want a different (shorter) output format are typically choosing
between outputting only function names or outputting only function
location in source file. Getting function name and function location
in source file are two different COM calls on Windows, so such
callback will not work great on Windows.

--
Best regards,
Antony Polukhin

Peter Dimov

unread,

Dec 17, 2016, 11:24:30 AM12/17/16

to bo...@lists.boost.org

> In
>
> std::string name = f.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 same problem exists in the Windows backend, where the COM machinery is
initialized/deinitialized on each call, and GetLineByOffset is called twice,
each time discarding half of the result.

Peter Dimov

unread,

Dec 17, 2016, 11:37:50 AM12/17/16

to bo...@lists.boost.org

Antony Polukhin wrote:

> Windows implementation of those functions is definitely impossible to make
> async safe, as the Windows backend uses COM. And that can not be changed
> :(

I'm not sure that the concept of async safety exists on Windows. Windows
doesn't have signals. Or, stated differently, everything is async safe on
Windows.

> I'd like to avoid functions that are async-safe on one platform and async
> unsafe on other. Mixing safety is a perfect way for producing issues in
> user code.

Well, at least some of the code will be correct then, as opposed to none
today.

> Yes, I'm planing to provide optimized versions of ostream operators after
> the review. Windows backend has exactly the same problem.

The problem is that the frame interface has these inherent performance
problems. You could optimize op<<, but any user code that doesn't use op<<
is still going to have issues. This is an indication that the provided
interface is not quite right.

> Getting function name and function location in source file are two
> different COM calls on Windows, so such callback will not work great on
> Windows.

The cost of initializing and shutting down COM each time dwarfs calling
GetNameByOffset. It shouldn't even be a contest.

It also occurs to me that it would be more natural for the backend interface
to use size_t get_name(char* buffer, size_t size) instead of string name(),
because all the low-level APIs the backends use actually already use the
former interface, or something closer to it. Again, not very C++ friendly,
but it probably should be at least an option for people who don't want to
touch the C/C++ runtime when printing their stack trace, as it could be in
an unpredictable state after a crash.

Antony Polukhin

unread,

Dec 17, 2016, 12:30:03 PM12/17/16

to boost@lists.boost.org List

2016-12-17 19:37 GMT+03:00 Peter Dimov <li...@pdimov.com>:
> Antony Polukhin wrote:
>
>> Windows implementation of those functions is definitely impossible to make
>> async safe, as the Windows backend uses COM. And that can not be changed :(
>
>
> I'm not sure that the concept of async safety exists on Windows. Windows
> doesn't have signals. Or, stated differently, everything is async safe on
> Windows.

Actually, it's the other way around: everything is async-signal unsafe
https://msdn.microsoft.com/en-us/en-en/library/xdkz3x12.aspx :

"Do not use any function that generates a system call (for example,
_getcwd or time)."

>> Yes, I'm planing to provide optimized versions of ostream operators after
>> the review. Windows backend has exactly the same problem.
>
>
> The problem is that the frame interface has these inherent performance
> problems. You could optimize op<<, but any user code that doesn't use op<<
> is still going to have issues. This is an indication that the provided
> interface is not quite right.

This could be optimized and cached, but in that case library won't be
header only and it will get dependencies on boost_thread. Most of the
people won't like it. Optimized ostream operators seem to be a lesser
evil.

>> Getting function name and function location in source file are two
>> different COM calls on Windows, so such callback will not work great on
>> Windows.
>
>
> The cost of initializing and shutting down COM each time dwarfs calling
> GetNameByOffset. It shouldn't even be a contest.
>
> It also occurs to me that it would be more natural for the backend interface
> to use size_t get_name(char* buffer, size_t size) instead of string name(),
> because all the low-level APIs the backends use actually already use the
> former interface, or something closer to it. Again, not very C++ friendly,
> but it probably should be at least an option for people who don't want to
> touch the C/C++ runtime when printing their stack trace, as it could be in
> an unpredictable state after a crash.

Probably I'm missing some point, but async-signal-safety means almost
the same thing. If function is not async-signal-safe you shall not use
it in unpredictable states after crash. Anyway, if you do not wish to
use C++ runtime after a crash, you definitely would not like to use
COM in that place (so std::string in the interface does not make a big
difference).

--
Best regards,
Antony Polukhin

Peter Dimov

unread,

Dec 17, 2016, 12:47:31 PM12/17/16

to bo...@lists.boost.org

Antony Polukhin wrote:
> Actually, it's the other way around: everything is async-signal unsafe
> https://msdn.microsoft.com/en-us/en-en/library/xdkz3x12.aspx :
>
> "Do not use any function that generates a system call (for example,
> _getcwd or time)."

There are no signals on Windows. They just don't exist. The signal function
is provided by the C runtime and emulates signals.

The closest thing to a signal are the Ctrl+C and Ctrl+Break keyboard
interrupts, and they spawn a new thread to call the handler, instead of
reusing one as a POSIX signal would do.

> Anyway, if you do not wish to use C++ runtime after a crash, you
> definitely would not like to use COM in that place (so std::string in the
> interface does not make a big difference).

The runtime and COM have nothing in common. If your program crashes
somewhere in the runtime, the runtime may be unusable from that point on,
but this does not affect COM at all. And if you crash somewhere in a COM
component, that particular component may or may not be usable from that
point on, but this does not necessarily affect COM as a whole. You can still
use COM.

Antony Polukhin

unread,

Dec 17, 2016, 1:02:04 PM12/17/16

to boost@lists.boost.org List

2016-12-17 20:47 GMT+03:00 Peter Dimov <li...@pdimov.com>:
> Antony Polukhin wrote:
>>
>> Actually, it's the other way around: everything is async-signal unsafe
>> https://msdn.microsoft.com/en-us/en-en/library/xdkz3x12.aspx :
>>
>> "Do not use any function that generates a system call (for example,
>> _getcwd or time)."
>
>
> There are no signals on Windows. They just don't exist. The signal function
> is provided by the C runtime and emulates signals.
>
> The closest thing to a signal are the Ctrl+C and Ctrl+Break keyboard
> interrupts, and they spawn a new thread to call the handler, instead of
> reusing one as a POSIX signal would do.

Ok, but the signal documentation does not allow to use a lot of
functions in signal handlers (syscalls, malloc, printf...). Am I
allowed to use COM functions in signal handlers?

--
Best regards,
Antony Polukhin

Niall Douglas

unread,

Dec 17, 2016, 1:17:30 PM12/17/16

to bo...@lists.boost.org

On 17 Dec 2016 at 19:47, Peter Dimov wrote:

> Antony Polukhin wrote:
> > Actually, it's the other way around: everything is async-signal unsafe
> > https://msdn.microsoft.com/en-us/en-en/library/xdkz3x12.aspx :
> >
> > "Do not use any function that generates a system call (for example,
> > _getcwd or time)."
>
> There are no signals on Windows. They just don't exist. The signal function
> is provided by the C runtime and emulates signals.

There seems to be a lot of muddled discussion here.

Of course Windows has signals, as already referred to by myself
earlier it's called vectored exception handling which is exactly the
same as a signal implementation. In an exception handler you cannot
call any async unsafe routine such as anything in MSVCRT nor anything
implemented by kernel32.dll in userspace. As on POSIX, almost all
syscalls implemented entirely in kernel space are safe.

With respect to Stacktrace, that means that if you installed a
SIGSEGV handler and wanted to print a stacktrace from it before fatal
exit, you have exactly the same limitations on Windows as on POSIX.
You can't call printf, you can't call malloc. You can't use ostream.

This is why I hinted at the usage of std::string in Stacktrace's API.
As far as I can tell, this API design choice precludes the use of
Stacktrace in any SIGSEGV handlers.

Antony makes the valid point that on Windows there are race problems
with the DbgHelp library, in fact not only is it not async-unsafe,
it's also thread-unsafe. Trying to symbolise a stacktrace from
multiple threads concurrently on Windows will therefore misoperate.

Is this okay with the Boost community or do you feel it is a problem?
Specifically:

1. Should Stacktrace be async safe so it can be used to print a
stacktrace from a signal/exception handler?

2. Is its lack of thread safety on Windows a problem?

Niall

--
ned Productions Limited Consulting
http://www.nedproductions.biz/
http://ie.linkedin.com/in/nialldouglas/

Niall Douglas

unread,

Dec 17, 2016, 1:17:44 PM12/17/16

to bo...@lists.boost.org

On 17 Dec 2016 at 17:31, Peter Dimov 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.

Thanks Peter, you did a great job covering my own concerns here.

I can tell you it is entirely possible to write a stacktrace printer
which is async safe on POSIX. I've done it before. The Windows case
is much harder to achieve, you basically need to bring in non system
libraries because DbgHelp is crap.

Niall

--
ned Productions Limited Consulting
http://www.nedproductions.biz/
http://ie.linkedin.com/in/nialldouglas/

Peter Dimov

unread,

Dec 17, 2016, 1:25:12 PM12/17/16

to bo...@lists.boost.org

Antony Polukhin wrote:

> Ok, but the signal documentation does not allow to use a lot of functions
> in signal handlers (syscalls, malloc, printf...). Am I allowed to use COM
> functions in signal handlers?

The signal handlers are fake, they aren't really signal handlers. A crash
generates a structured exception on Windows, and the runtime calls the
appropriate signal handler when handling that exception. I'm not 100% sure
what can be done in a SEH handler, but calling Windows API functions is
certainly possible, and this specific COM use - loading an inproc handler
and calling it - should be fine too. (In some rare situations, such as
crashes in constructors of static objects in DLLs, the loader lock may cause
a deadlock, but that's life.)

Peter Dimov

unread,

Dec 17, 2016, 1:33:48 PM12/17/16

to bo...@lists.boost.org

Niall Douglas wrote:

> In an exception handler you cannot call any async unsafe routine such as
> anything in MSVCRT nor anything
implemented by kernel32.dll in userspace. As on POSIX, almost all syscalls
implemented entirely in kernel space are safe.

Thanks Niall. Do you know which Windows API functions are safe and which
aren't? I couldn't find a list anywhere.

> Antony makes the valid point that on Windows there are race problems with
> the DbgHelp library, in fact not only is it not async-unsafe, it's also
> thread-unsafe.

He doesn't use DbgHelp in the Windows backend though, he uses Dbgeng.h. This
is not the same thing, I think?

> Of course Windows has signals, as already referred to by myself earlier
> it's called vectored exception handling which is exactly the same as a
> signal implementation.

Not quite. A signal immediately suspends the thread and calls the handler in
it. Windows exception handling, in contrast, unwinds the stack. So if the
kernel crashes somewhere deep, it can unwind itself to a usable state before
the program gets to handle the exception. Or at least that's my
understanding.

Andrey Semashev

unread,

Dec 17, 2016, 4:30:20 PM12/17/16

to bo...@lists.boost.org

On Sat, Dec 17, 2016 at 9:17 PM, Niall Douglas
<s_sour...@nedprod.com> wrote:
>
> Specifically:
>
> 1. Should Stacktrace be async safe so it can be used to print a
> stacktrace from a signal/exception handler?

That depends on whether any backend supports decoding the stacktrace
in signal handlers. Note that if we also want to load debug symbols
from a separate file while doing this, the task seems to become
impossible.

> 2. Is its lack of thread safety on Windows a problem?

Yes, I think that is a serious problem. Though I'm not sure if
Boost.Stacktrace can do something about it, except to introduce a
global lock (sigh).

Niall Douglas

unread,

Dec 18, 2016, 8:56:26 AM12/18/16

to bo...@lists.boost.org

On 17 Dec 2016 at 20:22, Peter Dimov wrote:

> > Ok, but the signal documentation does not allow to use a lot of functions
> > in signal handlers (syscalls, malloc, printf...). Am I allowed to use COM
> > functions in signal handlers?
>
> The signal handlers are fake, they aren't really signal handlers. A crash
> generates a structured exception on Windows, and the runtime calls the
> appropriate signal handler when handling that exception. I'm not 100% sure
> what can be done in a SEH handler, but calling Windows API functions is
> certainly possible, and this specific COM use - loading an inproc handler
> and calling it - should be fine too. (In some rare situations, such as
> crashes in constructors of static objects in DLLs, the loader lock may cause
> a deadlock, but that's life.)

Reentering the COM machinery is definitely not safe, it deadlocks
randomly depending on the threading model used.

I have found that if you preload the COM object and precall all the
functions you'll call from the async handler, you preexecute all the
delay loading and lazy binding code. When the async handler is
called, it will then not reenter the COM machinery, thus avoiding
deadlocks.

Niall

--
ned Productions Limited Consulting
http://www.nedproductions.biz/
http://ie.linkedin.com/in/nialldouglas/

Niall Douglas

unread,

Dec 18, 2016, 8:56:43 AM12/18/16

to bo...@lists.boost.org

On 18 Dec 2016 at 0:29, Andrey Semashev wrote:

> > 1. Should Stacktrace be async safe so it can be used to print a
> > stacktrace from a signal/exception handler?
>
> That depends on whether any backend supports decoding the stacktrace
> in signal handlers. Note that if we also want to load debug symbols
> from a separate file while doing this, the task seems to become
> impossible.

If you preload the symbols and prebind the COM functions you'll use
in the async unsafe handler, I've found decoding the stacktrace to be
reliable in the past on Windows.

On POSIX launching a subprocess and parsing its output is async safe.
I have to admit I've always launched addr2line or llvm-symbolizer.
It's nasty and slow but it works.

> > 2. Is its lack of thread safety on Windows a problem?
>
> Yes, I think that is a serious problem. Though I'm not sure if
> Boost.Stacktrace can do something about it, except to introduce a
> global lock (sigh).

Thankfully as I just recently discovered Stacktrace is using Dbgeng
not Dbghelp. That means it's threadsafe (yay!).

Niall

--
ned Productions Limited Consulting
http://www.nedproductions.biz/
http://ie.linkedin.com/in/nialldouglas/

Niall Douglas

unread,

Dec 18, 2016, 8:57:39 AM12/18/16

to bo...@lists.boost.org

On 17 Dec 2016 at 20:32, Peter Dimov wrote:

> > In an exception handler you cannot call any async unsafe routine such as
> > anything in MSVCRT nor anything
> implemented by kernel32.dll in userspace. As on POSIX, almost all syscalls
> implemented entirely in kernel space are safe.
>
> Thanks Niall. Do you know which Windows API functions are safe and which
> aren't? I couldn't find a list anywhere.

I don't think there is even a list internal to Microsoft. There are
various user compiled lists around the internet, the ReactOS team
also have a good list somewhere on their mailing list.

I've seen my code which worked perfectly on Win7 rarely and randomly
deadlock on Win10 and one time, vice versa. WOW64 also has a very
different safe list to native Win32.

There are some obvious things not to call: anything which obviously
runs code in userspace.

> > Antony makes the valid point that on Windows there are race problems with
> > the DbgHelp library, in fact not only is it not async-unsafe, it's also
> > thread-unsafe.
>
> He doesn't use DbgHelp in the Windows backend though, he uses Dbgeng.h. This
> is not the same thing, I think?

Woohoo!

That is amazing news, and congrats to Antony for getting Dbgeng
working. Last month when I prereviewed Stacktrace I mentioned that
DbgHelp was a steaming pile of poo and that I had had much more
reliable experience with the thoroughly superior Dbgeng. Antony asked
for some example code because Dbgeng is barely documented, I no
longer had access to the code I wrote many years ago which used it. I
tried to cobble something together and I made some progress over what
Antony had, but I ran out of time due to needing to mind the recent
new baby. I'm not actually sure what he changed from what I sent him,
mine and his look very close, I must have missed something very
small.

I hadn't realised Antony figured it out and had assumed he was still
on DbgHelp, and the fact he's using Dbgeng makes Stacktrace much
superior to 99% of the windows stack trace implementations out there.
Particular benefits include:

* Dbgeng understands non-native stack frames, so mixed .NET, WinRT
and C++/CLI stack traces just work.

* Dbgeng is threadsafe.

* Dbgeng doesn't randomly fail and randomly work next time you call
it.

> > Of course Windows has signals, as already referred to by myself earlier
> > it's called vectored exception handling which is exactly the same as a
> > signal implementation.
>
> Not quite. A signal immediately suspends the thread and calls the handler in
> it. Windows exception handling, in contrast, unwinds the stack. So if the
> kernel crashes somewhere deep, it can unwind itself to a usable state before
> the program gets to handle the exception. Or at least that's my
> understanding.

You're missing a few steps.

1. RaiseException() is like kill() and starts the signal handling
process with parameters. Hardware exceptions raise an exception at
the immediately point of ocurrance i.e. inside any locks etc.

2. Any installed vectored exception handlers are like sigaction()
except they are called for all exception codes. The handler returns
whether it handled it or whether to keep searching. Vectored
exception handlers are process wide.

3. If still unhandled, the Thread Information Block (TIB) for the
thread where the exception occurred is asked for the current
thread-local TEH (Table Exception Handling) on x64 or SEH (Structured
Exception Handling) on x86. A search for all handlers installed for
all code in the stack until the point of exception are called in
reverse order. Each handler may handle the exception, or say to keep
searching.

4. Every thread is always begun with a default exception handler
which opens that famous dialog box and terminates the process, so if
your exception reaches this the right thing happens.

5. C++ exceptions are implemented 100% a client of the same TEH and
SEH framework. In fact they are simply a RaiseException(0xE06D7363,
...). If you dive into the implementation of __CxxThrowException()
you'll see that.

What's very important to note is that all this occurs without
unwinding the stack and at the point of the exception with any locks
still held. This is the source of the reentrancy which causes the
deadlocks if you call any userspace implemented code from an
exception handler. The reason it doesn't unwind is because a handler
may choose to restart execution of the failed operation. There was a
very clever commercial C++ object-to-disk system a very long time ago
which serialised C++ objects out to disk and removed the RAM storage.
When the C++ program faulted on accessing them, the SEH handler would
deserialise the object back into RAM and restart the failed
instruction. Worked beautifully.

Niall

--
ned Productions Limited Consulting
http://www.nedproductions.biz/
http://ie.linkedin.com/in/nialldouglas/

Peter Dimov

unread,

Dec 18, 2016, 9:03:55 AM12/18/16

to bo...@lists.boost.org

Niall Douglas wrote:
> Reentering the COM machinery is definitely not safe, it deadlocks randomly
> depending on the threading model used.
>
> I have found that if you preload the COM object and precall all the
> functions you'll call from the async handler, you preexecute all the delay
> loading and lazy binding code. When the async handler is called, it will
> then not reenter the COM machinery, thus avoiding deadlocks.

This is not COM, but the loader lock. If you try to load a DLL from DllMain,
it deadlocks. This in C++ terms means that constructors of static objects in
a DLL can't load DLLs, so in our case, would not be able to resolve stack
traces. This is an acceptable limitation.

That said, I'm actually not sure whether the debug engine is in-process.

Antony Polukhin

unread,

Dec 18, 2016, 11:55:30 AM12/18/16

to boost@lists.boost.org List

2016-12-18 16:56 GMT+03:00 Niall Douglas <s_sour...@nedprod.com>:
<...>

>> > Antony makes the valid point that on Windows there are race problems with
>> > the DbgHelp library, in fact not only is it not async-unsafe, it's also
>> > thread-unsafe.
>>
>> He doesn't use DbgHelp in the Windows backend though, he uses Dbgeng.h. This
>> is not the same thing, I think?
>
> Woohoo!
>
> That is amazing news, and congrats to Antony for getting Dbgeng
> working. Last month when I prereviewed Stacktrace I mentioned that
> DbgHelp was a steaming pile of poo and that I had had much more
> reliable experience with the thoroughly superior Dbgeng. Antony asked
> for some example code because Dbgeng is barely documented, I no
> longer had access to the code I wrote many years ago which used it. I
> tried to cobble something together and I made some progress over what
> Antony had, but I ran out of time due to needing to mind the recent
> new baby. I'm not actually sure what he changed from what I sent him,
> mine and his look very close, I must have missed something very
> small.

I've took Nialls code that uses COM as a base and mixed it with the
code I already had that collects stacktrace without COM and DbgHelp.
Some hammering produced a class that *probably* async-safely collects
stacktrace and can output it using async-unsafe COM.

--
Best regards,
Antony Polukhin

Antony Polukhin

unread,

Dec 18, 2016, 12:08:28 PM12/18/16

to boost@lists.boost.org List

2016-12-18 17:03 GMT+03:00 Peter Dimov <li...@pdimov.com>:
> Niall Douglas wrote:
>>
>> Reentering the COM machinery is definitely not safe, it deadlocks randomly
>> depending on the threading model used.
>>
>> I have found that if you preload the COM object and precall all the
>> functions you'll call from the async handler, you preexecute all the delay
>> loading and lazy binding code. When the async handler is called, it will
>> then not reenter the COM machinery, thus avoiding deadlocks.
>
>
> This is not COM, but the loader lock. If you try to load a DLL from DllMain,
> it deadlocks. This in C++ terms means that constructors of static objects in
> a DLL can't load DLLs, so in our case, would not be able to resolve stack
> traces. This is an acceptable limitation.
>
> That said, I'm actually not sure whether the debug engine is in-process.

The bad thing is that any printf like function, kernel calls
(including multithreading) or malloc are not async signal handler
safe. I'm quite sure that COM functions allocate memory and do some
synchronization.

It is possible to make an async-signal-safe stacktrace printing on
Linux... But on Windows it seems impossible. If anyone knows a way to
move data from signal handler into a usual environment I can create a
safe_print() function that
* will do safe printing for Linux/POSIX
* will do some extremely heavy machinery of moving the stacktrace
outside the signal handler and printing it for windows.

Any ideas on how to do that?

--
Best regards,
Antony Polukhin

Antony Polukhin

unread,

Dec 18, 2016, 12:16:26 PM12/18/16

to boost@lists.boost.org List

2016-12-18 0:29 GMT+03:00 Andrey Semashev <andrey....@gmail.com>:
> On Sat, Dec 17, 2016 at 9:17 PM, Niall Douglas
> <s_sour...@nedprod.com> wrote:
>>
>> Specifically:
>>
>> 1. Should Stacktrace be async safe so it can be used to print a
>> stacktrace from a signal/exception handler?
>
> That depends on whether any backend supports decoding the stacktrace
> in signal handlers. Note that if we also want to load debug symbols
> from a separate file while doing this, the task seems to become
> impossible.

Spawning a new subprocess in signal handler (or at the beginning of
the program) would be OK for you? If I'll came up with an
async_safe_print(stacktrace()); function that may fix the broken
example in docs, provide much more motivation for having non
allocating stacktrace class.

>> 2. Is its lack of thread safety on Windows a problem?
>
> Yes, I think that is a serious problem. Though I'm not sure if
> Boost.Stacktrace can do something about it, except to introduce a
> global lock (sigh).

It is thread safe now (was not thread safe 2 months ago).

--
Best regards,
Antony Polukhin

unread,

Dec 18, 2016, 5:15:02 PM12/18/16

to boost@lists.boost.org List

2016-12-18 20:26 GMT+03:00 Peter Dimov <li...@pdimov.com>:
> After this discussion, I can suggest the following changes to the library:
>
> - the part that captures a stack trace and the part that resolves it to
> function/file/line should be separate, and usable without each other.

Already.

> - ideally, both ought to be async safe, or as async safe as possible on the
> target architecture.

I'm trying to unify behavior rather than provoking people to write
unportable code. Both must be as async safe, if *all* the platforms
allow to do that.

> - there should be a low-level API that is async safe, and a high level API
> that is not necessarily such.

There's no way to get function location in async safe way onWindows.

> - macOS should be supported.

OK.

> - the library should not #include <windows.h>. The part of the Windows
> backend that needs <windows.h> should be a separately-compiled library.

I'd rather take the boost/winapi path.

> What I can suggest as a low-level API is:
>
> // capture
>
> typedef void (*stacktrace_frame)(); // a bit on the pedantic side, but why
> not
>
> size_t stacktrace_capture( stacktrace_frame buffer[], size_t size );

That's doable. But how are you going to use it?

> // resolution
>
> void stracktrace_resolve_frame( stacktrace_frame address, char function[],
> size_t fnsize, char file[], size_t filesize, size_t & line );

What if the user wants only the function name?

> typedef void (*stacktrace_resolve_callback)( stacktrace_frame address, char
> const * function, char const * file, size_t line );
>
> void stacktrace_resolve( stacktrace_frame const[] trace, size_t size, char
> function[], size_t fnsize, char file[], size_t filesize,
> stacktrace_resolve_callback callback );
>
> On Windows, I see stacktrace_capture as header-only and async-safe, and
> stacktrace_resolve as separately compiled and maybe-mostly-safe, as today.

As today - it is unsafe. We may assume the safety, but there's no
guarantee. Moreover, I'm pretty sure that it is unsafe.

> On Mac OS X, stacktrace_capture can use ::backtrace and stacktrace_resolve
> can use the atos utility instead of addr2line.

glibc ::backtrace() implementation is not async-signal-safe. So
probably I will need to discover another way of doing that thing.

> https://developer.apple.com/legacy/library/documentation/Darwin/Reference/ManPages/man1/atos.1.html

--
Best regards,
Antony Polukhin

Antony Polukhin

unread,

Dec 18, 2016, 5:19:24 PM12/18/16

to boost@lists.boost.org List

2016-12-18 20:38 GMT+03:00 Peter Dimov <li...@pdimov.com>:
> Antony Polukhin wrote:
>
>> The bad thing is that any printf like function, kernel calls (including
>> multithreading) or malloc are not async signal handler safe. I'm quite sure
>> that COM functions allocate memory and do some synchronization.
>
>
> We can't speak of "COM functions" as such. Every COM function does its own
> thing. In the in-process case, COM is basically just an ABI. What Dbgeng.dll
> does is what it does. It certainly doesn't allocate using (the program's)
> malloc though. And if it does synchronize, it does it with its own
> CRITICAL_SECTIONs, and those can be used in a structured exception handler.

Those are very generous assumptions. As long as we have no strict
documentation or source codes of the component - it is UB to use it in
async-signal-handler.

--
Best regards,
Antony Polukhin

Antony Polukhin

unread,

Dec 18, 2016, 5:26:54 PM12/18/16

to boost@lists.boost.org List

It's not that simple. After a fork() you are still in the
async-signal-processing mode. You have to call one of the exec*
functions to safely print the backtrace.

Windows has no fork(). Looks like the only way to do that safely on
Windows, is to create a process before the signal, make a pipe and
wait for data on a pipe. As soon as the signal appears - dump data
into the pipe and die. The child process have to output the content...
it seems to me that such out-of-the-box functionality won't make all
the users happy. They would definitely want to tune multiple things
that I'm failing to predict.

--
Best regards,
Antony Polukhin