-
Notifications
You must be signed in to change notification settings - Fork 17.9k
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
cmd/cgo: nested structure has too much alignment padding #28896
Comments
Program output Aligned struct requiring padding |
There is no way to represent a packed struct in Go, so the bug is that cgo is generating an incorrect struct rather than an error. Slightly smaller test case. The two lines should have the same numbers. On amd64 this program currently prints
package main
import (
"fmt"
"unsafe"
)
/*
#include <stdio.h>
#include <stdlib.h>
#include <stdint.h>
#pragma pack(push, _header, 1)
// Should have size of 8 + 4 = 0xc
typedef struct {
void * elem1;
uint32_t elem2;
} InnerNotAligned;
// Should have size of 8 + 8 = 0x10
typedef struct {
void * elem1;
uint64_t elem2;
} InnerAligned;
#pragma pack(pop, _header)
// Should have size of 0xc + 4 (pad) + 8 = 0x18
typedef struct {
InnerNotAligned inner;
// requires ONLY 4 pad bytes to align the next element
uint64_t elem3;
} OuterNotAligned;
// Should have size of 0x10 + 8 = 0x18
typedef struct {
InnerAligned inner;
// requires no padding
uint64_t elem3;
} OuterAligned;
void printSizes() {
printf("C %zu %zu\n", offsetof(OuterNotAligned, elem3), offsetof(OuterAligned, elem3));
}
*/
import "C"
func main() {
var notAligned C.OuterNotAligned
var aligned C.OuterAligned
fmt.Printf("Go %d %d\n", unsafe.Offsetof(notAligned.elem3), unsafe.Offsetof(aligned.elem3))
C.printSizes()
} |
Change https://golang.org/cl/150602 mentions this issue: |
I can't follow the gopherbot included link, but this is the issue referred to: https://go-review.googlesource.com/c/go/+/150602 |
Can this be considered for backport to 1.11 ? |
@gopherbot Please open an issue to backport to 1.11. |
Backport issue(s) opened: #28916 (for 1.11). Remember to create the cherry-pick CL(s) as soon as the patch is submitted to master, according to https://golang.org/wiki/MinorReleases. |
Change https://golang.org/cl/151778 mentions this issue: |
…ld offset The old code ignored the field alignment, and only looked at the field offset: if the field offset required padding, cgo added padding. But while that approach works for Go (at least with the gc toolchain) it doesn't work for C code using packed structs. With a packed struct the added padding may leave the struct at a misaligned position, and the inserted alignment, which cgo is not considering, may introduce additional, unexpected, padding. Padding that ignores alignment is not a good idea when the struct is not packed, and Go structs are never packed. So don't ignore alignment. Updates #28896 Fixes #28916 Change-Id: Ie50ea15fa6dc35557497097be9fecfecb11efd8a Reviewed-on: https://go-review.googlesource.com/c/150602 Run-TryBot: Ian Lance Taylor <iant@golang.org> Reviewed-by: Bryan C. Mills <bcmills@google.com> (cherry picked from commit fbdaa96) Reviewed-on: https://go-review.googlesource.com/c/151778 TryBot-Result: Gobot Gobot <gobot@golang.org>
What version of Go are you using (
go version
)?Does this issue reproduce with the latest release?
Yes
What operating system and processor architecture are you using (
go env
)?go env
OutputWhat did you do?
The attached file is a minimal reproducible test case for a complicated set of nested includes in our codebase. A "packed" structure that ends 'unaligned' is nested into an "unpacked" structure with a subsequent element requiring 8 byte alignment.
What did you expect to see?
I expected that only four bytes of padding would be inserted and be visible via fmt.Printf("%+v") resulting in a data structure that matched our external C code.
What did you see instead?
An extra "invisible" 8 bytes of padding are inserted breaking interoperability
main.txt
The text was updated successfully, but these errors were encountered: