Review Manager needed for stacktrace library
Hello, Stacktrace is a C++98 library for storing and printing backtraces. Supports major platforms, does demangling, has API for accessing individual frames, supports hashing and comparison operators. Library: https://github.com/apolukhin/stacktrace Docs: http://apolukhin.github.io/stacktrace/index.html Anyone wishing to become the Review Manager for the library? -- Best regards, Antony Polukhin
Hello, I have added Stacktrace to the review schedule. Best, Ron
On Oct 13, 2016, at 11:22 PM, Antony Polukhin <antoshkka@gmail.com> wrote:
Hello,
Stacktrace is a C++98 library for storing and printing backtraces.
Supports major platforms, does demangling, has API for accessing individual frames, supports hashing and comparison operators.
Library: https://github.com/apolukhin/stacktrace Docs: http://apolukhin.github.io/stacktrace/index.html
Anyone wishing to become the Review Manager for the library?
-- Best regards, Antony Polukhin
_______________________________________________ Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
On Fri, Oct 14, 2016 at 2:22 AM, Antony Polukhin <antoshkka@gmail.com> wrote: Stacktrace is a C++98 library for storing and printing backtraces.
Supports major platforms, does demangling, has API for accessing individual frames, supports hashing and comparison operators.
Library: https://github.com/apolukhin/stacktrace Docs: http://apolukhin.github.io/stacktrace/index.html
Anyone wishing to become the Review Manager for the library?
Maybe? I already have a couple requests. I do love the idea of a portable facility to capture and/or display stack traces at any desired point in a running program. Seems way overdue... It would be even more valuable if line number and source file information were available. I recognize that you just can't get that under all circumstances -- but maybe you could document the per-platform circumstances in which that *could* be retrieved? If function name, source file and line number were (conditionally) available, I'd like to be able to retrieve an individual entry that has accessors returning each of those things -- instead of operator[] returning only a std::string for function name. Perhaps the source file and line number would have to be wrapped in boost::optional. I suggest an object with accessors returning those things, rather than a struct, so no source code will come to rely on the size or specific data layout of the returned entry. That should hopefully permit extension, if there were ever still *more* information available. (In the Glorious Future, perhaps we could retrieve function parameter values or something.) Each entry should support streaming to ostream to get reasonable formatting in the presence or absence of source file and line number information. The overall stacktrace ostream operator would rely on the entry ostream operator. And finally, I'd like both begin()/end() and rbegin()/rend(). size() and operator[] are great, but they don't support range for. I hope all the above sounds reasonable...
2016-10-24 4:09 GMT+03:00 Nat Goodspeed <nat@lindenlab.com>:
On Fri, Oct 14, 2016 at 2:22 AM, Antony Polukhin <antoshkka@gmail.com> wrote: <...>
Library: https://github.com/apolukhin/stacktrace Docs: http://apolukhin.github.io/stacktrace/index.html
Anyone wishing to become the Review Manager for the library?
Maybe? I already have a couple requests.
Thank you! But Niall already agreed to be the Review Manager: http://www.boost.org/community/review_schedule.html
I do love the idea of a portable facility to capture and/or display stack traces at any desired point in a running program. Seems way overdue...
It would be even more valuable if line number and source file information were available. I recognize that you just can't get that under all circumstances -- but maybe you could document the per-platform circumstances in which that *could* be retrieved?
The idea is good and I'm definitely going to experiment with that for Windows platform and for the Linux platforms soon. But I'm afraid that it may lead to bigger backtraces that do not fit onto screen and are harder to read.
If function name, source file and line number were (conditionally) available, I'd like to be able to retrieve an individual entry that has accessors returning each of those things -- instead of operator[] returning only a std::string for function name. Perhaps the source file and line number would have to be wrapped in boost::optional.
I'm writing stacktrace library as a prototype for further adoption into the C++ Standard (I've got big ambitions and groundless optimism). Additional dependency to std::optional could make the adoption harder.
I suggest an object with accessors returning those things, rather than a struct, so no source code will come to rely on the size or specific data layout of the returned entry. That should hopefully permit extension, if there were ever still *more* information available. (In the Glorious Future, perhaps we could retrieve function parameter values or something.)
Each entry should support streaming to ostream to get reasonable formatting in the presence or absence of source file and line number information. The overall stacktrace ostream operator would rely on the entry ostream operator.
I've tried to walk that way. Unfortunately outputs differ a lot depending on the backtrace backend. For example instruction address may be reprsented as: * absolute address for currently loaded executable * offset from the stack frame * offset from the section in the binary It's hard to unify even those. More problems will arise with source files and line numbers. May be you'd like a different solution? What for are you willing to have such precise control?
And finally, I'd like both begin()/end() and rbegin()/rend(). size() and operator[] are great, but they don't support range for.
Sounds right. I'll add iterators and begin,end,rbegin,rend. Great thanks! -- Best regards, Antony Polukhin
On 10/24/2016 02:05 PM, Antony Polukhin wrote: >> I do love the idea of a portable facility to capture and/or display stack >> traces at any desired point in a running program. Seems way overdue... >> >> It would be even more valuable if line number and source file information >> were available. I recognize that you just can't get that under all >> circumstances -- but maybe you could document the per-platform >> circumstances in which that *could* be retrieved? > The idea is good and I'm definitely going to experiment with that for > Windows platform and for the Linux platforms soon. But I'm afraid that > it may lead to bigger backtraces that do not fit onto screen and are > harder to read. >> I suggest an object with accessors returning those things, rather than a >> struct, so no source code will come to rely on the size or specific data >> layout of the returned entry. That should hopefully permit extension, if >> there were ever still *more* information available. (In the Glorious >> Future, perhaps we could retrieve function parameter values or something.) >> >> Each entry should support streaming to ostream to get reasonable formatting >> in the presence or absence of source file and line number information. The >> overall stacktrace ostream operator would rely on the entry ostream >> operator. > I've tried to walk that way. Unfortunately outputs differ a lot > depending on the backtrace backend. For example instruction address > may be reprsented as: > * absolute address for currently loaded executable > * offset from the stack frame > * offset from the section in the binary > > It's hard to unify even those. More problems will arise with source > files and line numbers. May be you'd like a different solution? What > for are you willing to have such precise control? I'll just chime in and agree with all of Nat's suggestions. stacktrace would be an excellent addition to the standard library and I can think of a lot of times in the past where I wished I had it on the shelf already. I do agree though that file/line information is critical (although I appreciate that it's not always available). I think it would definitely be worth your time to add that functionality, even if it's only on some platforms (like Linux). Jason
What about giving access to the list of ids, RVAs relative virtual address? This way this could be saved in the case of a release build that doesn't have the debug information and then paired external to the program with pdb/dwarf file to reconstruct the stack trace? ________________________________ From: Boost <boost-bounces@lists.boost.org> on behalf of Jason Roehm <jasonr@3db-labs.com> Sent: Monday, October 24, 2016 2:18:24 PM To: boost@lists.boost.org Subject: Re: [boost] Review Manager needed for stacktrace library On 10/24/2016 02:05 PM, Antony Polukhin wrote: >> I do love the idea of a portable facility to capture and/or display stack >> traces at any desired point in a running program. Seems way overdue... >> >> It would be even more valuable if line number and source file information >> were available. I recognize that you just can't get that under all >> circumstances -- but maybe you could document the per-platform >> circumstances in which that *could* be retrieved? > The idea is good and I'm definitely going to experiment with that for > Windows platform and for the Linux platforms soon. But I'm afraid that > it may lead to bigger backtraces that do not fit onto screen and are > harder to read. >> I suggest an object with accessors returning those things, rather than a >> struct, so no source code will come to rely on the size or specific data >> layout of the returned entry. That should hopefully permit extension, if >> there were ever still *more* information available. (In the Glorious >> Future, perhaps we could retrieve function parameter values or something.) >> >> Each entry should support streaming to ostream to get reasonable formatting >> in the presence or absence of source file and line number information. The >> overall stacktrace ostream operator would rely on the entry ostream >> operator. > I've tried to walk that way. Unfortunately outputs differ a lot > depending on the backtrace backend. For example instruction address > may be reprsented as: > * absolute address for currently loaded executable > * offset from the stack frame > * offset from the section in the binary > > It's hard to unify even those. More problems will arise with source > files and line numbers. May be you'd like a different solution? What > for are you willing to have such precise control? I'll just chime in and agree with all of Nat's suggestions. stacktrace would be an excellent addition to the standard library and I can think of a lot of times in the past where I wished I had it on the shelf already. I do agree though that file/line information is critical (although I appreciate that it's not always available). I think it would definitely be worth your time to add that functionality, even if it's only on some platforms (like Linux). Jason _______________________________________________ Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
On Mon, Oct 24, 2016 at 2:05 PM, Antony Polukhin <antoshkka@gmail.com> wrote:
2016-10-24 4:09 GMT+03:00 Nat Goodspeed <nat@lindenlab.com>:
On Fri, Oct 14, 2016 at 2:22 AM, Antony Polukhin <antoshkka@gmail.com> wrote:
It would be even more valuable if line number and source file information were available. I recognize that you just can't get that under all circumstances -- but maybe you could document the per-platform circumstances in which that *could* be retrieved?
The idea is good and I'm definitely going to experiment with that for Windows platform and for the Linux platforms soon. But I'm afraid that it may lead to bigger backtraces that do not fit onto screen and are harder to read.
Hence the desire to be able to retrieve individual components of a stacktrace entry. A consumer could extract just the parts s/he cares about. I do want operator<<(std::ostream&, const stacktrace_entry&) to provide default formatting, but being able to extract individual fields (without parsing your output!) would allow a consumer to format in whatever way s/he needs.
If function name, source file and line number were (conditionally) available, I'd like to be able to retrieve an individual entry that has accessors returning each of those things -- instead of operator[] returning only a std::string for function name. Perhaps the source file and line number would have to be wrapped in boost::optional.
I'm writing stacktrace library as a prototype for further adoption into the C++ Standard (I've got big ambitions and groundless optimism). Additional dependency to std::optional could make the adoption harder.
Okay, that would be a reason to avoid std::optional. You could define a distinguished value to mean "not available."
I suggest an object with accessors returning those things, rather than a struct, so no source code will come to rely on the size or specific data layout of the returned entry. That should hopefully permit extension, if there were ever still *more* information available. (In the Glorious Future, perhaps we could retrieve function parameter values or something.)
Each entry should support streaming to ostream to get reasonable formatting in the presence or absence of source file and line number information. The overall stacktrace ostream operator would rely on the entry ostream operator.
I've tried to walk that way. Unfortunately outputs differ a lot depending on the backtrace backend. For example instruction address may be reprsented as: * absolute address for currently loaded executable * offset from the stack frame * offset from the section in the binary
It's hard to unify even those.
Well, to be honest, I doubt I care much about the instruction address per se -- only the line number in the source file that it indicates. But the wrapper layer for each backend could "do the right thing" in extracting the instruction in whatever common form you decide, could it not?
More problems will arise with source files and line numbers. May be you'd like a different solution? What for are you willing to have such precise control?
As I said, I want to be able to extract particular pieces of information without having to maintain backend-specific parsing of the string you emit. Ultimately, of course, it's all for human consumption, but you can't know the format I will consider ideal. It's great to be able to extract a single stacktrace entry, and it's good to have a "good enough" default formatting, but I'm just telling you what I would like even better. ;-)
2016-10-24 21:49 GMT+03:00 Nat Goodspeed <nat@lindenlab.com>:
On Mon, Oct 24, 2016 at 2:05 PM, Antony Polukhin <antoshkka@gmail.com> wrote: <...>
More problems will arise with source files and line numbers. May be you'd like a different solution? What for are you willing to have such precise control?
As I said, I want to be able to extract particular pieces of information without having to maintain backend-specific parsing of the string you emit.
Ultimately, of course, it's all for human consumption, but you can't know the format I will consider ideal.
It's great to be able to extract a single stacktrace entry, and it's good to have a "good enough" default formatting, but I'm just telling you what I would like even better. ;-)
Started implementing that: added begin/end functions, iterator and class frame. Is it ok to have a nonmovable type for a stack frame? -- Best regards, Antony Polukhin
2016-10-25 22:00 GMT+03:00 Nat Goodspeed <nat@lindenlab.com>:
On Tue, Oct 25, 2016 at 2:57 PM, Antony Polukhin <antoshkka@gmail.com> wrote:
Started implementing that: added begin/end functions, iterator and class frame. Is it ok to have a nonmovable type for a stack frame?
Could it be move-only?
Yes, but that will make the situation worse :( Class stacktrace does not hold actual `frame` instances, it holds a raw pointers or some high-level stuff that has different size depending on the backtrace backend. If I make the `frame` class copyable, it will either * break the ability to disable stacktraces without recompilation; will not allow to change backends transparently * lead to nonintuitive situation, that `frame` is just a reference to actual frame data that is kept inside stcktrace class. So a copy of `frame` becomes invalid after the stacktrace destruction. I'd rather avoid such design. -- Best regards, Antony Polukhin
On Tue, Oct 25, 2016 at 3:14 PM, Antony Polukhin <antoshkka@gmail.com> wrote:
2016-10-25 22:00 GMT+03:00 Nat Goodspeed <nat@lindenlab.com>:
On Tue, Oct 25, 2016 at 2:57 PM, Antony Polukhin <antoshkka@gmail.com> wrote:
Is it ok to have a nonmovable type for a stack frame?
Could it be move-only?
So a copy of `frame` becomes invalid after the stacktrace destruction.
Maybe a frame should be a weak_ptr<internal_class>, accessed with pointer semantics?
2016-10-25 22:17 GMT+03:00 Nat Goodspeed <nat@lindenlab.com>:
On Tue, Oct 25, 2016 at 3:14 PM, Antony Polukhin <antoshkka@gmail.com> wrote:
2016-10-25 22:00 GMT+03:00 Nat Goodspeed <nat@lindenlab.com>:
On Tue, Oct 25, 2016 at 2:57 PM, Antony Polukhin <antoshkka@gmail.com> wrote:
Is it ok to have a nonmovable type for a stack frame?
Could it be move-only?
So a copy of `frame` becomes invalid after the stacktrace destruction.
Maybe a frame should be a weak_ptr<internal_class>, accessed with pointer semantics?
Memory allocation will happen for each backend in that case, so users would like to specify allocators... and that's what I'd like to avoid even more than dangling references. -- Best regards, Antony Polukhin
On Tue, Oct 25, 2016 at 3:19 PM, Antony Polukhin <antoshkka@gmail.com> wrote:
2016-10-25 22:00 GMT+03:00 Nat Goodspeed <nat@lindenlab.com>:
On Tue, Oct 25, 2016 at 2:57 PM, Antony Polukhin <antoshkka@gmail.com> wrote:
Is it ok to have a nonmovable type for a stack frame?
Could it be move-only?
Memory allocation will happen for each backend in that case, so users would like to specify allocators... and that's what I'd like to avoid even more than dangling references.
So okay, propose the frame type as immovable and let's see how reviewers respond. I think it would be good if your documentation explains the reasons for this decision: the tradeoffs you rejected.
2016-10-25 22:28 GMT+03:00 Nat Goodspeed <nat@lindenlab.com>:
On Tue, Oct 25, 2016 at 3:19 PM, Antony Polukhin <antoshkka@gmail.com> wrote:
2016-10-25 22:00 GMT+03:00 Nat Goodspeed <nat@lindenlab.com>:
On Tue, Oct 25, 2016 at 2:57 PM, Antony Polukhin <antoshkka@gmail.com> wrote:
Is it ok to have a nonmovable type for a stack frame?
Could it be move-only?
Memory allocation will happen for each backend in that case, so users would like to specify allocators... and that's what I'd like to avoid even more than dangling references.
So okay, propose the frame type as immovable and let's see how reviewers respond. I think it would be good if your documentation explains the reasons for this decision: the tradeoffs you rejected.
Good point! I'll polish the implementation and docs soon. -- Best regards, Antony Polukhin
On 26/10/2016 08:14, Antony Polukhin wrote:
Could it be move-only?
Yes, but that will make the situation worse :( Class stacktrace does not hold actual `frame` instances, it holds a raw pointers or some high-level stuff that has different size depending on the backtrace backend. If I make the `frame` class copyable, it will either * break the ability to disable stacktraces without recompilation; will not allow to change backends transparently * lead to nonintuitive situation, that `frame` is just a reference to actual frame data that is kept inside stcktrace class. So a copy of `frame` becomes invalid after the stacktrace destruction.
Not sure if this helps or hinders, but these are the things I would consider to be most important in a stacktrace library: 1. Capturing the current state/trace should be as fast as possible. 2. This captured context should be able to be copied/moved into an exception or into a container and passed to a different thread for later use. (It is reasonable to only be able to do so for the context as a whole, not individual frames, although it's also useful to be able to "trim" the captured context before storing it (eg. omit the first few frames which are inside stack-trace-capturing methods rather than places of interest).) 3. In this secondary location it should be possible to symbolise the stack trace (ie. turn raw addresses into function/source/line info). And in particular this should be done only on such request, not at the original point of capture (to make that faster). 4. It would be useful, though not essential, to have a serialisation format such that the context from #1 can be saved to a file and a separate utility on a separate machine can be used to do #3 (similar to addr2line, but better). It's reasonable to require that the original binaries and symbol files are available to perform the symbol decoding, and it's expected that the serialisation format would have to include not just the trace addresses but also the module load addresses, since they might have loaded in different locations than their binary headers specify, especially on machines with ASLR enabled. (re: #1, I'm thinking in part of the ETfW sampling, which is able to capture stack traces from all threads every millisecond and every context switch, among other events, without noticeably slowing down even a high-performance application. Comes in surprisingly handy at times to have capturing that fast, even when doing it more ad hoc rather than sampling.)
2016-10-26 1:13 GMT+03:00 Gavin Lambert <gavinl@compacsort.com>:
On 26/10/2016 08:14, Antony Polukhin wrote:
Could it be move-only?
Yes, but that will make the situation worse :( Class stacktrace does not hold actual `frame` instances, it holds a raw pointers or some high-level stuff that has different size depending on the backtrace backend. If I make the `frame` class copyable, it will either * break the ability to disable stacktraces without recompilation; will not allow to change backends transparently * lead to nonintuitive situation, that `frame` is just a reference to actual frame data that is kept inside stcktrace class. So a copy of `frame` becomes invalid after the stacktrace destruction.
Not sure if this helps or hinders, but these are the things I would consider to be most important in a stacktrace library:
1. Capturing the current state/trace should be as fast as possible.
Already done. + stack capturing is noexcept. Thou not all the backends have great performance. I'll improve the docs by highlighting the performance differences for backends.
2. This captured context should be able to be copied/moved into an exception or into a container and passed to a different thread for later use. (It is reasonable to only be able to do so for the context as a whole, not individual frames, although it's also useful to be able to "trim" the captured context before storing it (eg. omit the first few frames which are inside stack-trace-capturing methods rather than places of interest).)
Already done. + copy constructor is noexcept.
3. In this secondary location it should be possible to symbolise the stack trace (ie. turn raw addresses into function/source/line info). And in particular this should be done only on such request, not at the original point of capture (to make that faster).
Already done for function names, source and line info will hopefully come soon.
4. It would be useful, though not essential, to have a serialisation format such that the context from #1 can be saved to a file and a separate utility on a separate machine can be used to do #3 (similar to addr2line, but better). It's reasonable to require that the original binaries and symbol files are available to perform the symbol decoding, and it's expected that the serialisation format would have to include not just the trace addresses but also the module load addresses, since they might have loaded in different locations than their binary headers specify, especially on machines with ASLR enabled.
That's interesting! I'll make it possible to have that functionality by adding an get_address() function to the frame class. I'll try to implement that and provide an example in docs soon. Great thanks!
(re: #1, I'm thinking in part of the ETfW sampling, which is able to capture stack traces from all threads every millisecond and every context switch, among other events, without noticeably slowing down even a high-performance application. Comes in surprisingly handy at times to have capturing that fast, even when doing it more ad hoc rather than sampling.)
Not sure that the library is meant for that: such functionality requires additional care, permissions and probably special compiler support. -- Best regards, Antony Polukhin
Polished the code, changed the API a little bit again, added more docs. Thanks to all of you, especially Nat Goodspeed, for good comments. Now the library looks mature! Any other comments are welcomed! -- Best regards, Antony Polukhin
On 26/10/2016 19:14, Antony Polukhin wrote:
(re: #1, I'm thinking in part of the ETfW sampling, which is able to capture stack traces from all threads every millisecond and every context switch, among other events, without noticeably slowing down even a high-performance application. Comes in surprisingly handy at times to have capturing that fast, even when doing it more ad hoc rather than sampling.)
Not sure that the library is meant for that: such functionality requires additional care, permissions and probably special compiler support.
I wasn't meaning to imply that it would, just trying to illustrate that capturing traces can be really fast, especially if symbolising them is deferred until later. I'll be interested to see that backend performance information when it's ready (no hurry though).
Antony Polukhin wrote:
I'm writing stacktrace library as a prototype for further adoption into the C++ Standard (I've got big ambitions and groundless optimism). Additional dependency to std::optional could make the adoption harder.
In what way could a dependency on std::optional make the adoption harder? You can't get stacktrace into C++14, can you? C++20 is the earliest possible. And C++17 already has optional. (Of course you could always return pointers instead.)
2016-10-24 22:04 GMT+03:00 Peter Dimov <lists@pdimov.com>:
Antony Polukhin wrote:
I'm writing stacktrace library as a prototype for further adoption into the C++ Standard (I've got big ambitions and groundless optimism). Additional dependency to std::optional could make the adoption harder.
In what way could a dependency on std::optional make the adoption harder? You can't get stacktrace into C++14, can you? C++20 is the earliest possible. And C++17 already has optional.
I've heard rumors a long time ago that std::type_info has no method returning demngled name because that method will add dependency to <string> header for <typeinfo> header. So I prefer to depend on a minimal subset of headers. -- Best regards, Antony Polukhin
Antony Polukhin wrote:
I've heard rumors a long time ago that std::type_info has no method returning demngled name because that method will add dependency to <string> header for <typeinfo> header. So I prefer to depend on a minimal subset of headers.
type_info instances are generated by the compiler, so this header is a special case. Either way, the original request was for file/line info, right? const char* file() const can return NULL, int line() const can return 0 or -1, when not present. There's no need for optional here.
On 24 Oct 2016 at 21:05, Antony Polukhin wrote:
I do love the idea of a portable facility to capture and/or display stack traces at any desired point in a running program. Seems way overdue...
It would be even more valuable if line number and source file information were available. I recognize that you just can't get that under all circumstances -- but maybe you could document the per-platform circumstances in which that *could* be retrieved?
The idea is good and I'm definitely going to experiment with that for Windows platform and for the Linux platforms soon. But I'm afraid that it may lead to bigger backtraces that do not fit onto screen and are harder to read.
I haven't done a deep dive into Backtrace yet, I'm hoping this weekend. But the lack of iterators problem is one I'd already noticed. I was also going to propose lazy symbolisation, so only if the user specifically asks for source code file or line number is llvm-symbolizer (or the equivalent for some platform) invoked per stack frame. That implies a dependency on Boost.Process, but Boost-lite actually has a lightweight zero dependency embedded process manager which like all Boost-lite libraries is drag and drop in friendly. I was going to suggest embedding a copy of that.
If function name, source file and line number were (conditionally) available, I'd like to be able to retrieve an individual entry that has accessors returning each of those things -- instead of operator[] returning only a std::string for function name. Perhaps the source file and line number would have to be wrapped in boost::optional.
I'm writing stacktrace library as a prototype for further adoption into the C++ Standard (I've got big ambitions and groundless optimism). Additional dependency to std::optional could make the adoption harder.
It's also a pointless optimisation. Reliable symbolisation on some platforms is a very, very heavy overhead, it involves launching a whole new process, waiting for it to complete and parsing its output. A std::string for the source file name which is null until lazy filled is plenty. BTW Antony are you planning for Backtrace to allow portable runtime *modification* of stack unwinds? Every C++ implementation has an API to do that in its runtime, it's just not normally exposed to the running program. I've been a long time advocate that it ought to be, mainly because of the raw undiluted untrammeled power for potential abuse it hands to the programmer (yes, yes, I really can handle the power, and no, I really really promise to not do evil ... :). Many years ago I tried to persuade WG14 to standardise that API as it's generally in the major C compilers too, but they refused on the basis that such a facility would be horribly abused and it must not be encouraged. Which sounds magic to my ears. Niall -- ned Productions Limited Consulting http://www.nedproductions.biz/ http://ie.linkedin.com/in/nialldouglas/
participants (8)
-
Antony Polukhin -
Gavin Lambert -
Jarrad Waterloo -
Jason Roehm -
Nat Goodspeed -
Niall Douglas -
Peter Dimov -
Ronald Garcia