DWARF: Add logging for unexpected CFA opcodes

internal/dwarf/frame/table.go

@@ -84,26 +84,29 @@ func (ici *InstructionContextIterator) HasNext() bool {
  return !ici.done
 }
 
-func (ici *InstructionContextIterator) Next() *InstructionContext {
+func (ici *InstructionContextIterator) Next() (*InstructionContext, error) {
  for ici.ctx.buf.Len() > 0 {
  lastPcBefore := ici.ctx.lastInsCtx.loc
- executeDWARFInstruction(ici.ctx)
+ err := executeDWARFInstruction(ici.ctx)
+ if err != nil {
+ return ici.ctx.lastInsCtx, err
+ }
  lastPcAfter := ici.ctx.lastInsCtx.loc
  // We are at an instruction boundary when there's a program counter change.
  if lastPcBefore != lastPcAfter {
- return ici.ctx.lastInsCtx
+ return ici.ctx.lastInsCtx, nil
  }
  }
 
  // Account for the last instruction boundary.
  if !ici.lastReached {
  ici.lastReached = true
- return ici.ctx.currInsCtx
+ return ici.ctx.currInsCtx, nil
  }
 
  // We are done iterating.
  ici.done = true
- return nil
+ return nil, nil //nolint:nilnil
 }
 
 // RowState is a stack where `DW_CFA_remember_state` pushes
@@ -262,29 +265,39 @@ func NewContext() *Context {
  }
 }
 
-func executeCIEInstructions(cie *CommonInformationEntry, context *Context) *Context {
+func executeCIEInstructions(cie *CommonInformationEntry, context *Context) (*Context, error) {
  if context == nil {
  context = NewContext()
  }
 
  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)
+ if err != nil {
+ return nil, err
+ }
  ctx.order = fde.order
  frame := ctx.currentInstruction()
  frame.loc = fde.Begin()
- return ctx.Execute(fde.Instructions)
+ 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.
@@ -296,21 +309,26 @@ 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)
+ if err != nil {
+ return fmt.Errorf("DWARF CFA rule is not valid. This should never happen :%w", err)
+ }
  fn(ctx)
+
+ return nil
 }
 
-func lookupFunc(instruction byte, ctx *Context) instruction {
+func lookupFunc(instruction byte, ctx *Context) (instruction, error) {
  const high_2_bits = 0xc0
  var restoreOpcode bool
 
@@ -401,10 +419,9 @@ func lookupFunc(instruction byte, ctx *Context) instruction {
  case DW_CFA_GNU_window_save:
  fn = gnuwindowsave
  default:
- panic(fmt.Sprintf("Encountered an unexpected DWARF CFA opcode: %#v", instruction))
+ return nil, fmt.Errorf("encountered an unexpected DWARF CFA opcode instruction %d", instruction)
  }
-
- return fn
+ return fn, nil
 }
 
 // TODO(sylfrena): Reuse types.

pkg/stack/unwind/compact_unwind_table.go

@@ -153,16 +153,28 @@ func BuildCompactUnwindTable(fdes frame.FrameDescriptionEntries, arch elf.Machin
  })
  }
 
- frameContext := frame.ExecuteDWARFProgram(fde, context)
- for insCtx := frameContext.Next(); frameContext.HasNext(); insCtx = frameContext.Next() {
+ frameContext, err := frame.ExecuteDWARFProgram(fde, context)
+ if err != nil {
+ return CompactUnwindTable{}, err
+ }
+
+ for {
+ insCtx, err := frameContext.Next()
+ if err != nil {
+ return CompactUnwindTable{}, err
+ }
+
+ if !frameContext.HasNext() {
+ break
+ }
+
  row := unwindTableRow(insCtx)
  compactRow, err := rowToCompactRow(row, arch)
  if err != nil {
  return CompactUnwindTable{}, err
  }
  table = append(table, compactRow)
  }
-
  lastFunctionPc = fde.End()
  }
  // Add a synthetic row at the end of the unwind table. It is fine
@@ -282,7 +294,7 @@ func GenerateCompactUnwindTable(fullExecutablePath string) (CompactUnwindTable,
  // Generate the compact unwind table.
  ut, err = BuildCompactUnwindTable(fdes, arch)
  if err != nil {
- return ut, arch, err
+ return ut, arch, fmt.Errorf("build compact unwind table for executable %q: %w", fullExecutablePath, err)
  }
 
  // This should not be necessary, as per the sorting above, but

pkg/stack/unwind/unwind_table.go

@@ -102,8 +102,20 @@ func (ptb *UnwindTableBuilder) PrintTable(writer io.Writer, path string, compact
 
  fmt.Fprintf(writer, "=> Function start: %x, Function end: %x\n", fde.Begin(), fde.End())
 
- frameContext := frame.ExecuteDWARFProgram(fde, unwindContext)
- for insCtx := frameContext.Next(); frameContext.HasNext(); insCtx = frameContext.Next() {
+ frameContext, err := frame.ExecuteDWARFProgram(fde, unwindContext)
+ if err != nil {
+ return err
+ }
+
+ for {
+ insCtx, err := frameContext.Next()
+ if err != nil {
+ return err
+ }
+ if !frameContext.HasNext() {
+ break
+ }
+
  unwindRow := unwindTableRow(insCtx)
 
  if unwindRow == nil {
@@ -222,17 +234,29 @@ func ReadFDEs(path string) (frame.FrameDescriptionEntries, elf.Machine, error) {
  return fdes, arch, nil
 }
 
-func BuildUnwindTable(fdes frame.FrameDescriptionEntries) UnwindTable {
+func BuildUnwindTable(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, nil)
- for insCtx := frameContext.Next(); frameContext.HasNext(); insCtx = frameContext.Next() {
+ frameContext, err := frame.ExecuteDWARFProgram(fde, nil)
+ if err != nil {
+ return UnwindTable{}, err
+ }
+
+ for {
+ insCtx, err := frameContext.Next()
+ if err != nil {
+ return UnwindTable{}, err
+ }
+
+ if !frameContext.HasNext() {
+ break
+ }
  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

@@ -22,11 +22,13 @@ import (
  "github.com/parca-dev/parca-agent/internal/dwarf/frame"
 )
 
+// TODO(Sylfrena): Add equivalent test for arm64.
 func TestBuildUnwindTable(t *testing.T) {
  fdes, _, err := ReadFDEs("../../../testdata/out/x86/basic-cpp")
  require.NoError(t, err)
 
- unwindTable := BuildUnwindTable(fdes)
+ unwindTable, err := BuildUnwindTable(fdes)
+ require.NoError(t, err)
  require.Len(t, unwindTable, 38)
 
  require.Equal(t, uint64(0x401020), unwindTable[0].Loc)
@@ -47,12 +49,14 @@ func TestSpecialOpcodes(t *testing.T) {
  executable: "testdata/cfa_gnu_window_save",
  },
  }
+
  for _, tt := range tests {
  t.Run(tt.name, func(t *testing.T) {
  fdes, _, err := ReadFDEs(tt.executable)
  require.NoError(t, err)
 
- unwindTable := BuildUnwindTable(fdes)
+ unwindTable, err := BuildUnwindTable(fdes)
+ require.NoError(t, err)
  require.NotEmpty(t, unwindTable)
  })
  }
@@ -74,8 +78,21 @@ func benchmarkParsingDWARFUnwindInformation(b *testing.B, executable string) {
 
  unwindContext := frame.NewContext()
  for _, fde := range fdes {
- frameContext := frame.ExecuteDWARFProgram(fde, unwindContext)
- for insCtx := frameContext.Next(); frameContext.HasNext(); insCtx = frameContext.Next() {
+ frameContext, err := frame.ExecuteDWARFProgram(fde, unwindContext)
+ if err != nil {
+ b.Fail()
+ }
+
+ for {
+ insCtx, err := frameContext.Next()
+ if err != nil {
+ b.Fail()
+ }
+
+ if !frameContext.HasNext() {
+ break
+ }
+
  unwindRow := unwindTableRow(insCtx)
  if unwindRow.RBP.Rule == frame.RuleUndefined || unwindRow.RBP.Offset == 0 {
  // u