grafana / grafana/pyroscope-jfr-parser
bug: missing continue in T_ALLOC_SAMPLE skip branch causes nil pointer dereference
- Dominant language
- Go
- Stars
- 49
- Forks
- 24
- PR merge metrics
- No merged PRs in 30d
Description
## Description
In `parser/parser.go`, the `T_ALLOC_SAMPLE` case in `ParseEvent` is missing a `continue` statement after the skip branch, unlike every other event type. This means when `bindAllocSample == nil` (i.e. the event type was not found in the JFR metadata), the code skips to the correct position **but then immediately falls through** to call `Parse` with a nil `bind` pointer.
## Code
```go
// parser/parser.go ~line 199
case p.TypeMap.T_ALLOC_SAMPLE:
if p.bindAllocSample == nil {
p.pos = pp + int(size) // skip
// ← missing `continue` here!
}
_, err := p.ObjectAllocationSample.Parse(p.buf[p.pos:], p.bindAllocSample, &p.TypeMap)
```
Compare to every other event type, e.g.:
```go
case p.TypeMap.T_LIVE_OBJECT:
if p.bindLiveObject == nil {
p.pos = pp + int(size) // skip
continue // ← present here
}
```
## Impact
When `jdk.ObjectAllocationSample` is absent from the JFR metadata (which is valid — it's an optional event), `bindAllocSample` is nil and `T_ALLOC_SAMPLE` is set to `-1`. However, if another event type happens to have TypeID `-1`, the nil `bind` will be passed into `ObjectAllocationSample.Parse`, which will likely panic or produce corrupted output.
In practice `T_ALLOC_SAMPLE == -1` means the switch case is never matched under normal conditions (since no real event has type -1), so this bug is latent. It would surface if the JFR event type ID space ever collided with `-1`, or if the default value for unset TypeIDs is changed.
## Fix
Add `continue` after the skip branch:
```go
case p.TypeMap.T_ALLOC_SAMPLE:
if p.bindAllocSample == nil {
p.pos = pp + int(size) // skip
continue
}
_, err := p.ObjectAllocationSample.Parse(p.buf[p.pos:], p.bindAllocSample, &p.TypeMap)
```
Contributor guide
No contributing guide indexed for this repository
Assessment
This issue has not been assessed yet.