Skip to content
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

Fix issues with how we compute DST size #3687

Open
wants to merge 6 commits into
base: main
Choose a base branch
from

Conversation

celinval
Copy link
Contributor

@celinval celinval commented Nov 6, 2024

Sorry for the big change. They were all somewhat connected to size of DST.

  1. Add proper bounds check for size_of_val and align_of_val intrinsics.
  2. Fix how we compute size of the object in our mem predicates.
  3. Remove trait requirement for memory predicate functions.
  4. Provide user visible methods to try to retrieve the size of the object if known and valid (checked_size_of_raw and checked_align_of_raw.
  5. Fix aggregate for slices which is used in from_raw_ptr in our tests.

Fixes #3612
Fixes #3616
Fixes #3627
Fixes #3638
Fixes #3615

Call-outs

There's a bit of refactoring here on how to handle size_of_val intrinsics to reuse the same logic as the new predicates. Otherwise we would have to apply two fixes and keep maintaining them both. Note that I haven't deleted the code used by
the intrinsic yet, since that is still used in the raw pointer to reference checks. I will do that in a follow up PR.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 and MIT licenses.

1. Add proper bounds check for `size_of_val` intrinsics.
2. Refactor how we compute size of the object in our mem predicates.
3. Provide user visible methods to try to retrieve the size of the
   object if known and valid.
@github-actions github-actions bot added the Z-BenchCI Tag a PR to run benchmark CI label Nov 6, 2024
@celinval celinval marked this pull request as ready for review November 12, 2024 21:03
@celinval celinval requested a review from a team as a code owner November 12, 2024 21:03
}
RigidTy::Tuple(tys) => {
// We have to recurse and get the maximum alignment of all sized portions.
let last_ty = tys.last().expect("Expected unsized tail");
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note to self: Fix this. The order of fields is not guaranteed, and we need to use similar logic as the Adt logic to get the last field.

@celinval celinval marked this pull request as draft November 13, 2024 01:28
- Assume for now that we cannot compute foreign object size.
let model = match intrinsic {
Intrinsic::SizeOfVal => self.models[&KaniModel::SizeOfVal],
Intrinsic::MinAlignOfVal => self.models[&KaniModel::AlignOfVal],
// The rest is handled in hooks.
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
// The rest is handled in hooks.
// The rest is handled in codegen.

@celinval celinval marked this pull request as ready for review November 13, 2024 21:48
@celinval
Copy link
Contributor Author

FYI, I'm going to create a fix for the std in a separate PR. Also I think I could break down this PR into two if needed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment