diff --git a/cmd/debug-info/main.go b/cmd/debug-info/main.go index d0a5bb1ed..b9f6230ee 100644 --- a/cmd/debug-info/main.go +++ b/cmd/debug-info/main.go @@ -110,6 +110,9 @@ func main() { }) case "extract ": g.Add(func() error { + if err := os.RemoveAll(flags.Extract.OutputDir); err != nil { + return fmt.Errorf("failed to clean output dir, %s: %w", flags.Extract.OutputDir, err) + } if err := os.MkdirAll(flags.Extract.OutputDir, 0755); err != nil { return fmt.Errorf("failed to create output dir, %s: %w", flags.Extract.OutputDir, err) } @@ -133,10 +136,11 @@ func main() { } for _, f := range files { + // ./out//debuginfo _, p := filepath.Split(filepath.Dir(f)) output := filepath.Join(flags.Extract.OutputDir, filepath.Base(p)) if err := os.Rename(f, output); err != nil { - level.Error(logger).Log("msg", "failed to move file", "file", output) + level.Error(logger).Log("msg", "failed to move file", "file", output, "err", err) continue } level.Info(logger).Log("msg", "debug information extracted", "file", output) diff --git a/pkg/agent/profile.go b/pkg/agent/profile.go index 598651090..7b2e00b73 100644 --- a/pkg/agent/profile.go +++ b/pkg/agent/profile.go @@ -22,6 +22,7 @@ import ( "io" "os" "runtime" + "strings" "sync" "time" "unsafe" @@ -33,21 +34,16 @@ import ( "github.com/go-kit/log/level" "github.com/google/pprof/profile" profilestorepb "github.com/parca-dev/parca/gen/proto/go/parca/profilestore/v1alpha1" + "github.com/prometheus/common/model" "golang.org/x/sys/unix" "google.golang.org/grpc" "github.com/parca-dev/parca-agent/pkg/byteorder" "github.com/parca-dev/parca-agent/pkg/debuginfo" - "github.com/parca-dev/parca-agent/pkg/ksym" "github.com/parca-dev/parca-agent/pkg/maps" "github.com/parca-dev/parca-agent/pkg/perf" ) -import ( - "strings" - - "github.com/prometheus/common/model" -) //go:embed parca-agent.bpf.o var bpfObj []byte @@ -485,12 +481,10 @@ func (p *CgroupProfiler) profileLoop(ctx context.Context, now time.Time, counts, var labeloldformat []*profilestorepb.Label for key, value := range labels { - labeloldformat = append(labeloldformat, &profilestorepb.Label{Name: string(key), Value: string(value), }) - } _, err = p.writeClient.WriteRaw(ctx, &profilestorepb.WriteRawRequest{ diff --git a/pkg/debuginfo/debuginfo.go b/pkg/debuginfo/debuginfo.go index fe50863a3..5af9f3df3 100644 --- a/pkg/debuginfo/debuginfo.go +++ b/pkg/debuginfo/debuginfo.go @@ -243,29 +243,28 @@ func (di *Extractor) extract(ctx context.Context, buildID string, file string) ( level.Debug(di.logger).Log("msg", "failed to determine if binary is a Go binary", "path", file, "err", err) } - var ( - cmd *exec.Cmd - debugInfoFile string - ) + toRemove, err := sectionsToRemove(file) + if err != nil { + level.Debug(di.logger).Log("msg", "failed to determine sections to remove", "path", file, "err", err) + } + + var debugInfoFile string switch { case hasDWARF: - cmd, debugInfoFile = di.useStrip(ctx, tmpDir, file) + debugInfoFile, err = di.useStrip(ctx, tmpDir, file, toRemove) case isGo: - cmd, debugInfoFile = di.useObjcopy(ctx, tmpDir, file) + debugInfoFile, err = di.useObjcopy(ctx, tmpDir, file, toRemove) default: - cmd, debugInfoFile = di.useStrip(ctx, tmpDir, file) + debugInfoFile, err = di.useStrip(ctx, tmpDir, file, toRemove) } - if out, err := cmd.CombinedOutput(); err != nil { - level.Error(di.logger).Log( - "msg", "external binutils command call failed", - "output", strings.ReplaceAll(string(out), "\n", ""), - "file", file, - ) - return "", fmt.Errorf("failed to extract debug information from binary: %w", err) + if err != nil { + level.Error(di.logger).Log("msg", "external command call failed", "err", err, "file", file) + return "", err } + // Check if the debug information file is actually created. if exists, err := exists(debugInfoFile); !exists { - const msg = "external binutils command failed to extract debug information from binary" + const msg = "external binary utility command failed to extract debug information from binary" if err != nil { return "", fmt.Errorf(msg+": %w", err) } @@ -274,44 +273,61 @@ func (di *Extractor) extract(ctx context.Context, buildID string, file string) ( return debugInfoFile, nil } -func (di *Extractor) useStrip(ctx context.Context, dir string, file string) (*exec.Cmd, string) { +func (di *Extractor) useStrip(ctx context.Context, dir string, file string, toRemove []string) (string, error) { level.Debug(di.logger).Log("msg", "using eu-strip", "file", file) // Extract debug symbols. // If we have DWARF symbols, they are enough for us to symbolize the profiles. // We observed that having DWARF debug symbols and symbol table together caused us problem in certain cases. // As DWARF symbols enough on their own we just extract those. // eu-strip --strip-debug extracts the .debug/.zdebug sections from the object files. + args := []string{"--strip-debug"} + for _, s := range toRemove { + args = append(args, "--remove-section", s) + } debugInfoFile := path.Join(dir, "debuginfo") interimFile := path.Join(dir, "binary.stripped") - cmd := exec.CommandContext(ctx, - "eu-strip", "--strip-debug", - "--remove-section", ".debug_gdb_scripts", // causes some trouble when it's set to SHT_NOBITS - "-f", debugInfoFile, - "-o", interimFile, - file) - defer func() { - os.Remove(interimFile) - }() - return cmd, debugInfoFile + args = append(args, "-f", debugInfoFile, "-o", interimFile, file) + cmd := exec.CommandContext(ctx, "eu-strip", args...) + defer os.Remove(interimFile) + + if out, err := cmd.CombinedOutput(); err != nil { + level.Debug(di.logger).Log( + "msg", "external eu-strip command call failed", + "output", strings.ReplaceAll(string(out), "\n", ""), + "file", file, + ) + return "", fmt.Errorf("failed to extract debug information from binary: %w", err) + } + + return debugInfoFile, nil } -func (di *Extractor) useObjcopy(ctx context.Context, dir string, file string) (*exec.Cmd, string) { - debugInfoFile := path.Join(dir, "debuginfo") +func (di *Extractor) useObjcopy(ctx context.Context, dir string, file string, toRemove []string) (string, error) { level.Debug(di.logger).Log("msg", "using objcopy", "file", file) // Go binaries has a special case. They use ".gopclntab" section to symbolize addresses. // We need to keep ".note.go.buildid", ".symtab" and ".gopclntab", // however it doesn't hurt to keep rather small sections. - return exec.CommandContext(ctx, - "objcopy", - // NOTICE: Keep debug information till we find a better for symbolizing Go binaries without DWARF. - //"-R", ".zdebug_*", - //"-R", ".debug_*", - "--remove-section", ".text", // executable - "--remove-section", ".rodata*", // constants - "--remove-section", ".debug_gdb_scripts", // causes some trouble when it's set to SHT_NOBITS + args := []string{} + toRemove = append(toRemove, ".text", ".rodata*") + for _, s := range toRemove { + args = append(args, "--remove-section", s) + } + debugInfoFile := path.Join(dir, "debuginfo") + args = append(args, file, // source debugInfoFile, // destination - ), debugInfoFile + ) + cmd := exec.CommandContext(ctx, "objcopy", args...) + if out, err := cmd.CombinedOutput(); err != nil { + level.Debug(di.logger).Log( + "msg", "external objcopy command call failed", + "output", strings.ReplaceAll(string(out), "\n", ""), + "file", file, + ) + return "", fmt.Errorf("failed to extract debug information from binary: %w", err) + } + + return debugInfoFile, nil } func (di *Extractor) uploadDebugInfo(ctx context.Context, buildID string, file string) error { @@ -347,13 +363,13 @@ func hasDebugInfo(path string) (bool, error) { } func hasDWARF(path string) (bool, error) { - exe, err := elf.Open(path) + f, err := elf.Open(path) if err != nil { return false, fmt.Errorf("failed to open elf: %w", err) } - defer exe.Close() + defer f.Close() - data, err := getDWARF(exe) + data, err := dwarf(f) if err != nil { return false, fmt.Errorf("failed to read DWARF sections: %w", err) } @@ -361,35 +377,35 @@ func hasDWARF(path string) (bool, error) { return len(data) > 0, nil } -// A simplified and modified version of debug/elf.DWARF(). -func getDWARF(f *elf.File) (map[string][]byte, error) { - dwarfSuffix := func(s *elf.Section) string { - switch { - case strings.HasPrefix(s.Name, ".debug_"): - return s.Name[7:] - case strings.HasPrefix(s.Name, ".zdebug_"): - return s.Name[8:] - case strings.HasPrefix(s.Name, "__debug_"): // macos - return s.Name[8:] - default: - return "" - } +var dwarfSuffix = func(s *elf.Section) string { + switch { + case strings.HasPrefix(s.Name, ".debug_"): + return s.Name[7:] + case strings.HasPrefix(s.Name, ".zdebug_"): + return s.Name[8:] + case strings.HasPrefix(s.Name, "__debug_"): // macos + return s.Name[8:] + default: + return "" } +} +// A simplified and modified version of debug/elf.DWARF(). +func dwarf(f *elf.File) (map[string][]byte, error) { // There are many DWARf sections, but these are the ones // the debug/dwarf package started with "abbrev", "info", "str", "line", "ranges". // Possible canditates for future: "loc", "loclists", "rnglists" sections := map[string]*string{"abbrev": nil, "info": nil, "str": nil, "line": nil, "ranges": nil} data := map[string][]byte{} - for _, s := range f.Sections { - suffix := dwarfSuffix(s) + for _, sec := range f.Sections { + suffix := dwarfSuffix(sec) if suffix == "" { continue } if _, ok := sections[suffix]; !ok { continue } - b, err := s.Data() + b, err := sec.Data() if err != nil { return nil, fmt.Errorf("failed to read debug section: %w", err) } @@ -399,24 +415,40 @@ func getDWARF(f *elf.File) (map[string][]byte, error) { return data, nil } +func sectionsToRemove(path string) ([]string, error) { + var sections []string + f, err := elf.Open(path) + if err != nil { + return sections, fmt.Errorf("failed to open elf file: %w", err) + } + defer f.Close() + + for _, sec := range f.Sections { + if dwarfSuffix(sec) != "" && sec.Type == elf.SHT_NOBITS { // causes some trouble when it's set to SHT_NOBITS + sections = append(sections, sec.Name) + } + } + return sections, nil +} + func isSymbolizableGoBinary(path string) (bool, error) { // Checks ".note.go.buildid" section and symtab better to keep those sections in object file. - exe, err := elf.Open(path) + f, err := elf.Open(path) if err != nil { return false, fmt.Errorf("failed to open elf: %w", err) } - defer exe.Close() + defer f.Close() isGo := false - for _, s := range exe.Sections { - if s.Name == ".note.go.buildid" { + for _, sec := range f.Sections { + if sec.Name == ".note.go.buildid" { isGo = true } } // In case ".note.go.buildid" section is stripped, check for symbols. if !isGo { - syms, err := exe.Symbols() + syms, err := f.Symbols() if err != nil { return false, fmt.Errorf("failed to read symbols: %w", err) } @@ -438,7 +470,7 @@ func isSymbolizableGoBinary(path string) (bool, error) { // Check if the Go binary symbolizable. // Go binaries has a special case. They use ".gopclntab" section to symbolize addresses. var pclntab []byte - if sec := exe.Section(".gopclntab"); sec != nil { + if sec := f.Section(".gopclntab"); sec != nil { pclntab, err = sec.Data() if err != nil { return false, fmt.Errorf("could not find .gopclntab section: %w", err)