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

LLVM and Rust u128 alignment update breaks #[account(zero_copy)] with u128 #3114

Open
JakkuSakura opened this issue Jul 24, 2024 · 3 comments
Labels
bug Something isn't working lang

Comments

@JakkuSakura
Copy link

Long story short. The previous versions of rust and llvm treat u128 as 8 alignments incorrectly on x86_64. so does #[Account(zero_copy)]. A recent update fixed this issue, but breaks all existing #[account(zero_copy)] with u128

I'm surprised that nobody mentioned this issue here. So I'm just links related issues.
https://blog.rust-lang.org/2024/03/30/i128-layout-update.html
drift-labs/protocol-v2#891
rust-lang/rust#54341

A workaround would be defining a struct u128([u64; 2]) and replace all primitive type u128, inspired by https://solana.stackexchange.com/questions/7720/using-u128-without-sacrificing-alignment-8

@acheroncrypto acheroncrypto added bug Something isn't working lang labels Jul 24, 2024
@acheroncrypto
Copy link
Collaborator

Thanks for creating the issue but Anchor itself doesn't have any alignment assumptions when using #[account(zero_copy)] (internal bytemuck does).

Solana usually keeps layouts consistent — it might be worth creating an issue in https://github.com/anza-xyz/llvm-project.

@matt-allan
Copy link

The issue isn't that the Solana layout changed -- it's that it didn't change while everyone else's did.

If you are trying to deserialize #[zero_copy(unsafe)] anchor data containing a u128 or i128 with a Rust 1.77+ program, it won't work because the data was serialized using an old Rust layout that's no longer compatible.

Anchor itself doesn't have any alignment assumptions when using #[account(zero_copy)] (internal bytemuck does).

Older Anchor programs (and new ones using #[zero_copy(unsafe)]) are not defined as repr(C), so by default they use repr(Rust). That is the assumption causing the issue here.

While technically bytemuck is doing the heavy lifting its anchor that adds the repr. And as per the bytemuck docs:

The type needs to be repr(C) or repr(transparent).

Anchor fixed this issue in 0.20 but a lot of existing programs cannot change their representations. There are a lot of important programs using #[zero_copy(unsafe)] (and thus repr(Rust)) because they were deployed before Anchor 0.20 was released.

@matt-allan
Copy link

I pushed a self-contained reproduction here: https://github.com/matt-allan/anchor-layout-issue

In this case the issue between 1.77 and stable is not the change in alignment but that the order of the fields changed.

On 1.77 I get the expected value for the first field:

TickArray {
    start_tick_index: -1232,

The offset is 0, which is expected:

Offset of start_tick_index in TickArray: 0
Alignment of TickArray: 1
Alignment of Tick: 1
Offset of liquidity_gross in Tick: 17

On stable I don't get the expected value:

TickArray {
    start_tick_index: 0,

The offset of the field has changed:

Offset of start_tick_index in TickArray: 9944
Alignment of TickArray: 1
Alignment of Tick: 1
Offset of liquidity_gross in Tick: 17

9944 is the size of Tick (113) x the # of array elements (88), so it's moved.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working lang
Projects
None yet
Development

No branches or pull requests

3 participants