Skip to content

Commit edde575

Browse files
Clement Skaucommit-bot@chromium.org
Clement Skau
authored andcommitted
[SDK] Adds an SDK hash to kernels and the VM.
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 <[email protected]> Reviewed-by: Jens Johansen <[email protected]> Reviewed-by: Martin Kustermann <[email protected]>
1 parent d512020 commit edde575

27 files changed

+478
-183
lines changed

DEPS

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -512,7 +512,7 @@ deps = {
512512
"packages": [
513513
{
514514
"package": "dart/cfe/dart2js_dills",
515-
"version": "binary_version:42",
515+
"version": "binary_version:43_2",
516516
}
517517
],
518518
"dep_type": "cipd",

pkg/front_end/test/binary_md_dill_reader.dart

Lines changed: 26 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -473,31 +473,33 @@ class BinaryMdDillReader {
473473
type = type.substring(0, type.indexOf("["));
474474
type = _lookupGenericType(typeNames, type, types);
475475

476-
int intCount = -1;
477-
if (vars[count] != null && vars[count] is int) {
478-
intCount = vars[count];
479-
} else if (count.contains(".")) {
480-
List<String> countData =
481-
count.split(regExpSplit).map((s) => s.trim()).toList();
482-
if (vars[countData[0]] != null) {
483-
dynamic v = vars[countData[0]];
484-
if (v is Map &&
485-
countData[1] == "last" &&
486-
v["items"] is List &&
487-
v["items"].last is int) {
488-
intCount = v["items"].last;
489-
} else if (v is Map && v[countData[1]] != null) {
490-
v = v[countData[1]];
491-
if (v is Map && v[countData[2]] != null) {
492-
v = v[countData[2]];
493-
if (v is int) intCount = v;
494-
} else if (v is int &&
495-
countData.length == 4 &&
496-
countData[2] == "+") {
497-
intCount = v + int.parse(countData[3]);
476+
int intCount = int.tryParse(count) ?? -1;
477+
if (intCount == -1) {
478+
if (vars[count] != null && vars[count] is int) {
479+
intCount = vars[count];
480+
} else if (count.contains(".")) {
481+
List<String> countData =
482+
count.split(regExpSplit).map((s) => s.trim()).toList();
483+
if (vars[countData[0]] != null) {
484+
dynamic v = vars[countData[0]];
485+
if (v is Map &&
486+
countData[1] == "last" &&
487+
v["items"] is List &&
488+
v["items"].last is int) {
489+
intCount = v["items"].last;
490+
} else if (v is Map && v[countData[1]] != null) {
491+
v = v[countData[1]];
492+
if (v is Map && v[countData[2]] != null) {
493+
v = v[countData[2]];
494+
if (v is int) intCount = v;
495+
} else if (v is int &&
496+
countData.length == 4 &&
497+
countData[2] == "+") {
498+
intCount = v + int.parse(countData[3]);
499+
}
500+
} else {
501+
throw "Unknown dot to int ($count)";
498502
}
499-
} else {
500-
throw "Unknown dot to int ($count)";
501503
}
502504
}
503505
}

pkg/front_end/test/spell_checking_list_code.txt

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -313,6 +313,7 @@ disallow
313313
disambiguator
314314
disjoint
315315
dispatched
316+
distribute
316317
divided
317318
dmitryas
318319
doc
@@ -325,6 +326,7 @@ downloaded
325326
downloading
326327
dq
327328
dquote
329+
dsdk
328330
dst
329331
dummy
330332
dupdate
@@ -435,12 +437,14 @@ futured
435437
futureor
436438
g
437439
gardening
440+
gen
438441
generation
439442
gets
440443
getter1a
441444
getter1b
442445
getting
443446
gft
447+
git
444448
github
445449
glb
446450
glob

pkg/kernel/binary.md

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -143,7 +143,8 @@ type CanonicalName {
143143

144144
type ComponentFile {
145145
UInt32 magic = 0x90ABCDEF;
146-
UInt32 formatVersion = 42;
146+
UInt32 formatVersion = 43;
147+
Byte[10] shortSdkHash;
147148
List<String> problemsAsJson; // Described in problems.md.
148149
Library[] libraries;
149150
UriSource sourceMap;

pkg/kernel/lib/binary/ast_from_binary.dart

Lines changed: 27 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ library kernel.ast_from_binary;
55

66
import 'dart:core' hide MapEntry;
77
import 'dart:developer';
8+
import 'dart:convert';
89
import 'dart:typed_data';
910

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

3031
String toString() {
31-
return 'Unexpected Kernel version ${version} '
32+
return 'Unexpected Kernel Format Version ${version} '
3233
'(expected ${Tag.BinaryFormatVersion}).';
3334
}
3435
}
3536

37+
class InvalidKernelSdkVersionError {
38+
final String version;
39+
40+
InvalidKernelSdkVersionError(this.version);
41+
42+
String toString() {
43+
return 'Unexpected Kernel SDK Version ${version} '
44+
'(expected ${expectedSdkHash}).';
45+
}
46+
}
47+
3648
class CompilationModeError {
3749
final String message;
3850

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

499+
void _readAndVerifySdkHash() {
500+
final sdkHash = ascii.decode(readBytes(sdkHashLength));
501+
if (!isValidSdkHash(sdkHash)) {
502+
throw InvalidKernelSdkVersionError(sdkHash);
503+
}
504+
}
505+
487506
/// Deserializes a kernel component and stores it in [component].
488507
///
489508
/// When linking with a non-empty component, canonical names must have been
@@ -511,6 +530,9 @@ class BinaryBuilder {
511530
if (version != Tag.BinaryFormatVersion) {
512531
throw InvalidKernelVersionError(version);
513532
}
533+
534+
_readAndVerifySdkHash();
535+
514536
_byteOffset = offset;
515537
List<int> componentFileSizes = _indexComponents();
516538
if (componentFileSizes.length > 1) {
@@ -694,6 +716,8 @@ class BinaryBuilder {
694716
throw InvalidKernelVersionError(formatVersion);
695717
}
696718

719+
_readAndVerifySdkHash();
720+
697721
// Read component index from the end of this ComponentFiles serialized data.
698722
_ComponentIndex index = _readComponentIndex(componentFileSize);
699723

@@ -718,6 +742,8 @@ class BinaryBuilder {
718742
throw InvalidKernelVersionError(formatVersion);
719743
}
720744

745+
_readAndVerifySdkHash();
746+
721747
List<String> problemsAsJson = readListOfStrings();
722748
if (problemsAsJson != null) {
723749
component.problemsAsJson ??= <String>[];

pkg/kernel/lib/binary/ast_to_binary.dart

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44
library kernel.ast_to_binary;
55

66
import 'dart:core' hide MapEntry;
7+
import 'dart:convert';
78
import 'dart:developer';
89
import 'dart:io' show BytesBuilder;
910
import 'dart:typed_data';
@@ -537,6 +538,7 @@ class BinaryPrinter implements Visitor<void>, BinarySink {
537538
final componentOffset = getBufferOffset();
538539
writeUInt32(Tag.ComponentFile);
539540
writeUInt32(Tag.BinaryFormatVersion);
541+
writeBytes(ascii.encode(expectedSdkHash));
540542
writeListOfStrings(component.problemsAsJson);
541543
indexLinkTable(component);
542544
_collectMetadata(component);

pkg/kernel/lib/binary/tag.dart

Lines changed: 25 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -149,7 +149,7 @@ class Tag {
149149
/// Internal version of kernel binary format.
150150
/// Bump it when making incompatible changes in kernel binaries.
151151
/// Keep in sync with runtime/vm/kernel_binary.h, pkg/kernel/binary.md.
152-
static const int BinaryFormatVersion = 42;
152+
static const int BinaryFormatVersion = 43;
153153
}
154154

155155
abstract class ConstantTag {
@@ -169,3 +169,27 @@ abstract class ConstantTag {
169169
static const int UnevaluatedConstant = 12;
170170
// 13 is occupied by [SetConstant]
171171
}
172+
173+
const int sdkHashLength = 10; // Bytes, a Git "short hash".
174+
175+
const String sdkHashNull = '0000000000';
176+
177+
// Will be correct hash for Flutter SDK / Dart SDK we distribute.
178+
// If non-null we will validate when consuming kernel, will use when producing
179+
// kernel.
180+
// If null, local development setting (e.g. run gen_kernel.dart from source),
181+
// we put 0x00..00 into when producing, do not validate when consuming.
182+
String get expectedSdkHash {
183+
final sdkHash =
184+
const String.fromEnvironment('sdk_hash', defaultValue: sdkHashNull);
185+
if (sdkHash.length != 10) {
186+
throw '-Dsdk_hash=<hash> must be a 10 byte string!';
187+
}
188+
return sdkHash;
189+
}
190+
191+
bool isValidSdkHash(String sdkHash) {
192+
return (sdkHash == sdkHashNull ||
193+
expectedSdkHash == sdkHashNull ||
194+
sdkHash == expectedSdkHash);
195+
}

runtime/bin/BUILD.gn

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -964,9 +964,11 @@ prebuilt_dart_action("gen_kernel_bytecode_dill") {
964964

965965
abs_depfile = rebase_path(depfile)
966966
rebased_output = rebase_path(output, root_build_dir)
967+
967968
vm_args = [
968969
"--depfile=$abs_depfile",
969970
"--depfile_output_filename=$rebased_output",
971+
"-Dsdk_hash=$sdk_hash",
970972
]
971973

972974
args = [
Lines changed: 88 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,88 @@
1+
// Copyright (c) 2020, the Dart project authors. Please see the AUTHORS file
2+
// for details. All rights reserved. Use of this source code is governed by a
3+
// BSD-style license that can be found in the LICENSE file.
4+
5+
import 'dart:async';
6+
import 'dart:convert';
7+
import 'dart:io';
8+
import 'dart:typed_data';
9+
10+
import 'package:path/path.dart' as path;
11+
import 'package:expect/expect.dart';
12+
13+
import 'snapshot_test_helper.dart';
14+
15+
// Keep in sync with pkg/kernel/lib/binary/tag.dart:
16+
const tagComponentFile = [0x90, 0xAB, 0xCD, 0xEF];
17+
const tagBinaryFormatVersion = [0x00, 0x00, 0x00, 43];
18+
19+
Future<void> main(List<String> args) async {
20+
if (args.length == 1 && args[0] == '--child') {
21+
print('Hello, SDK Hash!');
22+
return;
23+
}
24+
25+
final String sourcePath =
26+
path.join('runtime', 'tests', 'vm', 'dart_2', 'sdk_hash_test.dart');
27+
28+
await withTempDir((String tmp) async {
29+
final String dillPath = path.join(tmp, 'test.dill');
30+
31+
{
32+
final result = await Process.run(dart, [
33+
genKernel,
34+
'--platform',
35+
platformDill,
36+
'-o',
37+
dillPath,
38+
sourcePath,
39+
]);
40+
Expect.equals('', result.stderr);
41+
Expect.equals(0, result.exitCode);
42+
Expect.equals('', result.stdout);
43+
}
44+
45+
{
46+
final result = await Process.run(dart, [dillPath, '--child']);
47+
Expect.equals('', result.stderr);
48+
Expect.equals(0, result.exitCode);
49+
Expect.equals('Hello, SDK Hash!\n', result.stdout);
50+
}
51+
52+
// Invalidate the SDK hash in the kernel dill:
53+
{
54+
final myFile = File(dillPath);
55+
Uint8List bytes = myFile.readAsBytesSync();
56+
// The SDK Hash is located after the ComponentFile and BinaryFormatVersion
57+
// tags (both UInt32).
58+
Expect.listEquals(tagComponentFile, bytes.sublist(0, 4));
59+
Expect.listEquals(tagBinaryFormatVersion, bytes.sublist(4, 8));
60+
// Flip the first byte in the hash:
61+
bytes[8] ^= bytes[8];
62+
myFile.writeAsBytesSync(bytes);
63+
}
64+
65+
{
66+
final result = await Process.run(dart, [dillPath, '--child']);
67+
Expect.equals(
68+
'Can\'t load Kernel binary: Invalid SDK hash.\n', result.stderr);
69+
Expect.equals(253, result.exitCode);
70+
Expect.equals('', result.stdout);
71+
}
72+
73+
// Zero out the SDK hash in the kernel dill to disable the check:
74+
{
75+
final myFile = File(dillPath);
76+
Uint8List bytes = myFile.readAsBytesSync();
77+
bytes.setRange(8, 18, ascii.encode('0000000000'));
78+
myFile.writeAsBytesSync(bytes);
79+
}
80+
81+
{
82+
final result = await Process.run(dart, [dillPath, '--child']);
83+
Expect.equals('', result.stderr);
84+
Expect.equals(0, result.exitCode);
85+
Expect.equals('Hello, SDK Hash!\n', result.stdout);
86+
}
87+
});
88+
}

runtime/tests/vm/dart/snapshot_test_helper.dart

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -48,6 +48,7 @@ final String executableSuffix = Platform.isWindows ? ".exe" : "";
4848
final String buildDir = p.dirname(Platform.executable);
4949
final String platformDill = p.join(buildDir, "vm_platform_strong.dill");
5050
final String genSnapshot = p.join(buildDir, "gen_snapshot${executableSuffix}");
51+
final String dart = p.join(buildDir, "dart${executableSuffix}");
5152
final String dartPrecompiledRuntime =
5253
p.join(buildDir, "dart_precompiled_runtime${executableSuffix}");
5354
final String genKernel = p.join("pkg", "vm", "bin", "gen_kernel.dart");

0 commit comments

Comments
 (0)