[stacktrace] Two days remaining before review ends

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
10 messages Options
Reply | Threaded
Open this post in threaded view
|

[stacktrace] Two days remaining before review ends

Boost - Dev mailing list
Current summary of votes cast so far:

* Peter Dimov: Unconditional acceptance
* Vladimir Batov: Unconditional acceptance

Am I missing any?

If you are going to make a review and a recommendation, please do so soon.

Niall

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


_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
Reply | Threaded
Open this post in threaded view
|

Re: [stacktrace] Two days remaining before review ends

mbartosik
Maybe the author can answer the issues that I raised:

Here:
http://boost.2283326.n4.nabble.com/Stacktrace-review-comments-td4692904.html

* Control of how aggressive stack walking is.
* Ability to walk stack of another thread including in another process.
* boost::stacktrace::frame::to_string should be efficient for 2nd and 3rd and multiple calls, but not require more than a single pointer to be stored in the boost::stacktrace::frame. This implies either a different class for symbol resolution or external caching.
* Control over what boost::stacktrace::frame::to_string includes (or define it).
* Ability to work with post-mortem data. e.g. serialize a boost::stacktrace plus other information (a list of modules) and then deserialize and resolve symbols after the process has exited.
* Work with boost serialization

If I have a vote it would depend on the answers to these questions, without good answers it would be a no vote.
I'm getting on a flight soon, so I'll won't be able to comment more until Sunday.


Reply | Threaded
Open this post in threaded view
|

Re: [stacktrace] Two days remaining before review ends

Boost - Dev mailing list
2017-03-25 12:02 GMT+03:00 mbartosik via Boost <[hidden email]>:
> Maybe the author can answer the issues that I raised:
>
> Here:
> http://boost.2283326.n4.nabble.com/Stacktrace-review-comments-td4692904.html
>
> * Control of how aggressive stack walking is.

I know only one stack walking strategy - the least aggressive. It's
the user responsibility to make stack walking possible by building the
debug version of the library, or providing OS specific files, or
building with specific flags. Otherwise no stack will be collected.

> * Ability to walk stack of another thread including in another process.

You can collect stack in current thread and move/copy it to other
thread using interthread/inteprocess communications.

Walking the stack of a thread that you have no control of is out of
current library scope as there's no known to me portable ways to do
so. Any suggestions would be appreciated.


> * boost::stacktrace::frame::to_string should be efficient for 2nd and 3rd
> and multiple calls, but not require more than a single pointer to be stored
> in the boost::stacktrace::frame. This implies either a different class for
> symbol resolution or external caching.

No out of the box caching, but user can write code for external
caching. I'd appreciate suggestions on how to implement caching on a
library level but without new class additions.


> * Control over what boost::stacktrace::frame::to_string includes (or define
> it).

Interface of boost::stacktrace::frame provides all the functionality
for writing your own to_string() functions. Can not really understand
your request.


> * Ability to work with post-mortem data. e.g. serialize a boost::stacktrace
> plus other information (a list of modules) and then deserialize and resolve
> symbols after the process has exited.

boost::stacktrace::safe_dump_to functions implement that functionality.


> * Work with boost serialization

This must be done in Boost.Serialization library, not in the Stacktrace library.


> If I have a vote it would depend on the answers to these questions, without
> good answers it would be a no vote.
> I'm getting on a flight soon, so I'll won't be able to comment more until
> Sunday.

Thanks in advance for the review!


--
Best regards,
Antony Polukhin

_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
Reply | Threaded
Open this post in threaded view
|

Re: [stacktrace] Two days remaining before review ends

mbartosik
Address space layout randomization
How does boost stacktrace cope with ASLR (address space layout randomization)?
This is where the load address of a module is randomized by the operating system loader to at an unpredictable address to help protect from viruses using techniques like buffer overrun attacks.
The use case that I am concerned about is calling boost::stacktrace::safe_dump_to and later resolving the function names with boost::stacktrace::stacktrace::from_dump. Without the module load addresses (and the module paths) I do not see how the frame symbols can be resolved. I think that it is possible to address this by a future enhancement like having a process modules class possibly with a similar interface to boost stacktrace, and then to boost::stacktrace::stacktrace::from_dump might be extended take an additional argument of a 'process modules' class which has similarly been reconstructed.

Having said this since I understand your expectation is possibly not to use in production (as build flags need to be set) then this may not be so serious, for use in production I would consider it a serious problem. I would like to be able to use this in production with both ASLR enabled and with code that may have optimizations on that make the stack harder to trace (including third party code for which I have no control of build settings).

