cmd/cgo: alignment issue with int128 inside of a struct #69086

mwetterw · 2024-08-27T15:19:56Z

Go version

go version go1.23.0 linux/amd64

Output of `go env` in your module/workspace:

GO111MODULE=''
GOARCH='amd64'
GOBIN=''
GOCACHE='/home/mwetterw/.cache/go-build'
GOENV='/home/mwetterw/.config/go/env'
GOEXE=''
GOEXPERIMENT=''
GOFLAGS=''
GOHOSTARCH='amd64'
GOHOSTOS='linux'
GOINSECURE=''
GOMODCACHE='/home/mwetterw/go/pkg/mod'
GONOPROXY=''
GONOSUMDB=''
GOOS='linux'
GOPATH='/home/mwetterw/go'
GOPRIVATE=''
GOPROXY='https://proxy.golang.org,direct'
GOROOT='/usr/lib/go'
GOSUMDB='sum.golang.org'
GOTMPDIR=''
GOTOOLCHAIN='auto'
GOTOOLDIR='/usr/lib/go/pkg/tool/linux_amd64'
GOVCS=''
GOVERSION='go1.23.0'
GODEBUG=''
GOTELEMETRY='local'
GOTELEMETRYDIR='/home/mwetterw/.config/go/telemetry'
GCCGO='gccgo'
GOAMD64='v1'
AR='ar'
CC='gcc'
CXX='g++'
CGO_ENABLED='1'
GOMOD='/home/mwetterw/testGo/int128/go.mod'
GOWORK=''
CGO_CFLAGS='-O2 -g'
CGO_CPPFLAGS=''
CGO_CXXFLAGS='-O2 -g'
CGO_FFLAGS='-O2 -g'
CGO_LDFLAGS='-O2 -g'
PKG_CONFIG='pkg-config'
GOGCCFLAGS='-fPIC -m64 -pthread -Wl,--no-gc-sections -fmessage-length=0 -ffile-prefix-map=/tmp/go-build3621668412=/tmp/go-build -gno-record-gcc-switches'

What did you do?

From Go, I passed to C a struct containing one int128 (and another field before it so that Go needs to guess the padding between the first member and that int128 member).
From C, I wanted to read what I got from Go.

go.mod

module int128

go 1.23.0

main.go

package main

import (
        "fmt"
        "unsafe"
)

// #include "int128.h"
import "C"

func toC() [16]byte {
        return [16]byte{17, 18, 19, 20, 21, 22, 23, 24, 25, 26, 27, 28, 29, 30, 31, 32}
}

func main() {
        x := C.get_uint128()
        fmt.Printf("%[1]T --- %[1]v\n", x)

        C.set_uint128([16]byte{1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16})
        fmt.Printf("%[1]T --- %[1]v\n", C.get_uint128())

        s := C.get_s128()
        fmt.Printf("\ns128 sz=%d sz_x=%d off_x=%d off_num=%d off_len=%d\n",
                unsafe.Sizeof(s), unsafe.Sizeof(s.x), unsafe.Offsetof(s.x), unsafe.Offsetof(s.num), unsafe.Offsetof(s.len))
        fmt.Printf("%[1]T --- %[1]v\n", s)
        fmt.Printf("%[1]T --- %[1]v\n", s.num)
        fmt.Printf("%[1]T --- %[1]v\n", s.len)

        C.set_s128(C.struct_s128{
                num: toC(),
                len: 56,
        })
        s = C.get_s128()
        fmt.Printf("\n%[1]T --- %[1]v\n", s)
        fmt.Printf("%[1]T --- %[1]v\n", s.num)
        fmt.Printf("%[1]T --- %[1]v\n", s.len)

        C.print_s128(C.struct_s128{
                num: toC(),
                len: 56,
        })
}

int128.h

#pragma once

void set_uint128(unsigned __int128 n);
unsigned __int128 get_uint128();

struct s128 {
    int x;
    unsigned __int128 num;
    unsigned char len;
};

void set_s128(struct s128 n);
struct s128 get_s128();

void print_s128(struct s128 x);

int128.c

