Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

::findjsobjects doesn't work properly with V8 4.5.x #32

Closed
misterdjules opened this issue Sep 10, 2015 · 4 comments
Closed

::findjsobjects doesn't work properly with V8 4.5.x #32

misterdjules opened this issue Sep 10, 2015 · 4 comments
Labels

Comments

@misterdjules
Copy link
Contributor

Even though I thought that #24 (which landed as 5a189dd) fixed mdb_v8's support for V8 4.5.x, it turns out that ::findjsobjects is still broken.

::findjsobjects misses a significant part of valid JavaScript objects:

[root@dev ~/mdb_v8]# node -p 'process.versions'
{ http_parser: '2.5.0',
  node: '4.0.0',
  v8: '4.5.103.30',
  uv: '1.7.3',
  zlib: '1.2.8',
  ares: '1.10.1-DEV',
  modules: '46',
  openssl: '1.0.2d' }
[root@dev ~/mdb_v8]# rm /var/cores/core.node.61936 
[root@dev ~/mdb_v8]# node -e 'function Foo() {}; var fooInstance = new Foo(); fooInstance.bar = 42; process.abort();'
Abort (core dumped)
[root@dev ~/mdb_v8]# mdb /var/cores/core.node.61939 
Loading modules: [ libumem.so.1 libc.so.1 ld.so.1 ]
> ::load /root/mdb_v8/build/ia32/mdb_v8.so
mdb_v8 version: 1.0.0 (dev)
V8 version: 4.5.103.30
Autoconfigured V8 support from target
C++ symbol demangling enabled
> ::findjsobjects -c Foo
> ::findjsobjects -p bar
> ::findjsobjects ! grep foo
> ::findjsobjects ! grep Foo
> ::findjsobjects ! grep bar
> 

However, when an object is created in a similar way, but without adding a property, ::findjsobjects is able to find it and filtering by constructor name also works as expected:

[root@dev ~/mdb_v8]# node -e 'function Foo() {}; var fooInstance = new Foo(); process.abort();'
Abort (core dumped)
[root@dev ~/mdb_v8]# mdb /var/cores/core.node.61936 
Loading modules: [ libumem.so.1 libc.so.1 ld.so.1 ]
> ::load /root/mdb_v8/build/ia32/mdb_v8.so
mdb_v8 version: 1.0.0 (dev)
V8 version: 4.5.103.30
Autoconfigured V8 support from target
C++ symbol demangling enabled
> ::findjsobjects -c Foo
9b07d541
> 

My theory so far is that the way an object's map is referenced from an object has changed. The constructor for an object used to be stored in the Map directly available from the object, but now one has to traverse the whole "transition tree" to find the original Map that contains some information about that object, including its constructor. The relevant upstream change seems to be https://codereview.chromium.org/950283002.

Before this issue is fixed, mdb_v8 1.0.0 will be pretty much unable to inspect objects on the heap.

#24 still had all tests passing because the changes in the tests that use ::findjsobjects (https://github.com/joyent/mdb_v8/pull/24/files#diff-76b702a3fc1358227ab75b86a1a39284R46 and https://github.com/joyent/mdb_v8/pull/24/files#diff-1f7f4b61869c469eb7471735f5412183R68) silently prevented the actual tests from running, which lead me to believe that they were passing. Moreover, when running manual tests, ::findjsobjects still found a significant number of objects, didn't output any garbage and in some cases found all objects so everything appeared to be working as expected. These tests will need to be fixed too.

misterdjules added a commit to misterdjules/mdb_v8 that referenced this issue Sep 16, 2015
This change is a work in progress to fix ::findjsobjects for core dumps
generated by Node.js 4.0.x (and possibly later).

A number of changes have been made to the layout of objects in V8 from
V8 4.3.x and later, and these changes prevent ::findjsobjects from
working as expected.

