Skip to content

VM: Restore checking fingerprints of recognized methods #36376

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
alexmarkov opened this issue Mar 28, 2019 · 2 comments
Closed

VM: Restore checking fingerprints of recognized methods #36376

alexmarkov opened this issue Mar 28, 2019 · 2 comments
Assignees
Labels
area-vm Use area-vm for VM related issues, including code coverage, and the AOT and JIT backends. NNBD Issues related to NNBD Release

Comments

@alexmarkov
Copy link
Contributor

VM used to check source fingerprints of 'recognized' methods in core libraries.

Condition in Function::CheckSourceFingerprint includes kernel_offset() <= 0 which is false since Dart 2. We need to restore these checks.

/cc @aartbik @mraleph @a-siva @rmacnak-google

@alexmarkov alexmarkov added the area-vm Use area-vm for VM related issues, including code coverage, and the AOT and JIT backends. label Mar 28, 2019
dart-bot pushed a commit that referenced this issue Sep 11, 2019
Bug: #36376
Change-Id: I8102d5d10abc98e4410c16d0d08bf5015e459b46
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/116481
Reviewed-by: Aart Bik <[email protected]>
Reviewed-by: Alexander Markov <[email protected]>
Commit-Queue: Ryan Macnak <[email protected]>
@ghost
Copy link

ghost commented Jul 7, 2020

I believe this (temporarily) invalidates the comment here:

// When adding a new function, add a 0 as the fingerprint and run the build in
// debug mode to get the correct fingerprint from the mismatch error.

Since the code to generate the new fingerprints has been disabled:

sdk/runtime/vm/object.cc

Lines 9165 to 9170 in c5ea3e8

#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

Unless there's some other mechanism I'm unaware of we should probably amend the first comment to clarify expectations.

@alexmarkov
Copy link
Contributor Author

Fingerprints checking was disabled once again when we had 2 versions of core libraries (sdk and sdk_nnbd).
After SDK unforking we have only one version of core libraries, so we should re-enable fingerprints checking by removing this #if and updating all fingerprints.

@alexmarkov alexmarkov added the NNBD Issues related to NNBD Release label Jul 8, 2020
@alexmarkov alexmarkov self-assigned this Jul 8, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-vm Use area-vm for VM related issues, including code coverage, and the AOT and JIT backends. NNBD Issues related to NNBD Release
Projects
None yet
Development

No branches or pull requests

1 participant