On Wed, Dec 14, 2016 at 3:44 PM, Andrey Semashev 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

On 15/12/2016 12:44, Andrey Semashev wrote:

On 12/15/16 02:27, 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'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.

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. And I've previously asked about this and been assured that this library does do this in two separate phases to reduce performance impact at the trace site. 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 haven't looked at this library in detail yet so I don't know if it takes advantage of this or not.

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

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.

And I've previously asked about this and been assured that this library does do this in two separate phases to reduce performance impact at the trace site.

Could you take a closer look at the actual implementation code please?

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. Niall -- ned Productions Limited Consulting http://www.nedproductions.biz/ http://ie.linkedin.com/in/nialldouglas/

On Thu, Dec 15, 2016 at 3:36 PM, 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.

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 overhead in structured exception handling comes from translating hardware exceptions to C++ exceptions. Under this (quite stupid) model, the compiler has to generate code that is able to throw C++ exceptions if any arbitrary instruction happens to crash (instead, obviously, in C++ only the throw keyword can throw and exception.) Emil

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/

On Wed, Dec 14, 2016 at 11:46 PM, Niall Douglas 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

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

On Wed, Dec 14, 2016 at 5:47 PM, Peter Dimov wrote:

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.

Also its dependence on Assert and Config has nothing to do with Exception. Emil

On Wed, Dec 14, 2016 at 10:03 PM, Robert Ramey wrote:

On 12/14/16 5:47 PM, Peter Dimov wrote:

...

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: from

config: from from

I meant that every library which used BOOST_THROW_EXCEPTION now depends upon boost/throw_exception. Am I wrong about that?

The macro BOOST_THROW_EXCEPTION did not exist prior to Boost Exception. It was introduced so that the __FILE__ and __LINE__ of the throw are captured in the exception object. This new functionality does not depend on the Boost Exception submodule. You should look at how the git submodules are structured: boost/throw_exception.hpp is in its own minimodule which also includes boost/exception/exception.hpp. Emil

On Wed, Dec 14, 2016 at 7:53 PM, 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.

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_EXCEPTIO... What I want from BOOST_THROW_EXCEPTION is to embed the current source file, line number and function name in the thrown exception -- which is the closest I can get to a "stack trace" with current portable facilities.

Now it's being suggested that BOOST_THROW_EXCEPTION be repurposed/enhanced yet again?

I suggested that there is potential for integration between (the proposed) Boost.Stacktrace and Boost.Exception. I'm flexible about exactly what form that integration will take. I rather like Emil's suggestion of embedding the stacktrace from within boost::throw_exception, since it can do that. It need not happen within the BOOST_THROW_EXCEPTION macro.

On Wed, Dec 14, 2016 at 10:14 PM, Robert Ramey wrote:

On 12/14/16 6:06 PM, Nat Goodspeed wrote:

...

On Wed, Dec 14, 2016 at 7:53 PM, 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.

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/BOOS T_THROW_EXCEPTION.html

Right. Because the semantics of BOOST_THROW_EXCEPTION were changed and the documentation updated to the new semantics.

False, the semantics of BOOST_THROW_EXCEPTION have never changed. Emil

On 12/15/16 5:37 AM, Peter Dimov wrote:

Robert Ramey wrote:

...

Because the semantics of BOOST_THROW_EXCEPTION were changed and the documentation updated to the new semantics.

BOOST_THROW_EXCEPTION did not exist before Boost.Exception, so no, it wasn't changed.

Sorry, my mistake. 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. Robert Ramey

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

On Thu, Dec 15, 2016 at 11:26 AM, Robert Ramey wrote:

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.

These are false claims -- the change you're talking about went through the review process and it was immediately reflected in the documentation of boost::throw_exception, even though it didn't break existing code. 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. Specifically, if you want to use boost::throw_exception, the Boost Exception submodule is NOT necessary.

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.

I agree 200% with your concern about coupling, so let's discuss that, rather than throwing around unsubstantiated claims. Any additions to boost::throw_exception should be self-contained. Even basic Boost components like function or shared_ptr should be off limits. 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. Emil

