apache / apache/arrow-go

arm64 assembly breaks CPU profiling

Open
#983 3 comments 0 reactions 0 assignees View on GitHub
Type: bug
Dominant language
Assembly
Stars
404
Forks
145
Avg merge
2d 4h
Merged PRs (30d)
87

Description

### 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

Image

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:

Image

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](https://cs.opensource.google/go/go/+/master:src/cmd/compile/abi-internal.md). 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 https://github.com/golang/go/issues/62086, where unwinding gets stuck, but I haven't reproduced that specific failure mode yet.

### Component(s)

Other

Contributor guide

Open the contributing guide

Research direction

Start by reproducing the profiling failures with go test -bench=. -cpuprofile=cpu.pprof ./arrow/memory and the BenchmarkUnpack32 benchmark in parquet/internal/utils, including CGO_ENABLED=0. Inspect the arm64 NEON assembly for _memset_neon and _unpack32_neon, then verify that profiles retain valid callers and the cgo-disabled benchmark no longer crashes.

Written by the indexing model from the issue text.

Assessment

Tech stack
go
Domain
performance
Issue type
Bug
Difficulty
4/5
Estimated time
3-5 days
Activity status
Active
Clarity
Mostly clear
Newbie friendliness
52/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.