#include <stdio.h>  // printf
#include <stddef.h> // offsetof

#include "int128.h"

unsigned __int128 x = ((unsigned __int128)0x0102030405060708ULL << 64)
                                        | 0x090a0b0c0d0e0f10ULL;
struct s128 s = {
    .num = ((unsigned __int128)0x0102030405060708ULL << 64)
                             | 0x090a0b0c0d0e0f10ULL,
    .len = 42
};

void print_s128(struct s128 x)
{
    printf("C(sz=%zu sz_x=%zu off_x=%zu off_num=%zu off_len=%zu) => "
           "%hhu.%hhu.%hhu.%hhu.%hhu.%hhu.%hhu.%hhu.%hhu.%hhu.%hhu.%hhu.%hhu.%hhu.%hhu.%hhu/%hhu\n",
           sizeof(x),
           sizeof(x.x),
           offsetof(struct s128, x),
           offsetof(struct s128, num),
           offsetof(struct s128, len),
           (unsigned char)(x.num >> 120) & 255,
           (unsigned char)(x.num >> 112) & 255,
           (unsigned char)(x.num >> 104) & 255,
           (unsigned char)(x.num >>  96) & 255,
           (unsigned char)(x.num >>  88) & 255,
           (unsigned char)(x.num >>  80) & 255,
           (unsigned char)(x.num >>  72) & 255,
           (unsigned char)(x.num >>  64) & 255,
           (unsigned char)(x.num >>  56) & 255,
           (unsigned char)(x.num >>  48) & 255,
           (unsigned char)(x.num >>  40) & 255,
           (unsigned char)(x.num >>  32) & 255,
           (unsigned char)(x.num >>  24) & 255,
           (unsigned char)(x.num >>  16) & 255,
           (unsigned char)(x.num >>   8) & 255,
           (unsigned char)x.num          & 255,
           x.len);
}

void set_uint128(unsigned __int128 n)
{
    x = n;
}

unsigned __int128 get_uint128()
{
    return x;
}

void set_s128(struct s128 n)
{
    s = n;
}

struct s128 get_s128()
{
    return s;
}

What did you see happen?

The result is corrupted: C doesn't receive the correct int128.
This is because Go seems to not correctly guess the padding for int128 inside of a struct.

main._Ctype___int128unsigned --- [16 15 14 13 12 11 10 9 8 7 6 5 4 3 2 1]
main._Ctype___int128unsigned --- [1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16]

s128 sz=44 sz_x=4 off_x=0 off_num=12 off_len=28
main._Ctype_struct_s128 --- {0 [0 0 0 0 0 0 0 0] [0 0 0 0 16 15 14 13 12 11 10 9 8 7 6 5] 4 [3 2 1 42 0 0 0 0 0 0 0 0 0 0 0]}
main._Ctype___int128unsigned --- [0 0 0 0 16 15 14 13 12 11 10 9 8 7 6 5]
main._Ctype_uchar --- 4

main._Ctype_struct_s128 --- {0 [0 0 0 0 0 0 0 0] [17 18 19 20 21 22 23 24 25 26 27 28 29 30 31 32] 56 [0 0 0 0 0 0 0 0 0 0 0 0 0 0 0]}
main._Ctype___int128unsigned --- [17 18 19 20 21 22 23 24 25 26 27 28 29 30 31 32]
main._Ctype_uchar --- 56
C(sz=48 sz_x=4 off_x=0 off_num=16 off_len=32) => 0.0.0.56.32.31.30.29.28.27.26.25.24.23.22.21/0

Look how C and Go both have a different conception of that C struct:

For C, the struct is 48 bytes long, and the int128 field is at offset 16.
For Go, the struct is 44 bytes long, and the int128 field is at offset 12.

What did you expect to see?

I expected Go to correctly guess the padding of the C struct, and to receive the correct int128 from C perspective.

C self-aligns the int128: i.e. it is aligned to a multiple of 16, which is its own size.

I would have expected to see this result:

main._Ctype___int128unsigned --- [16 15 14 13 12 11 10 9 8 7 6 5 4 3 2 1]
main._Ctype___int128unsigned --- [1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16]