The current status of this change fixes how mdb_v8 gets retrieves the
constructor of a JavaScript object given its Map. There are other
problems remaining, one of them being that the layout of instance
descriptors seems to have changed too, and thus it is currently not
possible to retrieve an instance's properties correctly with mdb_v8.
misterdjules added a commit to misterdjules/mdb_v8 that referenced this issue Sep 16, 2015
misterdjules added a commit to misterdjules/mdb_v8 that referenced this issue Sep 16, 2015
@misterdjules
Copy link
Contributor Author

I've made a bit of progress, and at least now I'm able to retrieve the constructor for all objects, and thus ::findjsobjects -c Foo now works in some cases where it was previously broken:

$ node -e 'function Foo() { this.someProp = "foo" }; var fooInstance = new Foo(); process.abort();'
Abort (core dumped)
$ mdb /var/cores/core.node-4.0-64.13400 
Loading modules: [ libumem.so.1 libc.so.1 ld.so.1 ]
> ::load /root/mdb_v8/build/amd64/mdb_v8.so
mdb_v8 version: 1.0.0 (dev)
V8 version: 4.5.103.30
Autoconfigured V8 support from target
C++ symbol demangling enabled
> ::findjsobjects -c Foo | ::findjsobjects | ::jsprint
{
    "someProp": "foo",
}
> 

However, it still fails in other cases. One of the failure cases that I found is the following:

$ node -e 'function Foo() {}; var fooInstance = new Foo(); fooInstance.my_buffer = new Buffer("foobarbaz"); process.abort();'
Abort (core dumped)
$ mdb /var/cores/core.node-4.0-64.13407 
Loading modules: [ libumem.so.1 libc.so.1 ld.so.1 ]
> ::load /root/mdb_v8/build/amd64/mdb_v8.so
mdb_v8 version: 1.0.0 (dev)
V8 version: 4.5.103.30
Autoconfigured V8 support from target
C++ symbol demangling enabled
> ::findjsobjects -c Foo
> ::findjsobjects -c Buffer
1788040f86b1
> ::findjsobjects -c Buffer | ::findjsobjects -r
1788040f86b1 is not referred to by a known object.
>

In other words, it seems that adding a property whose value is an instance of Buffer to any object breaks ::findjsobjects -c somehow.

::findjsobjects -p and access to properties' values is now also fixed. It was broken due to a typo in v8dbg_prop_index_mask and v8dbg_prop_index_shift.

However, as for ::findjsobjects -c, ::findjsobjects -p seems to be broken when using Buffer instances:

[root@dev ~/mdb_v8]# node -e 'function Foo() {}; var fooInstance = new Foo(); fooInstance.my_buffer = new Buffer("foobarbaz"); process.abort();'
Abort (core dumped)
[root@dev ~/mdb_v8]# mdb /var/cores/core.node-4.0-64.34992 
Loading modules: [ libumem.so.1 libc.so.1 ld.so.1 ]
> ::load /root/mdb_v8/build/amd64/mdb_v8.so
mdb_v8 version: 1.0.0 (dev)
V8 version: 4.5.103.30
Autoconfigured V8 support from target
C++ symbol demangling enabled
> ::findjsobjects -p my_buffer
>

I'm investigating these issues, but any hint or help would definitely be appreciated.

