Dump more stack frames to perf log when executing a C++ function.

Dump more stack frames to perf log when executing a C++ function.

JavaScriptFrameIterator is templatized on the iterator type and renamed to JavaScriptFrameIteratorTemp.
The original JSFI is now a typedef for JavaScriptFrameIteratorTemp<StackFrameIterator>. Because of templatizing, JSFI code is moved to frames-inl.h

StackTraceFrameIterator moved to frames.*

Implemented SafeStackFrameIterator which wraps StackFrameIterator and have the same interface. It performs additional checks of stack addresses prior to delegating to StackFrameIterator. SafeSFI is used in an another specialization of JavaScriptFrameIteratorTemp template to perform safe JS frames iteration on sampler ticks.

I haven't took an advantage of having multiple stack frames in tickprocessor yet.

Committed: http://code.google.com/p/v8/source/detail?r=1404

[Mikhail Naganov, 2009-03-03 10:32:59]

Kasper, please look at this CL as Søren said that you wrote some of stack
unwinding code before.

[Søren Thygesen Gjesse, 2009-03-03 11:30:10]

LGTM

http://codereview.chromium.org/39009/diff/1001/1002
File src/frames-inl.h (right):

http://codereview.chromium.org/39009/diff/1001/1002#newcode187
Line 187: while (true) {
while (true) -> while(!done()).

http://codereview.chromium.org/39009/diff/1001/1004
File src/frames.h (right):

http://codereview.chromium.org/39009/diff/1001/1004#newcode627
Line 627: typedef JavaScriptFrameIteratorTemp<SafeStackFrameIterator>
SafeJSFrameIterator;
SafeJSFrameIterator -> SafeJavaScriptFrameIterator?

http://codereview.chromium.org/39009/diff/1001/1004#newcode629
Line 629: 
Add #ifdef ENABLE_LOGGING_AND_PROFILING for this class

http://codereview.chromium.org/39009/diff/1001/1004#newcode630
Line 630: class SafeStackTraceFrameIterator: public SafeJSFrameIterator {
Maybe remove the Safe prefix (there is no unsafe version)

http://codereview.chromium.org/39009/diff/1001/1005
File src/log.cc (right):

http://codereview.chromium.org/39009/diff/1001/1005#newcode147
Line 147: if (Top::c_entry_fp(Top::GetCurrentThread())) {
Please add != NULL

http://codereview.chromium.org/39009/diff/1001/1005#newcode153
Line 153: for (; !it.done() && depth <= kMaxStackFrames; ++depth, it.Advance());
Most V8 code uses a while loop for this type of code.

[Mikhail Naganov, 2009-03-03 11:54:41]

http://codereview.chromium.org/39009/diff/1001/1002
File src/frames-inl.h (right):

http://codereview.chromium.org/39009/diff/1001/1002#newcode187
Line 187: while (true) {
On 2009/03/03 11:30:11, Søren Gjesse wrote:
> while (true) -> while(!done()).

Done.

http://codereview.chromium.org/39009/diff/1001/1004
File src/frames.h (right):

http://codereview.chromium.org/39009/diff/1001/1004#newcode627
Line 627: typedef JavaScriptFrameIteratorTemp<SafeStackFrameIterator>
SafeJSFrameIterator;
On 2009/03/03 11:30:11, Søren Gjesse wrote:
> SafeJSFrameIterator -> SafeJavaScriptFrameIterator?

Done.

http://codereview.chromium.org/39009/diff/1001/1004#newcode629
Line 629: 
On 2009/03/03 11:30:11, Søren Gjesse wrote:
> Add #ifdef ENABLE_LOGGING_AND_PROFILING for this class

Done.

http://codereview.chromium.org/39009/diff/1001/1004#newcode630
Line 630: class SafeStackTraceFrameIterator: public SafeJSFrameIterator {
On 2009/03/03 11:30:11, Søren Gjesse wrote:
> Maybe remove the Safe prefix (there is no unsafe version)

Actually, there is one, see above around line 589. I moved it from top.cc to
keep related things together.

http://codereview.chromium.org/39009/diff/1001/1005
File src/log.cc (right):

http://codereview.chromium.org/39009/diff/1001/1005#newcode147
Line 147: if (Top::c_entry_fp(Top::GetCurrentThread())) {
On 2009/03/03 11:30:11, Søren Gjesse wrote:
> Please add != NULL

Done.

http://codereview.chromium.org/39009/diff/1001/1005#newcode153
Line 153: for (; !it.done() && depth <= kMaxStackFrames; ++depth, it.Advance());
On 2009/03/03 11:30:11, Søren Gjesse wrote:
> Most V8 code uses a while loop for this type of code.

Done.

[Kasper Lund, 2009-03-03 13:28:43]

I'm okay with this change even though I'm not completely convinced that the
template trick is superior to simply specializing an abstract iterator, but it
still LGTM.

http://codereview.chromium.org/39009/diff/1001/1003
File src/frames.cc (right):

http://codereview.chromium.org/39009/diff/1001/1003#newcode171
Line 171: if (!iteration_done_) {
How about inverting this and returning with an early bailout?

http://codereview.chromium.org/39009/diff/1001/1003#newcode173
Line 173: if (!iterator_.done()) {
Ditto.

http://codereview.chromium.org/39009/diff/1001/1004
File src/frames.h (right):

http://codereview.chromium.org/39009/diff/1001/1004#newcode561
Line 561: explicit JavaScriptFrameIteratorTemp(Address low_bound, Address
high_bound) :
Nit: Doesn't have to be explicit.

http://codereview.chromium.org/39009/diff/1001/1004#newcode611
Line 611: static bool IsInBounds(
Maybe IsWithinBounds?

http://codereview.chromium.org/39009/diff/1001/1004#newcode615
Line 615: bool IsGoodStackAddress(Address addr) const {
Maybe IsValidStackAddress?

http://codereview.chromium.org/39009/diff/1001/1006
File src/log.h (right):

http://codereview.chromium.org/39009/diff/1001/1006#newcode282
Line 282: // Maximum number of stack frames to capture
Terminate comment with .

http://codereview.chromium.org/39009/diff/1001/1007
File src/platform.h (right):

http://codereview.chromium.org/39009/diff/1001/1007#newcode484
Line 484: if (depth) {
depth != 0 (or maybe >)