s128 sz=48 sz_x=4 off_x=0 off_num=16 off_len=32
main._Ctype_struct_s128 --- {0 [0 0 0 0 0 0 0 0 0 0 0 0] [16 15 14 13 12 11 10 9 8 7 6 5 4 3 2 1] 42 [0 0 0 0 0 0 0 0 0 0 0 0 0 0 0]}
main._Ctype___int128unsigned --- [16 15 14 13 12 11 10 9 8 7 6 5 4 3 2 1]
main._Ctype_uchar --- 42

main._Ctype_struct_s128 --- {0 [0 0 0 0 0 0 0 0 0 0 0 0] [17 18 19 20 21 22 23 24 25 26 27 28 29 30 31 32] 56 [0 0 0 0 0 0 0 0 0 0 0 0 0 0 0]}
main._Ctype___int128unsigned --- [17 18 19 20 21 22 23 24 25 26 27 28 29 30 31 32]
main._Ctype_uchar --- 56
C(sz=48 sz_x=4 off_x=0 off_num=16 off_len=32) => 32.31.30.29.28.27.26.25.24.23.22.21.20.19.18.17/56

Correct behavior can be obtained by making the C padding explicit and visible to Go:

struct s128 {
  int x;
  unsigned char _pad[12];
  unsigned __int128 num;
  unsigned char len;
};

The text was updated successfully, but these errors were encountered:

gopherbot · 2024-08-27T17:23:11Z

Change https://go.dev/cl/608815 mentions this issue: cmd/cgo: correct padding required by alignment

prattmic · 2024-08-27T19:06:07Z

cc @golang/compiler

mwetterw · 2024-08-28T15:47:20Z

Not sure if it's the same bug or another one.
Feel free to tell me if you want a separate issue for this.

I noticed the same kind of problem, but this time it looks to me like a function call ABI issue when a struct containing an int128 is passed as fourth argument of a C function.

go.mod

module int128

go 1.23.0

main.go

package main

// #include "int128.h"
import "C"

func main() {
	ul1 := C.ulong(42)
	ul2 := C.ulong(43)
	u := C.unsigned(44)

	int128 := [16]byte{1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16}
	len := 96
	s := C.struct_s128{int128: int128, len: C.uchar(len)}

	C.int128_test(ul1, ul2, u, s)
}

int128.h

#pragma once

struct s128 {
    unsigned __int128 int128;
    unsigned char len;
};

int int128_test(unsigned long ul1, unsigned long ul2, unsigned u, struct s128 s);

int128.c

#include <stdio.h>

#include "int128.h"


int int128_test(unsigned long ul1, unsigned long ul2, unsigned u, struct s128 s)
{
    printf("ul1: %lu ul2: %lu, u:%u\n", ul1, ul2, u);

    printf("s: %hhu %hhu %hhu %hhu %hhu %hhu %hhu %hhu %hhu %hhu %hhu %hhu %hhu %hhu %hhu %hhu [%hhu]\n",
            (unsigned char)(s.int128 >> 120 & 0xff),
            (unsigned char)(s.int128 >> 112 & 0xff),
            (unsigned char)(s.int128 >> 104 & 0xff),
            (unsigned char)(s.int128 >>  96 & 0xff),
            (unsigned char)(s.int128 >>  88 & 0xff),
            (unsigned char)(s.int128 >>  80 & 0xff),
            (unsigned char)(s.int128 >>  72 & 0xff),
            (unsigned char)(s.int128 >>  64 & 0xff),
            (unsigned char)(s.int128 >>  56 & 0xff),
            (unsigned char)(s.int128 >>  48 & 0xff),
            (unsigned char)(s.int128 >>  40 & 0xff),
            (unsigned char)(s.int128 >>  32 & 0xff),
            (unsigned char)(s.int128 >>  24 & 0xff),
            (unsigned char)(s.int128 >>  16 & 0xff),
            (unsigned char)(s.int128 >>   8 & 0xff),
            (unsigned char)(s.int128        & 0xff),
            s.len);

    return 0;
}

