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

_intoFFI padding #656

Closed
ambiguousname opened this issue Aug 23, 2024 · 2 comments · Fixed by #660
Closed

_intoFFI padding #656

ambiguousname opened this issue Aug 23, 2024 · 2 comments · Fixed by #660
Labels
B-js JS backend

Comments

@ambiguousname
Copy link
Member

ambiguousname commented Aug 23, 2024

Should double check with LLVM output, but basically:

struct Foo {
  a : u8,
  b : u8,
  c : u32
}

Pads with 2 x i8s. But

struct Bar {
  b : u16,
  c : u32
}

Pads with 1 x i16. The padding is determined by the previous type, and then it all gets converted into i32s for WASM, regardless of how much sense that makes.

So, the fix should go in generate_fields, to grab what the previous field's align is. Then update the padding to be use something like (next_offset - curr_offset - field_layout.size()) / previous_align

@ambiguousname ambiguousname added the B-js JS backend label Aug 23, 2024
@Manishearth
Copy link
Contributor

The padding is determined by the previous type

Or the previous set of types, (u8, u16, u32) will have an i8 padding. It's unclear to me if this is a global decision or per-alignment-section. I'll write out a bunch of test structs to reverse engineer this.

Also the example was incorrect, Bar should have no u8s.

@Manishearth
Copy link
Contributor

Or the previous set of types, (u8, u16, u32) will have an i8 padding. It's unclear to me if this is a global decision or per-alignment-section. I'll write out a bunch of test structs to reverse engineer this.

Nope, it pads with an i8 and then an i16. Previous type is right, the u16 needs to be padded too.

Manishearth added a commit to Manishearth/diplomat that referenced this issue Aug 27, 2024
This fixes rust-diplomat#656 but not rust-diplomat#657

The scalarpair struct part of the second test happens to pass since rust-diplomat#657
is only relevant for argument passing
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
B-js JS backend
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants