New issue
Advanced search Search tips

Issue 759831 link

Starred by 1 user
Status: Fixed
Owner:
jbroman@chromium.org
Closed: Sep 2017
Cc:
yangguo@chromium.org
peria@chromium.org
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug



Sign in to add a comment

V8 context snapshot external reference table is non-const

Project Member Reported by jbroman@chromium.org, Aug 28 2017 
This array currently resides in the (writable) .data ELF segment.

If we make it const, it can be stored in the .data.rel.ro segment, which is guaranteed to be read-only and shared between processes to the extent possible.

This requires changes both in V8 and Chromium, but seems straightforward (and there does not appear to be a need for V8 to modify it, so const is intuitively correct as well).

Comment 1 by peria@chromium.org, Aug 29 2017 

SGTM, and the change should be easy enough.
Project Member

Comment 2 by bugdroid1@chromium.org, Aug 29 2017 

The following revision refers to this bug:
  https://chromium.googlesource.com/v8/v8.git/+/779bb1b1724c02a7b88480db857d8d4c3f5ede74

commit 779bb1b1724c02a7b88480db857d8d4c3f5ede74
Author: Jeremy Roman <jbroman@chromium.org>
Date: Tue Aug 29 14:15:10 2017

[snapshot] Make the API external reference table const.

Chromium side: https://chromium-review.googlesource.com/c/chromium/src/+/639552

Bug:  chromium:759831 
Cq-Include-Trybots: master.tryserver.chromium.linux:linux_chromium_rel_ng
Change-Id: I1b217c4fa4c930733dcfab982879bf41936a3a83
Reviewed-on: https://chromium-review.googlesource.com/639551
Commit-Queue: Jeremy Roman <jbroman@chromium.org>
Reviewed-by: Yang Guo <yangguo@chromium.org>
Cr-Commit-Position: refs/heads/master@{#47683}
[modify] https://crrev.com/779bb1b1724c02a7b88480db857d8d4c3f5ede74/include/v8.h
[modify] https://crrev.com/779bb1b1724c02a7b88480db857d8d4c3f5ede74/src/api.cc
[modify] https://crrev.com/779bb1b1724c02a7b88480db857d8d4c3f5ede74/src/isolate.h
[modify] https://crrev.com/779bb1b1724c02a7b88480db857d8d4c3f5ede74/src/snapshot/serializer-common.cc
[modify] https://crrev.com/779bb1b1724c02a7b88480db857d8d4c3f5ede74/src/snapshot/serializer-common.h
Project Member

Comment 3 by bugdroid1@chromium.org, Sep 11 2017 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/7262266cc081cf7755e6f25ffe0179b812c91744

commit 7262266cc081cf7755e6f25ffe0179b812c91744
Author: Jeremy Roman <jbroman@chromium.org>
Date: Mon Sep 11 03:51:57 2017

Make the V8 external reference list const.

V8 side: https://chromium-review.googlesource.com/c/v8/v8/+/639551

Bug:  759831 
Change-Id: Ie2e21ec3b5b2cde7632d93528594619e797a5014
Reviewed-on: https://chromium-review.googlesource.com/639552
Reviewed-by: Kentaro Hara <haraken@chromium.org>
Reviewed-by: Hitoshi Yoshida <peria@chromium.org>
Commit-Queue: Jeremy Roman <jbroman@chromium.org>
Cr-Commit-Position: refs/heads/master@{#500833}
[modify] https://crrev.com/7262266cc081cf7755e6f25ffe0179b812c91744/gin/isolate_holder.cc
[modify] https://crrev.com/7262266cc081cf7755e6f25ffe0179b812c91744/gin/public/isolate_holder.h
[modify] https://crrev.com/7262266cc081cf7755e6f25ffe0179b812c91744/third_party/WebKit/Source/bindings/core/v8/V8ContextSnapshot.cpp
[modify] https://crrev.com/7262266cc081cf7755e6f25ffe0179b812c91744/third_party/WebKit/Source/bindings/core/v8/V8ContextSnapshot.h
[modify] https://crrev.com/7262266cc081cf7755e6f25ffe0179b812c91744/third_party/WebKit/Source/bindings/core/v8/V8Initializer.cpp
[modify] https://crrev.com/7262266cc081cf7755e6f25ffe0179b812c91744/third_party/WebKit/Source/bindings/core/v8/V8Initializer.h
[modify] https://crrev.com/7262266cc081cf7755e6f25ffe0179b812c91744/third_party/WebKit/Source/bindings/modules/v8/V8ContextSnapshotExternalReferences.h
[modify] https://crrev.com/7262266cc081cf7755e6f25ffe0179b812c91744/third_party/WebKit/Source/bindings/templates/external_reference_table.cpp.tmpl
[modify] https://crrev.com/7262266cc081cf7755e6f25ffe0179b812c91744/third_party/WebKit/Source/platform/bindings/V8PerIsolateData.cpp
[modify] https://crrev.com/7262266cc081cf7755e6f25ffe0179b812c91744/third_party/WebKit/Source/platform/bindings/V8PerIsolateData.h

Comment 4 by jbroman@chromium.org, Sep 11 2017

Status: Fixed (was: Started)
►

Sign in to add a comment