When struct s128 is the fourth parameter of the C function, Go doesn't correctly pass the struct to C.
C sees following (incorrect):

ul1: 42 ul2: 43, u:44
s: 0 0 0 96 16 15 14 13 12 11 10 9 8 7 6 5 [0]

When struct s128 is the first, the second or the third argument of the C function, Go correctly passes the struct to C.
C sees following (correct):

ul1: 42 ul2: 43, u:44
s: 16 15 14 13 12 11 10 9 8 7 6 5 4 3 2 1 [96]

(Note that here, adding a padding to the struct doesn't change the issue.)

ianlancetaylor · 2024-08-28T16:12:01Z

@mwetterw Can you post the corresponding Go code for the problem passing a struct as an argument? Thanks.

mwetterw · 2024-08-28T16:16:53Z

@ianlancetaylor Sorry, completely forgot that one!
I've edited my message just above with the missing main.go file.

gabyhelp · 2024-08-28T21:11:17Z

Related Issues and Documentation

cmd/cgo: miscalculates C array offset when C struct has __attribute__((__packed__)) #46675
cmd/compile: wrong calculation result for bit operation that's inlined and has all constant shifts in rewrite rules #32680 (closed)
cmd/compile: internal compiler error: 'New': can't find source #32681 (closed)
cmd/compile: multiple LittleEndian.Uint* calls on the same buffer block load merging #52708 (closed)
cmd/compile: encoding/binary.PutUint32 generates unaligned data storage on arm64 arch #59856 (closed)
cmd/vet: false positive for uint128 ([2]uint64) #46843 (closed)
cmd/compile: Use wide integer load/store instructions if possible #11819 (closed)
cmd/cgo: cannot load DWARF output from ./_cgo_.o: applyRelocations: not implemented #18959 (closed)
cmd/compile: regression: unnecessary spilling to the stack #61356 (closed)
cmd/cgo: Writing to C pointer causes Go heap corruption #26288 (closed)

_{(Emoji vote if this was helpful or unhelpful; more detailed feedback welcome in this discussion.)}

maxatome · 2024-09-02T12:49:48Z

Thanks for the fix!
Do you think it could be possible to backport it to the next 1.23 minor release?

ianlancetaylor · 2024-09-03T00:06:51Z

@gopherbot Please backport this issue.

This never worked so it is not a regression. But it seems worth fixing because without it it's quite hard to use a C struct with an int128 field.

gopherbot · 2024-09-03T00:07:35Z

Backport issue(s) opened: #69218 (for 1.22), #69219 (for 1.23).

Remember to create the cherry-pick CL(s) as soon as the patch is submitted to master, according to https://go.dev/wiki/MinorReleases.

mwetterw · 2024-09-03T15:40:21Z

@ianlancetaylor Thanks for the fix!
I tested it with gotip and I confirm that my original issues are fixed. :)

gopherbot · 2024-09-05T21:24:51Z

Change https://go.dev/cl/611296 mentions this issue: [release-branch.go1.23] cmd/cgo: correct padding required by alignment

gopherbot · 2024-09-05T21:30:34Z

Change https://go.dev/cl/611297 mentions this issue: [release-branch.go1.22] cmd/cgo: correct padding required by alignment

If the aligned offset isn't sufficient for the field offset, we were padding based on the aligned offset. We need to pad based on the original offset instead. Also set the Go alignment correctly for int128. We were defaulting to the maximum alignment, but since we translate int128 into an array of uint8 the correct Go alignment is 1. For #69086 Fixes #69219 Change-Id: I23ce583335c81beac2ac51f7f9336ac97ccebf09 Reviewed-on: https://go-review.googlesource.com/c/go/+/608815 Reviewed-by: Damien Neil <dneil@google.com> LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com> Reviewed-by: Cherry Mui <cherryyz@google.com> Auto-Submit: Ian Lance Taylor <iant@golang.org> (cherry picked from commit c209892) Reviewed-on: https://go-review.googlesource.com/c/go/+/611296 Reviewed-by: Ian Lance Taylor <iant@google.com> Auto-Submit: Ian Lance Taylor <iant@google.com> Commit-Queue: Ian Lance Taylor <iant@google.com>

