postmortem: export private field offsets

postmortem: export private field offsets

Export heap spaces offsets that are useful for generating code map in
postmortem/live process.

BUG=
R=machenbach@chromium.org

[fedor.indutny, 2015-11-16 16:47:34]

Kindly reminding about this.

[Jakob Kummerow, 2015-11-17 09:57:22]

jkummerow@chromium.org changed reviewers:
+ jkummerow@chromium.org

[Jakob Kummerow, 2015-11-17 09:57:23]

Michael is OOO.

I don't think this approach is going to find approval.

[fedor.indutny, 2015-11-17 18:20:57]

On 2015/11/17 09:57:23, Jakob wrote:
> Michael is OOO.
> 
> I don't think this approach is going to find approval.

Jakob,

Of course, I totally understand it. May I ask you if you have any suggestions on
other ways to export this information? I'm currently using it (in hard-coded
way) in llnode, to iterate through the old space (SharedFunctionInfo) and print
all code addresses with their function names. This seems to be very useful,
especially in postmortem.

I wish I could do it without changing headers in `src/`, but many of the
exported fields are private.

Thank you,
Fedor.

[Jakob Kummerow, 2015-11-18 16:05:18]

No, I don't have a better suggestion.

[fedor.indutny, 2015-11-18 16:07:57]

On 2015/11/18 16:05:18, Jakob wrote:
> No, I don't have a better suggestion.

Heh. This is quite sad to hear. May I at least ask you to provide me more
details on why do you think it is not going to find approval?

Thank you,
Fedor.

[Jakob Kummerow, 2015-11-18 16:28:51]

> Heh. This is quite sad to hear. May I at least ask you to provide me more
> details on why do you think it is not going to find approval?

Well, it's been on the mailing list for almost a week and nobody has approved
it... possible reasons might be:

- You're adding dead code without tests or documentation
- You're exposing one project's internal details to another (arguably even in
both directions)
- The OWNERS of src/heap/ don't like random external components accessing their
private fields (I wouldn't either, if I was them)

[fedor.indutny, 2015-11-18 16:35:33]

Jakob,

I totally agree with you on this.

- The state of postmortem in V8 is rather sad indeed, and I think it could be
fixed by adding something like `llnode` to the test suite. This is why I think
it is very important to have external tool that just works with the `lldb`.
- I understand your frustration on this too, so I think it may be worth
explaining it a bit. There is no code that actually uses this in any destructive
way. In fact, there is no code that is using it at all. The gen-postmortem will
just create something like this:

class PostMortem {
 public:
  static const int kIsolate__heap_ = offsetof(Isolate, heap_);

  ...
}

And there are no uses of the private fields except the constants. I hope that
this makes it look a bit more transparent.

If I will write unit tests for this functionality as a part of this CL, will you
or anyone else be willing to review it and accept this CL (in one form or any
other)?

Thank you for your time,
Fedor.

[ofrobots, 2015-11-18 16:56:31]

On 2015/11/18 16:28:51, Jakob wrote:
> - The OWNERS of src/heap/ don't like random external components accessing
their
> private fields (I wouldn't either, if I was them)

In my opinion, this should not be a concern. This is exactly what a debugger
does, and we don't have qualms about that. 

If you look at the current set of changes in CL, the offsets are just enabling
an post-mortem debugger, or a debugger that otherwise cannot extract/find DWARF
debug info in the binary to continue functioning well. 

The things that do need be clarified are:
1. The internal structures of the VM, being internal, may change at any time,
and there is no guarantee of any stability for the post-mortem data.
2. Testing.

[fedor.indutny, 2015-11-18 17:03:17]

On 2015/11/18 16:56:31, ofrobots wrote:
> On 2015/11/18 16:28:51, Jakob wrote:
> > - The OWNERS of src/heap/ don't like random external components accessing
> their
> > private fields (I wouldn't either, if I was them)
> 
> In my opinion, this should not be a concern. This is exactly what a debugger
> does, and we don't have qualms about that. 
> 
> If you look at the current set of changes in CL, the offsets are just enabling
> an post-mortem debugger, or a debugger that otherwise cannot extract/find
DWARF
> debug info in the binary to continue functioning well. 
> 
> The things that do need be clarified are:
> 1. The internal structures of the VM, being internal, may change at any time,
> and there is no guarantee of any stability for the post-mortem data.

This is totally understandable. I already use major/minor version to polyfill
some of the constants on previous V8 versions, so using them to decide what kind
of structure to expect should be fine too. At least I will have a chance to do
it!

> 2. Testing.

Again, this is very reasonable. Especially, since
https://codereview.chromium.org/1458583003/ has landed recently. I see several
ways of doing it:

- Simple - just check that all required constants are in-place
- Full - build (or download) something like llnode, and run it on `d8` process
to dump some information

Full way is rather complicated at the moment, because there is most likely
(correct me if I am wrong) no lldb binary in the chromium's buckets.

[fedor.indutny, 2015-11-25 07:32:46]

Hello again,

Sorry for bothering you with this, but I believe that these changes has a value
for postmortem debugging that is otherwise unobtainable. I would appreciate any
extra feedback.

If the only thing that is blocking you from landing this is a test - let's
discuss the way it could be implemented.

Thank you again,
Fedor.

[fedor.indutny, 2015-11-26 00:25:17]

A question:

Perhaps, I could live with just `friend class PostMortem` added in these places.
It should not make any difference for V8 structure, are you OK with this
changes?

Thanks,
Fedor.

[Jakob Kummerow, 2015-11-28 10:40:57]

If your reasoning is "a debugger should be able to access everything", consider
doing this in your code:

#define private public
#define protected public
#include v8/src/heap/heap.h
#include v8/src/heap/spaces.h

No changes to V8 required. And the extent to which you're going beyond any form
of code architecture rules would be appropriately expressed.