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

deps: backport 56a0a79 from V8 upstream #3057

Conversation

misterdjules
Copy link

Backport 56a0a797f210e04746f2888116365d29a4bb6afc from V8 upstream to
include post-mortem metadata used by mdb_v8 to support V8 4.6.

Original commit message:

Update post-mortem metadata generation

mdb_v8, a post-mortem debugger for Node.js, now uses JSArrayBuffer's
backing_store property and JSArrayBufferView's byte_offset property to
get access to the content of Buffer instances in node (which are
Uint8Array instances). This change adds post-mortem metadata for these
two properties.

This change also fixes a typo in
inobject_properties_of_constructor_function_index_offset that was added
to gen-postmortem-metadata in a previous change. It should be named
inobject_properties_or_constructor_function_index instead.

R=[email protected]

Review URL: https://codereview.chromium.org/1363403003

Cr-Commit-Position: refs/heads/master@{#30926}

/cc @targos @ofrobots @nodejs/post-mortem

See related mdb_v8 change here: misterdjules/mdb_v8@853e337.

Backport 56a0a797f210e04746f2888116365d29a4bb6afc from V8 upstream to
include post-mortem metadata used by mdb_v8 to support V8 4.6.

Original commit message:

  Update post-mortem metadata generation

  mdb_v8, a post-mortem debugger for Node.js, now uses JSArrayBuffer's
  backing_store property and JSArrayBufferView's byte_offset property to
  get access to the content of Buffer instances in node (which are
  Uint8Array instances). This change adds post-mortem metadata for these
  two properties.

  This change also fixes a typo in
  inobject_properties_of_constructor_function_index_offset that was added
  to gen-postmortem-metadata in a previous change. It should be named
  inobject_properties_or_constructor_function_index instead.

  [email protected]

  Review URL: https://codereview.chromium.org/1363403003

  Cr-Commit-Position: refs/heads/master@{nodejs#30926}
@misterdjules misterdjules added v8 engine Issues and PRs related to the V8 dependency. post-mortem Issues and PRs related to the post-mortem diagnostics of Node.js. labels Sep 25, 2015
@ofrobots
Copy link
Contributor

LGTM.

BTW, is there a way to somehow test the metadata file generated by gen-postmortem-metadata.py?

@targos
Copy link
Member

targos commented Sep 25, 2015

LGTM

1 similar comment
@jasnell
Copy link
Member

jasnell commented Sep 25, 2015

LGTM

@misterdjules
Copy link
Author

CI tests fail on a lot of platforms on test-stringbytes-external.js, even though the vee-eight-4.6 branch seems to have a change that landed to make that test faster.

Also, one of the FreeBSD Jenkins agents is down, and thus the Jenkins job cannot complete.

Landing anyway, as I can't imagine how this change would cause these regressions.

misterdjules pushed a commit that referenced this pull request Sep 25, 2015
Backport 56a0a797f210e04746f2888116365d29a4bb6afc from V8 upstream to
include post-mortem metadata used by mdb_v8 to support V8 4.6.

Original commit message:

  Update post-mortem metadata generation

  mdb_v8, a post-mortem debugger for Node.js, now uses JSArrayBuffer's
  backing_store property and JSArrayBufferView's byte_offset property to
  get access to the content of Buffer instances in node (which are
  Uint8Array instances). This change adds post-mortem metadata for these
  two properties.

  This change also fixes a typo in
  inobject_properties_of_constructor_function_index_offset that was added
  to gen-postmortem-metadata in a previous change. It should be named
  inobject_properties_or_constructor_function_index instead.

  [email protected]

  Review URL: https://codereview.chromium.org/1363403003

  Cr-Commit-Position: refs/heads/master@{#30926}

PR: #3057
PR-URL: #3057
Reviewed-By: Michaël Zasso <[email protected]>
Reviewed-By: Ali Ijaz Sheikh <[email protected]>
@misterdjules
Copy link
Author

Landed in 7ffbfd8.

@misterdjules
Copy link
Author

@ofrobots Regarding testing generated post-mortem data: that's a very good and important question.

Currently, this is done my mdb_v8's tests suite, because the only way to test if the post-mortem metadata is generated properly is to run post-mortem debugging tools on known programs and check that inspected data structures match what we expect.

We could run mdb_v8's tests suite as part of node's tests suite before a release, but:

  • mdb_v8 was removed from node's code base with io.js, and hasn't been put back yet. I don't know if it will be put back in the near future, because I'm not sure yet if there's a benefit in doing that.
  • mdb_v8 only works on SmartOS, because it's a "plug-in" for mdb (what we call dmods). So that restricts the platforms on which we would be able to test the generated post-mortem metadata.

There's also the question of whether these tests should be ran by node's tests suite, or V8's tests suite. Ultimately, this post-mortem debugging data is generated by V8 and would be usable by any V8 embedder. That being said, mdb_v8 for instance has debugging commands that are specific to node, like ::nodebuffer, so it's a grey area.

Finally, @davepacheco started some work to allow parts of mdb_v8 that are not platform specific to be abstracted out in a reusable library. We could imagine building a small multi-platform binary that uses this library to run post-mortem debugging tests.

This is only me thinking out loud though, and I believe this would be a great topic to discuss with @nodejs/post-mortem, maybe during our next meeting?

Anyway, thank you for bringing that up! 👍

misterdjules pushed a commit that referenced this pull request Sep 25, 2015
Backport 56a0a797f210e04746f2888116365d29a4bb6afc from V8 upstream to
include post-mortem metadata used by mdb_v8 to support V8 4.6.

Original commit message:

  Update post-mortem metadata generation

  mdb_v8, a post-mortem debugger for Node.js, now uses JSArrayBuffer's
  backing_store property and JSArrayBufferView's byte_offset property to
  get access to the content of Buffer instances in node (which are
  Uint8Array instances). This change adds post-mortem metadata for these
  two properties.

  This change also fixes a typo in
  inobject_properties_of_constructor_function_index_offset that was added
  to gen-postmortem-metadata in a previous change. It should be named
  inobject_properties_or_constructor_function_index instead.

  [email protected]

  Review URL: https://codereview.chromium.org/1363403003

  Cr-Commit-Position: refs/heads/master@{#30926}

PR: #3057
PR-URL: #3057
Reviewed-By: Michaël Zasso <[email protected]>
Reviewed-By: Ali Ijaz Sheikh <[email protected]>
misterdjules pushed a commit that referenced this pull request Sep 26, 2015
Backport 56a0a797f210e04746f2888116365d29a4bb6afc from V8 upstream to
include post-mortem metadata used by mdb_v8 to support V8 4.6.

Original commit message:

  Update post-mortem metadata generation

  mdb_v8, a post-mortem debugger for Node.js, now uses JSArrayBuffer's
  backing_store property and JSArrayBufferView's byte_offset property to
  get access to the content of Buffer instances in node (which are
  Uint8Array instances). This change adds post-mortem metadata for these
  two properties.

  This change also fixes a typo in
  inobject_properties_of_constructor_function_index_offset that was added
  to gen-postmortem-metadata in a previous change. It should be named
  inobject_properties_or_constructor_function_index instead.

  [email protected]

  Review URL: https://codereview.chromium.org/1363403003

  Cr-Commit-Position: refs/heads/master@{#30926}

PR: #3057
PR-URL: #3057
Reviewed-By: Michaël Zasso <[email protected]>
Reviewed-By: Ali Ijaz Sheikh <[email protected]>
targos pushed a commit to targos/node that referenced this pull request Oct 5, 2015
Backport 56a0a797f210e04746f2888116365d29a4bb6afc from V8 upstream to
include post-mortem metadata used by mdb_v8 to support V8 4.6.

Original commit message:

  Update post-mortem metadata generation

  mdb_v8, a post-mortem debugger for Node.js, now uses JSArrayBuffer's
  backing_store property and JSArrayBufferView's byte_offset property to
  get access to the content of Buffer instances in node (which are
  Uint8Array instances). This change adds post-mortem metadata for these
  two properties.

  This change also fixes a typo in
  inobject_properties_of_constructor_function_index_offset that was added
  to gen-postmortem-metadata in a previous change. It should be named
  inobject_properties_or_constructor_function_index instead.

  [email protected]

  Review URL: https://codereview.chromium.org/1363403003

  Cr-Commit-Position: refs/heads/master@{nodejs#30926}

PR: nodejs#3057
PR-URL: nodejs#3057
Reviewed-By: Michaël Zasso <[email protected]>
Reviewed-By: Ali Ijaz Sheikh <[email protected]>
targos pushed a commit to targos/node that referenced this pull request Oct 6, 2015
Backport 56a0a797f210e04746f2888116365d29a4bb6afc from V8 upstream to
include post-mortem metadata used by mdb_v8 to support V8 4.6.

Original commit message:

  Update post-mortem metadata generation

  mdb_v8, a post-mortem debugger for Node.js, now uses JSArrayBuffer's
  backing_store property and JSArrayBufferView's byte_offset property to
  get access to the content of Buffer instances in node (which are
  Uint8Array instances). This change adds post-mortem metadata for these
  two properties.

  This change also fixes a typo in
  inobject_properties_of_constructor_function_index_offset that was added
  to gen-postmortem-metadata in a previous change. It should be named
  inobject_properties_or_constructor_function_index instead.

  [email protected]

  Review URL: https://codereview.chromium.org/1363403003

  Cr-Commit-Position: refs/heads/master@{nodejs#30926}

PR: nodejs#3057
PR-URL: nodejs#3057
Reviewed-By: Michaël Zasso <[email protected]>
Reviewed-By: Ali Ijaz Sheikh <[email protected]>
misterdjules pushed a commit that referenced this pull request Oct 8, 2015
Backport 56a0a797f210e04746f2888116365d29a4bb6afc from V8 upstream to
include post-mortem metadata used by mdb_v8 to support V8 4.6.

Original commit message:

  Update post-mortem metadata generation

  mdb_v8, a post-mortem debugger for Node.js, now uses JSArrayBuffer's
  backing_store property and JSArrayBufferView's byte_offset property to
  get access to the content of Buffer instances in node (which are
  Uint8Array instances). This change adds post-mortem metadata for these
  two properties.

  This change also fixes a typo in
  inobject_properties_of_constructor_function_index_offset that was added
  to gen-postmortem-metadata in a previous change. It should be named
  inobject_properties_or_constructor_function_index instead.

  [email protected]

  Review URL: https://codereview.chromium.org/1363403003

  Cr-Commit-Position: refs/heads/master@{#30926}

PR: #3057
PR-URL: #3057
Reviewed-By: Michaël Zasso <[email protected]>
Reviewed-By: Ali Ijaz Sheikh <[email protected]>
misterdjules pushed a commit that referenced this pull request Oct 11, 2015
Backport 56a0a797f210e04746f2888116365d29a4bb6afc from V8 upstream to
include post-mortem metadata used by mdb_v8 to support V8 4.6.

Original commit message:

  Update post-mortem metadata generation

  mdb_v8, a post-mortem debugger for Node.js, now uses JSArrayBuffer's
  backing_store property and JSArrayBufferView's byte_offset property to
  get access to the content of Buffer instances in node (which are
  Uint8Array instances). This change adds post-mortem metadata for these
  two properties.

  This change also fixes a typo in
  inobject_properties_of_constructor_function_index_offset that was added
  to gen-postmortem-metadata in a previous change. It should be named
  inobject_properties_or_constructor_function_index instead.

  [email protected]

  Review URL: https://codereview.chromium.org/1363403003

  Cr-Commit-Position: refs/heads/master@{#30926}

PR: #3057
PR-URL: #3057
Reviewed-By: Michaël Zasso <[email protected]>
Reviewed-By: Ali Ijaz Sheikh <[email protected]>
ofrobots pushed a commit to ofrobots/node that referenced this pull request Oct 14, 2015
Backport 56a0a797f210e04746f2888116365d29a4bb6afc from V8 upstream to
include post-mortem metadata used by mdb_v8 to support V8 4.6.

Original commit message:

  Update post-mortem metadata generation

  mdb_v8, a post-mortem debugger for Node.js, now uses JSArrayBuffer's
  backing_store property and JSArrayBufferView's byte_offset property to
  get access to the content of Buffer instances in node (which are
  Uint8Array instances). This change adds post-mortem metadata for these
  two properties.

  This change also fixes a typo in
  inobject_properties_of_constructor_function_index_offset that was added
  to gen-postmortem-metadata in a previous change. It should be named
  inobject_properties_or_constructor_function_index instead.

  [email protected]

  Review URL: https://codereview.chromium.org/1363403003

  Cr-Commit-Position: refs/heads/master@{nodejs#30926}

Ref: nodejs#3057
PR-URL: nodejs#3351
Reviewed-By: indutny - Fedor Indutny <[email protected]>
Reviewed-By: bnoordhuis - Ben Noordhuis <[email protected]>
misterdjules pushed a commit that referenced this pull request Oct 14, 2015
Backport 56a0a797f210e04746f2888116365d29a4bb6afc from V8 upstream to
include post-mortem metadata used by mdb_v8 to support V8 4.6.

Original commit message:

  Update post-mortem metadata generation

  mdb_v8, a post-mortem debugger for Node.js, now uses JSArrayBuffer's
  backing_store property and JSArrayBufferView's byte_offset property to
  get access to the content of Buffer instances in node (which are
  Uint8Array instances). This change adds post-mortem metadata for these
  two properties.

  This change also fixes a typo in
  inobject_properties_of_constructor_function_index_offset that was added
  to gen-postmortem-metadata in a previous change. It should be named
  inobject_properties_or_constructor_function_index instead.

  [email protected]

  Review URL: https://codereview.chromium.org/1363403003

  Cr-Commit-Position: refs/heads/master@{#30926}

Ref: #3057
PR-URL: #3351
Reviewed-By: indutny - Fedor Indutny <[email protected]>
Reviewed-By: bnoordhuis - Ben Noordhuis <[email protected]>
@misterdjules misterdjules deleted the vee-eight-4.6-postmortem-update branch July 24, 2017 17:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
post-mortem Issues and PRs related to the post-mortem diagnostics of Node.js. v8 engine Issues and PRs related to the V8 dependency.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants