On 2013/10/30 15:01:24, Yang wrote:
I do not agree with this change yet:
- We are introducing a new flag to the StackTraceFrameIterator and all its
parent classes just to disable an assert.
- Do we actually understand why the stackframe iterator cannot work when
being
called from a builtin (I assume that you are referring to C++ builtins in
like
ArrayShift)? I was under the assumption that the StackFrameIterator
should be
able to handle all types of frames on the stack.
The assert was there since the Crankshaft implementation was landed in
bleeding_edge. Digging into the history of this code before it was published
didn't give much additional info.
My understanding of this code is that determining frame type based on the
return
address saved on the stack may be more reliable than casting frame marker
value
to a function. Otherwise I don't see why not simply cast the marker to a
function and return its code type.
It must be the assert is there because the function may well be calling some
code stub without erecting a frame in which case return address on the stack
would point inside that code stub. Based on the stub's code's type we
couldn't
tell the type of the function's code. I believe the assert checks that we
cannot
end up in a situation where call stack contains JSFunction -> Builtin ->
JSFunction, in other words if we are inside a JSFunction then return
address on
the stack may belong only to a JSFunction's code (given the caller's frame
type
is JS).
I could relax this assert instead of plumbing the boolean flag through the
iterator constructors but even with the allocation stack traces the
built-ins
are allowed only on top of the call stack and I preferred to preserve the
check
for all current cases. Do you think it would make sense to relax that assert
instead?
https://codereview.chromium.org/43693002/diff/200001/src/x64/macro-assembler-x64.cc
File src/x64/macro-assembler-x64.cc (right):
https://codereview.chromium.org/43693002/diff/200001/src/x64/macro-assembler-x64.cc#newcode4097
src/x64/macro-assembler-x64.cc:4097: addq(rsp, Immediate(2 *
kPointerSize));
Use Drop(2) instead.
Done.
https://codereview.chromium.org/43693002/
--
--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev
---
You received this message because you are subscribed to the Google Groups "v8-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to [email protected].
For more options, visit https://groups.google.com/groups/opt_out.