Aggressiveness in walking stack:
It is inevitable that developers will want to use this in production code likely a release build with the optimizer on. Thus I was hoping for an option to use more aggressive algorithms (which could not be used within signal handlers). Using only the least aggressive method will significantly limit the usefulness -- but probably better to have a 1.0 version that requires compiler options than nothing maybe a 2.0 version will be able to support more aggressive algorithms for walking the stack.

Use on non-current threads:
To collect a stack in a different thread I know is possible and portably because I've seen it done on most platforms (this is what a debugger does when it dumps the stack of a target process). This is why the ReadMemory argument in Microsoft's StackWalk64 API is provided. All that is required is that we know that the thread being walked is paused. The idea being that if a deadlock is detected stacks for multiple threads can be logged but without of course requiring the deadlocked threads to do the walking. Potentially if stack tracing of other processes where supported it could be used to write watch dog processes, and even generate a stack trace for a stack overflow error (which is notoriously difficult to code for).

Out of the box caching:
Without adding extra classes the best I have is to add global map of address to string/symbol info (possibly with pruning of least used elements) for use by frame::name etc.
The concern I have is thus:
  Let's say to_string for a single frame takes ~ 0.1 seconds.
  Let's say our stack trace is 10 frames deep.
  The to_string for the entire stack trace takes ~ 1 second.
If a developer adds logging for some apparently rare exception in the hope of diagnosing a problem, but in production under some circumstance that exception occurs maybe once in 2 seconds, then the code will spend half it's wall time just logging stack traces. Basically it is too easy to accidentally grind some code to a halt by adding a diagnostic - which I think it just too dangerous.

So my only advice / idea is to provide a global thread safe cache for symbolic information that is configurable either by functions in the namespace boost::stacktrace (preferred) or by build options.

Control over what frame::to_string / frame::name do
What I was thinking was whether there should be some user control over how the name for the frame is reported e.g.
bar(int), void bar(int), void namespace::bar(int), bar(), __stdcall void bar(int), ?bar@@YAH@Z
This is not something that I consider particularly important, it might be food for thought for you.


I know that this is quite a bit of comment and that the official review period is over (I was getting married plus honey moon while the review was on). I do think this is an great addition to have in boost.
- Mark

Reply | Threaded
Open this post in threaded view
|

Re: [stacktrace] Two days remaining before review ends

Boost - Dev mailing list
I have a question on stacktrace: is it possible to decouple the capture of
stacktraces from the conversion to a human-readable format? These are two
different use cases and while most programs need both, that is not true for
libraries.

Emil

_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
Reply | Threaded
Open this post in threaded view
|

Re: [stacktrace] Two days remaining before review ends

Boost - Dev mailing list
In reply to this post by mbartosik
2017-04-11 1:41 GMT+03:00 mbartosik via Boost <[hidden email]>:
> *Address space layout randomization*

There problem here is that boost::stacktrace::safe_dump_to must be
async-signal-safe but getting the ASLR independent offset often
requires calling dl* functions that are not async-signal-safe. I'll
try to investigate further, probably there's some trick that I'm not
aware of.

> *Use on non-current threads:*

This is currently out of scope of the library. Main reasons are
* portability - this looks extremely unportable
* safety - this looks extremely unsafe and may lead to deadlocks/terminates
* usefulness - does not look like many users want that feature


> To collect a stack in a different thread I know is possible and portably
> because I've seen it done on most platforms (this is what a debugger does
> when it dumps the stack of a target process).

Think of it as of a surgery. It is kind-of simple to make a surgery if
you are not doing it to yourself. So yes, free standing debugger
process can stop threads, place breakpoints, investigate variables,
modify code on the fly, change registers from other threads. But doing
that from the process to itself... not sure that computer science is
ready for the experience of Leonid Rogozov. At least not in the
version 1.0 of the Boost.Stacktrace.

> This is why the ReadMemory
> argument in Microsoft's StackWalk64 API is provided.

Yep, DbgHelp library has lethal amount of irony in it: StackWalk64 has
interface to do threads debugging, but can not do that because it is
located in a DbgHelp library that is single threaded.

> *Out of the box caching:*

Oh, you're talking about that kind of caching. There's a plan to allow
that using Thread Local Storage
https://github.com/apolukhin/stacktrace/issues/11
I'll definitely do that some day soon.

> *Control over what frame::to_string / frame::name do *
> What I was thinking was whether there should be some user control over how
> the name for the frame is reported e.g.
> bar(int), void bar(int), void namespace::bar(int), bar(), __stdcall void
> bar(int), ?bar@@YAH@Z
> This is not something that I consider particularly important, it might be
> food for thought for you.

Such customization must be applied to a low level
boost/core/demangle.hpp header first. As soon as such customization
appear in there, I'll add that to the stacktrace interface.
>
> I know that this is quite a bit of comment and that the official review
> period is over (I was getting married plus honey moon while the review was
> on). I do think this is an great addition to have in boost.

Thanks a lot for the comments! I really appreciate those, they help to
make the library better.

--
Best regards,
Antony Polukhin

_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
Reply | Threaded
Open this post in threaded view
|

Re: [stacktrace] Two days remaining before review ends

Boost - Dev mailing list
On 13/04/2017 07:02, Antony Polukhin via Boost wrote:
> Yep, DbgHelp library has lethal amount of irony in it: StackWalk64 has
> interface to do threads debugging, but can not do that because it is
> located in a DbgHelp library that is single threaded.

You can use it to debug any thread, single or multiple.  You just have
to do it only one thread at a time.  There's no irony at all, just a mutex.



_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
Reply | Threaded
Open this post in threaded view
|

Re: [stacktrace] Two days remaining before review ends

Boost - Dev mailing list
In reply to this post by Boost - Dev mailing list
2017-04-11 4:20 GMT+03:00 Emil Dotchevski via Boost <[hidden email]>:
> I have a question on stacktrace: is it possible to decouple the capture of
> stacktraces from the conversion to a human-readable format? These are two
> different use cases and while most programs need both, that is not true for
> libraries.

Done. Now if you include boost/stacktrace/stacktrace.hpp you would be
able to catch stacktraces but would not be able to decode it.
Now boost/stacktrace/stacktrace.hpp does not include windows.h, COM,
LexicalCast and other "heavy" Boost libraries


--
Best regards,
Antony Polukhin

_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
Reply | Threaded
Open this post in threaded view
|

Re: [stacktrace] Two days remaining before review ends

Boost - Dev mailing list
On Sat, Apr 22, 2017 at 5:41 AM, Antony Polukhin via Boost
<[hidden email]> wrote:

> 2017-04-11 4:20 GMT+03:00 Emil Dotchevski via Boost <[hidden email]>:
>> I have a question on stacktrace: is it possible to decouple the capture of
>> stacktraces from the conversion to a human-readable format? These are two
>> different use cases and while most programs need both, that is not true for
>> libraries.
>
> Done. Now if you include boost/stacktrace/stacktrace.hpp you would be
> able to catch stacktraces but would not be able to decode it.
> Now boost/stacktrace/stacktrace.hpp does not include windows.h, COM,
> LexicalCast and other "heavy" Boost libraries
>

Similarly, what I'd like is to grab a stacktrace, which is basically
just addresses,
then later match that up with, for example, pdb files, to get function
names etc.

When I push as task onto an async queue, I'd like to capture the
stacktrace of the code doing the push,
and include that with the task in the queue.
So when the thread that performs the task throws or crashes, I can log
"who" pushed that task.
Then, back at work, decode that log and say "here, it's your fault".

But the capture needs to be fast and small.  Thus just addresses.

Tony

_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
Reply | Threaded
Open this post in threaded view
|

Re: [stacktrace] Two days remaining before review ends

Boost - Dev mailing list
2017-04-27 8:55 GMT+03:00 Gottlob Frege via Boost <[hidden email]>:

> On Sat, Apr 22, 2017 at 5:41 AM, Antony Polukhin via Boost
> <[hidden email]> wrote:
>> 2017-04-11 4:20 GMT+03:00 Emil Dotchevski via Boost <[hidden email]>:
>>> I have a question on stacktrace: is it possible to decouple the capture of
>>> stacktraces from the conversion to a human-readable format? These are two
>>> different use cases and while most programs need both, that is not true for
>>> libraries.
>>
>> Done. Now if you include boost/stacktrace/stacktrace.hpp you would be
>> able to catch stacktraces but would not be able to decode it.
>> Now boost/stacktrace/stacktrace.hpp does not include windows.h, COM,
>> LexicalCast and other "heavy" Boost libraries
>>
>
> Similarly, what I'd like is to grab a stacktrace, which is basically
> just addresses,
> then later match that up with, for example, pdb files, to get function
> names etc.
>
> When I push as task onto an async queue, I'd like to capture the
> stacktrace of the code doing the push,
> and include that with the task in the queue.
> So when the thread that performs the task throws or crashes, I can log
> "who" pushed that task.
> Then, back at work, decode that log and say "here, it's your fault".
>
> But the capture needs to be fast and small.  Thus just addresses.

Yes. That's how the library works. Captures and stores only addresses,
decoding is on demand.


--
Best regards,
Antony Polukhin

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