With my current changes, all tests but test/standalone/tst.postmortem_details.js succeed. Note that the tests now actually run as I reverted my previous changes that prevented them to run (See https://github.com/joyent/mdb_v8/compare/master...misterdjules:more-fixes-for-v8-4.x?expand=1#diff-1f7f4b61869c469eb7471735f5412183R68 and https://github.com/joyent/mdb_v8/compare/master...misterdjules:more-fixes-for-v8-4.x?expand=1#diff-76b702a3fc1358227ab75b86a1a39284R46).

misterdjules added a commit to misterdjules/mdb_v8 that referenced this issue Sep 18, 2015
misterdjules pushed a commit to misterdjules/mdb_v8 that referenced this issue Sep 18, 2015
misterdjules pushed a commit to misterdjules/mdb_v8 that referenced this issue Sep 18, 2015
misterdjules added a commit to misterdjules/mdb_v8 that referenced this issue Sep 18, 2015
This change is a work in progress to fix ::findjsobjects for core dumps
generated by Node.js 4.0.x (and possibly later).

The current status of this change fixes how mdb_v8 gets retrieves the
constructor of a JavaScript object given its Map.

It also fixes the problem of accessing objects' properties that was
caused by a typo in "v8db_propindex_mask" that should have been
"v8db_prop_index_mask" (note the underscore in "prop_index") in my
previous changes.

It has a "fallbacks" members for v8_offets so that we can have a
fallback for typed arrays' length's offset.

Finally, this change allows ::findjsobjects to find Buffer instances and
inspect them.

However, due to how Buffer instances are implemented in node v4.x and
later, they are currently seen by mdb_v8 as having a constructor named
"Uint8Array" instead of "Buffer". Even though Buffer instances are
actually now Uint8Array instances, their prototype is set to
Buffer.prototype, and someBufferInstance.constructor.name returns
'Buffer', so mdb_v8 should be able to get 'Buffer' as the constructor
name.

It fixes TritonDataCenter#32 in its current state, and it represents some
progress but I still want to investigate why we can't get the proper
constructor name for Buffer instances.
@misterdjules
Copy link
Contributor Author

#33 is a work in progress to fix this issue.

misterdjules added a commit to misterdjules/mdb_v8 that referenced this issue Sep 21, 2015
This change is a work in progress to fix ::findjsobjects for core dumps
generated by Node.js 4.0.x (and possibly later).

The current status of this change fixes how mdb_v8 gets retrieves the
constructor of a JavaScript object given its Map.

It also fixes the problem of accessing objects' properties that was
caused by a typo in "v8db_propindex_mask" that should have been
"v8db_prop_index_mask" (note the underscore in "prop_index") in my
previous changes.

It adds a "fallback" member for v8_offsets so that we can have a
fallback for typed arrays' length's offset.

Finally, this change allows ::findjsobjects to find Buffer instances and
inspect them. However, due to how Buffer instances are implemented in
node v4.x and later, they are currently seen by mdb_v8 as having a
constructor named "Uint8Array" instead of "Buffer". Even though Buffer
instances are actually now Uint8Array instances, their prototype is set
to Buffer.prototype, and someBufferInstance.constructor.name returns
'Buffer', so mdb_v8 should be able to get 'Buffer' as the constructor
name.

It fixes TritonDataCenter#32 in its current state, and it represents some
progress but I still want to investigate why we can't get the proper
constructor name for Buffer instances.
misterdjules pushed a commit to misterdjules/mdb_v8 that referenced this issue Sep 22, 2015
Do not warn if JSTypedArray type is missing.
Use a separate V8_OFF_MAP_CONSTRUCTOR_OR_BACKPOINTER constant to store
the constructor_or_backpointer offset, and set V8_OFF_MAP_CONSTRUCTOR
with thatvalue if V8_OFF_MAP_CONSTRUCTOR is -1. Before that change, the
same variable was used for both offsets, and for node versions < 4.x, it
would result in V8_OFF_MAP_CONSTRUCTOR always being -1, which would
break, among other things, get_map_constructor and thus ::findjsobjects.
misterdjules pushed a commit to misterdjules/mdb_v8 that referenced this issue Sep 22, 2015
Make changes according to dap's code review:
 - Formatting/indentation nits.
 - Print <Typed array of length N> when ::jsprinting a typed array.
 - Update tests accordingly.
misterdjules pushed a commit to misterdjules/mdb_v8 that referenced this issue Sep 22, 2015
Make changes according to dap's code review:
  - Do not warn if JSTypedArray type is missing.
  - Clarify/added comments.
  - Formatting/indentation nits.
  - Print <Typed array of length N> when ::jsprinting a typed array.
  - get_map_constructor fails if V8_OFF_MAP_CONSTRUCTOR is missing.
  - Update tests accordingly.

Also, use a separate V8_OFF_MAP_CONSTRUCTOR_OR_BACKPOINTER constant to
store the constructor_or_backpointer offset, and set
V8_OFF_MAP_CONSTRUCTOR with that value if V8_OFF_MAP_CONSTRUCTOR is -1.
Before that change, the same variable was used for both offsets, and
for node versions < 4.x, it would result in V8_OFF_MAP_CONSTRUCTOR
always being -1, which would break, among other things,
get_map_constructor and thus ::findjsobjects.
misterdjules added a commit to misterdjules/mdb_v8 that referenced this issue Sep 22, 2015
This fixes ::findjsobjects for core dumps generated by Node.js 4.0.x
(and possibly later).

This change fixes how mdb_v8 retrieves the constructor of a JavaScript
object given its Map.

It also fixes the problem of accessing objects' properties that was
caused by a typo in "v8db_propindex_mask" that should have been
"v8db_prop_index_mask" (note the underscore in "prop_index") in my
previous changes.

It adds a "fallback" member for v8_offsets so that we can have a
fallback for typed arrays' length's offset.

Finally, this change allows ::findjsobjects to find Buffer instances and
inspect them. However, due to how Buffer instances are implemented in
node v4.x and later, they are currently seen by mdb_v8 as having a
constructor named "Uint8Array" instead of "Buffer", so ::findjsobjects
-c Buffer won't find any actual Buffer instances, instead one has to use
::findjsobjects -c Uint8Array.
misterdjules pushed a commit to misterdjules/mdb_v8 that referenced this issue Sep 22, 2015
This change fixes:
 - 32 bit v4.x node binaries with improper JSTypedArray length offset.
 - v4.0.0 and v4.1.1 node binaries that miss Map's bit_field3's type
   metadata by hardcoding its type.
misterdjules added a commit to misterdjules/mdb_v8 that referenced this issue Sep 23, 2015
This fixes ::findjsobjects for core dumps generated by Node.js 4.0.x
(and possibly later).

This change fixes how mdb_v8 retrieves the constructor of a JavaScript
object given its Map.

It also fixes the problem of accessing objects' properties that was
caused by a typo in "v8db_propindex_mask" that should have been
"v8db_prop_index_mask" (note the underscore in "prop_index") in my
previous changes.

It adds a "fallback" member for v8_offsets so that we can have a
fallback for typed arrays' length's offset.

Finally, this change allows ::findjsobjects to find Buffer instances and
inspect them. However, due to how Buffer instances are implemented in
node v4.x and later, they are currently seen by mdb_v8 as having a
constructor named "Uint8Array" instead of "Buffer", so ::findjsobjects
-c Buffer won't find any actual Buffer instances, instead one has to use
::findjsobjects -c Uint8Array.
@misterdjules
Copy link
Contributor Author

#33 fixes ::findjsobjects for core dumps generated by Node.js 4.0.x (and possibly later).

This change fixes how mdb_v8 retrieves the constructor of a JavaScript object given its Map.

It also fixes the problem of accessing objects' properties that was caused by a typo in v8db_propindex_mask that should have been v8db_prop_index_mask (note the underscore in prop_index) in my previous changes.

It adds a v8o_fallback member to v8_offset so that we can have a fallback for typed arrays' length's offset.

Finally, this change allows ::findjsobjects to find Buffer instances and inspect them. However, due to how Buffer instances are implemented in node v4.x and later, they are currently seen by mdb_v8 as having a constructor named Uint8Array instead of Buffer, so ::findjsobjects -c Buffer won't find any actual Buffer instances, instead one has to use ::findjsobjects -c Uint8Array.

@davepacheco
Copy link
Contributor

Merged via #33. As part of testing this, I tested: v0.10.40, v0.12.7, and v4.0.0 on x86 and x64. I ran into #38, but also reproduced that on master.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants