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: unix.SysctlUint64 fails on 32-bit FreeBSD #15186

Closed
bradfitz opened this issue Apr 8, 2016 · 7 comments
Closed

x/sys/unix: unix.SysctlUint64 fails on 32-bit FreeBSD #15186

bradfitz opened this issue Apr 8, 2016 · 7 comments

Comments

@bradfitz
Copy link
Contributor

bradfitz commented Apr 8, 2016

The freebsd-386 builder has been failing on this x/sys/unix test for some time now:

https://storage.googleapis.com/go-build-log/68ac1f77/freebsd-386-gce101_03efe3a9.log

freebsd-386-gce101 at 68ac1f774624faf99e7f6ec6592acb50f23b7874 building sys at 4c24bcbb85d58313623349380489486f9b3934ec

?       golang.org/x/sys/plan9  [no test files]
--- FAIL: TestSysctUint64 (0.00s)
    syscall_freebsd_test.go:18: input/output error
FAIL
FAIL    golang.org/x/sys/unix   0.006s


Error: tests failed: exit status 1

/cc @adg @mikioh @minux

@bradfitz bradfitz added this to the Unreleased milestone Apr 8, 2016
gopherbot pushed a commit to golang/sys that referenced this issue Apr 8, 2016
Updates golang/go#15186

Change-Id: I02403cbc9b3f1c786ee8093e1480edff891ecf13
Reviewed-on: https://go-review.googlesource.com/21676
Reviewed-by: Andrew Gerrand <adg@golang.org>
@mikioh
Copy link
Contributor

mikioh commented Apr 9, 2016

EIO usually indicates an error of "size mismatched" or "unaligned access" to the kernel. I'm not sure why the test uses vm.max_kernel_address. Because vm usually consists of ton of machine-dependent stuff. Also freebsd-386-gce101 is actually a complicated "freebsd-386 on freebsd-amd64" host. It depends on the implementation whether the value is returned from 32-bit kernel part or 64-bit kernel part. It would be better to use more appropriate syscall entry in testing.

Also s/TestSysctUint64/TestSysctlUint64

@bradfitz
Copy link
Contributor Author

bradfitz commented Apr 9, 2016

vm.max_kernel_address doesn't look like it matters. It's an arbitrary sysctl name. The test doesn't even look at the value.

@mikioh
Copy link
Contributor

mikioh commented Apr 9, 2016

I guess vm.max_kernel_address is a read-only value inside the kernel, hm, am I missing something?

vm/kern_vm.c:

SYSCTL_ULONG(_vm, OID_AUTO, max_kernel_address, CTLFLAG_RD,
#if defined(__arm__) || defined(__sparc64__)
    &vm_max_kernel_address, 0,
#else
    SYSCTL_NULL_ULONG_PTR, VM_MAX_KERNEL_ADDRESS,
#endif
    "Max kernel address");

sys/sysctl.h:

/* definitions for sysctl_req 'flags' member */
#if defined(__amd64__) || defined(__ia64__) || defined(__powerpc64__) ||\
    (defined(__mips__) && defined(__mips_n64))
#define SCTL_MASK32     1       /* 32 bit emulation */
#endif

kern/kern_sysctl.c:

int
sysctl_handle_long(SYSCTL_HANDLER_ARGS)
{
(snip)
ifdef SCTL_MASK32
        if (req->flags & SCTL_MASK32) {
                tmpint = tmplong;
                error = SYSCTL_OUT(req, &tmpint, sizeof(int));
        } else
#endif
                error = SYSCTL_OUT(req, &tmplong, sizeof(long));

@bradfitz
Copy link
Contributor Author

bradfitz commented Apr 9, 2016

Why do we care whether it's read-only? All we're doing is reading it anyway.

@mikioh
Copy link
Contributor

mikioh commented Apr 9, 2016

Why do we care whether it's read-only?

Nope, I just mentioned it's a value because you said "The test doesn't even look at the value." As I mentioned above, the kernel returns either a 4-byte length value or an 8-byte length for vm.max_kernel_address. If the test (or API) expects an 8-byte length value, it should use an appropriate sysctl entry instead of vm.max_kernel_address.

@mikioh
Copy link
Contributor

mikioh commented Apr 11, 2016

This is a low hanging fruit, https://github.com/golang/sys/blob/master/unix/syscall_bsd.go#L532, for someone who wants to contribute this package. Please don't forget to fix the typos in test functions; s/TestSysct/TestSysctl/g

@gopherbot
Copy link

CL https://golang.org/cl/36058 mentions this issue.

@golang golang locked and limited conversation to collaborators Feb 1, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

3 participants