If the aligned offset isn't sufficient for the field offset, we were padding based on the aligned offset. We need to pad based on the original offset instead. Also set the Go alignment correctly for int128. We were defaulting to the maximum alignment, but since we translate int128 into an array of uint8 the correct Go alignment is 1. For #69086 Fixes #69218 Change-Id: I23ce583335c81beac2ac51f7f9336ac97ccebf09 Reviewed-on: https://go-review.googlesource.com/c/go/+/608815 Reviewed-by: Damien Neil <dneil@google.com> LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com> Reviewed-by: Cherry Mui <cherryyz@google.com> Auto-Submit: Ian Lance Taylor <iant@golang.org> (cherry picked from commit c209892) Reviewed-on: https://go-review.googlesource.com/c/go/+/611297 Auto-Submit: Ian Lance Taylor <iant@google.com> Reviewed-by: Ian Lance Taylor <iant@google.com>

mwetterw changed the title ~~CGO: Alignment issue with int128~~ CGO: Alignment issue with int128 inside of a struct Aug 27, 2024

ianlancetaylor changed the title ~~CGO: Alignment issue with int128 inside of a struct~~ cmd/cgo: alignment issue with int128 inside of a struct Aug 27, 2024

gopherbot added the compiler/runtime label Aug 27, 2024

prattmic added this to the Backlog milestone Aug 27, 2024

prattmic added the NeedsFix label Aug 27, 2024

dr2chase added this to Go Compiler / Runtime Aug 28, 2024

dr2chase assigned ianlancetaylor Aug 28, 2024

dr2chase moved this to In Progress in Go Compiler / Runtime Aug 28, 2024

gopherbot closed this as completed in c209892 Aug 30, 2024

github-project-automation bot moved this from In Progress to Done in Go Compiler / Runtime Aug 30, 2024

gopherbot mentioned this issue Sep 3, 2024

cmd/cgo: alignment issue with int128 inside of a struct [1.22 backport] #69218

Closed

gopherbot mentioned this issue Sep 3, 2024

cmd/cgo: alignment issue with int128 inside of a struct [1.23 backport] #69219

Closed

dmitshur modified the milestones: Backlog, Go1.24 Sep 4, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

cmd/cgo: alignment issue with int128 inside of a struct #69086

cmd/cgo: alignment issue with int128 inside of a struct #69086

mwetterw commented Aug 27, 2024 •

edited

Loading

gopherbot commented Aug 27, 2024

prattmic commented Aug 27, 2024

mwetterw commented Aug 28, 2024 •

edited

Loading

ianlancetaylor commented Aug 28, 2024

mwetterw commented Aug 28, 2024

gabyhelp commented Aug 28, 2024

maxatome commented Sep 2, 2024

ianlancetaylor commented Sep 3, 2024

gopherbot commented Sep 3, 2024

mwetterw commented Sep 3, 2024

gopherbot commented Sep 5, 2024

gopherbot commented Sep 5, 2024

cmd/cgo: alignment issue with int128 inside of a struct #69086

cmd/cgo: alignment issue with int128 inside of a struct #69086

Comments

mwetterw commented Aug 27, 2024 • edited Loading

Go version

Output of go env in your module/workspace:

What did you do?

go.mod

main.go

int128.h

int128.c

What did you see happen?

What did you expect to see?

gopherbot commented Aug 27, 2024

prattmic commented Aug 27, 2024

mwetterw commented Aug 28, 2024 • edited Loading

go.mod

main.go

int128.h

int128.c

ianlancetaylor commented Aug 28, 2024

mwetterw commented Aug 28, 2024

gabyhelp commented Aug 28, 2024

maxatome commented Sep 2, 2024

ianlancetaylor commented Sep 3, 2024

gopherbot commented Sep 3, 2024

mwetterw commented Sep 3, 2024

gopherbot commented Sep 5, 2024

gopherbot commented Sep 5, 2024

mwetterw commented Aug 27, 2024 •

edited

Loading

Output of `go env` in your module/workspace:

mwetterw commented Aug 28, 2024 •

edited

Loading