Skip to content

Make VM actually check for ABI compatibility when consuming kernel files #38969

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
mkustermann opened this issue Oct 18, 2019 · 2 comments
Closed
Assignees
Labels
area-vm Use area-vm for VM related issues, including code coverage, and the AOT and JIT backends. P1 A high priority bug; for example, a single project is unusable or has many test failures

Comments

@mkustermann
Copy link
Member

mkustermann commented Oct 18, 2019

Right now we have a vm-dartkb-linux-release-x64-abi and tools/VERSION with two ABI numbers in it (ABI_VERSION and OLDEST_SUPPORTED_ABI_VERSION). If the former bot turns red we just bump one or both abi numbers.

Then we have runtime/vm/kernel_binary.h which contains a kMinSupportedKernelFormatVersion and kMaxSupportedKernelFormatVersion. Though the kernel format hasn't had a breaking change in a long time, the [min,max] is [18,35] atm, so the format itself is not breaking very often.

When the VM consumes kernel files it will in fact check whether the given kernel files lie within the kMinSupported.* and kMaxSupported* range.
=> The VM will refuse to load incompatible kernel versions (it returns dart_api.h errors) - which is good.

Though the VM needs more than just a kernel file it can read and understand. It also expects the contents of kernel files to have certain contents. This is a tight contract between dart:* libraries and VM. Adding/Removing native methods, ... can break this contract. For this reason (I suppose) we have the ABI bots.
=> Yet the VM does not check what ABI a kernel file has when consuming it. It will simply crash if the assumptions are not met (see e.g. b/142533773).

Without the VM actually checking the ABI version when consuming kernel files, what is the purpose of the ABI bot?

I assume the answer is that we should embed the ABI version in the kernel files, embedding min/max ABI versions in the VM and start checking compatibility.

/cc @mraleph @a-siva

@mkustermann mkustermann added the area-vm Use area-vm for VM related issues, including code coverage, and the AOT and JIT backends. label Oct 18, 2019
@devoncarew
Copy link
Member

Bumping to P1 as per #41913 (comment).

@devoncarew devoncarew added the P1 A high priority bug; for example, a single project is unusable or has many test failures label May 20, 2020
@natebosch
Copy link
Member

I think this has been done now with the SDK hash stuff. Should we close this?

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. P1 A high priority bug; for example, a single project is unusable or has many test failures
Projects
None yet
Development

No branches or pull requests

4 participants