On Thu, Dec 15, 2016 at 1:24 PM, Robert Ramey 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/excep... .

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

On Thu, Dec 15, 2016 at 4:53 PM, Robert Ramey wrote:

On 12/15/16 3:33 PM, Emil Dotchevski wrote:

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?

This was discussed in a thread

http://boost.2283326.n4.nabble.com/Boost-and-exceptions- td4631441i140.html#a4631898

As I pointed out in my response there, this is not equivalent to what Boost Exception does. You are catching one exception object and throwing another, specifically you catch(...) and then throw a new object of the unrelated type exception_wrapper<>. To emulate the semantics of Boost Exception non-intrusively, you have to be able to do something like this: catch(...) { //somehow access the current exception object and associate arbitrary data with it throw; //re-throw the original exception object, not a new one. } With Boost Exception this works like so: catch( boost::exception & e ) { e << my_error_info(....); throw; } The problem is that this only works for exceptions of types that derive from boost::exception. The boost::throw_exception class template does NOTHING more than inject boost::exception as a base to the type used to instantiate it, and only if it isn't a base already. Emil

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

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.

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.

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

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

-----Original Message----- From: Boost [mailto:boost-bounces@lists.boost.org] On Behalf Of Robert Ramey Sent: 16 December 2016 16:47 To: boost@lists.boost.org Subject: Re: [boost] [Boost] [Stacktrace] review

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.

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

On Fri, Dec 16, 2016 at 8:46 AM, Robert Ramey 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

On Sat, Dec 17, 2016 at 12:54 AM, Robert Ramey 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.

On 12/16/16 2:53 PM, Emil Dotchevski wrote:

On Fri, Dec 16, 2016 at 2:05 PM, Andrey Semashev wrote:

...

On Sat, Dec 17, 2016 at 12:54 AM, Robert Ramey 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

Emil

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

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

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

On Fri, Dec 16, 2016 at 1:20 AM, Andrey Semashev 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.

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

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 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 in boost/throw_exception.hpp? That's a no-go. Making it not header-only module? Also a no-go.

Emil

On Fri, Dec 16, 2016 at 12:24 PM, Andrey Semashev

wrote:

On 12/16/16 22:32, Emil Dotchevski wrote:

...

On Fri, Dec 16, 2016 at 1:20 AM, Andrey Semashev < andrey.semashev@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

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.

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.

Yes, which is very old. :) Anyway, the point is that boost::throw_exception must remain extremely lightweight in terms of dependencies AND it must be very conservative in terms of using C++ features. Specifically, I am hinting that std synchronization facilities are off-limits (though to be fair capturing the stack trace can be commented-out for old compilers.)

Are you proposing to include 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.

Requiring to link the Windows kernel? That's a breaking change, too. Emil

On Fri, Dec 16, 2016 at 1:59 PM, Andrey Semashev wrote:

...

On Fri, Dec 16, 2016 at 12:24 PM, Andrey Semashev < andrey.semashev@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

On Sat, Dec 17, 2016 at 12:11 AM, Emil Dotchevski wrote: 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

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

On Fri, Dec 16, 2016 at 11:05 AM, 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.

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?

It seems that there is some disagreement on how costly each of the options is. I think that before a vote we should look at actual code, actual proposed changes, and actual costs in terms of coupling and speed/space. This shouldn't be difficult to do, given that Stacktrace already exists. Emil

This [the potential integration with throw_exception] 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.

Although, on second thought, there is one pertinent question that seems to have come up. Specifically, if the library should be extended to allow me to capture the trace on my own using RtlCaptureStackBackTrace or ::backtrace, then at some later point use the rest of the Stacktrace functionality. And additionally, can I do that for ::backtrace captures even when Stacktrace is using a different backend (unwind) as its default.

From a quick look at the code, it seems that the library cannot currently use more than one backend at the same time because they are all named 'class backend'.

As a note, the documentation states that backtrace is a POSIX function, but it seems to be Linux-specific. I can't find it in the POSIX standard. I doubt that unwind is also standard POSIX. Does the library support OS X?

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

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 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. If Dbgeng.dll itself crashes, it would most likely be unable to resolve its own crash, of course. For async safe printing itself, you should use write/WriteFile. Something like void frame::print_to_fd( int fd ); // POSIX void frame::print_to_fd( void* fd ); // Windows, void* is HANDLE Now the user will be able to pass GetStdHandle( STD_ERROR_HANDLE ) for stderr output, or a file handle for log output.

