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

x/sys/unix: BTRFS_SUPER_MAGIC overflows Statfs_t.Type's type on 32-bit platforms #52061

Open
greatroar opened this issue Mar 31, 2022 · 6 comments
Labels
compiler/runtime Issues related to the Go compiler and/or runtime. NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Milestone

Comments

@greatroar
Copy link

What version of Go are you using (go version)?

$ go version
go version go1.18 linux/amd64

What did you do?

func isBtrfs(info *unix.Statfs_t) bool { return info.Type == unix.BTRFS_SUPER_MAGIC }

What did you expect to see?

Code compiles on all platforms.

What did you see instead?

The code compiles on amd64 and arm64, but on 386 and arm it fails:

./fstype.go:7:62: unix.BTRFS_SUPER_MAGIC (untyped int constant 2435016766) overflows int32

Statfs_t.Type is an int32 or int64, depending on the platform. Maybe the constant should be typed accordingly?

Ref restic/restic#3687.

@gopherbot gopherbot added this to the Unreleased milestone Mar 31, 2022
@tklauser tklauser added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Mar 31, 2022
@tklauser tklauser changed the title x/sys: unix.BTRFS_SUPER_MAGIC overflows unix.Statfs_t.Type's type on 32-bit platforms x/sys/unix: BTRFS_SUPER_MAGIC overflows Statfs_t.Type's type on 32-bit platforms Mar 31, 2022
@ianlancetaylor
Copy link
Contributor

This seems like a specific case of a general problem, and I'm not sure what the general answer is. There is an awkward mismatch between Go and C when it comes to constant overflow. C silently wraps, Go does not.

@greatroar
Copy link
Author

greatroar commented Apr 1, 2022

Our ad hoc solution is to cast to int64(uint(info.Type)). Getting that cast right is tricky enough to warrant a convenience method, IMHO.

@fd0
Copy link

fd0 commented Apr 1, 2022

FWIW, I'm unsure if the types for the Type field in the struct (int32/int64) are correct. I don't know how the struct (for Go) is generated, but the manpage for statfs(2) it is advised to use an unsigned int:

The __fsword_t type used for various fields in the statfs structure definition is a glibc internal type, not intended for public use. This leaves the programmer in a bit of a conundrum when trying to copy or compare these fields to local variables in a program. Using unsigned int for such variables suffices on most systems.

@greatroar
Copy link
Author

The type on amd64 is correct:

#include <sys/vfs.h>
#include <stdio.h>

int main() {
    struct statfs buf;
    printf("%zd\n", sizeof(buf.f_type));
    return 0;
}

prints 8.

@fd0
Copy link

fd0 commented Apr 1, 2022

What I wanted to point out was that the manpage advises programmers to use an unsigned int, on amd64 in Go Type is of int64. On 386, the type is int32, for which the constant is too big. But it's not too big for an uint32.

Am I missing something?

@ianlancetaylor
Copy link
Contributor

The type of the f_type field in the C struct statfs is signed. That's why the Go field has a signed type.

@gopherbot gopherbot added the compiler/runtime Issues related to the Go compiler and/or runtime. label Jul 7, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
compiler/runtime Issues related to the Go compiler and/or runtime. NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Projects
Development

No branches or pull requests

5 participants