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

Miri's hardcoded PAGE_SIZE is sometimes inaccurate #2644

Closed
thomcc opened this issue Nov 2, 2022 · 2 comments · Fixed by #2721
Closed

Miri's hardcoded PAGE_SIZE is sometimes inaccurate #2644

thomcc opened this issue Nov 2, 2022 · 2 comments · Fixed by #2721
Labels
E-good-first-issue A good way to start contributing, mentoring is available

Comments

@thomcc
Copy link
Member

thomcc commented Nov 2, 2022

Miri always claims the page size is 4k, but on some targets this is not the case. Programs generally should query it at runtime, but a lot of code just hard-codes 4k (like miri), which is not right sometimes.

Plausibly this is an argument for it being configurable, but I think that's likely overkill (and deciding what values are allowable seems tricky). Thankfully, the targets I know of where this varies are seem to be statically fixed in practice, so Miri should just return the accurate-in-practice value. Specifically:

  • On aarch64-apple-darwin the page size seems to be 16k (apple made a point about this changing in the move to M1). (Unsure if miri supports it, but this is true for aarch64-apple-ios too, and perhaps all aarch64-apple-*).
  • The web assembly spec defines it as 64k (kinda wild, but so it goes): https://webassembly.github.io/spec/core/exec/runtime.html#memory-instances, and WASI seems to follow this.

I don't know of other targets which happen to have different values, and have looked around a bit (according to https://linux.die.net/man/2/getpagesize, on sun4 is a real example where it depended on the specific machine rather than the target, but Rust probably doesn't support it).

Anyway, currently it doesn't matter much, but if #2520 lands (CC @saethlin) it would matter more. It might make sense to do as part of that PR, but this is certainly not necessary, so I filed it as a separate thing rather than asking on that PR.

@RalfJung
Copy link
Member

RalfJung commented Nov 2, 2022

I don't know of other targets which happen to have different values, and have looked around a bit (according to https://linux.die.net/man/2/getpagesize, on sun4 is a real example where it depended on the specific machine rather than the target, but Rust probably doesn't support it).

Seems okay for Miri to define it as 4K then as long as that is a legal option for the target. ;)

But indeed if other targets define it to definitely be something else, Miri should respect that. A separate PR that replaces the constant by a function which takes into account the target would be appreciated.

@oli-obk oli-obk added the E-good-first-issue A good way to start contributing, mentoring is available label Nov 2, 2022
@bjorn3
Copy link
Member

bjorn3 commented Nov 2, 2022

On aarch64-apple-darwin the page size seems to be 16k (apple made a point about this changing in the move to M1). (Unsure if miri supports it, but this is true for aarch64-apple-ios too, and perhaps all aarch64-apple-*).

Even worse aarch64-unknown-linux-gnu on M1 is 16k too (as mmio has to be mapped with 16k pages and linux doesn't support mixing 4k and 16k pages), but on other systems it is 4k.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
E-good-first-issue A good way to start contributing, mentoring is available
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants