Skip to content

Commit

Permalink
[SDK] Adds an SDK hash to kernels and the VM.
Browse files Browse the repository at this point in the history
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.

Bug: #41802
Change-Id: I3cbb2d481239ee64dafdaa0e4aac36c80281931b
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/150343
Commit-Queue: Clement Skau <cskau@google.com>
Reviewed-by: Jens Johansen <jensj@google.com>
Reviewed-by: Martin Kustermann <kustermann@google.com>
  • Loading branch information
Clement Skau authored and commit-bot@chromium.org committed Jun 26, 2020
1 parent d512020 commit edde575
Show file tree
Hide file tree
Showing 27 changed files with 478 additions and 183 deletions.
2 changes: 1 addition & 1 deletion DEPS
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
Expand Up @@ -313,6 +313,7 @@ disallow
disambiguator
disjoint
dispatched
distribute
divided
dmitryas
doc
Expand All @@ -325,6 +326,7 @@ downloaded
downloading
dq
dquote
dsdk
dst
dummy
dupdate
Expand Down Expand Up @@ -435,12 +437,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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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 != 10) {
throw '-Dsdk_hash=<hash> must be a 10 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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
@@ -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!\n', result.stdout);
}

// 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.\n', result.stderr);
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!\n', result.stdout);
}
});
}
1 change: 1 addition & 0 deletions runtime/tests/vm/dart/snapshot_test_helper.dart
Original file line number Diff line number Diff line change
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
Loading

0 comments on commit edde575

Please sign in to comment.