Correctly report callstack when current function is FunctionCall builtin

Correctly report callstack when current function is FunctionCall builtin

When current function is FunctionCall builtin we have no reliable way to determine its caller function (in many cases the top of the sampled stack contains address of the caller but sometimes it does not). Instead of dropping the sample or its two top frames we simply mark the caller frame as '(unresolved function)'. It seems like a better approach that dropping whole sample as knowing the top function and the rest of the stack the user should be able to figure out what the caller was.

This change adds builtin id to CodeEntry objects. It will be used later to add similar top frame analysis for FunctionApply and probably other builtins.

BUG=None
R=jkummerow@chromium.org, loislo@chromium.org

Committed: https://code.google.com/p/v8/source/detail?r=15426

[loislo, 2013-07-01 12:00:20]

lgtm

https://codereview.chromium.org/18316004/diff/2001/src/profile-generator.cc
File src/profile-generator.cc (right):

https://codereview.chromium.org/18316004/diff/2001/src/profile-generator.cc#n...
src/profile-generator.cc:904: // When current function is FunctionCall builtin
tos is alway
tipo: alway

[Jakob Kummerow, 2013-07-01 12:16:05]

lgtm

https://codereview.chromium.org/18316004/diff/2001/src/cpu-profiler-inl.h
File src/cpu-profiler-inl.h (right):

https://codereview.chromium.org/18316004/diff/2001/src/cpu-profiler-inl.h#new...
src/cpu-profiler-inl.h:63: ASSERT(false);
nit: we have UNREACHABLE(); for this. Or you could just ASSERT(entry != NULL).

https://codereview.chromium.org/18316004/diff/2001/src/profile-generator.h
File src/profile-generator.h (right):

https://codereview.chromium.org/18316004/diff/2001/src/profile-generator.h#ne...
src/profile-generator.h:135: Builtins::Name builtin_id_ : 8;
Can you add a STATIC_ASSERT(Builtins::builtin_count < 256) or something like
that?

[yurys, 2013-07-01 12:50:16]

https://codereview.chromium.org/18316004/diff/2001/src/cpu-profiler-inl.h
File src/cpu-profiler-inl.h (right):

https://codereview.chromium.org/18316004/diff/2001/src/cpu-profiler-inl.h#new...
src/cpu-profiler-inl.h:63: ASSERT(false);
On 2013/07/01 12:16:06, Jakob wrote:
> nit: we have UNREACHABLE(); for this. Or you could just ASSERT(entry != NULL).

Done.

https://codereview.chromium.org/18316004/diff/2001/src/profile-generator.h
File src/profile-generator.h (right):

https://codereview.chromium.org/18316004/diff/2001/src/profile-generator.h#ne...
src/profile-generator.h:135: Builtins::Name builtin_id_ : 8;
On 2013/07/01 12:16:06, Jakob wrote:
> Can you add a STATIC_ASSERT(Builtins::builtin_count < 256) or something like
> that?

Compilation will fail with a error like the following

../src/profile-generator.h:135:32: error: 'v8::internal::CodeEntry::builtin_id_'
is too small to hold all values of 'enum v8::internal::Builtins::Name' [-Werror]

if 8 bits is not enough to fit all enum values so there is no need in extra
check.

[yurys, 2013-07-01 14:39:56]

It turned out that sometimes while we are in FunctionCall return address is not
on the top of the stack so we don't have a reliable way to determine the caller.
After offline discussion I updated the patch to simply mark the caller frame as
'(unresolved function)'

[loislo, 2013-07-01 14:42:21]

lgtm

[yurys, 2013-07-01 14:58:12]

Committed patchset #3 manually as r15426 (presubmit successful).