-
Notifications
You must be signed in to change notification settings - Fork 224
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
Starlark alloc alignment assertions fail following rustc 1.72 TypeId changes #408
Comments
cormacrelf
changed the title
Starlark alloc alignment assersions fail following rustc 1.72 TypeId changes
Starlark alloc alignment assertions fail following rustc 1.72 TypeId changes
Sep 6, 2023
Thanks for the report! We'll take a look. Kind of surprised our internal CI didn't bleat loudly at this, |
Edit: nevermind, I can reproduce it. |
|
Internal: should be fixed in D49019964. |
facebook-github-bot
pushed a commit
to facebook/starlark-rust
that referenced
this issue
Sep 6, 2023
Summary: * `TypeId` is `u128` [in Rust 1.72](rust-lang/rust#109953) * `u128` is 16 bytes aligned on Apple Silicon * we require 8 byte alignment for `StarlarkValue` Maybe we should fix heap allocations: insert padding to allow any alignment. But practically we don't need it and should not do it: generated machine code is identical for aligned and 8-byte aligned `u128`: [compiler explorer](https://rust.godbolt.org/z/96KsnjW77) (note it is identical if we load it from memory not pass by value, but this is what we do). So for now just force 8 byte alignment on `TypeId`. Fixes issue facebook/buck2#408 Reviewed By: ndmitchell, shayne-fletcher Differential Revision: D49019964 fbshipit-source-id: 62c8d7db83e9e4bdbe58fe7a62d5daae41996e61
facebook-github-bot
pushed a commit
that referenced
this issue
Sep 6, 2023
Summary: * `TypeId` is `u128` [in Rust 1.72](rust-lang/rust#109953) * `u128` is 16 bytes aligned on Apple Silicon * we require 8 byte alignment for `StarlarkValue` Maybe we should fix heap allocations: insert padding to allow any alignment. But practically we don't need it and should not do it: generated machine code is identical for aligned and 8-byte aligned `u128`: [compiler explorer](https://rust.godbolt.org/z/96KsnjW77) (note it is identical if we load it from memory not pass by value, but this is what we do). So for now just force 8 byte alignment on `TypeId`. Fixes issue #408 Reviewed By: ndmitchell, shayne-fletcher Differential Revision: D49019964 fbshipit-source-id: 62c8d7db83e9e4bdbe58fe7a62d5daae41996e61
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Latest builds are red, bisecting reveals 21381685c as the source. Weirdly only macos seems to be affected.
See rust-lang/rust#109953, which I am pretty sure this is related to. The alloc is now aligned to 16 bytes with no code changes. Compare the changelog https://releases.rs/docs/1.72.0/
I should mention this seems to happen when you run any command that evaluates starlark.
Backtrace showing the assertion failure
The text was updated successfully, but these errors were encountered: