DWARF: Add error handling for unexpected CFA opcodes

Signed-off-by: Sumera Priyadarsini <[email protected]>

cmd/eh-frame/main.go

@@ -55,7 +55,7 @@ func main() {
  }
 
  if flags.Final {
- ut, arch, err := unwind.GenerateCompactUnwindTable(logger, executablePath)
+ ut, arch, err := unwind.GenerateCompactUnwindTable(executablePath)
  if err != nil {
  // nolint:forbidigo
  fmt.Println("failed with:", err)
@@ -70,7 +70,7 @@ func main() {
  }
 
  ptb := unwind.NewUnwindTableBuilder(logger)
- err := ptb.PrintTable(logger, os.Stdout, executablePath, flags.Compact, pc)
+ err := ptb.PrintTable(os.Stdout, executablePath, flags.Compact, pc)
  if err != nil {
  // nolint:forbidigo
  fmt.Println("failed with:", err)

internal/dwarf/frame/table.go

@@ -4,10 +4,10 @@ package frame
 import (
  "bytes"
  "encoding/binary"
+ "fmt"
  "strings"
 
  "github.com/go-kit/log"
- "github.com/go-kit/log/level"
 
  "github.com/parca-dev/parca-agent/internal/dwarf/util"
 )
@@ -258,40 +258,49 @@ const low_6_offset = 0x3f
 
 type instruction func(ctx *Context)
 
-func NewContext(logger log.Logger, fullExecutablePath string) *Context {
+func NewContext(fullExecutablePath string) *Context {
  return &Context{
  currInsCtx: &InstructionContext{},
  lastInsCtx: &InstructionContext{},
  buf: &bytes.Reader{},
  rememberedState: newStateStack(),
  fullExecutablePath: fullExecutablePath,
- logger: log.With(logger, "component", "dwarf unwind table context"),
  }
 }
 
-func executeCIEInstructions(cie *CommonInformationEntry, context *Context) *Context {
+func executeCIEInstructions(cie *CommonInformationEntry, context *Context) (*Context, error) {
  if context == nil {
- context = NewContext(context.logger, context.fullExecutablePath)
+ context = NewContext(context.fullExecutablePath)
  }
 
  context.reset(cie)
- context.executeDWARFProgram()
- return context
+ err := context.executeDWARFProgram()
+ if err != nil {
+ return context, err
+ }
+ return context, nil
 }
 
 // ExecuteDWARFProgram evaluates the unwind opcodes for a function.
-func ExecuteDWARFProgram(fde *FrameDescriptionEntry, context *Context) *InstructionContextIterator {
- ctx := executeCIEInstructions(fde.CIE, context)
+func ExecuteDWARFProgram(fde *FrameDescriptionEntry, context *Context) (*InstructionContextIterator, error) {
+ ctx, err := executeCIEInstructions(fde.CIE, context)
  ctx.order = fde.order
  frame := ctx.currentInstruction()
  frame.loc = fde.Begin()
- return ctx.Execute(fde.Instructions)
+ if err != nil {
+ return ctx.Execute(fde.Instructions), err
+ }
+ return ctx.Execute(fde.Instructions), nil
 }
 
-func (ctx *Context) executeDWARFProgram() {
+func (ctx *Context) executeDWARFProgram() error {
  for ctx.buf.Len() > 0 {
- executeDWARFInstruction(ctx)
+ err := executeDWARFInstruction(ctx)
+ if err != nil {
+ return err
+ }
  }
+ return nil
 }
 
 // Execute execute dwarf instructions.
@@ -303,26 +312,30 @@ func (ctx *Context) Execute(instructions []byte) *InstructionContextIterator {
  }
 }
 
-func executeDWARFInstruction(ctx *Context) {
+func executeDWARFInstruction(ctx *Context) error {
  instruction, err := ctx.buf.ReadByte()
  if err != nil {
  panic("Could not read from instruction buffer")
  }
 
  if instruction == DW_CFA_nop {
- return
+ return nil
  }
 
- fn := lookupFunc(instruction, ctx)
+ fn, err := lookupFunc(instruction, ctx)
  fn(ctx)
+ if err != nil {
+ return fmt.Errorf(" DWARF CFA rule is not valid. This should never happen :%w", err)
+ }
+ return nil
 }
 
 // getPid gets the Process ID as a string from the fullExecutablePath provided
 func getPid(fullExecutablePath string) string {
  return strings.Split(fullExecutablePath, "/")[2]
 }
 
-func lookupFunc(instruction byte, ctx *Context) instruction {
+func lookupFunc(instruction byte, ctx *Context) (instruction, error) {
  const high_2_bits = 0xc0
  var restoreOpcode bool
 
@@ -413,9 +426,10 @@ func lookupFunc(instruction byte, ctx *Context) instruction {
  case DW_CFA_GNU_window_save:
  fn = gnuwindowsave
  default:
- level.Error(ctx.logger).Log("msg", "encountered an unexpected DWARF CFA opcode", "opcode instruction", instruction, "PID", getPid(ctx.fullExecutablePath), "comm", ctx.fullExecutablePath)
+ pid := getPid(ctx.fullExecutablePath)
+ return nil, fmt.Errorf("encountered an unexpected DWARF CFA opcode instruction %d for executable: %s with PID: %s", instruction, ctx.fullExecutablePath, pid)
  }
- return fn
+ return fn, nil
 }
 
 // TODO(sylfrena): Reuse types.

pkg/profiler/cpu/bpf/maps/maps.go

@@ -1556,7 +1556,7 @@ func (m *Maps) setUnwindTableForMapping(buf *profiler.EfficientBuffer, pid int,
  // Generate the unwind table.
  // PERF(javierhonduco): Not reusing a buffer here yet, let's profile and decide whether this
  // change would be worth it.
- ut, arch, err := unwind.GenerateCompactUnwindTable(m.logger, fullExecutablePath)
+ ut, arch, err := unwind.GenerateCompactUnwindTable(fullExecutablePath)
  level.Debug(m.logger).Log("msg", "found unwind entries", "executable", mapping.Executable, "len", len(ut))
 
  if err != nil {

pkg/stack/unwind/compact_unwind_table.go

@@ -20,8 +20,6 @@ import (
  "fmt"
  "sort"
 
- "github.com/go-kit/log"
-
  "github.com/parca-dev/parca-agent/internal/dwarf/frame"
 )
 
@@ -137,9 +135,9 @@ func (t CompactUnwindTable) RemoveRedundant() CompactUnwindTable {
 
 // BuildCompactUnwindTable produces a compact unwind table for the given
 // frame description entries.
-func BuildCompactUnwindTable(logger log.Logger, fdes frame.FrameDescriptionEntries, arch elf.Machine, fullExecutablePath string) (CompactUnwindTable, error) {
+func BuildCompactUnwindTable(fdes frame.FrameDescriptionEntries, arch elf.Machine, fullExecutablePath string) (CompactUnwindTable, error) {
  table := make(CompactUnwindTable, 0, 4*len(fdes)) // heuristic: we expect each function to have ~4 unwind entries.
- context := frame.NewContext(logger, fullExecutablePath)
+ context := frame.NewContext(fullExecutablePath)
  lastFunctionPc := uint64(0)
  for _, fde := range fdes {
  // Add a synthetic row at the end of the function but only
@@ -155,7 +153,10 @@ func BuildCompactUnwindTable(logger log.Logger, fdes frame.FrameDescriptionEntri
  })
  }
 
- frameContext := frame.ExecuteDWARFProgram(fde, context)
+ frameContext, err := frame.ExecuteDWARFProgram(fde, context)
+ if err != nil {
+ return CompactUnwindTable{}, err
+ }
  for insCtx := frameContext.Next(); frameContext.HasNext(); insCtx = frameContext.Next() {
  row := unwindTableRow(insCtx)
  compactRow, err := rowToCompactRow(row, arch)
@@ -268,7 +269,7 @@ func CompactUnwindTableRepresentation(unwindTable UnwindTable, arch elf.Machine)
 
 // GenerateCompactUnwindTable produces the compact unwind table for a given
 // executable.
-func GenerateCompactUnwindTable(logger log.Logger, fullExecutablePath string) (CompactUnwindTable, elf.Machine, error) {
+func GenerateCompactUnwindTable(fullExecutablePath string) (CompactUnwindTable, elf.Machine, error) {
  var ut CompactUnwindTable
 
  // Fetch FDEs.
@@ -282,7 +283,7 @@ func GenerateCompactUnwindTable(logger log.Logger, fullExecutablePath string) (C
  sort.Sort(fdes)
 
  // Generate the compact unwind table.
- ut, err = BuildCompactUnwindTable(logger, fdes, arch, fullExecutablePath)
+ ut, err = BuildCompactUnwindTable(fdes, arch, fullExecutablePath)
  if err != nil {
  return ut, arch, err
  }

pkg/stack/unwind/compact_unwind_table_test.go

@@ -470,10 +470,8 @@ func TestNoRepeatedPCs(t *testing.T) {
  matches, err := filepath.Glob("../../../testdata/vendored/x86/*")
  require.NoError(t, err)
 
- logger := logger.NewLogger("error", logger.LogFormatLogfmt, "compact-unwind-table-tests")
-
  for _, match := range matches {
- ut, _, err := GenerateCompactUnwindTable(logger, match)
+ ut, _, err := GenerateCompactUnwindTable(match)
  require.NoError(t, err)
  requireNoDuplicatedPC(t, ut)
  }
@@ -483,10 +481,8 @@ func TestNoRedundantRows(t *testing.T) {
  matches, err := filepath.Glob("../../../testdata/vendored/x86/*")
  require.NoError(t, err)
 
- logger := logger.NewLogger("error", logger.LogFormatLogfmt, "compact-unwind-table-tests")
-
  for _, match := range matches {
- ut, _, err := GenerateCompactUnwindTable(logger, match)
+ ut, _, err := GenerateCompactUnwindTable(match)
  require.NoError(t, err)
  requireNoRedundantRows(t, ut)
  }
@@ -499,12 +495,10 @@ func BenchmarkGenerateCompactUnwindTable(b *testing.B) {
 
  b.ReportAllocs()
 
- logger := logger.NewLogger("error", logger.LogFormatLogfmt, "compact-unwind-table-tests")
-
  var cut CompactUnwindTable
  var err error
  for n := 0; n < b.N; n++ {
- cut, _, err = GenerateCompactUnwindTable(logger, objectFilePath)
+ cut, _, err = GenerateCompactUnwindTable(objectFilePath)
  }
 
  require.NoError(b, err)

pkg/stack/unwind/unwind_table.go

@@ -79,7 +79,7 @@ func registerToString(reg uint64, arch elf.Machine) string {
 }
 
 // PrintTable is a debugging helper that prints the unwinding table to the given io.Writer.
-func (ptb *UnwindTableBuilder) PrintTable(logger log.Logger, writer io.Writer, path string, compact bool, pc *uint64) error {
+func (ptb *UnwindTableBuilder) PrintTable(writer io.Writer, path string, compact bool, pc *uint64) error {
  fdes, arch, err := ReadFDEs(path)
  if err != nil {
  return err
@@ -92,7 +92,7 @@ func (ptb *UnwindTableBuilder) PrintTable(logger log.Logger, writer io.Writer, p
  }
  }()
 
- unwindContext := frame.NewContext(logger, path)
+ unwindContext := frame.NewContext(path)
  for _, fde := range fdes {
  if pc != nil {
  if fde.Begin() > *pc || *pc > fde.End() {
@@ -102,7 +102,10 @@ func (ptb *UnwindTableBuilder) PrintTable(logger log.Logger, writer io.Writer, p
 
  fmt.Fprintf(writer, "=> Function start: %x, Function end: %x\n", fde.Begin(), fde.End())
 
- frameContext := frame.ExecuteDWARFProgram(fde, unwindContext)
+ frameContext, err := frame.ExecuteDWARFProgram(fde, unwindContext)
+ if err != nil {
+ return err
+ }
  for insCtx := frameContext.Next(); frameContext.HasNext(); insCtx = frameContext.Next() {
  unwindRow := unwindTableRow(insCtx)
 
@@ -222,17 +225,20 @@ func ReadFDEs(path string) (frame.FrameDescriptionEntries, elf.Machine, error) {
  return fdes, arch, nil
 }
 
-func BuildUnwindTable(ctx *frame.Context, fdes frame.FrameDescriptionEntries) UnwindTable {
+func BuildUnwindTable(ctx *frame.Context, fdes frame.FrameDescriptionEntries) (UnwindTable, error) {
  // The frame package can raise in case of malformed unwind data.
  table := make(UnwindTable, 0, 4*len(fdes)) // heuristic
 
  for _, fde := range fdes {
- frameContext := frame.ExecuteDWARFProgram(fde, ctx)
+ frameContext, err := frame.ExecuteDWARFProgram(fde, ctx)
+ if err != nil {
+ return UnwindTable{}, err
+ }
  for insCtx := frameContext.Next(); frameContext.HasNext(); insCtx = frameContext.Next() {
  table = append(table, *unwindTableRow(insCtx))
  }
  }
- return table
+ return table, nil
 }
 
 // UnwindTableRow represents a single row in the unwind table.

pkg/stack/unwind/unwind_table_test.go

@@ -15,10 +15,8 @@
 package unwind
 
 import (
- "os"
  "testing"
 
- "github.com/go-kit/log"
  "github.com/stretchr/testify/require"
 
  "github.com/parca-dev/parca-agent/internal/dwarf/frame"
@@ -29,11 +27,10 @@ func TestBuildUnwindTable(t *testing.T) {
  fdes, _, err := ReadFDEs("../../../testdata/out/x86/basic-cpp")
  require.NoError(t, err)
 
- logger := log.NewLogfmtLogger(log.NewSyncWriter(os.Stderr))
- logger = log.With(logger, "component", "unwind table test")
- unwindContext := frame.NewContext(logger, "../../../testdata/out/x86/basic-cpp")
+ unwindContext := frame.NewContext("../../../testdata/out/x86/basic-cpp")
 
- unwindTable := BuildUnwindTable(unwindContext, fdes)
+ unwindTable, err := BuildUnwindTable(unwindContext, fdes)
+ require.NoError(t, err)
  require.Len(t, unwindTable, 38)
 
  require.Equal(t, uint64(0x401020), unwindTable[0].Loc)
@@ -55,16 +52,14 @@ func TestSpecialOpcodes(t *testing.T) {
  },
  }
 
- logger := log.NewLogfmtLogger(log.NewSyncWriter(os.Stderr))
- logger = log.With(logger, "component", "unwind table test")
-
  for _, tt := range tests {
  t.Run(tt.name, func(t *testing.T) {
  fdes, _, err := ReadFDEs(tt.executable)
  require.NoError(t, err)
 
- ctx := frame.NewContext(logger, tt.executable)
- unwindTable := BuildUnwindTable(ctx, fdes)
+ ctx := frame.NewContext(tt.executable)
+ unwindTable, err := BuildUnwindTable(ctx, fdes)
+ require.NoError(t, err)
  require.NotEmpty(t, unwindTable)
  })
  }
@@ -78,18 +73,18 @@ func benchmarkParsingDWARFUnwindInformation(b *testing.B, executable string) {
 
  var rbpOffset int64
 
- logger := log.NewLogfmtLogger(log.NewSyncWriter(os.Stderr))
- logger = log.With(logger, "component", "unwind table test")
-
  for n := 0; n < b.N; n++ {
  fdes, _, err := ReadFDEs(executable)
  if err != nil {
  panic("could not read FDEs")
  }
 
- unwindContext := frame.NewContext(logger, executable)
+ unwindContext := frame.NewContext(executable)
  for _, fde := range fdes {
- frameContext := frame.ExecuteDWARFProgram(fde, unwindContext)
+ frameContext, err := frame.ExecuteDWARFProgram(fde, unwindContext)
+ if err != nil {
+ panic("unable to evaluate DWARF unwind codes")
+ }
  for insCtx := frameContext.Next(); frameContext.HasNext(); insCtx = frameContext.Next() {
  unwindRow := unwindTableRow(insCtx)
  if unwindRow.RBP.Rule == frame.RuleUndefined || unwindRow.RBP.Offset == 0 {