code review 95460046: syscall: fix stack frame sizes in assembly

syscall: fix stack frame sizes in assembly

for GOOS in darwin freebsd linux nacl netbsd openbsd plan9 solaris windows
do
        for GOARCH in 386 amd64 amd64p32 arm
        do
                go vet
        done
done

These are all real mistakes being corrected, but none
of them should be able to cause problems today
due to the NOSPLIT on the functions.

However, vet has also identified a few important problems.
I'm sending this CL to get rid of the trivial 'go vet' results
before attacking the real ones.

[rsc, 2014-05-15 20:31:08]

Hello golang-codereviews@googlegroups.com (cc: bradfitz, r),

I'd like you to review this change to
https://code.google.com/p/go/

[r, 2014-05-15 20:41:29]

LGTM

https://codereview.appspot.com/95460046/diff/60001/src/pkg/syscall/asm_linux_...
File src/pkg/syscall/asm_linux_amd64.s (right):

https://codereview.appspot.com/95460046/diff/60001/src/pkg/syscall/asm_linux_...
src/pkg/syscall/asm_linux_amd64.s:113: TEXT ·Gettimeofday(SB),NOSPLIT,$0-24
why isn't this 16 and the next one 24?

[bradfitz, 2014-05-15 20:43:49]

https://codereview.appspot.com/95460046/diff/60001/src/pkg/syscall/asm_linux_...
File src/pkg/syscall/asm_linux_amd64.s (right):

https://codereview.appspot.com/95460046/diff/60001/src/pkg/syscall/asm_linux_...
src/pkg/syscall/asm_linux_amd64.s:113: TEXT ·Gettimeofday(SB),NOSPLIT,$0-24
On 2014/05/15 20:41:29, r wrote:
> why isn't this 16 and the next one 24?

*Timeval + 2 words in the error interface result parameter = 3 * 8, no?

[rsc, 2014-05-15 20:45:11]

https://codereview.appspot.com/95460046/diff/60001/src/pkg/syscall/asm_linux_...
File src/pkg/syscall/asm_linux_amd64.s (right):

https://codereview.appspot.com/95460046/diff/60001/src/pkg/syscall/asm_linux_...
src/pkg/syscall/asm_linux_amd64.s:113: TEXT ·Gettimeofday(SB),NOSPLIT,$0-24
On 2014/05/15 20:41:29, r wrote:
> why isn't this 16 and the next one 24?

This is a bug to be fixed in the next CL. The Go function is declared to return
'err error' but this code is returning 'errno uintptr'. If gettimeofday
succeeds, errno is 0, we write a 0 to the first word of the two-word error
value, that counts as a nil error (which no one checks anyway, probably) and all
is well. If gettimeofday were to fail, errno would be non-zero, we'd write that
to the type word of the interface, and calling err.Error() would blow up. Lucky
for us, gettimeofday probably never fails.

Time has the same problem, as does Seek on linux/arm and linux/386.

go vet flagged these problems too (because it doesn't expect assembly to be able
to fill out an interface values) but I want to do them in a followup.

[r, 2014-05-15 20:45:47]

https://codereview.appspot.com/95460046/diff/60001/src/pkg/syscall/asm_linux_...
File src/pkg/syscall/asm_linux_amd64.s (right):

https://codereview.appspot.com/95460046/diff/60001/src/pkg/syscall/asm_linux_...
src/pkg/syscall/asm_linux_amd64.s:113: TEXT ·Gettimeofday(SB),NOSPLIT,$0-24
On 2014/05/15 20:43:49, bradfitz wrote:
> On 2014/05/15 20:41:29, r wrote:
> > why isn't this 16 and the next one 24?
> 
> *Timeval + 2 words in the error interface result parameter = 3 * 8, no?

got it

[rsc, 2014-05-15 20:47:55]

*** Submitted as https://code.google.com/p/go/source/detail?r=b02fede60a4c ***

syscall: fix stack frame sizes in assembly

for GOOS in darwin freebsd linux nacl netbsd openbsd plan9 solaris windows
do
        for GOARCH in 386 amd64 amd64p32 arm
        do
                go vet
        done
done

These are all real mistakes being corrected, but none
of them should be able to cause problems today
due to the NOSPLIT on the functions.

