Fix generator layout and sizes#1607
Conversation
| ($which: ident) => {{ | ||
| let tp_ty = instance.substs.type_at(0); | ||
| if tp_ty.is_generator() { | ||
| let e = self.codegen_unimplemented_expr( |
There was a problem hiding this comment.
Was this previously missed because sitting inside a macro made it hard to find references? Just curious.
There was a problem hiding this comment.
No, the macro had no effect on this as far as I can tell.
| name: field_name, | ||
| typ: ctx.codegen_ty(field_ty), | ||
| }); | ||
| offset += field_size; |
There was a problem hiding this comment.
This is the core of the fix right?
There was a problem hiding this comment.
Yes, that was the bug.
| // But it also fails for WASM (https://github.com/rust-lang/rust/issues/62807), | ||
| // so it is probably not a big problem: | ||
| assert_eq!(1026, std::mem::size_of_val(&move_before_yield_with_noop())); | ||
| // With panic=unwind, the following assertion passes: |
There was a problem hiding this comment.
Please improve this doc. Maybe add as a document of the harness itself.
| typ: ctx.codegen_ty(field_ty), | ||
| }); | ||
| offset += field_size; | ||
| offset = field_offset + field_size; |
There was a problem hiding this comment.
Can you please add a comment here? You really only care about the last field, right?
There was a problem hiding this comment.
What kind of comment do you mean? We also use the offset earlier to figure out how much padding to insert before a field.
There was a problem hiding this comment.
Gotcha. That's fine, thanks
Description of changes:
This PR fixes a bug in how Kani lays out generators. Previously, Kani got some offsets (and as a consequence, the size) wrong sometimes.
In the process, @celinval also discovered that the size of a generator may depend on the panic strategy, which explains the discrepancy of the sizes in #1395. This issue is resolved here as well.
Resolved issues:
Resolves #1395
Resolves #1593
Testing:
How is this change tested? Added a regression test for Assertion failure in
check_vtable_size#1593, and got the failing tests from Duplicate lang item in cratestderrors while building the std library #1359 working.Is this a refactor change? No
Checklist
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 and MIT licenses.