Skip to content

Commit

Permalink
Revert "[SDK] Adds an SDK hash to kernels and the VM."
Browse files Browse the repository at this point in the history
This reverts commit edde575.

Reason for revert: Breaks the Dart to Flutter roll and golem

Original change's description:
> [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]>

[email protected],[email protected],[email protected]

Change-Id: I34cc7d378e2babdaaca4d932d19c19d0f35422fc
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: #41802
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/152703
Reviewed-by: Siva Annamalai <[email protected]>
Commit-Queue: Siva Annamalai <[email protected]>
  • Loading branch information
a-siva authored and [email protected] committed Jun 26, 2020
1 parent a558d57 commit bb8d145
Show file tree
Hide file tree
Showing 27 changed files with 184 additions and 478 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:43_2",
"version": "binary_version:42",
}
],
"dep_type": "cipd",
Expand Down
50 changes: 24 additions & 26 deletions pkg/front_end/test/binary_md_dill_reader.dart
Original file line number Diff line number Diff line change
Expand Up @@ -473,33 +473,31 @@ class BinaryMdDillReader {
type = type.substring(0, type.indexOf("["));
type = _lookupGenericType(typeNames, type, types);

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)";
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]);
}
} else {
throw "Unknown dot to int ($count)";
}
}
}
Expand Down
4 changes: 0 additions & 4 deletions pkg/front_end/test/spell_checking_list_code.txt
Original file line number Diff line number Diff line change
Expand Up @@ -313,7 +313,6 @@ disallow
disambiguator
disjoint
dispatched
distribute
divided
dmitryas
doc
Expand All @@ -326,7 +325,6 @@ downloaded
downloading
dq
dquote
dsdk
dst
dummy
dupdate
Expand Down Expand Up @@ -437,14 +435,12 @@ futured
futureor
g
gardening
gen
generation
gets
getter1a
getter1b
getting
gft
git
github
glb
glob
Expand Down
3 changes: 1 addition & 2 deletions pkg/kernel/binary.md
Original file line number Diff line number Diff line change
Expand Up @@ -143,8 +143,7 @@ type CanonicalName {

type ComponentFile {
UInt32 magic = 0x90ABCDEF;
UInt32 formatVersion = 43;
Byte[10] shortSdkHash;
UInt32 formatVersion = 42;
List<String> problemsAsJson; // Described in problems.md.
Library[] libraries;
UriSource sourceMap;
Expand Down
28 changes: 1 addition & 27 deletions pkg/kernel/lib/binary/ast_from_binary.dart
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@ 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 @@ -29,22 +28,11 @@ class InvalidKernelVersionError {
InvalidKernelVersionError(this.version);

String toString() {
return 'Unexpected Kernel Format Version ${version} '
return 'Unexpected Kernel 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 @@ -496,13 +484,6 @@ 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 @@ -530,9 +511,6 @@ class BinaryBuilder {
if (version != Tag.BinaryFormatVersion) {
throw InvalidKernelVersionError(version);
}

_readAndVerifySdkHash();

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

_readAndVerifySdkHash();

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

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

_readAndVerifySdkHash();

List<String> problemsAsJson = readListOfStrings();
if (problemsAsJson != null) {
component.problemsAsJson ??= <String>[];
Expand Down
2 changes: 0 additions & 2 deletions pkg/kernel/lib/binary/ast_to_binary.dart
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@
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 @@ -538,7 +537,6 @@ 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: 1 addition & 25 deletions 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 = 43;
static const int BinaryFormatVersion = 42;
}

abstract class ConstantTag {
Expand All @@ -169,27 +169,3 @@ 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: 0 additions & 2 deletions runtime/bin/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -964,11 +964,9 @@ 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: 0 additions & 88 deletions runtime/tests/vm/dart/sdk_hash_test.dart

This file was deleted.

1 change: 0 additions & 1 deletion runtime/tests/vm/dart/snapshot_test_helper.dart
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,6 @@ 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 bb8d145

Please sign in to comment.