Join GitHub today
GitHub is home to over 40 million developers working together to host and review code, manage projects, and build software together.
Sign upsrc: fix crash with SyntheticModule#setExports #30062
Closed
Conversation
This comment has been minimized.
This comment has been minimized.
|
lol I was just about to open a pr for this |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
Original commit message:
Make SetSyntheticModuleExport throw instead of crash for nonexistent export name
Per spec, Module::SetSyntheticModuleExport should throw a ReferenceError
when called with an export name that was not supplied when constructing
that SyntheticModule. Instead, the current implementation crashes with
a failed CHECK().
Add a new Module::SyntheticModuleSetExport that throws (without an ensuing
crash) for this case, and deprecate the old
Module::SetSyntheticModuleExport.
Bug: v8:9828
Change-Id: I3b3d353064c3851882781818099bd8f6ee74c809
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1860996
Reviewed-by: Adam Klein <adamk@chromium.org>
Reviewed-by: Georg Neis <neis@chromium.org>
Commit-Queue: Dan Clark <daniec@microsoft.com>
Cr-Commit-Position: refs/heads/master@{#64438}
Refs: v8/v8@777fa98
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
Trott
added a commit
that referenced
this pull request
Oct 26, 2019
Original commit message:
Make SetSyntheticModuleExport throw instead of crash for nonexistent export name
Per spec, Module::SetSyntheticModuleExport should throw a ReferenceError
when called with an export name that was not supplied when constructing
that SyntheticModule. Instead, the current implementation crashes with
a failed CHECK().
Add a new Module::SyntheticModuleSetExport that throws (without an ensuing
crash) for this case, and deprecate the old
Module::SetSyntheticModuleExport.
Bug: v8:9828
Change-Id: I3b3d353064c3851882781818099bd8f6ee74c809
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1860996
Reviewed-by: Adam Klein <adamk@chromium.org>
Reviewed-by: Georg Neis <neis@chromium.org>
Commit-Queue: Dan Clark <daniec@microsoft.com>
Cr-Commit-Position: refs/heads/master@{#64438}
Refs: v8/v8@777fa98
PR-URL: #30062
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com>
Reviewed-By: Gus Caplan <me@gus.host>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Trott
added a commit
that referenced
this pull request
Oct 26, 2019
Use the new non-deprecated V8 API for that. PR-URL: #30062 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com> Reviewed-By: Gus Caplan <me@gus.host> Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com>
This comment has been minimized.
This comment has been minimized.
|
Landed in 7b2de22...ab78d4d |
targos
added a commit
that referenced
this pull request
Oct 28, 2019
Original commit message:
Make SetSyntheticModuleExport throw instead of crash for nonexistent export name
Per spec, Module::SetSyntheticModuleExport should throw a ReferenceError
when called with an export name that was not supplied when constructing
that SyntheticModule. Instead, the current implementation crashes with
a failed CHECK().
Add a new Module::SyntheticModuleSetExport that throws (without an ensuing
crash) for this case, and deprecate the old
Module::SetSyntheticModuleExport.
Bug: v8:9828
Change-Id: I3b3d353064c3851882781818099bd8f6ee74c809
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1860996
Reviewed-by: Adam Klein <adamk@chromium.org>
Reviewed-by: Georg Neis <neis@chromium.org>
Commit-Queue: Dan Clark <daniec@microsoft.com>
Cr-Commit-Position: refs/heads/master@{#64438}
Refs: v8/v8@777fa98
PR-URL: #30062
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com>
Reviewed-By: Gus Caplan <me@gus.host>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
targos
added a commit
that referenced
this pull request
Oct 28, 2019
Use the new non-deprecated V8 API for that. PR-URL: #30062 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com> Reviewed-By: Gus Caplan <me@gus.host> Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com>
This comment has been minimized.
This comment has been minimized.
|
Should only land on v12.x if #29864 does. |
aduh95
added a commit
to aduh95/node
that referenced
this pull request
Jan 7, 2020
Original commit message:
Make SetSyntheticModuleExport throw instead of crash for nonexistent export name
Per spec, Module::SetSyntheticModuleExport should throw a ReferenceError
when called with an export name that was not supplied when constructing
that SyntheticModule. Instead, the current implementation crashes with
a failed CHECK().
Add a new Module::SyntheticModuleSetExport that throws (without an ensuing
crash) for this case, and deprecate the old
Module::SetSyntheticModuleExport.
Bug: v8:9828
Change-Id: I3b3d353064c3851882781818099bd8f6ee74c809
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1860996
Reviewed-by: Adam Klein <adamk@chromium.org>
Reviewed-by: Georg Neis <neis@chromium.org>
Commit-Queue: Dan Clark <daniec@microsoft.com>
Cr-Commit-Position: refs/heads/master@{#64438}
Refs: v8/v8@777fa98
PR-URL: nodejs#30062
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com>
Reviewed-By: Gus Caplan <me@gus.host>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
aduh95
added a commit
to aduh95/node
that referenced
this pull request
Jan 7, 2020
Use the new non-deprecated V8 API for that. PR-URL: nodejs#30062 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com> Reviewed-By: Gus Caplan <me@gus.host> Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com>
aduh95
added a commit
to aduh95/node
that referenced
this pull request
Jan 8, 2020
Original commit message:
Make SetSyntheticModuleExport throw instead of crash for nonexistent export name
Per spec, Module::SetSyntheticModuleExport should throw a ReferenceError
when called with an export name that was not supplied when constructing
that SyntheticModule. Instead, the current implementation crashes with
a failed CHECK().
Add a new Module::SyntheticModuleSetExport that throws (without an ensuing
crash) for this case, and deprecate the old
Module::SetSyntheticModuleExport.
Bug: v8:9828
Change-Id: I3b3d353064c3851882781818099bd8f6ee74c809
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1860996
Reviewed-by: Adam Klein <adamk@chromium.org>
Reviewed-by: Georg Neis <neis@chromium.org>
Commit-Queue: Dan Clark <daniec@microsoft.com>
Cr-Commit-Position: refs/heads/master@{#64438}
Refs: v8/v8@777fa98
PR-URL: nodejs#30062
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com>
Reviewed-By: Gus Caplan <me@gus.host>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
aduh95
added a commit
to aduh95/node
that referenced
this pull request
Jan 8, 2020
Use the new non-deprecated V8 API for that. PR-URL: nodejs#30062 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com> Reviewed-By: Gus Caplan <me@gus.host> Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com>
targos
added a commit
that referenced
this pull request
Jan 8, 2020
Original commit message:
Make SetSyntheticModuleExport throw instead of crash for nonexistent export name
Per spec, Module::SetSyntheticModuleExport should throw a ReferenceError
when called with an export name that was not supplied when constructing
that SyntheticModule. Instead, the current implementation crashes with
a failed CHECK().
Add a new Module::SyntheticModuleSetExport that throws (without an ensuing
crash) for this case, and deprecate the old
Module::SetSyntheticModuleExport.
Bug: v8:9828
Change-Id: I3b3d353064c3851882781818099bd8f6ee74c809
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1860996
Reviewed-by: Adam Klein <adamk@chromium.org>
Reviewed-by: Georg Neis <neis@chromium.org>
Commit-Queue: Dan Clark <daniec@microsoft.com>
Cr-Commit-Position: refs/heads/master@{#64438}
Refs: v8/v8@777fa98
PR-URL: #30062
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com>
Reviewed-By: Gus Caplan <me@gus.host>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
targos
added a commit
that referenced
this pull request
Jan 8, 2020
Use the new non-deprecated V8 API for that. PR-URL: #30062 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com> Reviewed-By: Gus Caplan <me@gus.host> Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com>
BethGriggs
added a commit
that referenced
this pull request
Feb 6, 2020
Original commit message:
Make SetSyntheticModuleExport throw instead of crash for nonexistent export name
Per spec, Module::SetSyntheticModuleExport should throw a ReferenceError
when called with an export name that was not supplied when constructing
that SyntheticModule. Instead, the current implementation crashes with
a failed CHECK().
Add a new Module::SyntheticModuleSetExport that throws (without an ensuing
crash) for this case, and deprecate the old
Module::SetSyntheticModuleExport.
Bug: v8:9828
Change-Id: I3b3d353064c3851882781818099bd8f6ee74c809
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1860996
Reviewed-by: Adam Klein <adamk@chromium.org>
Reviewed-by: Georg Neis <neis@chromium.org>
Commit-Queue: Dan Clark <daniec@microsoft.com>
Cr-Commit-Position: refs/heads/master@{#64438}
Refs: v8/v8@777fa98
PR-URL: #30062
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com>
Reviewed-By: Gus Caplan <me@gus.host>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
BethGriggs
added a commit
that referenced
this pull request
Feb 6, 2020
Use the new non-deprecated V8 API for that. PR-URL: #30062 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com> Reviewed-By: Gus Caplan <me@gus.host> Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com>
Merged
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
targos commentedOct 22, 2019
Checklist
make -j4 test(UNIX), orvcbuild test(Windows) passes