[webkit-reviews] review granted: [Bug 179699] Fix a bit-rotted Interpreter::dumpRegisters() and make it more robust. : [Attachment 326936] proposed patch.

[bugzilla-daemon at webkit.org]

Michael Saboff <msaboff at apple.com> has granted Mark Lam <mark.lam at apple.com>'s
request for review:
Bug 179699: Fix a bit-rotted Interpreter::dumpRegisters() and make it more
robust.
https://bugs.webkit.org/show_bug.cgi?id=179699

Attachment 326936: proposed patch.

https://bugs.webkit.org/attachment.cgi?id=326936&action=review




--- Comment #3 from Michael Saboff <msaboff at apple.com> ---
Comment on attachment 326936
  --> https://bugs.webkit.org/attachment.cgi?id=326936
proposed patch.

View in context: https://bugs.webkit.org/attachment.cgi?id=326936&action=review

r=me after reversing the order of CallerFrame and ReturnPC.

> Source/JavaScriptCore/interpreter/Interpreter.cpp:-441
> +    dataLogF("[CallerFrame]		    | %10p | %p \n", it,
callFrame->callerFrame());
>      --it;
>  #if ENABLE(JIT)
>      AbstractPC pc = callFrame->abstractReturnPC(callFrame->vm());
>      if (pc.hasJITReturnAddress())
>	   dataLogF("[ReturnJITPC]		| %10p | %p \n", it,
pc.jitReturnAddress().value());
> -#endif

I think you need to flip the order of CallerFrame and ReturnJITPC as I'm pretty
sure that the CallerFrame (aka previous frame pointer) is pushed on after the
Return PC (or at a lower address for ARM64 as they are pushed simultaneously).

Also, change ReturnJITPC to ReturnPC as it may not be a JIT'ed PC.

> Source/JavaScriptCore/interpreter/Interpreter.cpp:465
> +		   ? "INVALID"

Nit - I'd make this "Unknown" or leave it blank.