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.

Sign up for GitHub

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

Jump to bottom

"wrong argument size" for uint16 arguments #191

Closed
ericlagergren opened this issue May 12, 2021 · 2 comments
Closed

"wrong argument size" for uint16 arguments #191

ericlagergren opened this issue May 12, 2021 · 2 comments
Labels
bug Something isn't working

Comments

@ericlagergren
Copy link

This snippet will cause compilation errors: foo: wrong argument size 8; expected $...-2

TEXT("foo", 0, "func(x uint16)")
RET()
@mmcloughlin mmcloughlin mentioned this issue Oct 30, 2021
Merged
@mmcloughlin
Copy link
Owner

Okay, #212 reproduces the bug. To clarify, it seems to be an asmdecl error not a compilation error.

So I'm pretty sure I've found the bug. If there are return values then the arguments structure is padded to max alignment before the first return value. avo is doing the padding even if there are no return values, as in this case. See on the avo side:

avo/gotypes/signature.go

Line 102 in 1d235fb

vs = append(vs, types.NewParam(token.NoPos, nil, "sentinel", types.Typ[types.Uint64]))

And in asmdecl:

https://github.com/golang/tools/blob/351c04c5a14afe1131e549831fcb29a0ca1df982/go/analysis/passes/asmdecl/asmdecl.go#L627

Fix should be simple, but I'm wondering about how to create a test suite that would more thoroughly test these issues, because #195 seems to be another instance.

@mmcloughlin mmcloughlin added the bug Something isn't working label Oct 30, 2021
mmcloughlin added a commit that referenced this issue Oct 30, 2021
@mmcloughlin
gotypes: fix argument size for signatures without return types (#212)
7de0251 
This fixes a bug in argument size calculation in the case where the function
has no return values. Previously it was padding the argument struct to max
alignment, but this only happens if there are return values following.

Updates #191
@mmcloughlin mmcloughlin mentioned this issue Oct 30, 2021
Closed
mmcloughlin added a commit that referenced this issue Oct 30, 2021
@mmcloughlin
gotypes,tests: test case for issue #195 (#214)
4f7179b 
Adds test cases for the bug in #195, similar to #212.

Updates #195
Updates #191
@mmcloughlin mmcloughlin mentioned this issue Oct 30, 2021
Merged
mmcloughlin added a commit that referenced this issue Oct 31, 2021
@mmcloughlin
tests: test functions with random signatures (#216)
6acd6a0 
Adds a test for function signature memory layout by generating functions with
random signatures. This confirms that the size and offsets computed by
`gotypes` agree with `asmdecl`.

Updates #191 #195
@mmcloughlin
Copy link
Owner

I believe this is fixed, with an extensive test added in #216. Tagged release v0.3.1.

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

No branches or pull requests
2 participants
@ericlagergren @mmcloughlin