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.

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: long long not working with gcc 4.7 on Windows #5603

Closed
gopherbot opened this issue May 31, 2013 · 8 comments

Comments

@gopherbot
Copy link

commented May 31, 2013

by zombie.fml:

Summary: __attribute__((__packed__)) is broken on Windows with gcc 4.7
using the default compiler options.

I noticed this problem when compiling go on Windows. The build failed when testing
misc/cgo/test at `testParallelSleep`. It reported a very long duration between
two parallel 1s sleeps.

This is my system information:

gcc.exe (GCC) 4.7.2/mingw ** NOTE gcc version is important
go version go1.1 windows/386
8g
Windows 7 32-bit

With gcc 4.7, Windows mingw targets are using the -mms-bitfields option by default.
See http://gcc.gnu.org/bugzilla/show_bug.cgi?id=52991 and
http://gcc.gnu.org/gcc-4.7/changes.html

This flag causes __attribute__((__packed__)) not working as intended on structs.
cgo generates code that relies on __attribute__((__packed__)) which is then broken.

I wrote a simple test code at http://play.golang.org/p/VnXvU4yQPw
The expected output is four 0x12345678. But the output is:

$ go run 64.go
x0 = 0x12345678, x1 = 0x1234567800000000, x2 = 0x12345678, x3 = 0x1234567800000000, x4 =
0x12345678

I modified cgo generated intermediate c code during compiling to output the
struct size. Like this one:

void
_cgo_f861c44e4fb7_Cfunc_foo1(void *v)
{
    struct foo {
        void* p0;
        long long int r;
    } __attribute__((__packed__)) *a = v;
    int i = 0;
    unsigned char *p = (unsigned char*)a;
    a->r = foo1((void*)a->p0);
    for (i = 0; i < sizeof(*a); i++, p++) {
        printf("0x%x ", *p);
    }
    printf("--foo1, sizeof = %d, offset(r) = %d\n", sizeof(*a), offsetof(struct foo, r));
}

And I got this output:

$ go run 64.go
x0 = 0x12345678, x1 = 0x1234567800000000, x2 = 0x12345678, x3 = 0x1234567800000000, x4 =
0x12345678
0x78 0x56 0x34 0x12 0x0 0x0 0x0 0x0 --foo0, sizeof = 8, offset(r) = 0
0x0 0x0 0x0 0x0 0x0 0x0 0x0 0x0 0x78 0x56 0x34 0x12 0x0 0x0 0x0 0x0 --foo1, sizeof = 16,
offset(r) = 8
0x0 0x0 0x0 0x0 0x0 0x0 0x0 0x0 0x78 0x56 0x34 0x12 0x0 0x0 0x0 0x0 --foo2, sizeof = 16,
offset(r) = 8
0x0 0x0 0x0 0x0 0x0 0x0 0x0 0x0 0x0 0x0 0x0 0x0 0x0 0x0 0x0 0x0 0x78 0x56 0x34 0x12 0x0
0x0 0x0 0x0 --foo3, sizeof = 24, offset(r) = 16
0x0 0x0 0x0 0x0 0x0 0x0 0x0 0x0 0x0 0x0 0x0 0x0 0x0 0x0 0x0 0x0 0x78 0x56 0x34 0x12 0x0
0x0 0x0 0x0 --foo4, sizeof = 24, offset(r) = 16

There's a 4-byte padding in foo1 and foo3 which is not expected. The ·_Cfunc_foo1
and ·_Cfunc_foo3 functions in _cgo_defun.c passes a 12 and 20 bytes buffer to 
_cgo_f861c44e4fb7_Cfunc_foo1 and _cgo_f861c44e4fb7_Cfunc_foo3 respectively. But
the actual struct sizes are 16 and 24 respectively.
@minux

This comment has been minimized.

Copy link
Member

commented May 31, 2013

Comment 1:

please see if https://golang.org/cl/9895043 fixes the problem.

Labels changed: added priority-later, removed priority-triage.

Status changed to Accepted.

@minux

This comment has been minimized.

Copy link
Member

commented May 31, 2013

Comment 2:

Owner changed to @minux.

Status changed to Started.

@gopherbot

This comment has been minimized.

Copy link
Author

commented May 31, 2013

Comment 3 by zombie.fml:

https://golang.org/cl/9895043 works fine for me.
@minux

This comment has been minimized.

Copy link
Member

commented May 31, 2013

Comment 4:

thank you for your confirmation. i will mail that CL.
@davecheney

This comment has been minimized.

Copy link
Contributor

commented Jun 5, 2013

Comment 5:

Issue #5638 has been merged into this issue.

@minux

This comment has been minimized.

Copy link
Member

commented Jun 9, 2013

Comment 6:

This issue was closed by revision 949228a.

Status changed to Fixed.

@davecheney

This comment has been minimized.

Copy link
Contributor

commented Jun 11, 2013

Comment 7:

Issue #5546 has been merged into this issue.

@ianlancetaylor

This comment has been minimized.

Copy link
Contributor

commented Sep 10, 2013

Comment 8:

Issue #6149 has been merged into this issue.

This issue was closed.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
4 participants
You can’t perform that action at this time.