Skip to content

Commit

Permalink
Revert of [gin] Unify snapshot loading on Windows and other platforms…
Browse files Browse the repository at this point in the history
…. (patchset chromium#2 id:20001 of https://codereview.chromium.org/2103903002/ )

Reason for revert:
Investigate effect on Win7 startup.cold.blank_page metrics.

BUG=501799,625516

Original issue's description:
> [gin] Unify snapshot loading on Windows and other platforms.
>
> Removes extra v8 snapshot and natives validation on Windows. All platforms
> have the same validation henceforth.
>
> BUG=501799
>
> Committed: https://crrev.com/10b5a96b363a6ff23176d27a2e7fb1bba300f089
> Cr-Commit-Position: refs/heads/master@{#403430}

TBR=jochen@chromium.org,ben@chromium.org
# Not skipping CQ checks because original CL landed more than 1 days ago.
BUG=501799

Review-Url: https://codereview.chromium.org/2141043002
Cr-Commit-Position: refs/heads/master@{#405111}
  • Loading branch information
ohodson authored and Commit bot committed Jul 13, 2016
1 parent dfc9cd8 commit 2a65319
Show file tree
Hide file tree
Showing 7 changed files with 260 additions and 11 deletions.
4 changes: 4 additions & 0 deletions BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -344,6 +344,10 @@ group("both_gn_and_gyp") {
deps += [ "//components/policy:policy_templates" ]
}

if (v8_use_external_startup_data) {
deps += [ "//gin:gin_v8_snapshot_fingerprint" ]
}

if (is_win) {
deps += [
"//chrome/installer/gcapi",
Expand Down
1 change: 1 addition & 0 deletions build/gn_migration.gypi
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,7 @@
'../content/content_shell_and_tests.gyp:content_unittests',
'../crypto/crypto.gyp:crypto_unittests',
'../device/device_tests.gyp:device_unittests',
'../gin/gin.gyp:gin_v8_snapshot_fingerprint',
'../gpu/gpu.gyp:angle_unittests',
'../gpu/gpu.gyp:gl_tests',
'../gpu/gpu.gyp:gpu_perftests',
Expand Down
35 changes: 35 additions & 0 deletions gin/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,11 @@ component("gin") {
"//base/third_party/dynamic_annotations",
"//crypto",
]
if (v8_use_external_startup_data && is_win) {
public_deps += [ ":gin_v8_snapshot_fingerprint" ]
sources += [ "$target_gen_dir/v8_snapshot_fingerprint.cc" ]
defines += [ "V8_VERIFY_EXTERNAL_STARTUP_DATA" ]
}

if (is_mac) {
libs = [ "CoreFoundation.framework" ]
Expand All @@ -94,6 +99,36 @@ component("gin") {
configs += [ "//v8:external_startup_data" ]
}

if (v8_use_external_startup_data) {
action("gin_v8_snapshot_fingerprint") {
script = "//gin/fingerprint/fingerprint_v8_snapshot.py"

snapshot_file = "$root_out_dir/snapshot_blob.bin"
natives_file = "$root_out_dir/natives_blob.bin"
output_file = "$target_gen_dir/v8_snapshot_fingerprint.cc"

args = [
"--snapshot_file",
rebase_path(snapshot_file, root_build_dir),
"--natives_file",
rebase_path(natives_file, root_build_dir),
"--output_file",
rebase_path(output_file, root_build_dir),
]
inputs = [
snapshot_file,
natives_file,
]
outputs = [
output_file,
]

deps = [
"//v8",
]
}
}

executable("gin_shell") {
sources = [
"shell/gin_main.cc",
Expand Down
47 changes: 47 additions & 0 deletions gin/fingerprint/fingerprint_v8_snapshot.gypi
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
# Copyright 2015 The Chromium Authors. All rights reserved.
# Use of this source code is governed by a BSD-style license that can be
# found in the LICENSE file.

# This file is meant to be included into a target to provide a rule that
# fingerprints the v8 snapshot and generates a .cc file which includes this
# fingerprint.
#
# To use this, create a gyp target with the following form:
# {
# 'target_name': 'gin_v8_snapshot_fingerprint',
# 'type': 'none',
# 'variables': {
# 'snapshot_file': 'snapshot blob file to be fingerprinted',
# 'natives_file': 'natives blob file to be fingerprinted',
# 'output_file': 'output .cc file to generate with fingerprints',
# },
# 'includes': [ '../gin/fingerprint/fingerprint_v8_snapshot.gypi' ],
# },
#

{
'conditions': [
['v8_use_external_startup_data==1', {
'actions': [
{
'action_name': 'Generate V8 snapshot fingerprint',
'message': 'Generating V8 snapshot fingerprint',
'inputs': [
'<(DEPTH)/gin/fingerprint/fingerprint_v8_snapshot.py',
'<(snapshot_file)',
'<(natives_file)',
],
'outputs': [
'<(output_file)',
],
'action': [
'python', '<(DEPTH)/gin/fingerprint/fingerprint_v8_snapshot.py',
'--snapshot_file=<(snapshot_file)',
'--natives_file=<(natives_file)',
'--output_file=<(output_file)',
],
}
],
}],
],
}
86 changes: 86 additions & 0 deletions gin/fingerprint/fingerprint_v8_snapshot.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,86 @@
#!/usr/bin/env python
#
# Copyright 2015 The Chromium Authors. All rights reserved.
# Use of this source code is governed by a BSD-style license that can be
# found in the LICENSE file.

"""Fingerprints the V8 snapshot blob files.
Constructs a SHA256 fingerprint of the V8 natives and snapshot blob files and
creates a .cc file which includes these fingerprint as variables.
"""

import hashlib
import optparse
import os
import sys

_HEADER = """// Copyright 2015 The Chromium Authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
// This file was generated by fingerprint_v8_snapshot.py.
namespace gin {
"""

_FOOTER = """
} // namespace gin
"""


def FingerprintFile(file_path):
input_file = open(file_path, 'rb')
sha256 = hashlib.sha256()
while True:
block = input_file.read(sha256.block_size)
if not block:
break
sha256.update(block)
return sha256.digest()


def WriteFingerprint(output_file, variable_name, fingerprint):
output_file.write('\nextern const unsigned char %s[] = { ' % variable_name)
for byte in fingerprint:
output_file.write(str(ord(byte)) + ', ')
output_file.write('};\n')


def WriteOutputFile(natives_fingerprint,
snapshot_fingerprint,
output_file_path):
output_dir_path = os.path.dirname(output_file_path)
if not os.path.exists(output_dir_path):
os.makedirs(output_dir_path)
output_file = open(output_file_path, 'w')

output_file.write(_HEADER)
WriteFingerprint(output_file, 'g_natives_fingerprint', natives_fingerprint)
output_file.write('\n')
WriteFingerprint(output_file, 'g_snapshot_fingerprint', snapshot_fingerprint)
output_file.write(_FOOTER)


def main():
parser = optparse.OptionParser()

parser.add_option('--snapshot_file',
help='The input V8 snapshot blob file path.')
parser.add_option('--natives_file',
help='The input V8 natives blob file path.')
parser.add_option('--output_file',
help='The path for the output cc file which will be write.')

options, _ = parser.parse_args()

natives_fingerprint = FingerprintFile(options.natives_file)
snapshot_fingerprint = FingerprintFile(options.snapshot_file)
WriteOutputFile(
natives_fingerprint, snapshot_fingerprint, options.output_file)

return 0


if __name__ == '__main__':
sys.exit(main())
24 changes: 24 additions & 0 deletions gin/gin.gyp
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,30 @@
'wrappable.h',
'wrapper_info.cc',
],
'conditions': [
['v8_use_external_startup_data==1 and OS=="win"', {
'dependencies': [
'gin_v8_snapshot_fingerprint',
'../crypto/crypto.gyp:crypto',
],
'sources': [
'<(gin_gen_path)/v8_snapshot_fingerprint.cc',
],
'defines': [
'V8_VERIFY_EXTERNAL_STARTUP_DATA',
]
}],
],
},
{
'target_name': 'gin_v8_snapshot_fingerprint',
'type': 'none',
'variables': {
'snapshot_file': '<(PRODUCT_DIR)/snapshot_blob.bin',
'natives_file': '<(PRODUCT_DIR)/natives_blob.bin',
'output_file': '<(gin_gen_path)/v8_snapshot_fingerprint.cc',
},
'includes': [ '../gin/fingerprint/fingerprint_v8_snapshot.gypi' ],
},
{
'target_name': 'gin_shell',
Expand Down
74 changes: 63 additions & 11 deletions gin/v8_initializer.cc
Original file line number Diff line number Diff line change
Expand Up @@ -191,6 +191,36 @@ static const OpenedFileMap::mapped_type OpenFileIfNecessary(
return opened;
}

#if defined(V8_VERIFY_EXTERNAL_STARTUP_DATA)
bool VerifyV8StartupFile(base::MemoryMappedFile** file,
const unsigned char* fingerprint) {
unsigned char output[crypto::kSHA256Length];
crypto::SHA256HashString(
base::StringPiece(reinterpret_cast<const char*>((*file)->data()),
(*file)->length()),
output, sizeof(output));
if (!memcmp(fingerprint, output, sizeof(output))) {
return true;
}

// TODO(oth): Remove this temporary diagnostics for http://crbug.com/501799
uint64_t input[sizeof(output)];
memcpy(input, fingerprint, sizeof(input));

base::debug::Alias(output);
base::debug::Alias(input);

const uint64_t* o64 = reinterpret_cast<const uint64_t*>(output);
const uint64_t* f64 = reinterpret_cast<const uint64_t*>(fingerprint);
LOG(FATAL) << "Natives length " << (*file)->length()
<< " H(computed) " << o64[0] << o64[1] << o64[2] << o64[3]
<< " H(expected) " << f64[0] << f64[1] << f64[2] << f64[3];

delete *file;
*file = NULL;
return false;
}
#endif // V8_VERIFY_EXTERNAL_STARTUP_DATA
#endif // V8_USE_EXTERNAL_STARTUP_DATA

bool GenerateEntropy(unsigned char* buffer, size_t amount) {
Expand All @@ -213,37 +243,47 @@ bool ShouldUseIgnition() {
} // namespace

#if defined(V8_USE_EXTERNAL_STARTUP_DATA)

namespace {
#if defined(V8_VERIFY_EXTERNAL_STARTUP_DATA)
// Defined in gen/gin/v8_snapshot_fingerprint.cc
extern const unsigned char g_natives_fingerprint[];
extern const unsigned char g_snapshot_fingerprint[];
#endif // V8_VERIFY_EXTERNAL_STARTUP_DATA

enum LoadV8FileResult {
V8_LOAD_SUCCESS = 0,
V8_LOAD_FAILED_OPEN,
V8_LOAD_FAILED_MAP,
V8_LOAD_FAILED_VERIFY, // Deprecated.
V8_LOAD_FAILED_VERIFY,
V8_LOAD_MAX_VALUE
};

LoadV8FileResult MapOpenedFile(
const OpenedFileMap::mapped_type& file_region,
base::MemoryMappedFile** mmapped_file_out) {
static LoadV8FileResult MapVerify(const OpenedFileMap::mapped_type& file_region,
#if defined(V8_VERIFY_EXTERNAL_STARTUP_DATA)
const unsigned char* fingerprint,
#endif
base::MemoryMappedFile** mmapped_file_out) {
if (file_region.first == base::kInvalidPlatformFile)
return V8_LOAD_FAILED_OPEN;
if (!MapV8File(file_region.first, file_region.second, mmapped_file_out))
return V8_LOAD_FAILED_MAP;
#if defined(V8_VERIFY_EXTERNAL_STARTUP_DATA)
if (!VerifyV8StartupFile(mmapped_file_out, fingerprint))
return V8_LOAD_FAILED_VERIFY;
#endif // V8_VERIFY_EXTERNAL_STARTUP_DATA
return V8_LOAD_SUCCESS;
}

} // namespace

// static
void V8Initializer::LoadV8Snapshot() {
if (g_mapped_snapshot)
return;

OpenFileIfNecessary(kSnapshotFileName);
LoadV8FileResult result = MapOpenedFile(GetOpenedFile(kSnapshotFileName),
&g_mapped_snapshot);
LoadV8FileResult result = MapVerify(GetOpenedFile(kSnapshotFileName),
#if defined(V8_VERIFY_EXTERNAL_STARTUP_DATA)
g_snapshot_fingerprint,
#endif
&g_mapped_snapshot);
// V8 can't start up without the source of the natives, but it can
// start up (slower) without the snapshot.
UMA_HISTOGRAM_ENUMERATION("V8.Initializer.LoadV8Snapshot.Result", result,
Expand All @@ -255,7 +295,10 @@ void V8Initializer::LoadV8Natives() {
return;

OpenFileIfNecessary(kNativesFileName);
LoadV8FileResult result = MapOpenedFile(GetOpenedFile(kNativesFileName),
LoadV8FileResult result = MapVerify(GetOpenedFile(kNativesFileName),
#if defined(V8_VERIFY_EXTERNAL_STARTUP_DATA)
g_natives_fingerprint,
#endif
&g_mapped_natives);
if (result != V8_LOAD_SUCCESS) {
LOG(FATAL) << "Couldn't mmap v8 natives data file, status code is "
Expand Down Expand Up @@ -283,6 +326,10 @@ void V8Initializer::LoadV8SnapshotFromFD(base::PlatformFile snapshot_pf,
LoadV8FileResult result = V8_LOAD_SUCCESS;
if (!MapV8File(snapshot_pf, snapshot_region, &g_mapped_snapshot))
result = V8_LOAD_FAILED_MAP;
#if defined(V8_VERIFY_EXTERNAL_STARTUP_DATA)
if (!VerifyV8StartupFile(&g_mapped_snapshot, g_snapshot_fingerprint))
result = V8_LOAD_FAILED_VERIFY;
#endif // V8_VERIFY_EXTERNAL_STARTUP_DATA
if (result == V8_LOAD_SUCCESS) {
g_opened_files.Get()[kSnapshotFileName] =
std::make_pair(snapshot_pf, snapshot_region);
Expand Down Expand Up @@ -310,6 +357,11 @@ void V8Initializer::LoadV8NativesFromFD(base::PlatformFile natives_pf,
if (!MapV8File(natives_pf, natives_region, &g_mapped_natives)) {
LOG(FATAL) << "Couldn't mmap v8 natives data file";
}
#if defined(V8_VERIFY_EXTERNAL_STARTUP_DATA)
if (!VerifyV8StartupFile(&g_mapped_natives, g_natives_fingerprint)) {
LOG(FATAL) << "Couldn't verify contents of v8 natives data file";
}
#endif // V8_VERIFY_EXTERNAL_STARTUP_DATA
g_opened_files.Get()[kNativesFileName] =
std::make_pair(natives_pf, natives_region);
}
Expand Down

0 comments on commit 2a65319

Please sign in to comment.