pkg/objectfile: reduce elf.Open() calls

- pass elf file as a field to Objectfile struct type
- update tests accordingly

pkg/objectfile/object_file.go

@@ -30,13 +30,25 @@ import (
 	"github.com/google/pprof/profile"
 
 	"github.com/parca-dev/parca-agent/internal/pprof/elfexec"
-
 	"github.com/parca-dev/parca-agent/pkg/buildid"
 )
 
-// Defined for testing.
 var elfOpen = elf.Open
 
+type ObjectFile struct {
+	Path    string
+	BuildID string
+	ElfFile *elf.File
+
+	// Ensures the base, baseErr and isData are computed once.
+	baseOnce sync.Once
+	base     uint64
+	baseErr  error
+
+	isData bool
+	m      *mapping
+}
+
 // Open opens the specified executable or library file from the given path.
 func Open(filePath string, m *profile.Mapping) (*ObjectFile, error) {
 	f, err := os.Open(filePath)
@@ -120,6 +132,7 @@ func open(filePath string, start, limit, offset uint64, relocationSymbol string)
 	return &ObjectFile{
 		Path:    filePath,
 		BuildID: buildID,
+		ElfFile: f,
 		m: &mapping{
 			start:        start,
 			limit:        limit,
@@ -129,19 +142,6 @@ func open(filePath string, start, limit, offset uint64, relocationSymbol string)
 	}, nil
 }
 
-type ObjectFile struct {
-	Path    string
-	BuildID string
-
-	// Ensures the base, baseErr and isData are computed once.
-	baseOnce sync.Once
-	base     uint64
-	baseErr  error
-
-	isData bool
-	m      *mapping
-}
-
 type MappedObjectFile struct {
 	*ObjectFile
 
@@ -171,11 +171,8 @@ func (f *ObjectFile) computeBase(addr uint64) error {
 	if addr < f.m.start || addr >= f.m.limit {
 		return fmt.Errorf("specified address %x is outside the mapping range [%x, %x] for ObjectFile %q", addr, f.m.start, f.m.limit, f.Path)
 	}
-	ef, err := elfOpen(f.Path)
-	if err != nil {
-		return fmt.Errorf("error parsing %s: %w", f.Path, err)
-	}
-	defer ef.Close()
+
+	ef := f.ElfFile
 
 	ph, err := f.m.findProgramHeader(ef, addr)
 	if err != nil {
@@ -191,8 +188,6 @@ func (f *ObjectFile) computeBase(addr uint64) error {
 	return nil
 }
 
-// mapping stores the parameters of a runtime mapping that are needed to
-// identify the ELF segment associated with a mapping.
 type mapping struct {
 	// Runtime mapping parameters.
 	start, limit, offset uint64

pkg/objectfile/object_file_test.go

@@ -25,16 +25,34 @@ import (
 	"github.com/google/pprof/profile"
 )
 
-func TestOpenMalformedELF(t *testing.T) {
-	// Test that opening a malformed ELF ObjectFile will report an error containing
-	// the word "ELF".
-	_, err := Open(filepath.Join("../../internal/pprof/binutils/testdata", "malformed_elf"), &profile.Mapping{})
-	if err == nil {
-		t.Fatalf("Open: unexpected success")
-	}
-	if !strings.Contains(err.Error(), "ELF") {
-		t.Errorf("Open: got %v, want error containing 'ELF'", err)
-	}
+func TestOpenELF(t *testing.T) {
+	t.Run("Malformed ELF", func(t *testing.T) {
+		// Test that opening a malformed ELF ObjectFile will report an error containing
+		// the word "ELF".
+		_, err := Open(filepath.Join("../../internal/pprof/binutils/testdata", "malformed_elf"), &profile.Mapping{})
+		if err == nil {
+			t.Fatalf("Open: unexpected success")
+		}
+		if !strings.Contains(err.Error(), "ELF") {
+			t.Errorf("Open: got %v, want error containing 'ELF'", err)
+		}
+	})
+
+	t.Run("ELF Open Error", func(t *testing.T) {
+		elfOpen = func(_ string) (*elf.File, error) {
+			return &elf.File{FileHeader: elf.FileHeader{Type: elf.ET_EXEC}}, errors.New("elf.Open failed")
+		}
+		t.Cleanup(func() {
+			elfOpen = elf.Open
+		})
+		_, err := open("", uint64(0x2000), uint64(0x5000), uint64(0x1000), "")
+		if err == nil {
+			t.Fatalf("open: unexpected success")
+		}
+		if !strings.Contains(err.Error(), "elf.Open failed") {
+			t.Errorf("Open: got %v, want error 'elf.Open failed'", err)
+		}
+	})
 }
 
 func TestComputeBase(t *testing.T) {
@@ -61,7 +79,6 @@ func TestComputeBase(t *testing.T) {
 	for _, tc := range []struct {
 		desc       string
 		file       *elf.File
-		openErr    error
 		mapping    *mapping
 		addr       uint64
 		wantError  bool
@@ -82,14 +99,6 @@ func TestComputeBase(t *testing.T) {
 			addr:      0x1000,
 			wantError: true,
 		},
-		{
-			desc:      "elf.Open failing means error",
-			file:      &elf.File{FileHeader: elf.FileHeader{Type: elf.ET_EXEC}},
-			openErr:   errors.New("elf.Open failed"),
-			mapping:   &mapping{start: 0x2000, limit: 0x5000, offset: 0x1000},
-			addr:      0x4000,
-			wantError: true,
-		},
 		{
 			desc:       "no loadable segments, no error",
 			file:       &elf.File{FileHeader: elf.FileHeader{Type: elf.ET_EXEC}},
@@ -145,13 +154,7 @@ func TestComputeBase(t *testing.T) {
 		},
 	} {
 		t.Run(tc.desc, func(t *testing.T) {
-			elfOpen = func(_ string) (*elf.File, error) {
-				return tc.file, tc.openErr
-			}
-			t.Cleanup(func() {
-				elfOpen = elf.Open
-			})
-			f := ObjectFile{m: tc.mapping}
+			f := ObjectFile{ElfFile: tc.file, m: tc.mapping}
 			err := f.computeBase(tc.addr)
 			if (err != nil) != tc.wantError {
 				t.Errorf("got error %v, want any error=%v", err, tc.wantError)