Skip to content

Commit

Permalink
[vm] Enable fingerprints checking of recognized methods
Browse files Browse the repository at this point in the history
Fixes #36376

Change-Id: I470b95a1f854ef6e3798422508a89bc61f71cd0c
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/153740
Reviewed-by: Martin Kustermann <[email protected]>
Commit-Queue: Alexander Markov <[email protected]>
  • Loading branch information
alexmarkov authored and [email protected] committed Jul 9, 2020
1 parent 2280e77 commit 0162e7c
Show file tree
Hide file tree
Showing 7 changed files with 391 additions and 405 deletions.
4 changes: 3 additions & 1 deletion runtime/vm/compiler/frontend/kernel_fingerprints.cc
Original file line number Diff line number Diff line change
Expand Up @@ -227,9 +227,11 @@ void KernelFingerprintHelper::CalculateDartTypeFingerprint() {
case kDynamicType:
case kVoidType:
case kBottomType:
case kNeverType:
// those contain nothing.
break;
case kNeverType:
BuildHash(static_cast<uint32_t>(ReadNullability()));
break;
case kInterfaceType:
CalculateInterfaceTypeFingerprint(false);
break;
Expand Down
7 changes: 2 additions & 5 deletions runtime/vm/compiler/method_recognizer.cc
Original file line number Diff line number Diff line change
Expand Up @@ -204,10 +204,7 @@ void MethodRecognizer::InitializeState() {
func = Library::GetFunction(libs, recognized_methods[i].class_name,
recognized_methods[i].function_name);
if (!func.IsNull()) {
CHECK_FINGERPRINT3(func, recognized_methods[i].class_name,
recognized_methods[i].function_name,
recognized_methods[i].enum_name,
recognized_methods[i].fp);
ASSERT(func.CheckSourceFingerprint(recognized_methods[i].fp));
func.set_recognized_kind(kind);
switch (kind) {
#define RECOGNIZE_METHOD(class_name, function_name, enum_name, fp) \
Expand All @@ -228,7 +225,7 @@ void MethodRecognizer::InitializeState() {
#define SET_FUNCTION_BIT(class_name, function_name, dest, fp, setter, value) \
func = Library::GetFunction(libs, #class_name, #function_name); \
if (!func.IsNull()) { \
CHECK_FINGERPRINT3(func, class_name, function_name, dest, fp); \
ASSERT(func.CheckSourceFingerprint(fp)); \
func.setter(value); \
} else if (!FLAG_precompiled_mode) { \
OS::PrintErr("Missing %s::%s\n", #class_name, #function_name); \
Expand Down
6 changes: 0 additions & 6 deletions runtime/vm/compiler/method_recognizer.h
Original file line number Diff line number Diff line change
Expand Up @@ -65,12 +65,6 @@ class MethodTokenRecognizer : public AllStatic {
static Token::Kind RecognizeTokenKind(const String& name);
};

#define CHECK_FINGERPRINT2(f, p0, p1, fp) \
ASSERT(f.CheckSourceFingerprint(#p0 ", " #p1, fp))

#define CHECK_FINGERPRINT3(f, p0, p1, p2, fp) \
ASSERT(f.CheckSourceFingerprint(#p0 ", " #p1 ", " #p2, fp))

// Class that recognizes factories and returns corresponding result cid.
class FactoryRecognizer : public AllStatic {
public:
Expand Down
762 changes: 381 additions & 381 deletions runtime/vm/compiler/recognized_methods_list.h

Large diffs are not rendered by default.

2 changes: 1 addition & 1 deletion runtime/vm/dart_api_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1160,7 +1160,7 @@ static Dart_Isolate CreateIsolate(IsolateGroup* group,
is_new_group ? nullptr : group, isolate_data));
if (error_obj.IsNull()) {
#if defined(DEBUG) && !defined(DART_PRECOMPILED_RUNTIME)
if (FLAG_check_function_fingerprints && source->kernel_buffer == NULL) {
if (FLAG_check_function_fingerprints && !FLAG_precompiled_mode) {
Library::CheckFunctionFingerprints();
}
#endif // defined(DEBUG) && !defined(DART_PRECOMPILED_RUNTIME).
Expand Down
13 changes: 3 additions & 10 deletions runtime/vm/object.cc
Original file line number Diff line number Diff line change
Expand Up @@ -9153,7 +9153,7 @@ void Function::SetDeoptReasonForAll(intptr_t deopt_id,
}
}

bool Function::CheckSourceFingerprint(const char* prefix, int32_t fp) const {
bool Function::CheckSourceFingerprint(int32_t fp) const {
if (Isolate::Current()->obfuscate() || FLAG_precompiled_mode ||
(Dart::vm_snapshot_kind() != Snapshot::kNone)) {
return true; // The kernel structure has been altered, skip checking.
Expand All @@ -9165,12 +9165,6 @@ bool Function::CheckSourceFingerprint(const char* prefix, int32_t fp) const {
return true;
}

#if 1
// The non-nullable experiment changes the fingerprints, we only track
// one fingerprint set, until we unfork and settle on a single snapshot
// version this check has to be bypassed.
// TODO(36376) - Restore checking fingerprints of recognized methods.
#else
if (SourceFingerprint() != fp) {
const bool recalculatingFingerprints = false;
if (recalculatingFingerprints) {
Expand All @@ -9189,7 +9183,6 @@ bool Function::CheckSourceFingerprint(const char* prefix, int32_t fp) const {
return false;
}
}
#endif
return true;
}

Expand Down Expand Up @@ -13520,7 +13513,7 @@ void Library::CheckFunctionFingerprints() {
has_errors = true; \
OS::PrintErr("Function not found %s.%s\n", #class_name, #function_name); \
} else { \
CHECK_FINGERPRINT3(func, class_name, function_name, dest, fp); \
ASSERT(func.CheckSourceFingerprint(fp)); \
}

#define CHECK_FINGERPRINTS2(class_name, function_name, dest, fp) \
Expand Down Expand Up @@ -13563,7 +13556,7 @@ void Library::CheckFunctionFingerprints() {
has_errors = true; \
OS::PrintErr("Function not found %s.%s\n", #class_name, #factory_name); \
} else { \
CHECK_FINGERPRINT2(func, symbol, cid, fp); \
ASSERT(func.CheckSourceFingerprint(fp)); \
}

all_libs.Add(&Library::ZoneHandle(Library::CoreLibrary()));
Expand Down
2 changes: 1 addition & 1 deletion runtime/vm/object.h
Original file line number Diff line number Diff line change
Expand Up @@ -3515,7 +3515,7 @@ class Function : public Object {
int32_t SourceFingerprint() const;

// Return false and report an error if the fingerprint does not match.
bool CheckSourceFingerprint(const char* prefix, int32_t fp) const;
bool CheckSourceFingerprint(int32_t fp) const;

// Works with map [deopt-id] -> ICData.
void SaveICDataMap(
Expand Down

0 comments on commit 0162e7c

Please sign in to comment.