Antony Polukhin wrote:

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

Yes, obviously. It's not clear though what do Linux users gain from the decision to keep their backend async-unsafe just because Windows is not strictly documented. And, FWIW, Windows is pretty resilient. Here for instance I crash in an APC that is called by the kernel, and it works, even though I can't see g() in the trace probably because Windows has handled the exception and rethrown it: #include #include <iostream> #include #include static void handler( int sig ) { boost::stacktrace::stacktrace st; std::cout << st << std::endl; _exit( 3 ); } VOID CALLBACK g( ULONG_PTR ) { std::cout << "g\n"; *(int*)0 = 0; } static void f() { std::cout << "f\n"; QueueUserAPC( g, GetCurrentThread(), 0 ); SleepEx( 1000, TRUE ); } int main() { signal( SIGSEGV, handler ); f(); }

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.

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

...

...

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

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/

On 12/18/16 20:15, Antony Polukhin wrote:

2016-12-18 0:29 GMT+03:00 Andrey Semashev :

...

On Sat, Dec 17, 2016 at 9:17 PM, Niall Douglas 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.

If that is only possible by forking a process then I'm not sure that accomplishes much. I mean, if there is a non-signal-safe formatting function for stacktraces, there's no problem to call `fork()` yourself in the signal handler, use that formatting function in the child process, and pipe its result back to the main process. I guess, a helper function that does this would be useful, just make sure to explicitly document that it forks. Also, the non-forking formatting implementation is still needed for all cases other than signal handlers.

On 19 Dec 2016 at 1:26, Antony Polukhin wrote:

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.

The NT kernel has a fork() implementation: RtlCloneUserProcess() You should note that much Win32 code is highly unreliable after being forked, only Nt*() calls are reliable but if you called NtTerminateProcess() on the clone when you're done so no Win32 code executes it should work. Niall -- ned Productions Limited Consulting http://www.nedproductions.biz/ http://ie.linkedin.com/in/nialldouglas/

Andrey Semashev wrote:

