Fix struct copying for structs with an unaligned size #387
Conversation
rinon
changed the title
rinon
force-pushed
the
sjc/fix_structs_test
branch
from
b0474e0 to
c5dc580
Compare
tools/rewriter/DetermineAbi.cpp
Outdated
| sig.ret.push_back(CAbiArg{ | ||
| .kind = CAbiArgKind::Memory, | ||
| .size = static_cast<unsigned>(num_regs) * 8, | ||
| .align = 8, | ||
| }); |
There was a problem hiding this comment.
Is this unchanged (in terms of still working with eightbytes) because the ABI behaves differently between arguments and returns, or because this PR alters behavior for multiple arguments being packed together but there is only ever one value returned?
There was a problem hiding this comment.
I believe branch only handles multiple integral values. Strangely sized structs are handled in abiSlotsForArg above.
rinon
force-pushed
the
sjc/fix_structs_test
branch
from
c5dc580 to
9e1dbe3
Compare
Clang started packing structs immediately after stack arguments, but we used to copy memory arguments as 8-byte chunks. Instead, copy only up to the size of the struct argument to avoid overwriting other bytes above it on the stack.
rinon
force-pushed
the
sjc/fix_structs_test
branch
from
9e1dbe3 to
6737b99
Compare
|
@fw-immunant I think I've addressed all your feedback, so this needs another look when you can. |
One last question, otherwise all looks good. |
|
I ported over the free space simplification that is going to happen for ARM anyway. Should be good now? |
Clang started packing structs immediately after
stack arguments, but we used to copy memory
arguments as 8-byte chunks. Instead, copy only up
to the size of the struct argument to avoid
overwriting other bytes above it on the stack.
Fixes #365