Author: [email protected]
Date: Tue Mar 3 03:56:44 2009
New Revision: 1404
Modified:
branches/bleeding_edge/src/frames-inl.h
branches/bleeding_edge/src/frames.cc
branches/bleeding_edge/src/frames.h
branches/bleeding_edge/src/log.cc
branches/bleeding_edge/src/log.h
branches/bleeding_edge/src/platform.h
branches/bleeding_edge/src/top.cc
branches/bleeding_edge/test/cctest/test-log-ia32.cc
Log:
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.
Review URL: http://codereview.chromium.org/39009
Modified: branches/bleeding_edge/src/frames-inl.h
==============================================================================
--- branches/bleeding_edge/src/frames-inl.h (original)
+++ branches/bleeding_edge/src/frames-inl.h Tue Mar 3 03:56:44 2009
@@ -169,7 +169,8 @@
}
-inline JavaScriptFrame* JavaScriptFrameIterator::frame() const {
+template<typename Iterator>
+inline JavaScriptFrame* JavaScriptFrameIteratorTemp<Iterator>::frame()
const {
// TODO(1233797): The frame hierarchy needs to change. It's
// problematic that we can't use the safe-cast operator to cast to
// the JavaScript frame type, because we may encounter arguments
@@ -177,6 +178,39 @@
StackFrame* frame = iterator_.frame();
ASSERT(frame->is_java_script() || frame->is_arguments_adaptor());
return static_cast<JavaScriptFrame*>(frame);
+}
+
+
+template<typename Iterator>
+JavaScriptFrameIteratorTemp<Iterator>::JavaScriptFrameIteratorTemp(
+ StackFrame::Id id) {
+ while (!done()) {
+ Advance();
+ if (frame()->id() == id) return;
+ }
+}
+
+
+template<typename Iterator>
+void JavaScriptFrameIteratorTemp<Iterator>::Advance() {
+ do {
+ iterator_.Advance();
+ } while (!iterator_.done() && !iterator_.frame()->is_java_script());
+}
+
+
+template<typename Iterator>
+void JavaScriptFrameIteratorTemp<Iterator>::AdvanceToArgumentsFrame() {
+ if (!frame()->has_adapted_arguments()) return;
+ iterator_.Advance();
+ ASSERT(iterator_.frame()->is_arguments_adaptor());
+}
+
+
+template<typename Iterator>
+void JavaScriptFrameIteratorTemp<Iterator>::Reset() {
+ iterator_.Reset();
+ if (!done()) Advance();
}
Modified: branches/bleeding_edge/src/frames.cc
==============================================================================
--- branches/bleeding_edge/src/frames.cc (original)
+++ branches/bleeding_edge/src/frames.cc Tue Mar 3 03:56:44 2009
@@ -74,6 +74,11 @@
frame_(NULL), handler_(NULL), thread_(t) {
Reset();
}
+StackFrameIterator::StackFrameIterator(bool reset)
+ : STACK_FRAME_TYPE_LIST(INITIALIZE_SINGLETON)
+ frame_(NULL), handler_(NULL), thread_(Top::GetCurrentThread()) {
+ if (reset) Reset();
+}
#undef INITIALIZE_SINGLETON
@@ -131,32 +136,77 @@
//
-------------------------------------------------------------------------
-JavaScriptFrameIterator::JavaScriptFrameIterator(StackFrame::Id id) {
+StackTraceFrameIterator::StackTraceFrameIterator() {
+ if (!done() && !frame()->function()->IsJSFunction()) Advance();
+}
+
+
+void StackTraceFrameIterator::Advance() {
while (true) {
- Advance();
- if (frame()->id() == id) return;
+ JavaScriptFrameIterator::Advance();
+ if (done()) return;
+ if (frame()->function()->IsJSFunction()) return;
}
}
-void JavaScriptFrameIterator::Advance() {
- do {
+//
-------------------------------------------------------------------------
+
+
+SafeStackFrameIterator::SafeStackFrameIterator(
+ Address low_bound, Address high_bound) :
+ low_bound_(low_bound), high_bound_(high_bound),
+ is_working_iterator_(IsInBounds(low_bound, high_bound,
+
Top::c_entry_fp(Top::GetCurrentThread()))),
+ iteration_done_(!is_working_iterator_),
iterator_(is_working_iterator_) {
+}
+
+
+void SafeStackFrameIterator::Advance() {
+ ASSERT(is_working_iterator_);
+ ASSERT(!done());
+ StackFrame* frame = iterator_.frame();
+ iteration_done_ =
+ !IsGoodStackAddress(frame->sp()) || !IsGoodStackAddress(frame->fp());
+ if (!iteration_done_) {
iterator_.Advance();
- } while (!iterator_.done() && !iterator_.frame()->is_java_script());
+ if (!iterator_.done()) {
+ // Check that we have actually moved to the previous frame in the
stack
+ StackFrame* prev_frame = iterator_.frame();
+ iteration_done_ =
+ prev_frame->sp() < frame->sp() || prev_frame->fp() < frame->fp();
+ }
+ }
+}
+
+
+void SafeStackFrameIterator::Reset() {
+ if (is_working_iterator_) {
+ iterator_.Reset();
+ iteration_done_ = false;
+ }
}
-void JavaScriptFrameIterator::AdvanceToArgumentsFrame() {
- if (!frame()->has_adapted_arguments()) return;
- iterator_.Advance();
- ASSERT(iterator_.frame()->is_arguments_adaptor());
+//
-------------------------------------------------------------------------
+
+
+#ifdef ENABLE_LOGGING_AND_PROFILING
+SafeStackTraceFrameIterator::SafeStackTraceFrameIterator(
+ Address low_bound, Address high_bound) :
+ SafeJavaScriptFrameIterator(low_bound, high_bound) {
+ if (!done() && !frame()->function()->IsJSFunction()) Advance();
}
-void JavaScriptFrameIterator::Reset() {
- iterator_.Reset();
- Advance();
+void SafeStackTraceFrameIterator::Advance() {
+ while (true) {
+ SafeJavaScriptFrameIterator::Advance();
+ if (done()) return;
+ if (frame()->function()->IsJSFunction()) return;
+ }
}
+#endif
//
-------------------------------------------------------------------------
Modified: branches/bleeding_edge/src/frames.h
==============================================================================
--- branches/bleeding_edge/src/frames.h (original)
+++ branches/bleeding_edge/src/frames.h Tue Mar 3 03:56:44 2009
@@ -509,6 +509,9 @@
// An iterator that iterates over a given thread's stack.
explicit StackFrameIterator(ThreadLocalTop* thread);
+ // An iterator that conditionally resets itself on init.
+ explicit StackFrameIterator(bool reset);
+
StackFrame* frame() const {
ASSERT(!done());
return frame_;
@@ -542,16 +545,21 @@
// Iterator that supports iterating through all JavaScript frames.
-class JavaScriptFrameIterator BASE_EMBEDDED {
+template<typename Iterator>
+class JavaScriptFrameIteratorTemp BASE_EMBEDDED {
public:
- JavaScriptFrameIterator() { if (!done()) Advance(); }
+ JavaScriptFrameIteratorTemp() { if (!done()) Advance(); }
- explicit JavaScriptFrameIterator(ThreadLocalTop* thread) :
iterator_(thread) {
+ explicit JavaScriptFrameIteratorTemp(ThreadLocalTop* thread) :
+ iterator_(thread) {
if (!done()) Advance();
}
// Skip frames until the frame with the given id is reached.
- explicit JavaScriptFrameIterator(StackFrame::Id id);
+ explicit JavaScriptFrameIteratorTemp(StackFrame::Id id);
+
+ explicit JavaScriptFrameIteratorTemp(Address low_bound, Address
high_bound) :
+ iterator_(low_bound, high_bound) { if (!done()) Advance(); }
inline JavaScriptFrame* frame() const;
@@ -567,8 +575,66 @@
void Reset();
private:
+ Iterator iterator_;
+};
+
+
+typedef JavaScriptFrameIteratorTemp<StackFrameIterator>
JavaScriptFrameIterator;
+
+
+// NOTE: The stack trace frame iterator is an iterator that only
+// traverse proper JavaScript frames; that is JavaScript frames that
+// have proper JavaScript functions. This excludes the problematic
+// functions in runtime.js.
+class StackTraceFrameIterator: public JavaScriptFrameIterator {
+ public:
+ StackTraceFrameIterator();
+ void Advance();
+};
+
+
+class SafeStackFrameIterator BASE_EMBEDDED {
+ public:
+ explicit SafeStackFrameIterator(Address low_bound, Address high_bound);
+
+ StackFrame* frame() const {
+ ASSERT(is_working_iterator_);
+ return iterator_.frame();
+ }
+
+ bool done() const { return iteration_done_ ? true : iterator_.done(); }
+
+ void Advance();
+ void Reset();
+
+ private:
+ static bool IsInBounds(
+ Address low_bound, Address high_bound, Address addr) {
+ return low_bound <= addr && addr <= high_bound;
+ }
+ bool IsGoodStackAddress(Address addr) const {
+ return IsInBounds(low_bound_, high_bound_, addr);
+ }
+
+ Address low_bound_;
+ Address high_bound_;
+ const bool is_working_iterator_;
+ bool iteration_done_;
StackFrameIterator iterator_;
};
+
+
+#ifdef ENABLE_LOGGING_AND_PROFILING
+typedef JavaScriptFrameIteratorTemp<SafeStackFrameIterator>
+ SafeJavaScriptFrameIterator;
+
+
+class SafeStackTraceFrameIterator: public SafeJavaScriptFrameIterator {
+ public:
+ explicit SafeStackTraceFrameIterator(Address low_bound, Address
high_bound);
+ void Advance();
+};
+#endif
class StackFrameLocator BASE_EMBEDDED {
Modified: branches/bleeding_edge/src/log.cc
==============================================================================
--- branches/bleeding_edge/src/log.cc (original)
+++ branches/bleeding_edge/src/log.cc Tue Mar 3 03:56:44 2009
@@ -137,14 +137,38 @@
// StackTracer implementation
//
void StackTracer::Trace(TickSample* sample) {
- // Assuming that stack grows from lower addresses
- if (sample->state != GC
- && (sample->sp < sample->fp && sample->fp < low_stack_bound_)) {
+ if (sample->state == GC) {
+ sample->InitStack(0);
+ return;
+ }
+
+ // If c_entry_fp is available, this means that we are inside a C++
+ // function and sample->fp value isn't reliable due to FPO
+ if (Top::c_entry_fp(Top::GetCurrentThread()) != NULL) {
+ SafeStackTraceFrameIterator it(
+ reinterpret_cast<Address>(sample->sp),
+ reinterpret_cast<Address>(low_stack_bound_));
+ // Pass 1: Calculate depth
+ int depth = 0;
+ while (!it.done() && depth <= kMaxStackFrames) {
+ ++depth;
+ it.Advance();
+ }
+ // Pass 2: Save stack
+ sample->InitStack(depth);
+ if (depth > 0) {
+ it.Reset();
+ for (int i = 0; i < depth && !it.done(); ++i, it.Advance()) {
+ sample->stack[i] = it.frame()->pc();
+ }
+ }
+ } else if (sample->sp < sample->fp && sample->fp < low_stack_bound_) {
+ // The check assumes that stack grows from lower addresses
sample->InitStack(1);
sample->stack[0] = Memory::Address_at(
(Address)(sample->fp + StandardFrameConstants::kCallerPCOffset));
} else {
- // GC runs or FP seems to be in some intermediate state,
+ // FP seems to be in some intermediate state,
// better discard this sample
sample->InitStack(0);
}
Modified: branches/bleeding_edge/src/log.h
==============================================================================
--- branches/bleeding_edge/src/log.h (original)
+++ branches/bleeding_edge/src/log.h Tue Mar 3 03:56:44 2009
@@ -279,6 +279,9 @@
: low_stack_bound_(low_stack_bound) { }
void Trace(TickSample* sample);
private:
+ // Maximum number of stack frames to capture
+ static const int kMaxStackFrames = 5;
+
unsigned int low_stack_bound_;
};
Modified: branches/bleeding_edge/src/platform.h
==============================================================================
--- branches/bleeding_edge/src/platform.h (original)
+++ branches/bleeding_edge/src/platform.h Tue Mar 3 03:56:44 2009
@@ -481,9 +481,11 @@
}
inline void InitStack(int depth) {
- stack = SmartPointer<Address>(NewArray<Address>(depth + 1));
- // null-terminate
- stack[depth] = 0;
+ if (depth) {
+ stack = SmartPointer<Address>(NewArray<Address>(depth + 1));
+ // null-terminate
+ stack[depth] = 0;
+ }
}
};
Modified: branches/bleeding_edge/src/top.cc
==============================================================================
--- branches/bleeding_edge/src/top.cc (original)
+++ branches/bleeding_edge/src/top.cc Tue Mar 3 03:56:44 2009
@@ -668,26 +668,6 @@
}
-// NOTE: The stack trace frame iterator is an iterator that only
-// traverse proper JavaScript frames; that is JavaScript frames that
-// have proper JavaScript functions. This excludes the problematic
-// functions in runtime.js.
-class StackTraceFrameIterator: public JavaScriptFrameIterator {
- public:
- StackTraceFrameIterator() {
- if (!done() && !frame()->function()->IsJSFunction()) Advance();
- }
-
- void Advance() {
- while (true) {
- JavaScriptFrameIterator::Advance();
- if (done()) return;
- if (frame()->function()->IsJSFunction()) return;
- }
- }
-};
-
-
void Top::PrintCurrentStackTrace(FILE* out) {
StackTraceFrameIterator it;
while (!it.done()) {
Modified: branches/bleeding_edge/test/cctest/test-log-ia32.cc
==============================================================================
--- branches/bleeding_edge/test/cctest/test-log-ia32.cc (original)
+++ branches/bleeding_edge/test/cctest/test-log-ia32.cc Tue Mar 3 03:56:44
2009
@@ -43,7 +43,7 @@
static void DoTrace(unsigned int fp) {
trace_env.sample->fp = fp;
// something that is less than fp
- trace_env.sample->sp = trace_env.sample->fp - sizeof(unsigned int);
+ trace_env.sample->sp = trace_env.sample->fp - 100;
trace_env.tracer->Trace(trace_env.sample);
}
@@ -217,15 +217,18 @@
" JSFuncDoTrace();"
"};\n"
"JSTrace();");
+ CHECK_NE(0, *(sample.stack));
+ CheckRetAddrIsInFunction(
+ reinterpret_cast<unsigned int>(sample.stack[0]),
+ reinterpret_cast<unsigned int>(call_trace_code->instruction_start()),
+ call_trace_code->instruction_size());
Handle<JSFunction> js_trace(JSFunction::cast(*(v8::Utils::OpenHandle(
*GetGlobalProperty("JSTrace")))));
v8::internal::Code* js_trace_code = js_trace->code();
CheckRetAddrIsInFunction(
- reinterpret_cast<unsigned int>(sample.stack[0]),
+ reinterpret_cast<unsigned int>(sample.stack[1]),
reinterpret_cast<unsigned int>(js_trace_code->instruction_start()),
js_trace_code->instruction_size());
- CHECK_EQ(0, sample.stack[1]);
}
#endif // ENABLE_LOGGING_AND_PROFILING
-
--~--~---------~--~----~------------~-------~--~----~
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev
-~----------~----~----~----~------~----~------~--~---
