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

misc/cgo: Test32579 breaks builds on aix/ppc64 and linux/s390x #32770

Closed
Helflym opened this issue Jun 25, 2019 · 2 comments
Closed

misc/cgo: Test32579 breaks builds on aix/ppc64 and linux/s390x #32770

Helflym opened this issue Jun 25, 2019 · 2 comments
Labels
FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done. release-blocker Soon This needs to be done soon. (regressions, serious bugs, outages) Testing An issue that has been verified to require only test changes, not just a test failure.
Milestone

Comments

@Helflym
Copy link
Contributor

Helflym commented Jun 25, 2019

Test32579, introduce by CL 183458, doesn't work on aix/ppc64 and linux/s390x:
https://build.golang.org/log/7390cf1f99fa31505bc0f13e1834c2e7a20cc62f
https://build.golang.org/log/7df224f96eeb375bacff23d3656b111b3ee38a98

As these are big-endian, memset(..., 1, 1) won't set the value to 1(=0x00000001) but to 16777216 (=0x01000000). This can be reproduced with a C program.

On aix/pc64 (big-endian):

$ cat memset.c 
#include <stdio.h>
#include <stdlib.h>
#include <string.h>

typedef struct S32579 { int data[1]; } S32579;

void main(){
        S32579 s[1];
        s[0].data[0] = 0; //clear
        memset(&s[0].data[0], 1, 1);
        printf("0x%08x\n", s[0].data[0]);
}
$ gcc -maix64 memset.c && a.out 
0x01000000

On linux/ppc64le (little-endian)

$ gcc -m64 memset.c && ./a.out
0x00000001

How should this test be fixed ? Should memset be changed in order to still have s[0].data[0] == 1 or can we simply check that for big-endian s[0].data[0] == 16777216 ?

/cc @ianlancetaylor @kawakami-o3

@kawakami-o3
Copy link
Contributor

I recommend to use unsigned char data[1], instead of int data[1].

@gopherbot
Copy link

Change https://golang.org/cl/183778 mentions this issue: misc/cgo/test: use char, not int, so test works on big-endian systems

@bcmills bcmills added release-blocker Soon This needs to be done soon. (regressions, serious bugs, outages) Testing An issue that has been verified to require only test changes, not just a test failure. labels Jun 25, 2019
@bcmills bcmills added this to the Go1.13 milestone Jun 25, 2019
@andybons andybons added the NeedsFix The path to resolution is known, but the work has not been done. label Jun 25, 2019
@golang golang locked and limited conversation to collaborators Jun 24, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done. release-blocker Soon This needs to be done soon. (regressions, serious bugs, outages) Testing An issue that has been verified to require only test changes, not just a test failure.
Projects
None yet
Development

No branches or pull requests

5 participants