runtime: non-atomic pointer writes in memmove/memclr on arm64

[mwhudson]

Full output here: http://paste.ubuntu.com/12319496/

I've had this happen on an ubuntu builder (with the 1.5 release) and with tip as of 5cbca8d

Seems to happen roughly every other time.

[mwhudson]

I ran it in a much longer loop: 17 failures out of 100 runs. And each run takes a minute...

[ianlancetaylor]

CC @RLH @aclements

[aclements]

@mwhudson, I just pushed some improvements to this error message to master. Could you try reproducing it at tip?

[mwhudson]

https://gist.github.com/mwhudson/db0c9def25e417c03cb1

Seems marginally more intermittent now (~10% but maybe that's just due to load or someting)

[aclements]

Excellent. Thanks.

Since it sounds like you're able to reproduce the failure regularly, could
you paste a few more panics? That'll help with establishing a pattern.
On Sep 20, 2015 6:06 PM, "Michael Hudson-Doyle" notifications@github.com
wrote:

https://gist.github.com/mwhudson/db0c9def25e417c03cb1

Seems marginally more intermittent now (~10% but maybe that's just due to
load or someting)

—
Reply to this email directly or view it on GitHub
#12552 (comment).

[mwhudson]

Yep sure, I'd noticed that the three I've made so far this morning do look slightly different. I'll let it run for a few hours and post what I get.

[mwhudson]

http://people.canonical.com/~mwh/oracle-fails.tgz is 41 failures from 314 runs. Have fun :-)

[aclements]

Interesting. This didn't at all show the sorts of patterns I was expecting to see, but it did show a different pattern: in every case, the bad pointer has changed between when we first read it from the object (and checked it) and when we printed the object it was found in. Furthermore, excepting the cases where it's 0 the second time, the first read is always a zero-padded prefix of the second read:

panic.txt.103:runtime: pointer 0x4824000000 to unallocated spanidx=0x2000 span.start=0x4829326000 span.limit=0x482932a000 span.state=4
panic.txt.103: _(object+8) = _0x48241811f0* <==

panic.txt.11:runtime: pointer 0x4825000000 to unused region of spanidx=0x2800 span.start=0x4820a8a000 span.limit=0x4820a8c000 span.state=0
panic.txt.11: _(object+8) = _0x4825a62640* <==

panic.txt.129:runtime: pointer 0x4821f80000 to unallocated spanidx=0xfc0 span.start=0x4821f7a000 span.limit=0x4821f7e000 span.state=2
panic.txt.129: _(object+296) = _0x4821f8a0c0* <==

Whether there are two or three bytes of prefix varies. I suspect there can also be one or four bytes of prefix, but that we don't see one because it lands outside the heap and we don't see four because it lands inside of a valid span (just not the right object in the span).

Almost certainly something is doing a byte-at-a-time copying and the garbage collector is catching it in the middle of copying a pointer. The runtime's arm64 memmove does byte copying, which could cause exactly this failure. @4ad, @minux, it looks like we need to implement doubleword memmove at least for doubleword-aligned pointers on arm64. It looks like the compiler's blockcopy does 8-byte moves, so it should be good. I'm not aware of anything else in the compiler or runtime that does bulk memory copying.

[aclements]

This also affects ppc64. It always uses byte copying.

This also affects amd64p32, though more subtly: it uses 8 byte moves and then falls back to 1 byte moves, so if a pointer is the last field in an object and it's 4-byte aligned, but not 8-byte aligned, it will be copied one byte at a time.

I believe the other architectures are okay.

[mwhudson]

Oh hah, that's one of those things that had never occurred to me but is totally obvious in retrospect. At least the fix is fairly simple even if it's a bunch of typing.

[aclements]

Yeah, I had the same reaction. :) It's a standard optimization, of course, but it hadn't occurred to me that it was a required optimization in Go.

[davecheney]

Awesome detecting work Michael and Austin. Adding a better memcpy to arm64
shouldn't be too hard. Do you think this would be a candidate for go 1.5.2 ?

Thanks

Dave
On 22 Sep 2015 7:32 am, "Austin Clements" notifications@github.com wrote:

Yeah, I had the same reaction. :) It's a standard optimization, of course,
but it hadn't occurred to me that it was a required optimization in Go.

—
Reply to this email directly or view it on GitHub
#12552 (comment).