Set base address on OSX (#313)

Set base address on OSX.

This fixes issue #311. The mapped address of libraries was never considered, nor was the load address of the segments. It also fixes a couple of issues in binutils.go, such as when the tail of the data wasn't handled right when grouping symbols at the same address.

internal/binutils/binutils.go

@@ -175,20 +175,35 @@ func (bu *Binutils) Open(name string, start, limit, offset uint64) (plugin.ObjFi
 func (b *binrep) openMachO(name string, start, limit, offset uint64) (plugin.ObjFile, error) {
 	of, err := macho.Open(name)
 	if err != nil {
-		return nil, fmt.Errorf("Parsing %s: %v", name, err)
+		return nil, fmt.Errorf("error parsing %s: %v", name, err)
 	}
 	defer of.Close()
 
+	// Subtract the load address of the __TEXT section. Usually 0 for shared
+	// libraries or 0x100000000 for executables. You can check this value by
+	// running `objdump -private-headers <file>`.
+
+	textSegment := of.Segment("__TEXT")
+	if textSegment == nil {
+		return nil, fmt.Errorf("could not identify base for %s: no __TEXT segment", name)
+	}
+	if textSegment.Addr > start {
+		return nil, fmt.Errorf("could not identify base for %s: __TEXT segment address (0x%x) > mapping start address (0x%x)",
+			name, textSegment.Addr, start)
+	}
+
+	base := start - textSegment.Addr
+
 	if b.fast || (!b.addr2lineFound && !b.llvmSymbolizerFound) {
-		return &fileNM{file: file{b: b, name: name}}, nil
+		return &fileNM{file: file{b: b, name: name, base: base}}, nil
 	}
-	return &fileAddr2Line{file: file{b: b, name: name}}, nil
+	return &fileAddr2Line{file: file{b: b, name: name, base: base}}, nil
 }
 
 func (b *binrep) openELF(name string, start, limit, offset uint64) (plugin.ObjFile, error) {
 	ef, err := elf.Open(name)
 	if err != nil {
-		return nil, fmt.Errorf("Parsing %s: %v", name, err)
+		return nil, fmt.Errorf("error parsing %s: %v", name, err)
 	}
 	defer ef.Close()
 
@@ -217,7 +232,7 @@ func (b *binrep) openELF(name string, start, limit, offset uint64) (plugin.ObjFi
 
 	base, err := elfexec.GetBase(&ef.FileHeader, elfexec.FindTextProgHeader(ef), stextOffset, start, limit, offset)
 	if err != nil {
-		return nil, fmt.Errorf("Could not identify base for %s: %v", name, err)
+		return nil, fmt.Errorf("could not identify base for %s: %v", name, err)
 	}
 
 	buildID := ""

internal/binutils/binutils_test.go

@@ -177,14 +177,20 @@ func TestSetFastSymbolization(t *testing.T) {
 
 func skipUnlessLinuxAmd64(t *testing.T) {
 	if runtime.GOOS != "linux" || runtime.GOARCH != "amd64" {
-		t.Skip("Disasm only tested on x86-64 linux")
+		t.Skip("This test only works on x86-64 Linux")
+	}
+}
+
+func skipUnlessDarwinAmd64(t *testing.T) {
+	if runtime.GOOS != "darwin" || runtime.GOARCH != "amd64" {
+		t.Skip("This test only works on x86-64 Mac")
 	}
 }
 
 func TestDisasm(t *testing.T) {
 	skipUnlessLinuxAmd64(t)
 	bu := &Binutils{}
-	insts, err := bu.Disasm(filepath.Join("testdata", "hello"), 0, math.MaxUint64)
+	insts, err := bu.Disasm(filepath.Join("testdata", "exe_linux_64"), 0, math.MaxUint64)
 	if err != nil {
 		t.Fatalf("Disasm: unexpected error %v", err)
 	}
@@ -199,6 +205,17 @@ func TestDisasm(t *testing.T) {
 	}
 }
 
+func findSymbol(syms []*plugin.Sym, name string) *plugin.Sym {
+	for _, s := range syms {
+		for _, n := range s.Name {
+			if n == name {
+				return s
+			}
+		}
+	}
+	return nil
+}
+
 func TestObjFile(t *testing.T) {
 	skipUnlessLinuxAmd64(t)
 	for _, tc := range []struct {
@@ -215,7 +232,7 @@ func TestObjFile(t *testing.T) {
 	} {
 		t.Run(tc.desc, func(t *testing.T) {
 			bu := &Binutils{}
-			f, err := bu.Open(filepath.Join("testdata", "hello"), tc.start, tc.limit, tc.offset)
+			f, err := bu.Open(filepath.Join("testdata", "exe_linux_64"), tc.start, tc.limit, tc.offset)
 			if err != nil {
 				t.Fatalf("Open: unexpected error %v", err)
 			}
@@ -225,17 +242,7 @@ func TestObjFile(t *testing.T) {
 				t.Fatalf("Symbols: unexpected error %v", err)
 			}
 
-			find := func(name string) *plugin.Sym {
-				for _, s := range syms {
-					for _, n := range s.Name {
-						if n == name {
-							return s
-						}
-					}
-				}
-				return nil
-			}
-			m := find("main")
+			m := findSymbol(syms, "main")
 			if m == nil {
 				t.Fatalf("Symbols: did not find main")
 			}
@@ -255,6 +262,65 @@ func TestObjFile(t *testing.T) {
 	}
 }
 
+func TestMachoFiles(t *testing.T) {
+	skipUnlessDarwinAmd64(t)
+
+	t.Skip("Disabled because of issues with addr2line (see https://github.com/google/pprof/pull/313#issuecomment-364073010)")
+
+	// Load `file`, pretending it was mapped at `start`. Then get the symbol
+	// table. Check that it contains the symbol `sym` and that the address
+	// `addr` gives the `expected` stack trace.
+	for _, tc := range []struct {
+		desc                 string
+		file                 string
+		start, limit, offset uint64
+		addr                 uint64
+		sym                  string
+		expected             []plugin.Frame
+	}{
+		{"normal mapping", "exe_mac_64", 0x100000000, math.MaxUint64, 0,
+			0x100000f50, "_main",
+			[]plugin.Frame{
+				{Func: "main", File: "/tmp/hello.c", Line: 3},
+			}},
+		{"other mapping", "exe_mac_64", 0x200000000, math.MaxUint64, 0,
+			0x200000f50, "_main",
+			[]plugin.Frame{
+				{Func: "main", File: "/tmp/hello.c", Line: 3},
+			}},
+		{"lib normal mapping", "lib_mac_64", 0, math.MaxUint64, 0,
+			0xfa0, "_bar",
+			[]plugin.Frame{
+				{Func: "bar", File: "/tmp/lib.c", Line: 6},
+			}},
+	} {
+		t.Run(tc.desc, func(t *testing.T) {
+			bu := &Binutils{}
+			f, err := bu.Open(filepath.Join("testdata", tc.file), tc.start, tc.limit, tc.offset)
+			if err != nil {
+				t.Fatalf("Open: unexpected error %v", err)
+			}
+			defer f.Close()
+			syms, err := f.Symbols(nil, 0)
+			if err != nil {
+				t.Fatalf("Symbols: unexpected error %v", err)
+			}
+
+			m := findSymbol(syms, tc.sym)
+			if m == nil {
+				t.Fatalf("Symbols: could not find symbol %v", tc.sym)
+			}
+			gotFrames, err := f.SourceLine(tc.addr)
+			if err != nil {
+				t.Fatalf("SourceLine: unexpected error %v", err)
+			}
+			if !reflect.DeepEqual(gotFrames, tc.expected) {
+				t.Fatalf("SourceLine for main: got %v; want %v\n", gotFrames, tc.expected)
+			}
+		})
+	}
+}
+
 func TestLLVMSymbolizer(t *testing.T) {
 	if runtime.GOOS != "linux" {
 		t.Skip("testtdata/llvm-symbolizer has only been tested on linux")

internal/binutils/disasm.go

@@ -34,24 +34,48 @@ var (
 func findSymbols(syms []byte, file string, r *regexp.Regexp, address uint64) ([]*plugin.Sym, error) {
 	// Collect all symbols from the nm output, grouping names mapped to
 	// the same address into a single symbol.
+
+	// The symbols to return.
 	var symbols []*plugin.Sym
+
+	// The current group of symbol names, and the address they are all at.
 	names, start := []string{}, uint64(0)
+
 	buf := bytes.NewBuffer(syms)
-	for symAddr, name, err := nextSymbol(buf); err == nil; symAddr, name, err = nextSymbol(buf) {
+
+	for {
+		symAddr, name, err := nextSymbol(buf)
+		if err == io.EOF {
+			// Done. If there was an unfinished group, append it.
+			if len(names) != 0 {
+				if match := matchSymbol(names, start, symAddr-1, r, address); match != nil {
+					symbols = append(symbols, &plugin.Sym{Name: match, File: file, Start: start, End: symAddr - 1})
+				}
+			}
+
+			// And return the symbols.
+			return symbols, nil
+		}
+
 		if err != nil {
+			// There was some kind of serious error reading nm's output.
 			return nil, err
 		}
-		if start == symAddr {
+
+		// If this symbol is at the same address as the current group, add it to the group.
+		if symAddr == start {
 			names = append(names, name)
 			continue
 		}
+
+		// Otherwise append the current group to the list of symbols.
 		if match := matchSymbol(names, start, symAddr-1, r, address); match != nil {
 			symbols = append(symbols, &plugin.Sym{Name: match, File: file, Start: start, End: symAddr - 1})
 		}
+
+		// And start a new group.
 		names, start = []string{name}, symAddr
 	}
-
-	return symbols, nil
 }
 
 // matchSymbol checks if a symbol is to be selected by checking its
@@ -62,7 +86,7 @@ func matchSymbol(names []string, start, end uint64, r *regexp.Regexp, address ui
 		return names
 	}
 	for _, name := range names {
-		if r.MatchString(name) {
+		if r == nil || r.MatchString(name) {
 			return []string{name}
 		}