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

Jump to bottom

runtime: netbsd/386 lwp_park seems wrong #22946

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

runtime: netbsd/386 lwp_park seems wrong #22946

bradfitz opened this issue Nov 30, 2017 · 7 comments
Labels
FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done. OS-NetBSD

Comments

@bradfitz
Copy link
Contributor

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
@bradfitz bradfitz added NeedsFix The path to resolution is known, but the work has not been done. OS-NetBSD labels Nov 30, 2017
@ianlancetaylor
Copy link
Contributor

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
Copy link
Contributor Author

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
Copy link
Contributor

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
Copy link
Contributor

ianlancetaylor commented Nov 30, 2017 •
edited

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
Copy link
Contributor Author

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
Copy link
Contributor Author

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
Copy link
Contributor

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 as completed 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.
Assignees
No one assigned
Labels
FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done. OS-NetBSD
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
3 participants
@bradfitz @ianlancetaylor @gopherbot