However, vet has also identified a few important problems.
I'm sending this CL to get rid of the trivial 'go vet' results
before attacking the real ones.

LGTM=r
R=golang-codereviews, r, bradfitz
CC=golang-codereviews
https://codereview.appspot.com/95460046

[dave_cheney.net, 2014-05-16 02:16:21]

> This is a bug to be fixed in the next CL. The Go function is declared to
> return 'err error' but this code is returning 'errno uintptr'. If
> gettimeofday succeeds, errno is 0, we write a 0 to the first word of the
> two-word error value, that counts as a nil error (which no one checks
> anyway, probably) and all is well.

Aram and I hit this problem a few times during the Solaris port due to
the number of adapter methods we had to write to call syscalls
correctly. Something like

func open(...) error // implemented in asm or c which returned a
uintptr, not an error interface value

func Open(..) error {
  return open(..)
}

would cause bizarre situations where the function would be a success,
ie 0 was returned, but when interpreted as an interface value the
second word would be some random garbage so e != nil == true even
though underlying type of the error was nil.

[rsc, 2014-05-16 03:46:36]

On Thu, May 15, 2014 at 10:16 PM, Dave Cheney <dave@cheney.net> wrote:

> > This is a bug to be fixed in the next CL. The Go function is declared to
> > return 'err error' but this code is returning 'errno uintptr'. If
> > gettimeofday succeeds, errno is 0, we write a 0 to the first word of the
> > two-word error value, that counts as a nil error (which no one checks
> > anyway, probably) and all is well.
>
> Aram and I hit this problem a few times during the Solaris port due to
> the number of adapter methods we had to write to call syscalls
> correctly. Something like
>
> func open(...) error // implemented in asm or c which returned a
> uintptr, not an error interface value
>

Run go vet early and often.

would cause bizarre situations where the function would be a success,
> ie 0 was returned, but when interpreted as an interface value the
> second word would be some random garbage so e != nil == true even
> though underlying type of the error was nil.
>

I'm a little confused by this.  A check for nil only looks at the first
word.

[dave_cheney.net, 2014-05-16 04:06:49]

> On 16 May 2014, at 13:46, Russ Cox <rsc@golang.org> wrote:
> 
>> On Thu, May 15, 2014 at 10:16 PM, Dave Cheney <dave@cheney.net> wrote:
>> > This is a bug to be fixed in the next CL. The Go function is declared to
>> > return 'err error' but this code is returning 'errno uintptr'. If
>> > gettimeofday succeeds, errno is 0, we write a 0 to the first word of the
>> > two-word error value, that counts as a nil error (which no one checks
>> > anyway, probably) and all is well.
>> 
>> Aram and I hit this problem a few times during the Solaris port due to
>> the number of adapter methods we had to write to call syscalls
>> correctly. Something like
>> 
>> func open(...) error // implemented in asm or c which returned a
>> uintptr, not an error interface value
> 
> Run go vet early and often.

Yes, this error was self inflicted. 

> 
>> would cause bizarre situations where the function would be a success,
>> ie 0 was returned, but when interpreted as an interface value the
>> second word would be some random garbage so e != nil == true even
>> though underlying type of the error was nil.
> 
> I'm a little confused by this.  A check for nil only looks at the first word.

I am probably miss remembering our diagnosis, Aram can probably remember the
details better as he figured out what we were doing wrong. 

> 
> 
> 

[aram, 2014-05-16 10:03:58]

Can't remember the exact details, and my hg foo is weak today, but
the problem was with functions that failed and err.Error() blew up.
When I realised the problem, I fixed all the Solaris code, or at
least I hope I did. In retrospect, I should have fixed all the code,
not only the Solaris code, I appologise for not being a good citized.

If you'll forgive me for the above, I'm sure you will not forgive
me for what I am about to say next. During the port, I have identified
at least a dozen "theoretical" problems with the other ports. Most
of the problems were of the similar nature to this one. I didn't
file bugs for most of them, instead I wrote them on a piece of
paper.

At FOSDEM, I had the piece of paper with me. I wanted to read from
it to exemplify how bad it is not to file bugs; I wanted to give
myself as a negative example and tell people to do better. In an
ironic twist of fate, I forgot the paper in my hotel room in Brussels.

I can't remember what was written on it.