Yes, if the only way to do that is fork+exec then that is not usable for me, not as the default implementation of the stacktrace decoding algorithm. If such functionality is provided, it should be strictly optional and disabled by default (i.e. on Windows don't start the helper process unless the user explicitly requests that in runtime). Let the decoding algorithm itself be not-signal-safe but in-process.

Why is spawning a process to do the decoding unacceptable? I'm sorry if you already explained this in a previous message.

On 12/19/16 18:58, Andrey Semashev wrote:

On 12/19/16 17:07, Peter Dimov wrote:

...

Andrey Semashev wrote:

...

Yes, if the only way to do that is fork+exec then that is not usable for me, not as the default implementation of the stacktrace decoding algorithm. If such functionality is provided, it should be strictly optional and disabled by default (i.e. on Windows don't start the helper process unless the user explicitly requests that in runtime). Let the decoding algorithm itself be not-signal-safe but in-process.

Why is spawning a process to do the decoding unacceptable?

I pointed out this in my review. One reason is because of security considerations. If the library executes a foreign executable with path lookup, it is possible to put a malicious executable with the parent

...it is possible to *run* a malicious executable...

process permissions. The parent process can be a daemon, running with elevated permissions, so this could potentially be devastating. This is more so a problem if the user of Boost.Stacktrace is not suspecting that the library executes a process under the hood.

In general, as an application developer, I always want to be in control when my application forks and what other processes it executes. If that happens, I want that to happen in very few and specific places of my code. In those places I can carefully setup environment and permissions for the process to run, as well as specify the full path to the executable, to reduce the possibility of a breach.

On 12/19/16 19:09, Peter Dimov wrote:

Andrey Semashev wrote:

...

...

Why is spawning a process to do the decoding unacceptable?

I pointed out this in my review. One reason is because of security considerations. If the library executes a foreign executable with path lookup, it is possible to put a malicious executable with the parent process permissions. The parent process can be a daemon, running with elevated permissions, so this could potentially be devastating. This is more so a problem if the user of Boost.Stacktrace is not suspecting that the library executes a process under the hood.

OK... but can you give some specific examples, on Linux and Windows, as to how such an attack would work? Complete with actual directory and file names.

Suppose someone placed $HOME/bin/addr2line of the following content: #!/bin/bash if [ $UID -eq 0 ] then echo Hello, root! else /usr/bin/addr2line "$@" fi And then there is your application that is linked with Boost.Stacktrace and at some point it executes `frame::source_file()` or `frame::source_line()` (for example, while processing an exception with attached stacktrace). It would work seemingly fine when executed under your user, but it will execute malicious code when executed under `sudo`. And there may be valid reasons to want to run your program under `sudo` at times. I'm less knowledgeable about Windows innerworkings, but I think something similar is possible there as well. Also, I'm not a security expert, so maybe there are other exploits. Bottom line: I just don't want a potential security issue where none is required. No fork+exec = no problem.

On 12/19/16 20:04, Peter Dimov wrote:

Andrey Semashev wrote:

...

Suppose someone placed $HOME/bin/addr2line of the following content:

That's only going to work if $HOME/bin is on the path before /usr/bin, which seems not very prudent from a security perspective. The user can type 'addr2line' (or anything else in /usr/bin such as 'ls') himself, after all. Hello root. So it's not that easy.

Well, $HOME/bin is before /usr/bin on my system. And I suppose, it is expected to be looked up first, precisely to be able to override system binaries with your own. Similar to how /usr/local/bin is expected to be looked up before /usr/bin.

In general, if the attacker has write access to a directory in $PATH, things are already not very secure. This also applies to Windows, because $PATH is searched for DLLs, although it has very low priority, so you need to find a DLL that the program attempts to load but isn't present in the system directories.

I think, a process could also modify the system-wide PATH variable, at least on some Windows versions. IIRC, some installers (Visual Studio? Intel compiler? I can't remember) did that at some point. I don't know if that's the case now.

That said, Stacktrace should probably not use $PATH at all for locating its helper process. On POSIX, execvp is not async safe anyway, so using /usr/bin/addr2line directly may be better.

That is somewhat better. It does exclude PATH from the lookup, that's for sure. But consider that the application could use a custom build of Boost and be installed in /opt. What if the application also wants to use a custom addr2line from its /opt/my_app/bin? It's possible to patch the path in Boost.Stacktrace, but that adds some maintenance burden of having to maintain the patch. A config macro perhaps? A runtime option? The full path does not solve the problem of setting up environment for the process though. E.g. I don't have a way to drop privileges of root before executing addr2line. Environment setup is also problematic - one has to configure and restore environment before and after every call of Boost.Stacktrace that executes a process.

And on Windows, the helper would generally be installed along with the program - it won't be a system utility such as addr2line. In which case it would be spawned from the directory of the program using a full path.

That imposes a requirement on the user's application layout. I'd say, it should be configurable as well. It should probably be a runtime configuration in order to support portable applications (i.e. those not requiring installation in a particular directory). Security issues aside, there is also a matter of controlling the child process. What if it hangs? Or crashes? How does it affect the master application? What if the master crashes (i.e. how can you ensure you don't leave a zombie process)? What will you do if you exhaust the limit of running processes in the system (i.e. if the app itself is not able to spawn its processes because of a bunch of running addr2line)? Then, there are performance implications of running a process for each function call. These kind of questions are difficult to answer in a generic library, which is also not focused on process management. I'm really not convinced that I have to execute a process to be able to decode a stacktrace. That may be the only way to do that from a signal handler, but in a general context that is really not the expected behavior of the library. IMO, the signal handler case is special enough to warrant a special API for that, and it may involve forking and executing a process. That API should likely include the necessary means to configure the process before `exec`ing the new process, including the full path, permissions, a running timeout and what not. For other cases an in-process solution should be available.

On 12/19/16 21:42, Peter Dimov wrote:

Andrey Semashev wrote:

...

I think, a process could also modify the system-wide PATH variable, at least on some Windows versions. IIRC, some installers (Visual Studio? Intel compiler? I can't remember) did that at some point. I don't know if that's the case now.

You need root for that.

But installers always execute as root on Windows, don't they? (I believe, the term is "with elevated privileges") The idea is that the user installed something that modified system PATH so that it can later be used to inject a malicious process into your application. Also, disabling UAC is a popular practice, I think. :p

...

The full path does not solve the problem of setting up environment for the process though. E.g. I don't have a way to drop privileges of root before executing addr2line.

Dropping privileges could perhaps work for addr2line which takes a filename rather than a PID but it wouldn't work for utilities that are PID-based such as atos. If the program is root, the utility would also need root in order to inspect its address space and locate the module based on the address. I think.

Aren't there capabilities that could be granted to the child process? If not, that is unfortunate. Not just because dropping privileges won't work on all platforms, but also because the user will have to depend on the library implementation details to know if it can work. I suppose, this can be somewhat mitigated in the backend reported if it is capable to decode the stacktrace with reduced privileges.

...

Security issues aside, there is also a matter of controlling the child process. What if it hangs? Or crashes? How does it affect the master application? What if the master crashes (i.e. how can you ensure you don't leave a zombie process)? What will you do if you exhaust the limit of running processes in the system (i.e. if the app itself is not able to spawn its processes because of a bunch of running addr2line)?

Very similar questions can be posed towards the current Windows implementation. There isn't that much of a difference between using a helper process and doing the same work in-process.

In case of COM, handling the out-of-process COM objects is Windows' job. The application itself need not handle surrogate processes running those objects, nor does it have to specify path to those process executables or the COM object's dll. You are quite right in that COM hides most of that complexity, and that answers many of the questions above.

In fact, if your process has just crashed, it's better to do the work from another process, because its memory would be intact.

That is true, but I reiterate that handling a crash (or a signal) is special enough to warrant a special solution. At this point performance doesn't really matter, and the host process is pretty much doomed anyway, so the issues affecting its survival are off the table. By the way, since we're discussing crash handling, I have to say that I'm currently using gdb for this purpose. The signal handler in my process does fork and exec gdb, which saves the backtraces of all threads of the application to a file. If compared to Boost.Stacktrace, this approach has significant benefits, such as that gdb loads debug symbols, if present on the system, and that it dumps all threads instead of just one. Of course, gdb is much heavier than Boost.Stacktrace, but still this is something that I'd like to be achievable with the library.

...

Then, there are performance implications of running a process for each function call.

That's what I tried to address with my API suggestions.

I don't think that fully solves the performance issue because it still executes a process per stacktrace entry. In my example with gdb above, all decoding is done in one process, and I have to wonder how Boost.Stacktrace doing the same amount of work would compare to that.

On 12/20/2016 09:11 AM, Niall Douglas wrote:

... Stacktrace should be able to serialise the stacktrace of an about to be terminated process to disc in an async-safe way in a format which other tooling such as addr2line or llvm-symbolizer can consume in combination with the debug symbols to produce an accurate source and line number stacktrace. Stacktrace does not need to parse the stacktrace in an async-safe way.

The need to have and to use an additional tool to actually see the stack seems like a considerable complication. To me personally printing out the stack-trace to the log was a quick and light-weight (but often sufficient) clue where to start my investigation. How is the suggested procedure better compared to the established on Linux -- retrieve the core of the failed app, run it with gdb, look at the debug symbols, stack-trace, etc.? I suspect a similar/same mechanism to already exist on Windows as well, right?

On 12/20/16 01:11, Niall Douglas wrote:

Back to Stacktrace. I think I am beginning to see a consensus emerge: that Stacktrace should be able to serialise the stacktrace of an about to be terminated process to disc in an async-safe way in a format which other tooling such as addr2line or llvm-symbolizer can consume in combination with the debug symbols to produce an accurate source and line number stacktrace. Stacktrace does not need to parse the stacktrace in an async-safe way.

Is this consensus acceptable to the Boost community?

Sorry for repeating myself, but in my review and several replies in the discussion I suggested that the library should provide two interfaces for decoding a stacktrace. One for use in normal context, outside signal handler, and the other for signal handlers. The latter has to be signal-safe and can fork the process and reuse the former interface internally. It could also be used to save the decoded stacktrace to a file. This approach would allow to obtain decoded stacktrace in every context of a program. What you describe is quite different. It doesn't allow to decode the stacktrace in a live process, but provides a way to obtain it postmortem. I'm not sure how useful that would be, given that it's already possible to produce a core dump and examine that. In my practice though I prefer decoded stacktraces to core dumps because it doesn't require me to have the exact same binaries on my system as those on the client's. That is most likely not limited with binaries shipped with my application. If I was developing a crash handler with Stacktrace, I would prefer it produce a decoded backtrace instead of intermediate output that needs further resolving using the involved binaries.

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. - ideally, both ought to be async safe, or as async safe as possible on the target architecture. - there should be a low-level API that is async safe, and a high level API that is not necessarily such. - macOS should be supported. - the library should not #include . The part of the Windows backend that needs should be a separately-compiled library. 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 ); // resolution void stracktrace_resolve_frame( stacktrace_frame address, char function[], size_t fnsize, char file[], size_t filesize, size_t & line ); 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. On Mac OS X, stacktrace_capture can use ::backtrace and stacktrace_resolve can use the atos utility instead of addr2line. https://developer.apple.com/legacy/library/documentation/Darwin/Reference/Ma...

Antony Polukhin wrote:

2016-12-18 20:26 GMT+03:00 Peter Dimov :

...

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?

The majority of users will just construct a stacktrace high-level object, as today, which will use this in the constructor to capture. But people who'd rather go lower level can use this directly, and people who'd rather go even lower can capture a stack trace via other means, without depending on Stacktrace for the capture part. throw_exception, for instance, may reimplement the capture part, because it doesn't need to resolve.

...

// 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?

You can check for filesize < 2. Or for fnsize < 2.

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

There are different levels of unsafety in practice, even though purists may claim that undefined is undefined and that's that. There's a whole spectrum from "almost always works" to "rarely works, if ever". If we're going to play the pedantry game, everything after a crash is already undefined behavior anyway. In practice, most important is to avoid the C/C++ runtime not just because this or that is not documented async-safe, but because a crash may be a symptom of random memory damage. Holding the loader lock is pretty much a guaranteed loss, apart from that, I'd estimate our chances as above average.

I'd rather take the boost/winapi path.

That's an option but, frankly, a separately compiled library is not that problematic, as long as you avoid autolinking it unless needed. That is, a simple capture should not attempt to link it. This is why I thought separating the capture and resolution into two headers worthwhile.

Gavin Lambert wrote:

But I don't think there should be any requirement to make resolving a trace be async-safe. There's rarely going to be anything useful you could do with such a trace that would also be async-safe; it'd be better to require that this be done in regular context instead.

::write is async-safe. You can write it to a file or stderr or a pipe or a socket.

Gavin Lambert wrote:

On 19/12/2016 11:57, Peter Dimov wrote:

...

::write is async-safe. You can write it to a file or stderr or a pipe or a socket.

Yes, but most application code would want to do that via a common logging library or something which probably isn't async-safe.

I'm not saying it couldn't be done, just that it increases the risk that application writers will do it wrong, like they've been getting signal handlers wrong for many years as it is.

Yeah, well, some/many will get it wrong, but how does it increase the risk? Currently the risk stands at 100% getting it wrong.

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/

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.

On 16/12/2016 02:39, Peter Dimov wrote:

Gavin Lambert wrote:

...

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

This file didn't exist at the time boost/throw_exception.hpp was written.

I know that; I'm just saying that perhaps adding such a macro there (simpler than throw_exception) might address Robert's concerns. I'm not personally worried about it though.

2016-12-15 5:15 GMT+03:00 Nat Goodspeed :

On Wed, Dec 14, 2016 at 2:42 PM, Nat Goodspeed 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#...

* 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

On 15/12/2016 19:52, Antony Polukhin wrote:

2016-12-14 22:42 GMT+03:00 Nat Goodspeed :

...

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.