Skip to content

Commit

Permalink
Reland "[SDK] Adds an SDK hash to kernels and the VM."
Browse files Browse the repository at this point in the history
Note: This is a reland of https://dart-review.googlesource.com/c/sdk/+/150343

Adds a new SDK hash to kernels and the VM which is optionally checked
to verify kernels are built for the same SDK as the VM.
This helps catch incompatibilities that are currently causing
subtle bugs and (not so subtle) crashes.

The SDK hash is encoded in kernels as a new field in components.
The hash is derived from the 10 byte git short hash.

This new check can be disabled via:
  tools/gn.py ... --no-verify-sdk-hash

This CL bumps the min. (and max.) supported kernel format version,
making the VM backwards incompatible from this point back.

This also bumps the min. and current ABI version.

Bug: #41802
Change-Id: I2f85945045a603eb9dcfd1f2c0d0d024bd84a956
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/152802
Commit-Queue: Clement Skau <cskau@google.com>
Reviewed-by: Martin Kustermann <kustermann@google.com>
  • Loading branch information
Clement Skau authored and commit-bot@chromium.org committed Jul 7, 2020
1 parent 315ecf2 commit 0ce8398
Show file tree
Hide file tree
Showing 28 changed files with 511 additions and 198 deletions.
2 changes: 1 addition & 1 deletion DEPS
Expand Up @@ -512,7 +512,7 @@ deps = {
"packages": [
{
"package": "dart/cfe/dart2js_dills",
"version": "binary_version:42",
"version": "binary_version:43_2",
}
],
"dep_type": "cipd",
Expand Down
50 changes: 26 additions & 24 deletions pkg/front_end/test/binary_md_dill_reader.dart
Expand Up @@ -473,31 +473,33 @@ class BinaryMdDillReader {
type = type.substring(0, type.indexOf("["));
type = _lookupGenericType(typeNames, type, types);

int intCount = -1;
if (vars[count] != null && vars[count] is int) {
intCount = vars[count];
} else if (count.contains(".")) {
List<String> countData =
count.split(regExpSplit).map((s) => s.trim()).toList();
if (vars[countData[0]] != null) {
dynamic v = vars[countData[0]];
if (v is Map &&
countData[1] == "last" &&
v["items"] is List &&
v["items"].last is int) {
intCount = v["items"].last;
} else if (v is Map && v[countData[1]] != null) {
v = v[countData[1]];
if (v is Map && v[countData[2]] != null) {
v = v[countData[2]];
if (v is int) intCount = v;
} else if (v is int &&
countData.length == 4 &&
countData[2] == "+") {
intCount = v + int.parse(countData[3]);
int intCount = int.tryParse(count) ?? -1;
if (intCount == -1) {
if (vars[count] != null && vars[count] is int) {
intCount = vars[count];
} else if (count.contains(".")) {
List<String> countData =
count.split(regExpSplit).map((s) => s.trim()).toList();
if (vars[countData[0]] != null) {
dynamic v = vars[countData[0]];
if (v is Map &&
countData[1] == "last" &&
v["items"] is List &&
v["items"].last is int) {
intCount = v["items"].last;
} else if (v is Map && v[countData[1]] != null) {
v = v[countData[1]];
if (v is Map && v[countData[2]] != null) {
v = v[countData[2]];
if (v is int) intCount = v;
} else if (v is int &&
countData.length == 4 &&
countData[2] == "+") {
intCount = v + int.parse(countData[3]);
}
} else {
throw "Unknown dot to int ($count)";
}
} else {
throw "Unknown dot to int ($count)";
}
}
}
Expand Down
4 changes: 4 additions & 0 deletions pkg/front_end/test/spell_checking_list_code.txt
Expand Up @@ -314,6 +314,7 @@ disallow
disambiguator
disjoint
dispatched
distribute
divided
dmitryas
doc
Expand All @@ -326,6 +327,7 @@ downloaded
downloading
dq
dquote
dsdk
dst
dummy
dupdate
Expand Down Expand Up @@ -437,12 +439,14 @@ futured
futureor
g
gardening
gen
generation
gets
getter1a
getter1b
getting
gft
git
github
glb
glob
Expand Down
3 changes: 2 additions & 1 deletion pkg/kernel/binary.md
Expand Up @@ -143,7 +143,8 @@ type CanonicalName {

type ComponentFile {
UInt32 magic = 0x90ABCDEF;
UInt32 formatVersion = 42;
UInt32 formatVersion = 43;
Byte[10] shortSdkHash;
List<String> problemsAsJson; // Described in problems.md.
Library[] libraries;
UriSource sourceMap;
Expand Down
28 changes: 27 additions & 1 deletion pkg/kernel/lib/binary/ast_from_binary.dart
Expand Up @@ -5,6 +5,7 @@ library kernel.ast_from_binary;

import 'dart:core' hide MapEntry;
import 'dart:developer';
import 'dart:convert';
import 'dart:typed_data';

import '../ast.dart';
Expand All @@ -28,11 +29,22 @@ class InvalidKernelVersionError {
InvalidKernelVersionError(this.version);

String toString() {
return 'Unexpected Kernel version ${version} '
return 'Unexpected Kernel Format Version ${version} '
'(expected ${Tag.BinaryFormatVersion}).';
}
}

class InvalidKernelSdkVersionError {
final String version;

InvalidKernelSdkVersionError(this.version);

String toString() {
return 'Unexpected Kernel SDK Version ${version} '
'(expected ${expectedSdkHash}).';
}
}

class CompilationModeError {
final String message;

Expand Down Expand Up @@ -484,6 +496,13 @@ class BinaryBuilder {
if (_bytes.length == 0) throw new StateError("Empty input given.");
}

void _readAndVerifySdkHash() {
final sdkHash = ascii.decode(readBytes(sdkHashLength));
if (!isValidSdkHash(sdkHash)) {
throw InvalidKernelSdkVersionError(sdkHash);
}
}

/// Deserializes a kernel component and stores it in [component].
///
/// When linking with a non-empty component, canonical names must have been
Expand Down Expand Up @@ -511,6 +530,9 @@ class BinaryBuilder {
if (version != Tag.BinaryFormatVersion) {
throw InvalidKernelVersionError(version);
}

_readAndVerifySdkHash();

_byteOffset = offset;
List<int> componentFileSizes = _indexComponents();
if (componentFileSizes.length > 1) {
Expand Down Expand Up @@ -694,6 +716,8 @@ class BinaryBuilder {
throw InvalidKernelVersionError(formatVersion);
}

_readAndVerifySdkHash();

// Read component index from the end of this ComponentFiles serialized data.
_ComponentIndex index = _readComponentIndex(componentFileSize);

Expand All @@ -718,6 +742,8 @@ class BinaryBuilder {
throw InvalidKernelVersionError(formatVersion);
}

_readAndVerifySdkHash();

List<String> problemsAsJson = readListOfStrings();
if (problemsAsJson != null) {
component.problemsAsJson ??= <String>[];
Expand Down
2 changes: 2 additions & 0 deletions pkg/kernel/lib/binary/ast_to_binary.dart
Expand Up @@ -4,6 +4,7 @@
library kernel.ast_to_binary;

import 'dart:core' hide MapEntry;
import 'dart:convert';
import 'dart:developer';
import 'dart:io' show BytesBuilder;
import 'dart:typed_data';
Expand Down Expand Up @@ -537,6 +538,7 @@ class BinaryPrinter implements Visitor<void>, BinarySink {
final componentOffset = getBufferOffset();
writeUInt32(Tag.ComponentFile);
writeUInt32(Tag.BinaryFormatVersion);
writeBytes(ascii.encode(expectedSdkHash));
writeListOfStrings(component.problemsAsJson);
indexLinkTable(component);
_collectMetadata(component);
Expand Down
26 changes: 25 additions & 1 deletion pkg/kernel/lib/binary/tag.dart
Expand Up @@ -149,7 +149,7 @@ class Tag {
/// Internal version of kernel binary format.
/// Bump it when making incompatible changes in kernel binaries.
/// Keep in sync with runtime/vm/kernel_binary.h, pkg/kernel/binary.md.
static const int BinaryFormatVersion = 42;
static const int BinaryFormatVersion = 43;
}

abstract class ConstantTag {
Expand All @@ -169,3 +169,27 @@ abstract class ConstantTag {
static const int UnevaluatedConstant = 12;
// 13 is occupied by [SetConstant]
}

const int sdkHashLength = 10; // Bytes, a Git "short hash".

const String sdkHashNull = '0000000000';

// Will be correct hash for Flutter SDK / Dart SDK we distribute.
// If non-null we will validate when consuming kernel, will use when producing
// kernel.
// If null, local development setting (e.g. run gen_kernel.dart from source),
// we put 0x00..00 into when producing, do not validate when consuming.
String get expectedSdkHash {
final sdkHash =
const String.fromEnvironment('sdk_hash', defaultValue: sdkHashNull);
if (sdkHash.length != sdkHashLength) {
throw '-Dsdk_hash=<hash> must be a ${sdkHashLength} byte string!';
}
return sdkHash;
}

bool isValidSdkHash(String sdkHash) {
return (sdkHash == sdkHashNull ||
expectedSdkHash == sdkHashNull ||
sdkHash == expectedSdkHash);
}
2 changes: 2 additions & 0 deletions runtime/bin/BUILD.gn
Expand Up @@ -964,9 +964,11 @@ prebuilt_dart_action("gen_kernel_bytecode_dill") {

abs_depfile = rebase_path(depfile)
rebased_output = rebase_path(output, root_build_dir)

vm_args = [
"--depfile=$abs_depfile",
"--depfile_output_filename=$rebased_output",
"-Dsdk_hash=$sdk_hash",
]

args = [
Expand Down
88 changes: 88 additions & 0 deletions runtime/tests/vm/dart/sdk_hash_test.dart
@@ -0,0 +1,88 @@
// Copyright (c) 2020, the Dart project authors. Please see the AUTHORS file
// for details. All rights reserved. Use of this source code is governed by a
// BSD-style license that can be found in the LICENSE file.

import 'dart:async';
import 'dart:convert';
import 'dart:io';
import 'dart:typed_data';

import 'package:path/path.dart' as path;
import 'package:expect/expect.dart';

import 'snapshot_test_helper.dart';

// Keep in sync with pkg/kernel/lib/binary/tag.dart:
const tagComponentFile = [0x90, 0xAB, 0xCD, 0xEF];
const tagBinaryFormatVersion = [0x00, 0x00, 0x00, 43];

Future<void> main(List<String> args) async {
if (args.length == 1 && args[0] == '--child') {
print('Hello, SDK Hash!');
return;
}

final String sourcePath =
path.join('runtime', 'tests', 'vm', 'dart_2', 'sdk_hash_test.dart');

await withTempDir((String tmp) async {
final String dillPath = path.join(tmp, 'test.dill');

{
final result = await Process.run(dart, [
genKernel,
'--platform',
platformDill,
'-o',
dillPath,
sourcePath,
]);
Expect.equals('', result.stderr);
Expect.equals(0, result.exitCode);
Expect.equals('', result.stdout);
}

{
final result = await Process.run(dart, [dillPath, '--child']);
Expect.equals('', result.stderr);
Expect.equals(0, result.exitCode);
Expect.equals('Hello, SDK Hash!', result.stdout.trim());
}

// Invalidate the SDK hash in the kernel dill:
{
final myFile = File(dillPath);
Uint8List bytes = myFile.readAsBytesSync();
// The SDK Hash is located after the ComponentFile and BinaryFormatVersion
// tags (both UInt32).
Expect.listEquals(tagComponentFile, bytes.sublist(0, 4));
Expect.listEquals(tagBinaryFormatVersion, bytes.sublist(4, 8));
// Flip the first byte in the hash:
bytes[8] ^= bytes[8];
myFile.writeAsBytesSync(bytes);
}

{
final result = await Process.run(dart, [dillPath, '--child']);
Expect.equals(
'Can\'t load Kernel binary: Invalid SDK hash.', result.stderr.trim());
Expect.equals(253, result.exitCode);
Expect.equals('', result.stdout);
}

// Zero out the SDK hash in the kernel dill to disable the check:
{
final myFile = File(dillPath);
Uint8List bytes = myFile.readAsBytesSync();
bytes.setRange(8, 18, ascii.encode('0000000000'));
myFile.writeAsBytesSync(bytes);
}

{
final result = await Process.run(dart, [dillPath, '--child']);
Expect.equals('', result.stderr);
Expect.equals(0, result.exitCode);
Expect.equals('Hello, SDK Hash!', result.stdout.trim());
}
});
}
1 change: 1 addition & 0 deletions runtime/tests/vm/dart/snapshot_test_helper.dart
Expand Up @@ -48,6 +48,7 @@ final String executableSuffix = Platform.isWindows ? ".exe" : "";
final String buildDir = p.dirname(Platform.executable);
final String platformDill = p.join(buildDir, "vm_platform_strong.dill");
final String genSnapshot = p.join(buildDir, "gen_snapshot${executableSuffix}");
final String dart = p.join(buildDir, "dart${executableSuffix}");
final String dartPrecompiledRuntime =
p.join(buildDir, "dart_precompiled_runtime${executableSuffix}");
final String genKernel = p.join("pkg", "vm", "bin", "gen_kernel.dart");
Expand Down

0 comments on commit 0ce8398

Please sign in to comment.