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

runtime: netbsd/386 lwp_park seems wrong #22946

Closed
bradfitz opened this issue Nov 30, 2017 · 7 comments

Comments

Projects
None yet
3 participants
@bradfitz
Copy link
Member

commented Nov 30, 2017

I keep seeing netbsd/386 failures like:
https://build.golang.org/log/d792068d5d8a6b035a050f0f6da9a266eff5f66e

Stuck in:

ok  	container/list	0.034s
ok  	container/ring	0.013s
SIGQUIT: quit
PC=0x8096617 m=5 sigcode=0

goroutine 0 [idle]:
runtime.lwp_park(0x0, 0x0, 0x185564c4, 0x0, 0x0, 0x0, 0x0, 0x0, 0x8053ce6, 0x0, ...)
	/tmp/workdir/go/src/runtime/sys_netbsd_386.s:351 +0x7
runtime.semasleep(0xffffffff, 0xffffffff, 0x18556280)
	/tmp/workdir/go/src/runtime/os_netbsd.go:141 +0xc4
runtime.notesleep(0x18556344)
	/tmp/workdir/go/src/runtime/lock_sema.go:167 +0xdc
runtime.stopm()
	/tmp/workdir/go/src/runtime/proc.go:1953 +0xba
runtime.findrunnable(0x1851b300, 0x0)
	/tmp/workdir/go/src/runtime/proc.go:2410 +0x34c
runtime.schedule()
	/tmp/workdir/go/src/runtime/proc.go:2536 +0x10b
runtime.park_m(0x18558540)
	/tmp/workdir/go/src/runtime/proc.go:2599 +0x8d
runtime.mcall(0x96575)
	/tmp/workdir/go/src/runtime/asm_386.s:406 +0x43

Go's sys_netbsd_386.s contains:

TEXT runtime·lwp_park(SB),NOSPLIT,$-4                                                                    
        MOVL    $434, AX                // sys__lwp_park                                                                                                                                                           
        INT     $0x80                                                                                    
        MOVL    AX, ret+16(FP)                                                                           
        RET                                                                                              

That implies it has no arguments.

But in the NetBSD source:

sys/kern/syscalls.c:    /* 434 */       "compat_60__lwp_park",
...

sys/sys/syscall.h:/* syscall: "compat_60__lwp_park" ret: "int" args: "const struct timespec *" "lwpid_t" "const void *" "const void *" */

...

/*                                                                                                                                                                                                                 
 * 'park' an LWP waiting on a user-level synchronisation object.  The LWP                                                                                                                                          
 * will remain parked until another LWP in the same process calls in and                                                                                                                                           
 * requests that it be unparked.                                                                                                                                                                                   
 */     
int
sys____lwp_park60(struct lwp *l, const struct sys____lwp_park60_args *uap,
    register_t *retval)
{       

This look wrong to anybody else?

/cc @ianlancetaylor @bsiegert @zoulasc @krytarowski @aclements

@ianlancetaylor

This comment has been minimized.

Copy link
Contributor

commented Nov 30, 2017

The code in sys_netbsd_386.s should work because on 386 the syscall arguments are passed on the stack,and Go does the same, and the Go function takes the exact arguments that the syscall takes.

@bradfitz

This comment has been minimized.

Copy link
Member Author

commented Nov 30, 2017

Then I guess I'm misreading the NetBSD comments.

When I saw args: "const struct timespec *" "lwpid_t" "const void *" "const void *" , I assumed that the lwp_park60 syscall took 4 parameters, not zero.

@ianlancetaylor

This comment has been minimized.

Copy link
Contributor

commented Nov 30, 2017

lwp_park does take four parameters. It''s just that the syscall is going to look for those parameters on the stack, and that is exactly where Go puts them.

@ianlancetaylor

This comment has been minimized.

Copy link
Contributor

commented Nov 30, 2017

Or, put it this way, if lwp_park is wrong, then so is read, write, closefd, etc. Admittedly they might all be wrong--they don't seem to be taking the pushed PC from the call into account--but then I would expect that nothing would have ever worked.

@bradfitz

This comment has been minimized.

Copy link
Member Author

commented Nov 30, 2017

Oh, duh. I hadn't looked at lwp_park anywhere else in runtime except the *.s file and had expected to see handling of parameters like elsewhere in the file.

Thanks.

@bradfitz

This comment has been minimized.

Copy link
Member Author

commented Nov 30, 2017

Well, read & closefd are only used by getRandomData, which doesn't seem to care about errors:

//go:nosplit                                                                                                                                                                        
func getRandomData(r []byte) {
        fd := open(&urandom_dev[0], 0 /* O_RDONLY */, 0)   
        n := read(fd, unsafe.Pointer(&r[0]), int32(len(r)))
        closefd(fd)
        extendRandom(r, int(n))
}
@ianlancetaylor

This comment has been minimized.

Copy link
Contributor

commented Nov 30, 2017

I think it's OK. runtime/sys_openbsd_386.s uses the same kind of code, and it seems to correspond to the code in syscall/asm_unix_386.s.

@bradfitz bradfitz closed this Dec 1, 2017

@golang golang locked and limited conversation to collaborators Dec 1, 2018

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
You can’t perform that action at this time.