Revision: 5467
Author: [email protected]
Date: Thu Sep 16 01:23:34 2010
Log: Enhance SafeStackFrameIterator to avoid triggering assertions in debug
mode.
When running profiling in debug mode, several assertions in frame
iterators that are undoubtedly useful when iterator is started from a
VM thread in a known "good" state, may fail when running over a stack
of a suspended VM thread. This patch makes SafeStackFrameIterator
to proactively check addresses and bail out from iteration early,
before an assertion will be triggered.
BUG=crbug/55565
Review URL: http://codereview.chromium.org/3436006
http://code.google.com/p/v8/source/detail?r=5467
Modified:
/branches/bleeding_edge/src/arm/frames-arm.cc
/branches/bleeding_edge/src/frames.cc
/branches/bleeding_edge/src/frames.h
/branches/bleeding_edge/src/ia32/frames-ia32.cc
/branches/bleeding_edge/src/log.cc
/branches/bleeding_edge/src/mips/frames-mips.cc
/branches/bleeding_edge/src/x64/frames-x64.cc
/branches/bleeding_edge/test/cctest/test-log-stack-tracer.cc
=======================================
--- /branches/bleeding_edge/src/arm/frames-arm.cc Mon Aug 30 01:54:43 2010
+++ /branches/bleeding_edge/src/arm/frames-arm.cc Thu Sep 16 01:23:34 2010
@@ -37,17 +37,8 @@
namespace internal {
-StackFrame::Type ExitFrame::GetStateForFramePointer(Address fp, State*
state) {
- if (fp == 0) return NONE;
- // Compute frame type and stack pointer.
- Address sp = fp + ExitFrameConstants::kSPOffset;
-
- // Fill in the state.
- state->sp = sp;
- state->fp = fp;
- state->pc_address = reinterpret_cast<Address*>(sp - 1 * kPointerSize);
- ASSERT(*state->pc_address != NULL);
- return EXIT;
+Address ExitFrame::ComputeStackPointer(Address fp) {
+ return fp + ExitFrameConstants::kSPOffset;
}
=======================================
--- /branches/bleeding_edge/src/frames.cc Tue Aug 31 05:20:22 2010
+++ /branches/bleeding_edge/src/frames.cc Thu Sep 16 01:23:34 2010
@@ -143,8 +143,8 @@
state.pc_address =
reinterpret_cast<Address*>(StandardFrame::ComputePCAddress(fp_));
type = StackFrame::ComputeType(&state);
- if (SingletonFor(type) == NULL) return;
- }
+ }
+ if (SingletonFor(type) == NULL) return;
frame_ = SingletonFor(type, &state);
}
@@ -203,18 +203,37 @@
//
-------------------------------------------------------------------------
+bool SafeStackFrameIterator::ExitFrameValidator::IsValidFP(Address fp) {
+ if (!validator_.IsValid(fp)) return false;
+ Address sp = ExitFrame::ComputeStackPointer(fp);
+ if (!validator_.IsValid(sp)) return false;
+ StackFrame::State state;
+ ExitFrame::FillState(fp, sp, &state);
+ if (!validator_.IsValid(reinterpret_cast<Address>(state.pc_address))) {
+ return false;
+ }
+ return *state.pc_address != NULL;
+}
+
+
SafeStackFrameIterator::SafeStackFrameIterator(
Address fp, Address sp, Address low_bound, Address high_bound) :
- maintainer_(), low_bound_(low_bound), high_bound_(high_bound),
- is_valid_top_(
- IsWithinBounds(low_bound, high_bound,
- Top::c_entry_fp(Top::GetCurrentThread())) &&
- Top::handler(Top::GetCurrentThread()) != NULL),
+ maintainer_(),
+ stack_validator_(low_bound, high_bound),
+ is_valid_top_(IsValidTop(low_bound, high_bound)),
is_valid_fp_(IsWithinBounds(low_bound, high_bound, fp)),
is_working_iterator_(is_valid_top_ || is_valid_fp_),
iteration_done_(!is_working_iterator_),
iterator_(is_valid_top_, is_valid_fp_ ? fp : NULL, sp) {
}
+
+
+bool SafeStackFrameIterator::IsValidTop(Address low_bound, Address
high_bound) {
+ Address fp = Top::c_entry_fp(Top::GetCurrentThread());
+ ExitFrameValidator validator(low_bound, high_bound);
+ if (!validator.IsValidFP(fp)) return false;
+ return Top::handler(Top::GetCurrentThread()) != NULL;
+}
void SafeStackFrameIterator::Advance() {
@@ -258,9 +277,8 @@
// sure that caller FP address is valid.
Address caller_fp = Memory::Address_at(
frame->fp() + EntryFrameConstants::kCallerFPOffset);
- if (!IsValidStackAddress(caller_fp)) {
- return false;
- }
+ ExitFrameValidator validator(stack_validator_);
+ if (!validator.IsValidFP(caller_fp)) return false;
} else if (frame->is_arguments_adaptor()) {
// See ArgumentsAdaptorFrame::GetCallerStackPointer. It assumes that
// the number of arguments is stored on stack as Smi. We need to check
@@ -413,6 +431,22 @@
Address ExitFrame::GetCallerStackPointer() const {
return fp() + ExitFrameConstants::kCallerSPDisplacement;
}
+
+
+StackFrame::Type ExitFrame::GetStateForFramePointer(Address fp, State*
state) {
+ if (fp == 0) return NONE;
+ Address sp = ComputeStackPointer(fp);
+ FillState(fp, sp, state);
+ ASSERT(*state->pc_address != NULL);
+ return EXIT;
+}
+
+
+void ExitFrame::FillState(Address fp, Address sp, State* state) {
+ state->sp = sp;
+ state->fp = fp;
+ state->pc_address = reinterpret_cast<Address*>(sp - 1 * kPointerSize);
+}
Address StandardFrame::GetExpressionAddress(int n) const {
=======================================
--- /branches/bleeding_edge/src/frames.h Mon Aug 30 01:54:43 2010
+++ /branches/bleeding_edge/src/frames.h Thu Sep 16 01:23:34 2010
@@ -202,6 +202,7 @@
protected:
struct State {
+ State() : sp(NULL), fp(NULL), pc_address(NULL) { }
Address sp;
Address fp;
Address* pc_address;
@@ -318,6 +319,8 @@
// pointer. Used when constructing the first stack frame seen by an
// iterator and the frames following entry frames.
static Type GetStateForFramePointer(Address fp, State* state);
+ static Address ComputeStackPointer(Address fp);
+ static void FillState(Address fp, Address sp, State* state);
protected:
explicit ExitFrame(StackFrameIterator* iterator) : StackFrame(iterator)
{ }
@@ -443,6 +446,7 @@
inline Object* function_slot_object() const;
friend class StackFrameIterator;
+ friend class StackTracer;
};
@@ -654,12 +658,36 @@
}
private:
+ class StackAddressValidator {
+ public:
+ StackAddressValidator(Address low_bound, Address high_bound)
+ : low_bound_(low_bound), high_bound_(high_bound) { }
+ bool IsValid(Address addr) const {
+ return IsWithinBounds(low_bound_, high_bound_, addr);
+ }
+ private:
+ Address low_bound_;
+ Address high_bound_;
+ };
+
+ class ExitFrameValidator {
+ public:
+ explicit ExitFrameValidator(const StackAddressValidator& validator)
+ : validator_(validator) { }
+ ExitFrameValidator(Address low_bound, Address high_bound)
+ : validator_(low_bound, high_bound) { }
+ bool IsValidFP(Address fp);
+ private:
+ StackAddressValidator validator_;
+ };
+
bool IsValidStackAddress(Address addr) const {
- return IsWithinBounds(low_bound_, high_bound_, addr);
+ return stack_validator_.IsValid(addr);
}
bool CanIterateHandles(StackFrame* frame, StackHandler* handler);
bool IsValidFrame(StackFrame* frame) const;
bool IsValidCaller(StackFrame* frame);
+ static bool IsValidTop(Address low_bound, Address high_bound);
// This is a nasty hack to make sure the active count is incremented
// before the constructor for the embedded iterator is invoked. This
@@ -674,8 +702,7 @@
ActiveCountMaintainer maintainer_;
static int active_count_;
- Address low_bound_;
- Address high_bound_;
+ StackAddressValidator stack_validator_;
const bool is_valid_top_;
const bool is_valid_fp_;
const bool is_working_iterator_;
=======================================
--- /branches/bleeding_edge/src/ia32/frames-ia32.cc Mon Aug 30 01:54:43 2010
+++ /branches/bleeding_edge/src/ia32/frames-ia32.cc Thu Sep 16 01:23:34 2010
@@ -35,16 +35,8 @@
namespace internal {
-StackFrame::Type ExitFrame::GetStateForFramePointer(Address fp, State*
state) {
- if (fp == 0) return NONE;
- // Compute the stack pointer.
- Address sp = Memory::Address_at(fp + ExitFrameConstants::kSPOffset);
- // Fill in the state.
- state->fp = fp;
- state->sp = sp;
- state->pc_address = reinterpret_cast<Address*>(sp - 1 * kPointerSize);
- ASSERT(*state->pc_address != NULL);
- return EXIT;
+Address ExitFrame::ComputeStackPointer(Address fp) {
+ return Memory::Address_at(fp + ExitFrameConstants::kSPOffset);
}
=======================================
--- /branches/bleeding_edge/src/log.cc Mon Aug 30 04:48:07 2010
+++ /branches/bleeding_edge/src/log.cc Thu Sep 16 01:23:34 2010
@@ -171,7 +171,9 @@
SafeStackTraceFrameIterator it(sample->fp, sample->sp,
sample->sp, js_entry_sp);
while (!it.done() && i < TickSample::kMaxFramesCount) {
- sample->stack[i++] = reinterpret_cast<Address>(it.frame()->function());
+ sample->stack[i++] =
+ reinterpret_cast<Address>(it.frame()->function_slot_object()) -
+ kHeapObjectTag;
it.Advance();
}
sample->frames_count = i;
=======================================
--- /branches/bleeding_edge/src/mips/frames-mips.cc Mon May 17 08:41:35 2010
+++ /branches/bleeding_edge/src/mips/frames-mips.cc Thu Sep 16 01:23:34 2010
@@ -52,9 +52,7 @@
}
-StackFrame::Type ExitFrame::GetStateForFramePointer(Address fp, State*
state) {
- if (fp == 0) return NONE;
- // Compute frame type and stack pointer.
+Address ExitFrame::ComputeStackPointer(Address fp) {
Address sp = fp + ExitFrameConstants::kSPDisplacement;
const int offset = ExitFrameConstants::kCodeOffset;
Object* code = Memory::Object_at(fp + offset);
@@ -62,11 +60,7 @@
if (is_debug_exit) {
sp -= kNumJSCallerSaved * kPointerSize;
}
- // Fill in the state.
- state->sp = sp;
- state->fp = fp;
- state->pc_address = reinterpret_cast<Address*>(sp - 1 * kPointerSize);
- return EXIT;
+ return sp;
}
=======================================
--- /branches/bleeding_edge/src/x64/frames-x64.cc Mon Aug 30 01:54:43 2010
+++ /branches/bleeding_edge/src/x64/frames-x64.cc Thu Sep 16 01:23:34 2010
@@ -35,18 +35,8 @@
namespace internal {
-
-
-StackFrame::Type ExitFrame::GetStateForFramePointer(Address fp, State*
state) {
- if (fp == 0) return NONE;
- // Compute the stack pointer.
- Address sp = Memory::Address_at(fp + ExitFrameConstants::kSPOffset);
- // Fill in the state.
- state->fp = fp;
- state->sp = sp;
- state->pc_address = reinterpret_cast<Address*>(sp - 1 * kPointerSize);
- ASSERT(*state->pc_address != NULL);
- return EXIT;
+Address ExitFrame::ComputeStackPointer(Address fp) {
+ return Memory::Address_at(fp + ExitFrameConstants::kSPOffset);
}
=======================================
--- /branches/bleeding_edge/test/cctest/test-log-stack-tracer.cc Wed Sep 1
06:13:31 2010
+++ /branches/bleeding_edge/test/cctest/test-log-stack-tracer.cc Thu Sep 16
01:23:34 2010
@@ -206,21 +206,8 @@
}
-static Local<Value> GetGlobalProperty(const char* name) {
- return env->Global()->Get(String::New(name));
-}
-
-
-static Handle<JSFunction> GetGlobalJSFunction(const char* name) {
- Handle<JSFunction> result(JSFunction::cast(
- *v8::Utils::OpenHandle(*GetGlobalProperty(name))));
- return result;
-}
-
-
-static void CheckObjectIsJSFunction(const char* func_name,
- Address addr) {
- i::Object* obj = reinterpret_cast<i::Object*>(addr);
+static void CheckJSFunctionAtAddress(const char* func_name, Address addr) {
+ i::Object* obj = i::HeapObject::FromAddress(addr);
CHECK(obj->IsJSFunction());
CHECK(JSFunction::cast(obj)->shared()->name()->IsString());
i::SmartPointer<char> found_name =
@@ -304,7 +291,6 @@
#endif
SetGlobalProperty(func_name, v8::ToApi<Value>(func));
- CHECK_EQ(*func, *GetGlobalJSFunction(func_name));
}
@@ -332,13 +318,13 @@
// script [JS]
// JSTrace() [JS]
// JSFuncDoTrace() [JS] [captures EBP value and encodes it as Smi]
- // trace(EBP encoded as Smi) [native (extension)]
+ // trace(EBP) [native (extension)]
// DoTrace(EBP) [native]
// StackTracer::Trace
CHECK_GT(sample.frames_count, 1);
// Stack tracing will start from the first JS function,
i.e. "JSFuncDoTrace"
- CheckObjectIsJSFunction("JSFuncDoTrace", sample.stack[0]);
- CheckObjectIsJSFunction("JSTrace", sample.stack[1]);
+ CheckJSFunctionAtAddress("JSFuncDoTrace", sample.stack[0]);
+ CheckJSFunctionAtAddress("JSTrace", sample.stack[1]);
}
@@ -370,19 +356,18 @@
// script [JS]
// OuterJSTrace() [JS]
// JSTrace() [JS]
- // JSFuncDoTrace() [JS] [captures EBP value and encodes it as Smi]
- // js_trace(EBP encoded as Smi) [native (extension)]
+ // JSFuncDoTrace() [JS]
+ // js_trace(EBP) [native (extension)]
// DoTraceHideCEntryFPAddress(EBP) [native]
// StackTracer::Trace
//
// The last JS function called. It is only visible through
// sample.function, as its return address is above captured EBP value.
- CHECK_EQ(GetGlobalJSFunction("JSFuncDoTrace")->address(),
- sample.function);
+ CheckJSFunctionAtAddress("JSFuncDoTrace", sample.function);
CHECK_GT(sample.frames_count, 1);
// Stack sampling will start from the caller of JSFuncDoTrace,
i.e. "JSTrace"
- CheckObjectIsJSFunction("JSTrace", sample.stack[0]);
- CheckObjectIsJSFunction("OuterJSTrace", sample.stack[1]);
+ CheckJSFunctionAtAddress("JSTrace", sample.stack[0]);
+ CheckJSFunctionAtAddress("OuterJSTrace", sample.stack[1]);
}
--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev
