4ad/go.arm64

runtime: memmove wrong!

minux opened this issue · 4 comments

http://play.golang.org/p/sXDvlW5z7A

package main

import "fmt"

func main() {
    testCases := []struct {
        dst0, dst1 int
        src0, src1 int
        want       string
    }{
        {0, 9, 0, 9, "012345678"},
        {0, 5, 4, 9, "45678"},
        {4, 9, 0, 5, "01230"},
        {1, 6, 3, 8, "34567"},
        {3, 8, 1, 6, "12121"},
        {0, 9, 3, 6, "345"},
        {3, 6, 0, 9, "012"},
        {1, 6, 0, 9, "00000"},
        {0, 4, 7, 8, "7"},
        {0, 1, 6, 8, "6"},
        {4, 4, 6, 9, ""},
        {2, 8, 6, 6, ""},
        {0, 0, 0, 0, ""},
    }
    for _, tc := range testCases {
        b := []byte("0123456789")
        n := tc.dst1 - tc.dst0
        if tc.src1-tc.src0 < n {
            n = tc.src1 - tc.src0
        }
        forwardCopy(b, tc.dst0, tc.src0, n)
        got := string(b[tc.dst0 : tc.dst0+n])
        if got != tc.want {
            fmt.Printf("dst=b[%d:%d], src=b[%d:%d]: got %q, want %q\n",
                tc.dst0, tc.dst1, tc.src0, tc.src1, got, tc.want)
        }
        // Check that the bytes outside of dst[:n] were not modified.
        for i, x := range b {
            if i >= tc.dst0 && i < tc.dst0+n {
                continue
            }
            if int(x) != '0'+i {
                fmt.Printf("dst=b[%d:%d], src=b[%d:%d]: copy overrun at b[%d]: got '%c', want '%c'\n",
                    tc.dst0, tc.dst1, tc.src0, tc.src1, i, x, '0'+i)
            }
        }
    }
}

// forwardCopy is like the built-in copy function except that it always goes
// forward from the start, even if the dst and src overlap.
// It is equivalent to:
//   for i := 0; i < n; i++ {
//     mem[dst+i] = mem[src+i]
//   }
func forwardCopy(mem []byte, dst, src, n int) {
    if dst <= src {
        copy(mem[dst:dst+n], mem[src:src+n])
        return
    }
    for {
        if dst >= src+n {
            copy(mem[dst:dst+n], mem[src:src+n])
            return
        }
        // There is some forward overlap.  The destination
        // will be filled with a repeated pattern of mem[src:src+k].
        // We copy one instance of the pattern here, then repeat.
        // Each time around this loop k will double.
        k := dst - src
        copy(mem[dst:dst+k], mem[src:src+k])
        n -= k
        dst += k
    }
}

Produced the following on arm64:

dst=b[0:5], src=b[4:9]: got "85678", want "45678"
dst=b[1:6], src=b[3:8]: got "76767", want "34567"

But shouldn't output anything.

I haven't taken a closer look yet.

I think cmd/7g failed to use backward copy when needed.

A minimum test case:

package main

import "fmt"

var t = []byte{0,1,2,3,4,5,6,7,8,9}

func main() {
    copy(t[0:5], t[4:9])
    fmt.Println(t)
}

arm:
[4 5 6 7 8 5 6 7 8 9]
arm64:
[8 5 6 7 8 5 6 7 8 9]

On the contrary, it's using backward copy incorrectly...

OK, our memmove_arm64.s is also wrong. The problem is that CMP order is different between ARM64 an Power64.
Fixed by minux@8a57a9a

Has been fixed.