From b799c8d791eb2b91302bd7b6493901026c87d1fb Mon Sep 17 00:00:00 2001 From: Maxime Brunet Date: Thu, 17 Nov 2022 16:28:36 -0800 Subject: [PATCH 01/20] feat(perf): support reading symbols from jitdump --- pkg/jit/jitdump.go | 386 +++++++++++++++++++++++++++++++++++++++++++++ pkg/perf/perf.go | 67 +++++++- 2 files changed, 446 insertions(+), 7 deletions(-) create mode 100644 pkg/jit/jitdump.go diff --git a/pkg/jit/jitdump.go b/pkg/jit/jitdump.go new file mode 100644 index 0000000000..baaf9eff58 --- /dev/null +++ b/pkg/jit/jitdump.go @@ -0,0 +1,386 @@ +// Copyright 2022 The Parca Authors +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +// Specification: +// https://github.com/torvalds/linux/blob/master/tools/perf/Documentation/jitdump-specification.txt +// Perf implementation: +// https://github.com/torvalds/linux/blob/master/tools/perf/util/jitdump.c +// https://github.com/torvalds/linux/blob/master/tools/perf/util/jitdump.h + +// Package jit provides a parser for Perf's JITDUMP files +package jit + +import ( + "bufio" + "bytes" + "encoding/binary" + "errors" + "fmt" + "io" + "strings" + + "github.com/go-kit/log" + "github.com/go-kit/log/level" +) + +// JITHeader represent a jitdump file header. +type JITHeader struct { + Magic uint32 // a magic number tagging the file type + Version uint32 // a 4-byte value representing the format version. It is currently set to 1 + TotalSize uint32 // size in bytes of file header + ElfMach uint32 // ELF architecture encoding (ELF e_machine value as specified in /usr/include/elf.h) + Pad1 uint32 // padding. Reserved for future use + Pid uint32 // JIT runtime process identification (OS specific) + Timestamp uint64 // timestamp of when the file was created + Flags uint64 // a bitmask of flags +} + +// JITHeaderVersion is the supported version of the JITDUMP specification. +const JITHeaderVersion = 1 + +// JITRecordType is the value identifying the record type. +type JITRecordType uint32 + +const ( + JITCodeLoad JITRecordType = 0 // record describing a jitted function + JITCodeMove = 1 // record describing an already jitted function which is moved + JITCodeDebugInfo = 2 // record describing the debug information for a jitted function + JITCodeClose = 3 // record marking the end of the jit runtime (optional) + JITCodeUnwindingInfo = 4 // record describing a function unwinding information + JITCodeMax = iota // maximum record type +) + +// JRPrefix describes the record that follows. +type JRPrefix struct { + ID JITRecordType // a value identifying the record type + TotalSize uint32 // the size in bytes of the record including the header + Timestamp uint64 // a timestamp of when the record was created +} + +const jrPrefixSize int = 16 // size of JRPrefix type + +// JRCodeLoad represents a JITCodeLoad record. +type JRCodeLoad struct { + Prefix *JRPrefix // the record header + PID uint32 // OS process id of the runtime generating the jitted code + TID uint32 // OS thread identification of the runtime thread generating the jitted code + VMA uint64 // virtual address of jitted code start + CodeAddr uint64 // code start address for the jitted code. By default VMA = CodeAddr + CodeSize uint64 // size in bytes of the generated jitted code + CodeIndex uint64 // unique identifier for the jitted code + Name string // function name in ASCII + Code []byte // raw byte encoding of the jitted code +} + +// JRCodeMove represents a JITCodeMove record. +type JRCodeMove struct { + Prefix *JRPrefix // the record header + PID uint32 // OS process id of the runtime generating the jitted code + TID uint32 // OS thread identification of the runtime thread generating the jitted code + VMA uint64 // new virtual address of jitted code start + OldCodeAddr uint64 // previous code address for the same function + NewCodeAddr uint64 // alternate new code started address for the jitted code. By default it should be equal to the VMA address. + CodeSize uint64 // size in bytes of the jitted code + CodeIndex uint64 // index referring to the JRCodeLoad CodeIndex record of when the function was initially jitted +} + +// DebugEntry reprensents an entry from a JITCodeDebugInfo record. +type DebugEntry struct { + Addr uint64 // address of function for which the debug information is generated + Lineno uint32 // source file line number (starting at 1) + Discrim uint32 // column discriminator, 0 is default + Name string // source file name in ASCII +} + +const debugEntryFixedSize int = 16 // size of DebugEntry fixed-sized fields + +// JRCodeDebugInfo represents a JITCodeDebugInfo record. +type JRCodeDebugInfo struct { + Prefix *JRPrefix // the record header + CodeAddr uint64 // address of function for which the debug information is generated + NREntry uint64 // number of debug entries for the function + Entries []*DebugEntry // array of NREntry debug entries for the function +} + +const jrCodeDebugInfoFixedSize int = jrPrefixSize + 16 // size of JRCodeDebugInfo fixed-sized fields + +// JRCodeUnwindingInfo represents a JITCodeUnwindingInfo record. +type JRCodeUnwindingInfo struct { + Prefix *JRPrefix // the record header + UnwindingSize uint64 // the size in bytes of the unwinding data table at the end of the record + EHFrameHDRSize uint64 // the size in bytes of the DWARF EH Frame Header at the start of the unwinding data table at the end of the record + MappedSize uint64 // the size of the unwinding data mapped in memory + UnwindingData []byte // an array of unwinding data, consisting of the EH Frame Header, followed by the actual EH Frame +} + +const jrCodeUnwindingInfoFixedSize int = jrPrefixSize + 24 // size of JRCodeUnwindingInfo fixed-sized fields + +// JITDump represents the loaded jitdump. +type JITDump struct { + Header *JITHeader // the jitdump file header + CodeLoads []*JRCodeLoad // JITCodeLoad records + CodeMoves []*JRCodeMove // JITCodeMove records + DebugInfo []*JRCodeDebugInfo // JITCodeDebugInfo records + UnwindingInfo []*JRCodeUnwindingInfo // JITCodeUnwindingInfo records +} + +// jitDumpParser is used to parse a jitdump file. +type jitDumpParser struct { + logger log.Logger + buf *bufio.Reader + endianness binary.ByteOrder +} + +// newParser initializes a jitDumpParser. +func newParser(logger log.Logger, rd io.Reader) (*jitDumpParser, error) { + p := &jitDumpParser{ + logger: logger, + } + + p.buf = bufio.NewReader(rd) + + magic, err := p.buf.Peek(4) + if err != nil { + return nil, fmt.Errorf("failed to read magic number from jitdump file: %w", err) + } + + switch { + case bytes.Equal(magic, []byte{'J', 'i', 'T', 'D'}): + p.endianness = binary.BigEndian + case bytes.Equal(magic, []byte{'D', 'T', 'i', 'J'}): + p.endianness = binary.LittleEndian + default: + return nil, fmt.Errorf("failed to detect JIT dump endianness from %#x magic number: %w", magic, err) + } + + return p, nil +} + +// read reads structured binary data from the jitdump file into data. +func (p *jitDumpParser) read(data any) error { + return binary.Read(p.buf, p.endianness, data) +} + +func (p *jitDumpParser) parseJITHeader() (*JITHeader, error) { + header := &JITHeader{} + if err := p.read(header); err != nil { + return nil, fmt.Errorf("failed to read jitdump header: %w", err) + } + + if header.Version > JITHeaderVersion { + return nil, fmt.Errorf("wrong jitdump version: %d (expected: %d)", header.Version, JITHeaderVersion) + } + + return header, nil +} + +func (p *jitDumpParser) parseJRCodeLoad(prefix *JRPrefix) *JRCodeLoad { + jr := &JRCodeLoad{Prefix: prefix} + + if err := p.read(&jr.PID); err != nil { + level.Warn(p.logger).Log("msg", "error while reading JIT Code Load PID", "err", err) + } + if err := p.read(&jr.TID); err != nil { + level.Warn(p.logger).Log("msg", "error while reading JIT Code Load TID", "err", err) + } + if err := p.read(&jr.VMA); err != nil { + level.Warn(p.logger).Log("msg", "error while reading JIT Code Load VMA", "err", err) + } + if err := p.read(&jr.CodeAddr); err != nil { + level.Warn(p.logger).Log("msg", "error while reading JIT Code Load CodeAddr", "err", err) + } + if err := p.read(&jr.CodeSize); err != nil { + level.Warn(p.logger).Log("msg", "error while reading JIT Code Load CodeSize", "err", err) + } + if err := p.read(&jr.CodeIndex); err != nil { + level.Warn(p.logger).Log("msg", "error while reading JIT Code Load CodeIndex", "err", err) + } + + name, err := p.buf.ReadString(0) // read the null termination + if err != nil { + level.Warn(p.logger).Log("msg", "error while reading JIT Code Load Name", "err", err) + } + jr.Name = strings.TrimSuffix(name, "\x00") // trim the null termination + + jr.Code = make([]byte, jr.CodeSize) + if _, err := io.ReadAtLeast(p.buf, jr.Code, len(jr.Code)); err != nil { + level.Warn(p.logger).Log("msg", "error while reading JIT Code Load Code", "err", err) + } + + return jr +} + +func (p *jitDumpParser) parseJRCodeMove(prefix *JRPrefix) *JRCodeMove { + jr := &JRCodeMove{Prefix: prefix} + + if err := p.read(&jr.PID); err != nil { + level.Warn(p.logger).Log("msg", "error while reading JIT Code Move PID", "err", err) + } + if err := p.read(&jr.TID); err != nil { + level.Warn(p.logger).Log("msg", "error while reading JIT Code Move TID", "err", err) + } + if err := p.read(&jr.VMA); err != nil { + level.Warn(p.logger).Log("msg", "error while reading JIT Code Move VMA", "err", err) + } + if err := p.read(&jr.OldCodeAddr); err != nil { + level.Warn(p.logger).Log("msg", "error while reading JIT Code Move OldCodeAddr", "err", err) + } + if err := p.read(&jr.NewCodeAddr); err != nil { + level.Warn(p.logger).Log("msg", "error while reading JIT Code Move NewCodeAddr", "err", err) + } + if err := p.read(&jr.CodeSize); err != nil { + level.Warn(p.logger).Log("msg", "error while reading JIT Code Move CodeSize", "err", err) + } + if err := p.read(&jr.CodeIndex); err != nil { + level.Warn(p.logger).Log("msg", "error while reading JIT Code Move CodeIndex", "err", err) + } + + return jr +} + +func (p *jitDumpParser) parseJRCodeDebugInfo(prefix *JRPrefix) *JRCodeDebugInfo { + jr := &JRCodeDebugInfo{Prefix: prefix} + + if err := p.read(&jr.CodeAddr); err != nil { + level.Warn(p.logger).Log("msg", "error while reading JIT Code Debug Info CodeAddr", "err", err) + } + if err := p.read(&jr.NREntry); err != nil { + level.Warn(p.logger).Log("msg", "error while reading JIT Code Debug Info NREntry", "err", err) + } + + size := jrCodeDebugInfoFixedSize + debugEntryFixedSize*int(jr.NREntry) + + jr.Entries = make([]*DebugEntry, jr.NREntry) + for i := uint64(0); i < jr.NREntry; i++ { + jr.Entries[i] = &DebugEntry{} + + if err := p.read(&jr.Entries[i].Addr); err != nil { + level.Warn(p.logger).Log("msg", "error while reading Debug Entry Addr", "err", err) + } + if err := p.read(&jr.Entries[i].Lineno); err != nil { + level.Warn(p.logger).Log("msg", "error while reading Debug Entry Lineno", "err", err) + } + if err := p.read(&jr.Entries[i].Discrim); err != nil { + level.Warn(p.logger).Log("msg", "error while reading Debug Entry Discrim", "err", err) + } + + name, err := p.buf.ReadString(0) // read until the null termination + if err != nil { + level.Warn(p.logger).Log("msg", "error while reading Debug Entry Name", "err", err) + } + size += len(name) + jr.Entries[i].Name = strings.TrimSuffix(name, "\x00") // trim the null termination + } + + // Discard padding if any + if _, err := p.buf.Discard(int(jr.Prefix.TotalSize) - size); err != nil { + level.Warn(p.logger).Log("msg", "failed to discard JIT Code Debug Info record padding", "err", err) + } + + return jr +} + +func (p *jitDumpParser) parseJRCodeUnwindingInfo(prefix *JRPrefix) *JRCodeUnwindingInfo { + jr := &JRCodeUnwindingInfo{Prefix: prefix} + + jr.Prefix = prefix + + if err := p.read(&jr.UnwindingSize); err != nil { + level.Warn(p.logger).Log("msg", "error while reading JIT Code Unwinding Info UnwindingSize", "err", err) + } + if err := p.read(&jr.EHFrameHDRSize); err != nil { + level.Warn(p.logger).Log("msg", "error while reading JIT Code Unwinding Info EHFrameHDRSize", "err", err) + } + if err := p.read(&jr.MappedSize); err != nil { + level.Warn(p.logger).Log("msg", "error while reading JIT Code Unwinding Info MappedSize", "err", err) + } + + jr.UnwindingData = make([]byte, jr.UnwindingSize) + if _, err := io.ReadAtLeast(p.buf, jr.UnwindingData, len(jr.UnwindingData)); err != nil { + level.Warn(p.logger).Log("msg", "error while reading JIT Code Unwinding Info UnwindingData", "err", err) + } + + // Discard padding if any + if _, err := p.buf.Discard(int(jr.Prefix.TotalSize) - jrCodeUnwindingInfoFixedSize - int(jr.UnwindingSize)); err != nil { + level.Warn(p.logger).Log("msg", "failed to discard JIT Code Unwinding Info record padding", "err", err) + } + + return jr +} + +func (p *jitDumpParser) parse() (*JITDump, error) { + dump := &JITDump{} + var err error + dump.Header, err = p.parseJITHeader() + if err != nil { + return nil, fmt.Errorf("failed to read JIT dump header: %w", err) + } + + dump.CodeLoads = make([]*JRCodeLoad, 0) + dump.CodeMoves = make([]*JRCodeMove, 0) + dump.DebugInfo = make([]*JRCodeDebugInfo, 0) + dump.UnwindingInfo = make([]*JRCodeUnwindingInfo, 0) + + for { + prefix := &JRPrefix{} + if err := p.read(prefix); errors.Is(err, io.EOF) { + return dump, nil + } else if err != nil { + return nil, fmt.Errorf("failed to read JIT record prefix: %w", err) + } + + if prefix.ID >= JITCodeMax { + level.Warn(p.logger).Log("msg", "unknown JIT record type, skipping", "ID", prefix.ID) + if _, err := p.buf.Discard(int(prefix.TotalSize)); err != nil { + level.Warn(p.logger).Log("msg", "failed to discard unknown JIT record", "err", err) + } + continue + } + + switch prefix.ID { + case JITCodeLoad: + jr := p.parseJRCodeLoad(prefix) + dump.CodeLoads = append(dump.CodeLoads, jr) + case JITCodeMove: + jr := p.parseJRCodeMove(prefix) + dump.CodeMoves = append(dump.CodeMoves, jr) + case JITCodeDebugInfo: + jr := p.parseJRCodeDebugInfo(prefix) + dump.DebugInfo = append(dump.DebugInfo, jr) + case JITCodeClose: + level.Debug(p.logger).Log("msg", "reached JIT Code Close record") + return dump, nil + case JITCodeUnwindingInfo: + jr := p.parseJRCodeUnwindingInfo(prefix) + dump.UnwindingInfo = append(dump.UnwindingInfo, jr) + default: + // skip unknown record (we have read them) + level.Debug(p.logger).Log("msg", "skipped unknown JIT record", "prefix", prefix) + } + } +} + +// LoadJITDump loads a jitdump file. +func LoadJITDump(logger log.Logger, rd io.Reader) (*JITDump, error) { + parser, err := newParser(logger, rd) + if err != nil { + return nil, fmt.Errorf("failed to instantiate JIT dump parser: %w", err) + } + + dump, err := parser.parse() + if err != nil { + return nil, fmt.Errorf("failed to parse JIT dump: %w", err) + } + + return dump, nil +} diff --git a/pkg/perf/perf.go b/pkg/perf/perf.go index 0cab77280b..7b576dad5b 100644 --- a/pkg/perf/perf.go +++ b/pkg/perf/perf.go @@ -27,6 +27,7 @@ import ( "github.com/go-kit/log" "github.com/parca-dev/parca-agent/pkg/hash" + "github.com/parca-dev/parca-agent/pkg/jit" ) type cache struct { @@ -98,6 +99,33 @@ func ReadMap(fs fs.FS, fileName string) (Map, error) { return Map{addrs: addrs}, s.Err() } +func MapFromDump(logger log.Logger, fs fs.FS, fileName string) (Map, error) { + fd, err := fs.Open(fileName) + if err != nil { + return Map{}, err + } + defer fd.Close() + + dump, err := jit.LoadJITDump(logger, fd) + if err != nil { + return Map{}, err + } + + addrs := make([]MapAddr, len(dump.CodeLoads)) + for i, cl := range dump.CodeLoads { + addrs[i] = MapAddr{cl.CodeAddr, cl.CodeAddr + cl.CodeSize, cl.Name} + } + + // Sorted by end address to allow binary search during look-up. End to find + // the (closest) address _before_ the end. This could be an inlined instruction + // within a larger blob. + sort.Slice(addrs, func(i, j int) bool { + return addrs[i].End < addrs[j].End + }) + + return Map{addrs: addrs}, nil +} + func (p *Map) Lookup(addr uint64) (string, error) { idx := sort.Search(len(p.addrs), func(i int) bool { return addr < p.addrs[i].End @@ -139,20 +167,45 @@ func (p *cache) MapForPID(pid int) (*Map, error) { nsPid = p.nsPID[pid] } - perfFile := fmt.Sprintf("/proc/%d/root/tmp/perf-%d.map", pid, nsPid) - h, err := hash.File(p.fs, perfFile) - if err != nil { - if os.IsNotExist(err) { - return nil, ErrPerfMapNotFound + fileFmts := []string{ + "/proc/%d/root/tmp/perf-%d.map", + "/proc/%d/root/tmp/jit-%d.dump", + "/proc/%d/cwd/jit-%d.dump", + } + + var perfFile string + var h uint64 + for _, f := range fileFmts { + perfFile = fmt.Sprintf(f, pid, nsPid) + var err error + h, err = hash.File(p.fs, perfFile) + if err != nil && !os.IsNotExist(err) { + return nil, err + } + if h != 0 { + break } - return nil, err + } + + if h == 0 { + return nil, ErrPerfMapNotFound } if p.pidMapHash[pid] == h { return p.cache[pid], nil } - m, err := ReadMap(p.fs, perfFile) + var m Map + var err error + switch { + case strings.HasSuffix(perfFile, ".map"): + m, err = ReadMap(p.fs, perfFile) + case strings.HasSuffix(perfFile, ".dump"): + m, err = MapFromDump(p.logger, p.fs, perfFile) + default: + // should never happen + return nil, ErrPerfMapNotFound + } if err != nil { return nil, err } From 92871998c4a43bdccb02630ad1151efbb0b55f9e Mon Sep 17 00:00:00 2001 From: Maxime Brunet Date: Fri, 2 Dec 2022 14:45:28 -0800 Subject: [PATCH 02/20] Add readString() func without use of string.TrimSuffix() --- pkg/jit/jitdump.go | 19 +++++++++++++------ 1 file changed, 13 insertions(+), 6 deletions(-) diff --git a/pkg/jit/jitdump.go b/pkg/jit/jitdump.go index baaf9eff58..e2ff7bc62f 100644 --- a/pkg/jit/jitdump.go +++ b/pkg/jit/jitdump.go @@ -27,7 +27,6 @@ import ( "errors" "fmt" "io" - "strings" "github.com/go-kit/log" "github.com/go-kit/log/level" @@ -171,6 +170,14 @@ func (p *jitDumpParser) read(data any) error { return binary.Read(p.buf, p.endianness, data) } +func (p *jitDumpParser) readString() (string, error) { + s, err := p.buf.ReadString(0) // read the null termination + if err != nil { + return s, err + } + return s[:len(s)-1], nil // trim the null termination +} + func (p *jitDumpParser) parseJITHeader() (*JITHeader, error) { header := &JITHeader{} if err := p.read(header); err != nil { @@ -206,11 +213,11 @@ func (p *jitDumpParser) parseJRCodeLoad(prefix *JRPrefix) *JRCodeLoad { level.Warn(p.logger).Log("msg", "error while reading JIT Code Load CodeIndex", "err", err) } - name, err := p.buf.ReadString(0) // read the null termination + var err error + jr.Name, err = p.readString() if err != nil { level.Warn(p.logger).Log("msg", "error while reading JIT Code Load Name", "err", err) } - jr.Name = strings.TrimSuffix(name, "\x00") // trim the null termination jr.Code = make([]byte, jr.CodeSize) if _, err := io.ReadAtLeast(p.buf, jr.Code, len(jr.Code)); err != nil { @@ -274,12 +281,12 @@ func (p *jitDumpParser) parseJRCodeDebugInfo(prefix *JRPrefix) *JRCodeDebugInfo level.Warn(p.logger).Log("msg", "error while reading Debug Entry Discrim", "err", err) } - name, err := p.buf.ReadString(0) // read until the null termination + var err error + jr.Entries[i].Name, err = p.readString() if err != nil { level.Warn(p.logger).Log("msg", "error while reading Debug Entry Name", "err", err) } - size += len(name) - jr.Entries[i].Name = strings.TrimSuffix(name, "\x00") // trim the null termination + size += len(jr.Entries[i].Name) + 1 // +1 accounts for trimmed null termination } // Discard padding if any From 04a3fda7c90e831e839ba54e2cb57991191567bf Mon Sep 17 00:00:00 2001 From: Maxime Brunet Date: Fri, 2 Dec 2022 15:02:32 -0800 Subject: [PATCH 03/20] Read header and record prefixes field by field --- pkg/jit/jitdump.go | 48 +++++++++++++++++++++++++++++++++++++++++----- 1 file changed, 43 insertions(+), 5 deletions(-) diff --git a/pkg/jit/jitdump.go b/pkg/jit/jitdump.go index e2ff7bc62f..4cbb2f3d10 100644 --- a/pkg/jit/jitdump.go +++ b/pkg/jit/jitdump.go @@ -180,8 +180,30 @@ func (p *jitDumpParser) readString() (string, error) { func (p *jitDumpParser) parseJITHeader() (*JITHeader, error) { header := &JITHeader{} - if err := p.read(header); err != nil { - return nil, fmt.Errorf("failed to read jitdump header: %w", err) + + if err := p.read(&header.Magic); err != nil { + return nil, fmt.Errorf("failed to read jitdump header Magic: %w", err) + } + if err := p.read(&header.Version); err != nil { + return nil, fmt.Errorf("failed to read jitdump header Version: %w", err) + } + if err := p.read(&header.TotalSize); err != nil { + return nil, fmt.Errorf("failed to read jitdump header TotalSize: %w", err) + } + if err := p.read(&header.ElfMach); err != nil { + return nil, fmt.Errorf("failed to read jitdump header ElfMach: %w", err) + } + if err := p.read(&header.Pad1); err != nil { + return nil, fmt.Errorf("failed to read jitdump header Pad1: %w", err) + } + if err := p.read(&header.Pid); err != nil { + return nil, fmt.Errorf("failed to read jitdump header Pid: %w", err) + } + if err := p.read(&header.Timestamp); err != nil { + return nil, fmt.Errorf("failed to read jitdump header Timestamp: %w", err) + } + if err := p.read(&header.Flags); err != nil { + return nil, fmt.Errorf("failed to read jitdump header Flags: %w", err) } if header.Version > JITHeaderVersion { @@ -191,6 +213,22 @@ func (p *jitDumpParser) parseJITHeader() (*JITHeader, error) { return header, nil } +func (p *jitDumpParser) parseJRPrefix() (*JRPrefix, error) { + prefix := &JRPrefix{} + + if err := p.read(&prefix.ID); err != nil { + return nil, fmt.Errorf("failed to read JIT record prefix ID: %w", err) + } + if err := p.read(&prefix.TotalSize); err != nil { + return nil, fmt.Errorf("failed to read JIT record prefix TotalSize: %w", err) + } + if err := p.read(&prefix.Timestamp); err != nil { + return nil, fmt.Errorf("failed to read JIT record prefix Timestamp: %w", err) + } + + return prefix, nil +} + func (p *jitDumpParser) parseJRCodeLoad(prefix *JRPrefix) *JRCodeLoad { jr := &JRCodeLoad{Prefix: prefix} @@ -339,11 +377,11 @@ func (p *jitDumpParser) parse() (*JITDump, error) { dump.UnwindingInfo = make([]*JRCodeUnwindingInfo, 0) for { - prefix := &JRPrefix{} - if err := p.read(prefix); errors.Is(err, io.EOF) { + prefix, err := p.parseJRPrefix() + if errors.Is(err, io.EOF) { return dump, nil } else if err != nil { - return nil, fmt.Errorf("failed to read JIT record prefix: %w", err) + return dump, err } if prefix.ID >= JITCodeMax { From 0bd32b3bde4400f8b77925d42e8f443475d588df Mon Sep 17 00:00:00 2001 From: Maxime Brunet Date: Fri, 2 Dec 2022 16:04:03 -0800 Subject: [PATCH 04/20] Rework error handling --- pkg/jit/jitdump.go | 137 +++++++++++++++++++++++++++------------------ 1 file changed, 81 insertions(+), 56 deletions(-) diff --git a/pkg/jit/jitdump.go b/pkg/jit/jitdump.go index 4cbb2f3d10..3ef94a1e13 100644 --- a/pkg/jit/jitdump.go +++ b/pkg/jit/jitdump.go @@ -32,6 +32,9 @@ import ( "github.com/go-kit/log/level" ) +// ErrWrongJITDumpVersion is the error return when the version in the JITDUMP header is not 1. +var ErrWrongJITDumpVersion = errors.New("wrong JITDUMP version") + // JITHeader represent a jitdump file header. type JITHeader struct { Magic uint32 // a magic number tagging the file type @@ -165,15 +168,25 @@ func newParser(logger log.Logger, rd io.Reader) (*jitDumpParser, error) { return p, nil } +// isUnexpectedIOError ensures all EOF are unexpected EOF errors. +func isUnexpectedIOError(err error) error { + if errors.Is(err, io.EOF) { + return io.ErrUnexpectedEOF + } + return err +} + // read reads structured binary data from the jitdump file into data. func (p *jitDumpParser) read(data any) error { return binary.Read(p.buf, p.endianness, data) } +// readString reads a string (until its null termination) from the jitdump file. func (p *jitDumpParser) readString() (string, error) { s, err := p.buf.ReadString(0) // read the null termination if err != nil { - return s, err + // EOF is always unexpected, strings should always end by a null termination + return s, isUnexpectedIOError(err) } return s[:len(s)-1], nil // trim the null termination } @@ -182,32 +195,32 @@ func (p *jitDumpParser) parseJITHeader() (*JITHeader, error) { header := &JITHeader{} if err := p.read(&header.Magic); err != nil { - return nil, fmt.Errorf("failed to read jitdump header Magic: %w", err) + return nil, fmt.Errorf("magic: %w", err) } if err := p.read(&header.Version); err != nil { - return nil, fmt.Errorf("failed to read jitdump header Version: %w", err) + return nil, fmt.Errorf("version: %w", err) } if err := p.read(&header.TotalSize); err != nil { - return nil, fmt.Errorf("failed to read jitdump header TotalSize: %w", err) + return nil, fmt.Errorf("totalSize: %w", err) } if err := p.read(&header.ElfMach); err != nil { - return nil, fmt.Errorf("failed to read jitdump header ElfMach: %w", err) + return nil, fmt.Errorf("elfMach: %w", err) } if err := p.read(&header.Pad1); err != nil { - return nil, fmt.Errorf("failed to read jitdump header Pad1: %w", err) + return nil, fmt.Errorf("pad1: %w", err) } if err := p.read(&header.Pid); err != nil { - return nil, fmt.Errorf("failed to read jitdump header Pid: %w", err) + return nil, fmt.Errorf("pid: %w", err) } if err := p.read(&header.Timestamp); err != nil { - return nil, fmt.Errorf("failed to read jitdump header Timestamp: %w", err) + return nil, fmt.Errorf("timestamp: %w", err) } if err := p.read(&header.Flags); err != nil { - return nil, fmt.Errorf("failed to read jitdump header Flags: %w", err) + return nil, fmt.Errorf("flags: %w", err) } if header.Version > JITHeaderVersion { - return nil, fmt.Errorf("wrong jitdump version: %d (expected: %d)", header.Version, JITHeaderVersion) + return nil, fmt.Errorf("%w: %d (expected: %d)", ErrWrongJITDumpVersion, header.Version, JITHeaderVersion) } return header, nil @@ -217,90 +230,90 @@ func (p *jitDumpParser) parseJRPrefix() (*JRPrefix, error) { prefix := &JRPrefix{} if err := p.read(&prefix.ID); err != nil { - return nil, fmt.Errorf("failed to read JIT record prefix ID: %w", err) + return nil, fmt.Errorf("id: %w", err) } if err := p.read(&prefix.TotalSize); err != nil { - return nil, fmt.Errorf("failed to read JIT record prefix TotalSize: %w", err) + return nil, fmt.Errorf("totalSize: %w", isUnexpectedIOError(err)) } if err := p.read(&prefix.Timestamp); err != nil { - return nil, fmt.Errorf("failed to read JIT record prefix Timestamp: %w", err) + return nil, fmt.Errorf("timestamp: %w", isUnexpectedIOError(err)) } return prefix, nil } -func (p *jitDumpParser) parseJRCodeLoad(prefix *JRPrefix) *JRCodeLoad { +func (p *jitDumpParser) parseJRCodeLoad(prefix *JRPrefix) (*JRCodeLoad, error) { jr := &JRCodeLoad{Prefix: prefix} if err := p.read(&jr.PID); err != nil { - level.Warn(p.logger).Log("msg", "error while reading JIT Code Load PID", "err", err) + return nil, fmt.Errorf("pid: %w", err) } if err := p.read(&jr.TID); err != nil { - level.Warn(p.logger).Log("msg", "error while reading JIT Code Load TID", "err", err) + return nil, fmt.Errorf("tid: %w", err) } if err := p.read(&jr.VMA); err != nil { - level.Warn(p.logger).Log("msg", "error while reading JIT Code Load VMA", "err", err) + return nil, fmt.Errorf("vma: %w", err) } if err := p.read(&jr.CodeAddr); err != nil { - level.Warn(p.logger).Log("msg", "error while reading JIT Code Load CodeAddr", "err", err) + return nil, fmt.Errorf("codeAddr: %w", err) } if err := p.read(&jr.CodeSize); err != nil { - level.Warn(p.logger).Log("msg", "error while reading JIT Code Load CodeSize", "err", err) + return nil, fmt.Errorf("codeSize: %w", err) } if err := p.read(&jr.CodeIndex); err != nil { - level.Warn(p.logger).Log("msg", "error while reading JIT Code Load CodeIndex", "err", err) + return nil, fmt.Errorf("codeIndex: %w", err) } var err error jr.Name, err = p.readString() if err != nil { - level.Warn(p.logger).Log("msg", "error while reading JIT Code Load Name", "err", err) + return nil, fmt.Errorf("name: %w", err) } jr.Code = make([]byte, jr.CodeSize) if _, err := io.ReadAtLeast(p.buf, jr.Code, len(jr.Code)); err != nil { - level.Warn(p.logger).Log("msg", "error while reading JIT Code Load Code", "err", err) + return nil, fmt.Errorf("code: %w", err) } - return jr + return jr, nil } -func (p *jitDumpParser) parseJRCodeMove(prefix *JRPrefix) *JRCodeMove { +func (p *jitDumpParser) parseJRCodeMove(prefix *JRPrefix) (*JRCodeMove, error) { jr := &JRCodeMove{Prefix: prefix} if err := p.read(&jr.PID); err != nil { - level.Warn(p.logger).Log("msg", "error while reading JIT Code Move PID", "err", err) + return nil, fmt.Errorf("pid: %w", err) } if err := p.read(&jr.TID); err != nil { - level.Warn(p.logger).Log("msg", "error while reading JIT Code Move TID", "err", err) + return nil, fmt.Errorf("tid: %w", err) } if err := p.read(&jr.VMA); err != nil { - level.Warn(p.logger).Log("msg", "error while reading JIT Code Move VMA", "err", err) + return nil, fmt.Errorf("vma: %w", err) } if err := p.read(&jr.OldCodeAddr); err != nil { - level.Warn(p.logger).Log("msg", "error while reading JIT Code Move OldCodeAddr", "err", err) + return nil, fmt.Errorf("oldCodeAddr: %w", err) } if err := p.read(&jr.NewCodeAddr); err != nil { - level.Warn(p.logger).Log("msg", "error while reading JIT Code Move NewCodeAddr", "err", err) + return nil, fmt.Errorf("newCodeAddr: %w", err) } if err := p.read(&jr.CodeSize); err != nil { - level.Warn(p.logger).Log("msg", "error while reading JIT Code Move CodeSize", "err", err) + return nil, fmt.Errorf("codeSize: %w", err) } if err := p.read(&jr.CodeIndex); err != nil { - level.Warn(p.logger).Log("msg", "error while reading JIT Code Move CodeIndex", "err", err) + return nil, fmt.Errorf("codeIndex: %w", err) } - return jr + return jr, nil } -func (p *jitDumpParser) parseJRCodeDebugInfo(prefix *JRPrefix) *JRCodeDebugInfo { +func (p *jitDumpParser) parseJRCodeDebugInfo(prefix *JRPrefix) (*JRCodeDebugInfo, error) { jr := &JRCodeDebugInfo{Prefix: prefix} if err := p.read(&jr.CodeAddr); err != nil { - level.Warn(p.logger).Log("msg", "error while reading JIT Code Debug Info CodeAddr", "err", err) + return nil, fmt.Errorf("codeAddr: %w", err) } if err := p.read(&jr.NREntry); err != nil { - level.Warn(p.logger).Log("msg", "error while reading JIT Code Debug Info NREntry", "err", err) + return nil, fmt.Errorf("nrEntry: %w", err) } size := jrCodeDebugInfoFixedSize + debugEntryFixedSize*int(jr.NREntry) @@ -310,57 +323,57 @@ func (p *jitDumpParser) parseJRCodeDebugInfo(prefix *JRPrefix) *JRCodeDebugInfo jr.Entries[i] = &DebugEntry{} if err := p.read(&jr.Entries[i].Addr); err != nil { - level.Warn(p.logger).Log("msg", "error while reading Debug Entry Addr", "err", err) + return nil, fmt.Errorf("debugEntry addr (index: %d): %w", i, err) } if err := p.read(&jr.Entries[i].Lineno); err != nil { - level.Warn(p.logger).Log("msg", "error while reading Debug Entry Lineno", "err", err) + return nil, fmt.Errorf("debugEntry lineno (index: %d): %w", i, err) } if err := p.read(&jr.Entries[i].Discrim); err != nil { - level.Warn(p.logger).Log("msg", "error while reading Debug Entry Discrim", "err", err) + return nil, fmt.Errorf("debugEntry discrim (index: %d): %w", i, err) } var err error jr.Entries[i].Name, err = p.readString() if err != nil { - level.Warn(p.logger).Log("msg", "error while reading Debug Entry Name", "err", err) + return nil, fmt.Errorf("debugEntry name (index: %d): %w", i, err) } size += len(jr.Entries[i].Name) + 1 // +1 accounts for trimmed null termination } // Discard padding if any if _, err := p.buf.Discard(int(jr.Prefix.TotalSize) - size); err != nil { - level.Warn(p.logger).Log("msg", "failed to discard JIT Code Debug Info record padding", "err", err) + return nil, fmt.Errorf("failed to discard JIT Code Debug Info record padding: %w", err) } - return jr + return jr, nil } -func (p *jitDumpParser) parseJRCodeUnwindingInfo(prefix *JRPrefix) *JRCodeUnwindingInfo { +func (p *jitDumpParser) parseJRCodeUnwindingInfo(prefix *JRPrefix) (*JRCodeUnwindingInfo, error) { jr := &JRCodeUnwindingInfo{Prefix: prefix} jr.Prefix = prefix if err := p.read(&jr.UnwindingSize); err != nil { - level.Warn(p.logger).Log("msg", "error while reading JIT Code Unwinding Info UnwindingSize", "err", err) + return nil, fmt.Errorf("unwindingSize: %w", err) } if err := p.read(&jr.EHFrameHDRSize); err != nil { - level.Warn(p.logger).Log("msg", "error while reading JIT Code Unwinding Info EHFrameHDRSize", "err", err) + return nil, fmt.Errorf("ehFrameHDRSize: %w", err) } if err := p.read(&jr.MappedSize); err != nil { - level.Warn(p.logger).Log("msg", "error while reading JIT Code Unwinding Info MappedSize", "err", err) + return nil, fmt.Errorf("mappedSize: %w", err) } jr.UnwindingData = make([]byte, jr.UnwindingSize) if _, err := io.ReadAtLeast(p.buf, jr.UnwindingData, len(jr.UnwindingData)); err != nil { - level.Warn(p.logger).Log("msg", "error while reading JIT Code Unwinding Info UnwindingData", "err", err) + return nil, fmt.Errorf("unwindingData: %w", err) } // Discard padding if any if _, err := p.buf.Discard(int(jr.Prefix.TotalSize) - jrCodeUnwindingInfoFixedSize - int(jr.UnwindingSize)); err != nil { - level.Warn(p.logger).Log("msg", "failed to discard JIT Code Unwinding Info record padding", "err", err) + return nil, fmt.Errorf("failed to discard JIT Code Unwinding Info record padding: %w", err) } - return jr + return jr, nil } func (p *jitDumpParser) parse() (*JITDump, error) { @@ -368,7 +381,7 @@ func (p *jitDumpParser) parse() (*JITDump, error) { var err error dump.Header, err = p.parseJITHeader() if err != nil { - return nil, fmt.Errorf("failed to read JIT dump header: %w", err) + return nil, fmt.Errorf("failed to read JIT dump header: %w", isUnexpectedIOError(err)) } dump.CodeLoads = make([]*JRCodeLoad, 0) @@ -381,32 +394,44 @@ func (p *jitDumpParser) parse() (*JITDump, error) { if errors.Is(err, io.EOF) { return dump, nil } else if err != nil { - return dump, err + return dump, fmt.Errorf("failed to read JIT Record Prefix: %w", err) } if prefix.ID >= JITCodeMax { level.Warn(p.logger).Log("msg", "unknown JIT record type, skipping", "ID", prefix.ID) if _, err := p.buf.Discard(int(prefix.TotalSize)); err != nil { - level.Warn(p.logger).Log("msg", "failed to discard unknown JIT record", "err", err) + return dump, fmt.Errorf("failed to discard unknown JIT record: %w", err) } continue } switch prefix.ID { case JITCodeLoad: - jr := p.parseJRCodeLoad(prefix) + jr, err := p.parseJRCodeLoad(prefix) + if err != nil { + return dump, fmt.Errorf("failed to read JIT Code Load: %w", isUnexpectedIOError(err)) + } dump.CodeLoads = append(dump.CodeLoads, jr) case JITCodeMove: - jr := p.parseJRCodeMove(prefix) + jr, err := p.parseJRCodeMove(prefix) + if err != nil { + return dump, fmt.Errorf("failed to read JIT Code Move: %w", isUnexpectedIOError(err)) + } dump.CodeMoves = append(dump.CodeMoves, jr) case JITCodeDebugInfo: - jr := p.parseJRCodeDebugInfo(prefix) + jr, err := p.parseJRCodeDebugInfo(prefix) + if err != nil { + return dump, fmt.Errorf("failed to read JIT Code Debug Info: %w", isUnexpectedIOError(err)) + } dump.DebugInfo = append(dump.DebugInfo, jr) case JITCodeClose: level.Debug(p.logger).Log("msg", "reached JIT Code Close record") return dump, nil case JITCodeUnwindingInfo: - jr := p.parseJRCodeUnwindingInfo(prefix) + jr, err := p.parseJRCodeUnwindingInfo(prefix) + if err != nil { + return dump, fmt.Errorf("failed to read JIT Code Unwinding Info: %w", isUnexpectedIOError(err)) + } dump.UnwindingInfo = append(dump.UnwindingInfo, jr) default: // skip unknown record (we have read them) @@ -424,7 +449,7 @@ func LoadJITDump(logger log.Logger, rd io.Reader) (*JITDump, error) { dump, err := parser.parse() if err != nil { - return nil, fmt.Errorf("failed to parse JIT dump: %w", err) + return dump, fmt.Errorf("failed to parse JIT dump: %w", err) } return dump, nil From 30e0edda8dcff2fd0edbf7154f9ec39305934aed Mon Sep 17 00:00:00 2001 From: Maxime Brunet Date: Fri, 2 Dec 2022 16:33:30 -0800 Subject: [PATCH 05/20] Iron out a few allocation issues in perf map integration --- pkg/perf/perf.go | 39 +++++++++++++++++++++++++-------------- 1 file changed, 25 insertions(+), 14 deletions(-) diff --git a/pkg/perf/perf.go b/pkg/perf/perf.go index 7b576dad5b..d8d217d3d5 100644 --- a/pkg/perf/perf.go +++ b/pkg/perf/perf.go @@ -18,6 +18,7 @@ import ( "bufio" "errors" "fmt" + "io" "io/fs" "os" "sort" @@ -25,6 +26,7 @@ import ( "strings" "github.com/go-kit/log" + "github.com/go-kit/log/level" "github.com/parca-dev/parca-agent/pkg/hash" "github.com/parca-dev/parca-agent/pkg/jit" @@ -51,11 +53,18 @@ type Map struct { type realfs struct{} var ( - ErrPerfMapNotFound = errors.New("perf-map not found") - ErrProcStatusNotFound = errors.New("process status not found") - ErrNoSymbolFound = errors.New("no symbol found") + ErrPerfMapNotFound = errors.New("perf-map not found") + ErrProcStatusNotFound = errors.New("process status not found") + ErrNoSymbolFound = errors.New("no symbol found") + ErrJITDumpCodeLoadsNil = errors.New("code load records list not initialized in JIT dump") ) +var perfFileFmts = []string{ + "/proc/%d/root/tmp/perf-%d.map", + "/proc/%d/root/tmp/jit-%d.dump", + "/proc/%d/cwd/jit-%d.dump", +} + func (f *realfs) Open(name string) (fs.File, error) { return os.Open(name) } @@ -107,13 +116,21 @@ func MapFromDump(logger log.Logger, fs fs.FS, fileName string) (Map, error) { defer fd.Close() dump, err := jit.LoadJITDump(logger, fd) - if err != nil { + if errors.Is(err, io.ErrUnexpectedEOF) { + // Some runtimes update their dump all the time (e.g. libperf_jvmti.so), + // making it nearly impossible to read a complete file + level.Warn(logger).Log("msg", "JIT dump file ended unexpectedly", "err", err) + if dump.CodeLoads == nil { + // It happens only if EOF was reached while reading the JIT dump header + return Map{}, ErrJITDumpCodeLoadsNil + } + } else if err != nil { return Map{}, err } - addrs := make([]MapAddr, len(dump.CodeLoads)) - for i, cl := range dump.CodeLoads { - addrs[i] = MapAddr{cl.CodeAddr, cl.CodeAddr + cl.CodeSize, cl.Name} + addrs := make([]MapAddr, 0, len(dump.CodeLoads)) + for _, cl := range dump.CodeLoads { + addrs = append(addrs, MapAddr{cl.CodeAddr, cl.CodeAddr + cl.CodeSize, cl.Name}) } // Sorted by end address to allow binary search during look-up. End to find @@ -167,15 +184,9 @@ func (p *cache) MapForPID(pid int) (*Map, error) { nsPid = p.nsPID[pid] } - fileFmts := []string{ - "/proc/%d/root/tmp/perf-%d.map", - "/proc/%d/root/tmp/jit-%d.dump", - "/proc/%d/cwd/jit-%d.dump", - } - var perfFile string var h uint64 - for _, f := range fileFmts { + for _, f := range perfFileFmts { perfFile = fmt.Sprintf(f, pid, nsPid) var err error h, err = hash.File(p.fs, perfFile) From 51d1862c79789e22cc175d1f99a1134581d821ad Mon Sep 17 00:00:00 2001 From: Maxime Brunet Date: Sat, 3 Dec 2022 09:28:44 -0800 Subject: [PATCH 06/20] Ensure partial dump is usable --- pkg/jit/jitdump.go | 2 +- pkg/perf/perf.go | 14 ++++++-------- 2 files changed, 7 insertions(+), 9 deletions(-) diff --git a/pkg/jit/jitdump.go b/pkg/jit/jitdump.go index 3ef94a1e13..b7afa58c6b 100644 --- a/pkg/jit/jitdump.go +++ b/pkg/jit/jitdump.go @@ -400,7 +400,7 @@ func (p *jitDumpParser) parse() (*JITDump, error) { if prefix.ID >= JITCodeMax { level.Warn(p.logger).Log("msg", "unknown JIT record type, skipping", "ID", prefix.ID) if _, err := p.buf.Discard(int(prefix.TotalSize)); err != nil { - return dump, fmt.Errorf("failed to discard unknown JIT record: %w", err) + return dump, fmt.Errorf("failed to discard unknown JIT record: %w", isUnexpectedIOError(err)) } continue } diff --git a/pkg/perf/perf.go b/pkg/perf/perf.go index d8d217d3d5..f8d3ee95d1 100644 --- a/pkg/perf/perf.go +++ b/pkg/perf/perf.go @@ -53,10 +53,9 @@ type Map struct { type realfs struct{} var ( - ErrPerfMapNotFound = errors.New("perf-map not found") - ErrProcStatusNotFound = errors.New("process status not found") - ErrNoSymbolFound = errors.New("no symbol found") - ErrJITDumpCodeLoadsNil = errors.New("code load records list not initialized in JIT dump") + ErrPerfMapNotFound = errors.New("perf-map not found") + ErrProcStatusNotFound = errors.New("process status not found") + ErrNoSymbolFound = errors.New("no symbol found") ) var perfFileFmts = []string{ @@ -117,13 +116,12 @@ func MapFromDump(logger log.Logger, fs fs.FS, fileName string) (Map, error) { dump, err := jit.LoadJITDump(logger, fd) if errors.Is(err, io.ErrUnexpectedEOF) { + if dump == nil || dump.CodeLoads == nil { + return Map{}, err + } // Some runtimes update their dump all the time (e.g. libperf_jvmti.so), // making it nearly impossible to read a complete file level.Warn(logger).Log("msg", "JIT dump file ended unexpectedly", "err", err) - if dump.CodeLoads == nil { - // It happens only if EOF was reached while reading the JIT dump header - return Map{}, ErrJITDumpCodeLoadsNil - } } else if err != nil { return Map{}, err } From 1cd4d6e3402b73757a0fbdb4aaeed53046766e14 Mon Sep 17 00:00:00 2001 From: Maxime Brunet Date: Sat, 3 Dec 2022 13:59:07 -0800 Subject: [PATCH 07/20] Lookup jitdump in proc maps --- pkg/perf/perf.go | 66 ++++++++++++++++++++++++++++---------------- pkg/symbol/symbol.go | 2 +- 2 files changed, 43 insertions(+), 25 deletions(-) diff --git a/pkg/perf/perf.go b/pkg/perf/perf.go index f8d3ee95d1..e84b397c9e 100644 --- a/pkg/perf/perf.go +++ b/pkg/perf/perf.go @@ -27,6 +27,7 @@ import ( "github.com/go-kit/log" "github.com/go-kit/log/level" + "github.com/prometheus/procfs" "github.com/parca-dev/parca-agent/pkg/hash" "github.com/parca-dev/parca-agent/pkg/jit" @@ -53,17 +54,11 @@ type Map struct { type realfs struct{} var ( - ErrPerfMapNotFound = errors.New("perf-map not found") - ErrProcStatusNotFound = errors.New("process status not found") - ErrNoSymbolFound = errors.New("no symbol found") + ErrPerfMapNotFound = errors.New("perf-map not found") + ErrProcNotFound = errors.New("process not found") + ErrNoSymbolFound = errors.New("no symbol found") ) -var perfFileFmts = []string{ - "/proc/%d/root/tmp/perf-%d.map", - "/proc/%d/root/tmp/jit-%d.dump", - "/proc/%d/cwd/jit-%d.dump", -} - func (f *realfs) Open(name string) (fs.File, error) { return os.Open(name) } @@ -173,7 +168,7 @@ func (p *cache) MapForPID(pid int) (*Map, error) { nsPids, err := findNSPIDs(p.fs, pid) if err != nil { if os.IsNotExist(err) { - return nil, ErrProcStatusNotFound + return nil, fmt.Errorf("%w when reading status", ErrProcNotFound) } return nil, err } @@ -182,22 +177,25 @@ func (p *cache) MapForPID(pid int) (*Map, error) { nsPid = p.nsPID[pid] } - var perfFile string - var h uint64 - for _, f := range perfFileFmts { - perfFile = fmt.Sprintf(f, pid, nsPid) - var err error - h, err = hash.File(p.fs, perfFile) - if err != nil && !os.IsNotExist(err) { + perfFile := fmt.Sprintf("/proc/%d/root/tmp/perf-%d.map", pid, nsPid) + h, err := hash.File(p.fs, perfFile) + if os.IsNotExist(err) { + perfFile, err = findJITDump(pid, nsPid) + if err != nil { + if os.IsNotExist(err) { + return nil, fmt.Errorf("%w when searching for JITDUMP", ErrProcNotFound) + } return nil, err } - if h != 0 { - break + h, err = hash.File(p.fs, perfFile) + if err != nil { + if os.IsNotExist(err) { + return nil, ErrPerfMapNotFound + } + return nil, err } - } - - if h == 0 { - return nil, ErrPerfMapNotFound + } else if err != nil { + return nil, err } if p.pidMapHash[pid] == h { @@ -205,7 +203,6 @@ func (p *cache) MapForPID(pid int) (*Map, error) { } var m Map - var err error switch { case strings.HasSuffix(perfFile, ".map"): m, err = ReadMap(p.fs, perfFile) @@ -269,3 +266,24 @@ func extractPIDsFromLine(line string) ([]int, error) { return pids, nil } + +func findJITDump(pid, nsPid int) (string, error) { + proc, err := procfs.NewProc(pid) + if err != nil { + return "", fmt.Errorf("failed to instantiate process: %w", err) + } + + procMaps, err := proc.ProcMaps() + if err != nil { + return "", fmt.Errorf("failed to read process maps: %w", err) + } + + jitDumpName := fmt.Sprintf("/jit-%d.dump", nsPid) + for _, m := range procMaps { + if strings.HasSuffix(m.Pathname, jitDumpName) { + return fmt.Sprintf("/proc/%d/root%s", pid, m.Pathname), nil + } + } + + return "", nil +} diff --git a/pkg/symbol/symbol.go b/pkg/symbol/symbol.go index e471cd0517..bfbed331df 100644 --- a/pkg/symbol/symbol.go +++ b/pkg/symbol/symbol.go @@ -69,7 +69,7 @@ func (s *Symbolizer) Symbolize(prof *profiler.Profile) error { if err != nil { // Often some processes exit before symbols can be looked up. // We also expect only a minority of processes to have a JIT and produce the perf map. - if errors.Is(err, perf.ErrProcStatusNotFound) || errors.Is(err, perf.ErrPerfMapNotFound) { + if errors.Is(err, perf.ErrProcNotFound) || errors.Is(err, perf.ErrPerfMapNotFound) { return nil } result = multierror.Append(result, fmt.Errorf("failed to resolve user JITed functions: %w", err)) From 19952129eec5085c4206a1249120ccf3ecad22bb Mon Sep 17 00:00:00 2001 From: Maxime Brunet Date: Sat, 3 Dec 2022 15:39:32 -0800 Subject: [PATCH 08/20] add filename to warning --- pkg/perf/perf.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/perf/perf.go b/pkg/perf/perf.go index e84b397c9e..43f07414ee 100644 --- a/pkg/perf/perf.go +++ b/pkg/perf/perf.go @@ -116,7 +116,7 @@ func MapFromDump(logger log.Logger, fs fs.FS, fileName string) (Map, error) { } // Some runtimes update their dump all the time (e.g. libperf_jvmti.so), // making it nearly impossible to read a complete file - level.Warn(logger).Log("msg", "JIT dump file ended unexpectedly", "err", err) + level.Warn(logger).Log("msg", "JIT dump file ended unexpectedly", "filename", fileName, "err", err) } else if err != nil { return Map{}, err } From 0cd9aaa6401090e857955417e2834d57fd7741de Mon Sep 17 00:00:00 2001 From: Maxime Brunet Date: Sat, 3 Dec 2022 20:54:57 -0800 Subject: [PATCH 09/20] Return ErrPerfMapNotFound --- pkg/perf/perf.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/perf/perf.go b/pkg/perf/perf.go index 43f07414ee..f31fcdc076 100644 --- a/pkg/perf/perf.go +++ b/pkg/perf/perf.go @@ -285,5 +285,5 @@ func findJITDump(pid, nsPid int) (string, error) { } } - return "", nil + return "", ErrPerfMapNotFound } From b03a713eaf70102783b512e197e924052db60e18 Mon Sep 17 00:00:00 2001 From: Maxime Brunet Date: Sun, 4 Dec 2022 11:04:24 -0800 Subject: [PATCH 10/20] Improve debug entry error format --- pkg/jit/jitdump.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/pkg/jit/jitdump.go b/pkg/jit/jitdump.go index b7afa58c6b..cdddb5c260 100644 --- a/pkg/jit/jitdump.go +++ b/pkg/jit/jitdump.go @@ -323,19 +323,19 @@ func (p *jitDumpParser) parseJRCodeDebugInfo(prefix *JRPrefix) (*JRCodeDebugInfo jr.Entries[i] = &DebugEntry{} if err := p.read(&jr.Entries[i].Addr); err != nil { - return nil, fmt.Errorf("debugEntry addr (index: %d): %w", i, err) + return nil, fmt.Errorf("entries[%d].addr: %w", i, err) } if err := p.read(&jr.Entries[i].Lineno); err != nil { - return nil, fmt.Errorf("debugEntry lineno (index: %d): %w", i, err) + return nil, fmt.Errorf("entries[%d].lineno: %w", i, err) } if err := p.read(&jr.Entries[i].Discrim); err != nil { - return nil, fmt.Errorf("debugEntry discrim (index: %d): %w", i, err) + return nil, fmt.Errorf("entries[%d].discrim: %w", i, err) } var err error jr.Entries[i].Name, err = p.readString() if err != nil { - return nil, fmt.Errorf("debugEntry name (index: %d): %w", i, err) + return nil, fmt.Errorf("entries[%d].name: %w", i, err) } size += len(jr.Entries[i].Name) + 1 // +1 accounts for trimmed null termination } From b1debb450fa9ee636bbb06edc3cfdc7f3d76a835 Mon Sep 17 00:00:00 2001 From: Maxime Brunet Date: Tue, 6 Dec 2022 20:55:44 -0800 Subject: [PATCH 11/20] Add ErrWrongJITDumpMagic error --- pkg/jit/jitdump.go | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/pkg/jit/jitdump.go b/pkg/jit/jitdump.go index cdddb5c260..a1f8f53611 100644 --- a/pkg/jit/jitdump.go +++ b/pkg/jit/jitdump.go @@ -32,8 +32,12 @@ import ( "github.com/go-kit/log/level" ) -// ErrWrongJITDumpVersion is the error return when the version in the JITDUMP header is not 1. -var ErrWrongJITDumpVersion = errors.New("wrong JITDUMP version") +var ( + // ErrWrongJITDumpVersion is the error returned when the version in the JITDUMP header is not 1. + ErrWrongJITDumpVersion = errors.New("wrong JITDUMP version") + // ErrWrongJITDumpMagic is the error returned when the magic in the JITDUMP header is not recognized. + ErrWrongJITDumpMagic = errors.New("wrong JITDUMP magic") +) // JITHeader represent a jitdump file header. type JITHeader struct { @@ -162,7 +166,7 @@ func newParser(logger log.Logger, rd io.Reader) (*jitDumpParser, error) { case bytes.Equal(magic, []byte{'D', 'T', 'i', 'J'}): p.endianness = binary.LittleEndian default: - return nil, fmt.Errorf("failed to detect JIT dump endianness from %#x magic number: %w", magic, err) + return nil, fmt.Errorf("%w: %#x", err, magic) } return p, nil From 84734e5fc6ade7a041c709b00c5f2402ba25e00a Mon Sep 17 00:00:00 2001 From: Maxime Brunet Date: Fri, 9 Dec 2022 10:30:19 -0800 Subject: [PATCH 12/20] alpha order for errors --- pkg/jit/jitdump.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/pkg/jit/jitdump.go b/pkg/jit/jitdump.go index a1f8f53611..baad6ecf97 100644 --- a/pkg/jit/jitdump.go +++ b/pkg/jit/jitdump.go @@ -33,10 +33,10 @@ import ( ) var ( - // ErrWrongJITDumpVersion is the error returned when the version in the JITDUMP header is not 1. - ErrWrongJITDumpVersion = errors.New("wrong JITDUMP version") // ErrWrongJITDumpMagic is the error returned when the magic in the JITDUMP header is not recognized. ErrWrongJITDumpMagic = errors.New("wrong JITDUMP magic") + // ErrWrongJITDumpVersion is the error returned when the version in the JITDUMP header is not 1. + ErrWrongJITDumpVersion = errors.New("wrong JITDUMP version") ) // JITHeader represent a jitdump file header. From 5e5d9b150e35fc14042c047870a360b40129b158 Mon Sep 17 00:00:00 2001 From: Maxime Brunet Date: Fri, 9 Dec 2022 10:31:14 -0800 Subject: [PATCH 13/20] Add tests --- pkg/jit/jitdump_test.go | 143 ++++++++++++++++++++++++++++++++++++++++ testdata | 2 +- 2 files changed, 144 insertions(+), 1 deletion(-) create mode 100644 pkg/jit/jitdump_test.go diff --git a/pkg/jit/jitdump_test.go b/pkg/jit/jitdump_test.go new file mode 100644 index 0000000000..1d6de5e511 --- /dev/null +++ b/pkg/jit/jitdump_test.go @@ -0,0 +1,143 @@ +// Copyright 2022 The Parca Authors +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package jit_test + +import ( + "bytes" + "encoding/json" + "fmt" + "io" + "os" + "testing" + + "github.com/go-kit/log" + "github.com/stretchr/testify/require" + + "github.com/parca-dev/parca-agent/pkg/jit" +) + +const testdataDir = "../../testdata/jitdump" + +func getCases() []struct { + runtime string + err error +} { + return []struct { + runtime string + err error + }{ + { + runtime: "dotnet", + err: nil, + }, + { + runtime: "erlang", + err: nil, + }, + { + runtime: "julia", + err: nil, + }, + { + runtime: "libperf-jvmti", + // JVM updates its JITted code all the time, + // it is close to impossible to catch a full dump. + err: io.ErrUnexpectedEOF, + }, + { + runtime: "nodejs", + err: nil, + }, + { + runtime: "php", + err: nil, + }, + { + runtime: "wasmtime", + err: nil, + }, + } +} + +func TestLoadJITDump(t *testing.T) { + logger := log.NewNopLogger() + + for _, tt := range getCases() { + t.Run(tt.runtime, func(t *testing.T) { + fixture := fmt.Sprintf("%s/%s.dump", testdataDir, tt.runtime) + snapshot := fmt.Sprintf("%s/%s.json", testdataDir, tt.runtime) + + // Read fixture + f, err := os.Open(fixture) + require.NoError(t, err) + defer f.Close() + + // Load JITDUMP from fixture + dump, err := jit.LoadJITDump(logger, f) + require.ErrorIs(t, err, tt.err) + + // Encode JITDUMP to JSON + buf := &bytes.Buffer{} + enc := json.NewEncoder(buf) + enc.SetEscapeHTML(false) + enc.SetIndent("", " ") + err = enc.Encode(dump) + require.NoError(t, err) + actual := buf.Bytes() + + // Read JSON snapshot + expected, err := os.ReadFile(snapshot) + if err != nil { + if !os.IsNotExist(err) { + require.NoError(t, err) + } + os.WriteFile(snapshot, actual, 0o600) + expected = []byte{} + } + + // Register cleanup function if snapshot needs update + t.Cleanup(func() { + if t.Failed() && os.Getenv("SNAPSHOT") == "overwrite" { + os.WriteFile(snapshot, actual, 0o600) + } + }) + + // Compare actual and expected dumps + require.Equal(t, string(expected), string(actual), "To update the snapshot, run `SNAPSHOT=overwrite go test ./pkg/jit`") + }) + } +} + +func BenchmarkLoadJITDump(b *testing.B) { + b.ReportAllocs() + + logger := log.NewNopLogger() + + for _, bb := range getCases() { + b.Run(bb.runtime, func(b *testing.B) { + for i := 0; i < b.N; i++ { + fixture := fmt.Sprintf("%s/%s.dump", testdataDir, bb.runtime) + + // Read fixture + f, err := os.Open(fixture) + require.NoError(b, err) + defer f.Close() + + // Load JITDUMP from fixture + _, err = jit.LoadJITDump(logger, f) + require.ErrorIs(b, err, bb.err) + } + }) + } +} diff --git a/testdata b/testdata index 7f9206570a..2643c04d47 160000 --- a/testdata +++ b/testdata @@ -1 +1 @@ -Subproject commit 7f9206570a743206fdb6190d6240ced355b9e523 +Subproject commit 2643c04d47f97b902bd4dfaf87e67043b12388bc From d26079ce862a642232a8e97954ff010316067d0a Mon Sep 17 00:00:00 2001 From: Maxime Brunet Date: Sat, 10 Dec 2022 10:37:55 -0800 Subject: [PATCH 14/20] Run tests in parallel --- pkg/jit/jitdump_test.go | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/pkg/jit/jitdump_test.go b/pkg/jit/jitdump_test.go index 1d6de5e511..2800fcff8c 100644 --- a/pkg/jit/jitdump_test.go +++ b/pkg/jit/jitdump_test.go @@ -71,10 +71,14 @@ func getCases() []struct { } func TestLoadJITDump(t *testing.T) { + t.Parallel() + logger := log.NewNopLogger() for _, tt := range getCases() { t.Run(tt.runtime, func(t *testing.T) { + t.Parallel() + fixture := fmt.Sprintf("%s/%s.dump", testdataDir, tt.runtime) snapshot := fmt.Sprintf("%s/%s.json", testdataDir, tt.runtime) From f97c569cc1a2a6a4b023aee8d18e7115376c315d Mon Sep 17 00:00:00 2001 From: "pre-commit-ci-lite[bot]" <117423508+pre-commit-ci-lite[bot]@users.noreply.github.com> Date: Wed, 1 Feb 2023 17:00:45 +0000 Subject: [PATCH 15/20] [pre-commit.ci lite] apply automatic fixes --- pkg/jit/jitdump.go | 2 +- pkg/jit/jitdump_test.go | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/pkg/jit/jitdump.go b/pkg/jit/jitdump.go index baad6ecf97..69c4476e7c 100644 --- a/pkg/jit/jitdump.go +++ b/pkg/jit/jitdump.go @@ -1,4 +1,4 @@ -// Copyright 2022 The Parca Authors +// Copyright 2022-2023 The Parca Authors // Licensed under the Apache License, Version 2.0 (the "License"); // you may not use this file except in compliance with the License. // You may obtain a copy of the License at diff --git a/pkg/jit/jitdump_test.go b/pkg/jit/jitdump_test.go index 2800fcff8c..97031592e5 100644 --- a/pkg/jit/jitdump_test.go +++ b/pkg/jit/jitdump_test.go @@ -1,4 +1,4 @@ -// Copyright 2022 The Parca Authors +// Copyright 2022-2023 The Parca Authors // Licensed under the Apache License, Version 2.0 (the "License"); // you may not use this file except in compliance with the License. // You may obtain a copy of the License at From 6052f43e6bdb922fcf5685523baa3efb8236c48b Mon Sep 17 00:00:00 2001 From: Maxime Brunet Date: Thu, 2 Feb 2023 09:51:24 -0800 Subject: [PATCH 16/20] Fix ErrWrongJITDumpMagic --- pkg/jit/jitdump.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/jit/jitdump.go b/pkg/jit/jitdump.go index 69c4476e7c..fc452c068f 100644 --- a/pkg/jit/jitdump.go +++ b/pkg/jit/jitdump.go @@ -166,7 +166,7 @@ func newParser(logger log.Logger, rd io.Reader) (*jitDumpParser, error) { case bytes.Equal(magic, []byte{'D', 'T', 'i', 'J'}): p.endianness = binary.LittleEndian default: - return nil, fmt.Errorf("%w: %#x", err, magic) + return nil, fmt.Errorf("%w: %#x", ErrWrongJITDumpMagic, magic) } return p, nil From 65c293ac73b2e4e1f746e72470e3b9d81e181498 Mon Sep 17 00:00:00 2001 From: Maxime Brunet Date: Thu, 2 Feb 2023 10:18:21 -0800 Subject: [PATCH 17/20] Fix lint checks --- pkg/jit/jitdump.go | 1 + pkg/jit/jitdump_test.go | 9 +++++---- 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/pkg/jit/jitdump.go b/pkg/jit/jitdump.go index fc452c068f..061206fecc 100644 --- a/pkg/jit/jitdump.go +++ b/pkg/jit/jitdump.go @@ -131,6 +131,7 @@ type JRCodeUnwindingInfo struct { const jrCodeUnwindingInfoFixedSize int = jrPrefixSize + 24 // size of JRCodeUnwindingInfo fixed-sized fields +// nolint: musttag // JSON is used for testing only // JITDump represents the loaded jitdump. type JITDump struct { Header *JITHeader // the jitdump file header diff --git a/pkg/jit/jitdump_test.go b/pkg/jit/jitdump_test.go index 97031592e5..bc70bc4076 100644 --- a/pkg/jit/jitdump_test.go +++ b/pkg/jit/jitdump_test.go @@ -76,11 +76,12 @@ func TestLoadJITDump(t *testing.T) { logger := log.NewNopLogger() for _, tt := range getCases() { - t.Run(tt.runtime, func(t *testing.T) { + tc := tt + t.Run(tc.runtime, func(t *testing.T) { t.Parallel() - fixture := fmt.Sprintf("%s/%s.dump", testdataDir, tt.runtime) - snapshot := fmt.Sprintf("%s/%s.json", testdataDir, tt.runtime) + fixture := fmt.Sprintf("%s/%s.dump", testdataDir, tc.runtime) + snapshot := fmt.Sprintf("%s/%s.json", testdataDir, tc.runtime) // Read fixture f, err := os.Open(fixture) @@ -89,7 +90,7 @@ func TestLoadJITDump(t *testing.T) { // Load JITDUMP from fixture dump, err := jit.LoadJITDump(logger, f) - require.ErrorIs(t, err, tt.err) + require.ErrorIs(t, err, tc.err) // Encode JITDUMP to JSON buf := &bytes.Buffer{} From 5e213c54460903c79f4a6111921f9c746b082359 Mon Sep 17 00:00:00 2001 From: Maxime Brunet Date: Thu, 2 Feb 2023 15:21:33 -0800 Subject: [PATCH 18/20] Use ByteOrder.Uint*() to read uint* fields --- pkg/jit/jitdump.go | 100 ++++++++++++++++++++++++++------------------- 1 file changed, 59 insertions(+), 41 deletions(-) diff --git a/pkg/jit/jitdump.go b/pkg/jit/jitdump.go index 061206fecc..177349d576 100644 --- a/pkg/jit/jitdump.go +++ b/pkg/jit/jitdump.go @@ -143,15 +143,20 @@ type JITDump struct { // jitDumpParser is used to parse a jitdump file. type jitDumpParser struct { - logger log.Logger - buf *bufio.Reader - endianness binary.ByteOrder + logger log.Logger // logger + buf *bufio.Reader // JITDUMP buffered io.Reader + endianness binary.ByteOrder // JITDUMP byte order + + bUint32 []byte // read buffer for uint32 + bUint64 []byte // read buffer for uint64 } // newParser initializes a jitDumpParser. func newParser(logger log.Logger, rd io.Reader) (*jitDumpParser, error) { p := &jitDumpParser{ - logger: logger, + logger: logger, + bUint32: make([]byte, 4), + bUint64: make([]byte, 8), } p.buf = bufio.NewReader(rd) @@ -181,9 +186,22 @@ func isUnexpectedIOError(err error) error { return err } -// read reads structured binary data from the jitdump file into data. -func (p *jitDumpParser) read(data any) error { - return binary.Read(p.buf, p.endianness, data) +// readUint32 reads a uint32 from the jitdump file into n. +func (p *jitDumpParser) readUint32(n *uint32) error { + if _, err := io.ReadFull(p.buf, p.bUint32); err != nil { + return err + } + *n = p.endianness.Uint32(p.bUint32) + return nil +} + +// readUint64 reads a uint64 from the jitdump file into n. +func (p *jitDumpParser) readUint64(n *uint64) error { + if _, err := io.ReadFull(p.buf, p.bUint64); err != nil { + return err + } + *n = p.endianness.Uint64(p.bUint64) + return nil } // readString reads a string (until its null termination) from the jitdump file. @@ -199,28 +217,28 @@ func (p *jitDumpParser) readString() (string, error) { func (p *jitDumpParser) parseJITHeader() (*JITHeader, error) { header := &JITHeader{} - if err := p.read(&header.Magic); err != nil { + if err := p.readUint32(&header.Magic); err != nil { return nil, fmt.Errorf("magic: %w", err) } - if err := p.read(&header.Version); err != nil { + if err := p.readUint32(&header.Version); err != nil { return nil, fmt.Errorf("version: %w", err) } - if err := p.read(&header.TotalSize); err != nil { + if err := p.readUint32(&header.TotalSize); err != nil { return nil, fmt.Errorf("totalSize: %w", err) } - if err := p.read(&header.ElfMach); err != nil { + if err := p.readUint32(&header.ElfMach); err != nil { return nil, fmt.Errorf("elfMach: %w", err) } - if err := p.read(&header.Pad1); err != nil { + if err := p.readUint32(&header.Pad1); err != nil { return nil, fmt.Errorf("pad1: %w", err) } - if err := p.read(&header.Pid); err != nil { + if err := p.readUint32(&header.Pid); err != nil { return nil, fmt.Errorf("pid: %w", err) } - if err := p.read(&header.Timestamp); err != nil { + if err := p.readUint64(&header.Timestamp); err != nil { return nil, fmt.Errorf("timestamp: %w", err) } - if err := p.read(&header.Flags); err != nil { + if err := p.readUint64(&header.Flags); err != nil { return nil, fmt.Errorf("flags: %w", err) } @@ -234,13 +252,13 @@ func (p *jitDumpParser) parseJITHeader() (*JITHeader, error) { func (p *jitDumpParser) parseJRPrefix() (*JRPrefix, error) { prefix := &JRPrefix{} - if err := p.read(&prefix.ID); err != nil { + if err := p.readUint32((*uint32)(&prefix.ID)); err != nil { return nil, fmt.Errorf("id: %w", err) } - if err := p.read(&prefix.TotalSize); err != nil { + if err := p.readUint32(&prefix.TotalSize); err != nil { return nil, fmt.Errorf("totalSize: %w", isUnexpectedIOError(err)) } - if err := p.read(&prefix.Timestamp); err != nil { + if err := p.readUint64(&prefix.Timestamp); err != nil { return nil, fmt.Errorf("timestamp: %w", isUnexpectedIOError(err)) } @@ -250,22 +268,22 @@ func (p *jitDumpParser) parseJRPrefix() (*JRPrefix, error) { func (p *jitDumpParser) parseJRCodeLoad(prefix *JRPrefix) (*JRCodeLoad, error) { jr := &JRCodeLoad{Prefix: prefix} - if err := p.read(&jr.PID); err != nil { + if err := p.readUint32(&jr.PID); err != nil { return nil, fmt.Errorf("pid: %w", err) } - if err := p.read(&jr.TID); err != nil { + if err := p.readUint32(&jr.TID); err != nil { return nil, fmt.Errorf("tid: %w", err) } - if err := p.read(&jr.VMA); err != nil { + if err := p.readUint64(&jr.VMA); err != nil { return nil, fmt.Errorf("vma: %w", err) } - if err := p.read(&jr.CodeAddr); err != nil { + if err := p.readUint64(&jr.CodeAddr); err != nil { return nil, fmt.Errorf("codeAddr: %w", err) } - if err := p.read(&jr.CodeSize); err != nil { + if err := p.readUint64(&jr.CodeSize); err != nil { return nil, fmt.Errorf("codeSize: %w", err) } - if err := p.read(&jr.CodeIndex); err != nil { + if err := p.readUint64(&jr.CodeIndex); err != nil { return nil, fmt.Errorf("codeIndex: %w", err) } @@ -276,7 +294,7 @@ func (p *jitDumpParser) parseJRCodeLoad(prefix *JRPrefix) (*JRCodeLoad, error) { } jr.Code = make([]byte, jr.CodeSize) - if _, err := io.ReadAtLeast(p.buf, jr.Code, len(jr.Code)); err != nil { + if _, err := io.ReadFull(p.buf, jr.Code); err != nil { return nil, fmt.Errorf("code: %w", err) } @@ -286,25 +304,25 @@ func (p *jitDumpParser) parseJRCodeLoad(prefix *JRPrefix) (*JRCodeLoad, error) { func (p *jitDumpParser) parseJRCodeMove(prefix *JRPrefix) (*JRCodeMove, error) { jr := &JRCodeMove{Prefix: prefix} - if err := p.read(&jr.PID); err != nil { + if err := p.readUint32(&jr.PID); err != nil { return nil, fmt.Errorf("pid: %w", err) } - if err := p.read(&jr.TID); err != nil { + if err := p.readUint32(&jr.TID); err != nil { return nil, fmt.Errorf("tid: %w", err) } - if err := p.read(&jr.VMA); err != nil { + if err := p.readUint64(&jr.VMA); err != nil { return nil, fmt.Errorf("vma: %w", err) } - if err := p.read(&jr.OldCodeAddr); err != nil { + if err := p.readUint64(&jr.OldCodeAddr); err != nil { return nil, fmt.Errorf("oldCodeAddr: %w", err) } - if err := p.read(&jr.NewCodeAddr); err != nil { + if err := p.readUint64(&jr.NewCodeAddr); err != nil { return nil, fmt.Errorf("newCodeAddr: %w", err) } - if err := p.read(&jr.CodeSize); err != nil { + if err := p.readUint64(&jr.CodeSize); err != nil { return nil, fmt.Errorf("codeSize: %w", err) } - if err := p.read(&jr.CodeIndex); err != nil { + if err := p.readUint64(&jr.CodeIndex); err != nil { return nil, fmt.Errorf("codeIndex: %w", err) } @@ -314,10 +332,10 @@ func (p *jitDumpParser) parseJRCodeMove(prefix *JRPrefix) (*JRCodeMove, error) { func (p *jitDumpParser) parseJRCodeDebugInfo(prefix *JRPrefix) (*JRCodeDebugInfo, error) { jr := &JRCodeDebugInfo{Prefix: prefix} - if err := p.read(&jr.CodeAddr); err != nil { + if err := p.readUint64(&jr.CodeAddr); err != nil { return nil, fmt.Errorf("codeAddr: %w", err) } - if err := p.read(&jr.NREntry); err != nil { + if err := p.readUint64(&jr.NREntry); err != nil { return nil, fmt.Errorf("nrEntry: %w", err) } @@ -327,13 +345,13 @@ func (p *jitDumpParser) parseJRCodeDebugInfo(prefix *JRPrefix) (*JRCodeDebugInfo for i := uint64(0); i < jr.NREntry; i++ { jr.Entries[i] = &DebugEntry{} - if err := p.read(&jr.Entries[i].Addr); err != nil { + if err := p.readUint64(&jr.Entries[i].Addr); err != nil { return nil, fmt.Errorf("entries[%d].addr: %w", i, err) } - if err := p.read(&jr.Entries[i].Lineno); err != nil { + if err := p.readUint32(&jr.Entries[i].Lineno); err != nil { return nil, fmt.Errorf("entries[%d].lineno: %w", i, err) } - if err := p.read(&jr.Entries[i].Discrim); err != nil { + if err := p.readUint32(&jr.Entries[i].Discrim); err != nil { return nil, fmt.Errorf("entries[%d].discrim: %w", i, err) } @@ -358,18 +376,18 @@ func (p *jitDumpParser) parseJRCodeUnwindingInfo(prefix *JRPrefix) (*JRCodeUnwin jr.Prefix = prefix - if err := p.read(&jr.UnwindingSize); err != nil { + if err := p.readUint64(&jr.UnwindingSize); err != nil { return nil, fmt.Errorf("unwindingSize: %w", err) } - if err := p.read(&jr.EHFrameHDRSize); err != nil { + if err := p.readUint64(&jr.EHFrameHDRSize); err != nil { return nil, fmt.Errorf("ehFrameHDRSize: %w", err) } - if err := p.read(&jr.MappedSize); err != nil { + if err := p.readUint64(&jr.MappedSize); err != nil { return nil, fmt.Errorf("mappedSize: %w", err) } jr.UnwindingData = make([]byte, jr.UnwindingSize) - if _, err := io.ReadAtLeast(p.buf, jr.UnwindingData, len(jr.UnwindingData)); err != nil { + if _, err := io.ReadFull(p.buf, jr.UnwindingData); err != nil { return nil, fmt.Errorf("unwindingData: %w", err) } From 353120e28518c743cb44eda28993656bf5773ea4 Mon Sep 17 00:00:00 2001 From: Maxime Brunet Date: Thu, 2 Feb 2023 16:16:57 -0800 Subject: [PATCH 19/20] Move allocation to caller --- pkg/jit/jitdump.go | 58 ++++++++++++++++++++++++++--------------- pkg/jit/jitdump_test.go | 5 ++-- pkg/perf/perf.go | 3 ++- 3 files changed, 42 insertions(+), 24 deletions(-) diff --git a/pkg/jit/jitdump.go b/pkg/jit/jitdump.go index 177349d576..36bc3cffe9 100644 --- a/pkg/jit/jitdump.go +++ b/pkg/jit/jitdump.go @@ -399,31 +399,47 @@ func (p *jitDumpParser) parseJRCodeUnwindingInfo(prefix *JRPrefix) (*JRCodeUnwin return jr, nil } -func (p *jitDumpParser) parse() (*JITDump, error) { - dump := &JITDump{} +func (p *jitDumpParser) parse(dump *JITDump) error { var err error dump.Header, err = p.parseJITHeader() if err != nil { - return nil, fmt.Errorf("failed to read JIT dump header: %w", isUnexpectedIOError(err)) + return fmt.Errorf("failed to read JIT dump header: %w", isUnexpectedIOError(err)) } - dump.CodeLoads = make([]*JRCodeLoad, 0) - dump.CodeMoves = make([]*JRCodeMove, 0) - dump.DebugInfo = make([]*JRCodeDebugInfo, 0) - dump.UnwindingInfo = make([]*JRCodeUnwindingInfo, 0) + // Initialize or reset slices in jitdump + if dump.CodeLoads == nil { + dump.CodeLoads = make([]*JRCodeLoad, 0) + } else { + dump.CodeLoads = dump.CodeLoads[:0] + } + if dump.CodeMoves == nil { + dump.CodeMoves = make([]*JRCodeMove, 0) + } else { + dump.CodeMoves = dump.CodeMoves[:0] + } + if dump.DebugInfo == nil { + dump.DebugInfo = make([]*JRCodeDebugInfo, 0) + } else { + dump.DebugInfo = dump.DebugInfo[:0] + } + if dump.UnwindingInfo == nil { + dump.UnwindingInfo = make([]*JRCodeUnwindingInfo, 0) + } else { + dump.UnwindingInfo = dump.UnwindingInfo[:0] + } for { prefix, err := p.parseJRPrefix() if errors.Is(err, io.EOF) { - return dump, nil + return nil } else if err != nil { - return dump, fmt.Errorf("failed to read JIT Record Prefix: %w", err) + return fmt.Errorf("failed to read JIT Record Prefix: %w", err) } if prefix.ID >= JITCodeMax { level.Warn(p.logger).Log("msg", "unknown JIT record type, skipping", "ID", prefix.ID) if _, err := p.buf.Discard(int(prefix.TotalSize)); err != nil { - return dump, fmt.Errorf("failed to discard unknown JIT record: %w", isUnexpectedIOError(err)) + return fmt.Errorf("failed to discard unknown JIT record: %w", isUnexpectedIOError(err)) } continue } @@ -432,28 +448,28 @@ func (p *jitDumpParser) parse() (*JITDump, error) { case JITCodeLoad: jr, err := p.parseJRCodeLoad(prefix) if err != nil { - return dump, fmt.Errorf("failed to read JIT Code Load: %w", isUnexpectedIOError(err)) + return fmt.Errorf("failed to read JIT Code Load: %w", isUnexpectedIOError(err)) } dump.CodeLoads = append(dump.CodeLoads, jr) case JITCodeMove: jr, err := p.parseJRCodeMove(prefix) if err != nil { - return dump, fmt.Errorf("failed to read JIT Code Move: %w", isUnexpectedIOError(err)) + return fmt.Errorf("failed to read JIT Code Move: %w", isUnexpectedIOError(err)) } dump.CodeMoves = append(dump.CodeMoves, jr) case JITCodeDebugInfo: jr, err := p.parseJRCodeDebugInfo(prefix) if err != nil { - return dump, fmt.Errorf("failed to read JIT Code Debug Info: %w", isUnexpectedIOError(err)) + return fmt.Errorf("failed to read JIT Code Debug Info: %w", isUnexpectedIOError(err)) } dump.DebugInfo = append(dump.DebugInfo, jr) case JITCodeClose: level.Debug(p.logger).Log("msg", "reached JIT Code Close record") - return dump, nil + return nil case JITCodeUnwindingInfo: jr, err := p.parseJRCodeUnwindingInfo(prefix) if err != nil { - return dump, fmt.Errorf("failed to read JIT Code Unwinding Info: %w", isUnexpectedIOError(err)) + return fmt.Errorf("failed to read JIT Code Unwinding Info: %w", isUnexpectedIOError(err)) } dump.UnwindingInfo = append(dump.UnwindingInfo, jr) default: @@ -463,17 +479,17 @@ func (p *jitDumpParser) parse() (*JITDump, error) { } } -// LoadJITDump loads a jitdump file. -func LoadJITDump(logger log.Logger, rd io.Reader) (*JITDump, error) { +// LoadJITDump loads a jitdump file into dump. +func LoadJITDump(logger log.Logger, rd io.Reader, dump *JITDump) error { parser, err := newParser(logger, rd) if err != nil { - return nil, fmt.Errorf("failed to instantiate JIT dump parser: %w", err) + return fmt.Errorf("failed to instantiate JIT dump parser: %w", err) } - dump, err := parser.parse() + err = parser.parse(dump) if err != nil { - return dump, fmt.Errorf("failed to parse JIT dump: %w", err) + return fmt.Errorf("failed to parse JIT dump: %w", err) } - return dump, nil + return nil } diff --git a/pkg/jit/jitdump_test.go b/pkg/jit/jitdump_test.go index bc70bc4076..909ce95bb0 100644 --- a/pkg/jit/jitdump_test.go +++ b/pkg/jit/jitdump_test.go @@ -89,7 +89,8 @@ func TestLoadJITDump(t *testing.T) { defer f.Close() // Load JITDUMP from fixture - dump, err := jit.LoadJITDump(logger, f) + dump := &jit.JITDump{} + err = jit.LoadJITDump(logger, f, dump) require.ErrorIs(t, err, tc.err) // Encode JITDUMP to JSON @@ -140,7 +141,7 @@ func BenchmarkLoadJITDump(b *testing.B) { defer f.Close() // Load JITDUMP from fixture - _, err = jit.LoadJITDump(logger, f) + err = jit.LoadJITDump(logger, f, &jit.JITDump{}) require.ErrorIs(b, err, bb.err) } }) diff --git a/pkg/perf/perf.go b/pkg/perf/perf.go index 4ae50ec158..b651ab5acb 100644 --- a/pkg/perf/perf.go +++ b/pkg/perf/perf.go @@ -109,7 +109,8 @@ func MapFromDump(logger log.Logger, fs fs.FS, fileName string) (Map, error) { } defer fd.Close() - dump, err := jit.LoadJITDump(logger, fd) + dump := &jit.JITDump{} + err = jit.LoadJITDump(logger, fd, dump) if errors.Is(err, io.ErrUnexpectedEOF) { if dump == nil || dump.CodeLoads == nil { return Map{}, err From d1a65b8883cfd6db91d843e27e149d6b82457b0c Mon Sep 17 00:00:00 2001 From: Maxime Brunet Date: Fri, 3 Feb 2023 19:49:53 -0800 Subject: [PATCH 20/20] fix comment --- pkg/jit/jitdump.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/jit/jitdump.go b/pkg/jit/jitdump.go index 36bc3cffe9..390fc335bf 100644 --- a/pkg/jit/jitdump.go +++ b/pkg/jit/jitdump.go @@ -206,7 +206,7 @@ func (p *jitDumpParser) readUint64(n *uint64) error { // readString reads a string (until its null termination) from the jitdump file. func (p *jitDumpParser) readString() (string, error) { - s, err := p.buf.ReadString(0) // read the null termination + s, err := p.buf.ReadString(0) // read until the null termination if err != nil { // EOF is always unexpected, strings should always end by a null termination return s, isUnexpectedIOError(err)