Describe the bug, including details regarding any error messages, version, and platform.
The arm64 NEON assembly implementations of functions in this library break CPU profiling. This breakage usually manifests as truncated and sometimes invalid tracebacks in the profile and sometimes crashes. The reason for this is that the functions tend to manipulate the stack pointer, but via WORD directives rather than normal instructions, e.g. WORD $0xa9ba7bfd // stp x29, x30, [sp, #-96]!. The Go assembler doesn't decode these instructions and thus doesn't see the stack pointer manipulation. But this means that the generated unwinding tables will be invalid. The tables encode how much to increase the stack pointer at a given instruction to find the next call frame.
Examples
For example, go test -bench=. -cpuprofile=cpu.pprof ./arrow/memory produces a profile like
Notice that most of the memory._memset_neon time is in a traceback with one frame, missing the callers.
For another example, consider this benchmark of parquet/internal/utils:
func BenchmarkUnpack32(b *testing.B) {
const batchSize = 512
input := make([]byte, batchSize*4)
if _, err := rand.Read(input); err != nil {
b.Fatal(err)
}
output := make([]uint64, batchSize)
reader := NewBitReader(bytes.NewReader(input))
for b.Loop() {
reader.Reset(bytes.NewReader(input))
reader.GetBatch(32, output)
}
}
The CPU time attributed to _unpack32_neon, which gets called here, looks like this in the CPU profile:
There are either no callers, or an invalid call sequence showing the runtime as the caller of this function.
The same benchmark actually crashes the CPU profiler when with cgo disabled, i.e. CGO_ENABLED=0 go test -bench=BenchmarkUnpack32 ./parquet/internal/utils. This is because _unpack32_neon writes to register R28/x28, which is reserved by the runtime to hold the current goroutine address. Ref. With cgo disabled, the CPU profiling signal handler reads the goroutine directly from this register, crashing because it is invalid. The benchmark was constructed to trigger this case.
Suggested fixes
- Get rid of any instructions that write to the stack pointer.
- For functions which need a stack frame, give them a stack frame by modifying the assembly function declaration. For example,
TEXT ·_unpack32_neon(SB), $0-40 would become TEXT ·_unpack32_neon(SB), $496-40 because it needs a 496 byte frame. (I think it could even be 480, but my current draft fix had accounted for )
- Regenerate/modify
_unpack32_neon to avoid using registers R28 and R18, which are reserved.
I can send a PR for the first two fixes. I haven't tried the third fix yet and it might be a bit more involved. An ideal long term fix would be to port all the WORD directives to actual instructions but that's 1) a big change and 2) blocked by touching R28 and R18, which the assembler normally doesn't allow.
NB: I think these bugs might cause crashes like in golang/go#62086, where unwinding gets stuck, but I haven't reproduced that specific failure mode yet.
Component(s)
Other
Describe the bug, including details regarding any error messages, version, and platform.
The arm64 NEON assembly implementations of functions in this library break CPU profiling. This breakage usually manifests as truncated and sometimes invalid tracebacks in the profile and sometimes crashes. The reason for this is that the functions tend to manipulate the stack pointer, but via
WORDdirectives rather than normal instructions, e.g.WORD $0xa9ba7bfd // stp x29, x30, [sp, #-96]!. The Go assembler doesn't decode these instructions and thus doesn't see the stack pointer manipulation. But this means that the generated unwinding tables will be invalid. The tables encode how much to increase the stack pointer at a given instruction to find the next call frame.Examples
For example,
go test -bench=. -cpuprofile=cpu.pprof ./arrow/memoryproduces a profile likeNotice that most of the
memory._memset_neontime is in a traceback with one frame, missing the callers.For another example, consider this benchmark of
parquet/internal/utils:The CPU time attributed to
_unpack32_neon, which gets called here, looks like this in the CPU profile:There are either no callers, or an invalid call sequence showing the runtime as the caller of this function.
The same benchmark actually crashes the CPU profiler when with cgo disabled, i.e.
CGO_ENABLED=0 go test -bench=BenchmarkUnpack32 ./parquet/internal/utils. This is because_unpack32_neonwrites to register R28/x28, which is reserved by the runtime to hold the current goroutine address. Ref. With cgo disabled, the CPU profiling signal handler reads the goroutine directly from this register, crashing because it is invalid. The benchmark was constructed to trigger this case.Suggested fixes
TEXT ·_unpack32_neon(SB), $0-40would becomeTEXT ·_unpack32_neon(SB), $496-40because it needs a 496 byte frame. (I think it could even be 480, but my current draft fix had accounted for )_unpack32_neonto avoid using registers R28 and R18, which are reserved.I can send a PR for the first two fixes. I haven't tried the third fix yet and it might be a bit more involved. An ideal long term fix would be to port all the
WORDdirectives to actual instructions but that's 1) a big change and 2) blocked by touching R28 and R18, which the assembler normally doesn't allow.NB: I think these bugs might cause crashes like in golang/go#62086, where unwinding gets stuck, but I haven't reproduced that specific failure mode yet.
Component(s)
Other