Improve regexp matching of mappings for legacy formats

Simplify regexps and define a recommended format:
  Start   End     object file name     offset(optional)   linker build id
  0x40000-0x80000 /path/to/binary      (@FF00)            abc123456

Also include some additional tests and move existing tests to legacy_profile_test.go,
closer to the code in legacy_profile.go.

profile/legacy_profile.go

@@ -46,9 +46,23 @@ var (
 	threadzStartRE = regexp.MustCompile(`--- threadz \d+ ---`)
 	threadStartRE  = regexp.MustCompile(`--- Thread ([[:xdigit:]]+) \(name: (.*)/(\d+)\) stack: ---`)
 
-	procMapsRE = regexp.MustCompile(`([[:xdigit:]]+)-([[:xdigit:]]+)\s+([-rwxp]+)\s+([[:xdigit:]]+)\s+([[:xdigit:]]+):([[:xdigit:]]+)\s+([[:digit:]]+)\s*(\S+)?`)
-
-	briefMapsRE = regexp.MustCompile(`\s*([[:xdigit:]]+)-([[:xdigit:]]+):\s*(\S+)(\s.*@)?([[:xdigit:]]+)?`)
+	// Regular expressions to parse process mappings. Support the format used by Linux /proc/.../maps and other tools.
+	// Recommended format:
+	// Start   End     object file name     offset(optional)   linker build id
+	// 0x40000-0x80000 /path/to/binary      (@FF00)            abc123456
+	spaceDigits = `\s+[[:digit:]]+`
+	hexPair     = `\s+[[:xdigit:]]+:[[:xdigit:]]+`
+	oSpace      = `\s*`
+	// Capturing expressions.
+	cHex           = `(?:0x)?([[:xdigit:]]+)`
+	cHexRange      = `\s*` + cHex + `[\s-]?` + oSpace + cHex + `:?`
+	cSpaceString   = `(?:\s+(\S+))?`
+	cSpaceHex      = `\s+([[:xdigit:]]+)`
+	cSpaceAtOffset = `(?:\s+\(@([[:xdigit:]]+)\))?`
+	cPerm          = `(?:\s+([-rwxp]+))?`
+
+	procMapsRE  = regexp.MustCompile(`^` + cHexRange + cPerm + cSpaceHex + hexPair + spaceDigits + cSpaceString + `\s*$`)
+	briefMapsRE = regexp.MustCompile(`^` + cHexRange + cPerm + cSpaceString + cSpaceAtOffset + cSpaceString + `\s*$`)
 )
 
 func isSpaceOrComment(line string) bool {
@@ -1008,45 +1022,36 @@ func (p *Profile) parseMemoryMapFromScanner(s *bufio.Scanner) error {
 }
 
 func parseMappingEntry(l string) (*Mapping, error) {
-	mapping := &Mapping{}
-	var err error
-	if me := procMapsRE.FindStringSubmatch(l); len(me) == 9 {
-		if !strings.Contains(me[3], "x") {
-			// Skip non-executable entries.
-			return nil, nil
-		}
-		if mapping.Start, err = strconv.ParseUint(me[1], 16, 64); err != nil {
-			return nil, errUnrecognized
-		}
-		if mapping.Limit, err = strconv.ParseUint(me[2], 16, 64); err != nil {
-			return nil, errUnrecognized
-		}
-		if me[4] != "" {
-			if mapping.Offset, err = strconv.ParseUint(me[4], 16, 64); err != nil {
-				return nil, errUnrecognized
-			}
-		}
-		mapping.File = me[8]
-		return mapping, nil
+	var start, end, perm, file, offset, buildID string
+	if me := procMapsRE.FindStringSubmatch(l); len(me) == 6 {
+		start, end, perm, offset, file = me[1], me[2], me[3], me[4], me[5]
+	} else if me := briefMapsRE.FindStringSubmatch(l); len(me) == 7 {
+		start, end, perm, file, offset, buildID = me[1], me[2], me[3], me[4], me[5], me[6]
+	} else {
+		return nil, errUnrecognized
 	}
 
-	if me := briefMapsRE.FindStringSubmatch(l); len(me) == 6 {
-		if mapping.Start, err = strconv.ParseUint(me[1], 16, 64); err != nil {
-			return nil, errUnrecognized
-		}
-		if mapping.Limit, err = strconv.ParseUint(me[2], 16, 64); err != nil {
+	var err error
+	mapping := &Mapping{
+		File:    file,
+		BuildID: buildID,
+	}
+	if perm != "" && !strings.Contains(perm, "x") {
+		// Skip non-executable entries.
+		return nil, nil
+	}
+	if mapping.Start, err = strconv.ParseUint(start, 16, 64); err != nil {
+		return nil, errUnrecognized
+	}
+	if mapping.Limit, err = strconv.ParseUint(end, 16, 64); err != nil {
+		return nil, errUnrecognized
+	}
+	if offset != "" {
+		if mapping.Offset, err = strconv.ParseUint(offset, 16, 64); err != nil {
 			return nil, errUnrecognized
 		}
-		mapping.File = me[3]
-		if me[5] != "" {
-			if mapping.Offset, err = strconv.ParseUint(me[5], 16, 64); err != nil {
-				return nil, errUnrecognized
-			}
-		}
-		return mapping, nil
 	}
-
-	return nil, errUnrecognized
+	return mapping, nil
 }
 
 type sectionType int

profile/legacy_profile_test.go

@@ -118,3 +118,110 @@ func profileOfType(sampleTypes []string) *Profile {
 	}
 	return p
 }
+
+func TestParseMappingEntry(t *testing.T) {
+	for _, test := range []*struct {
+		entry string
+		want  *Mapping
+	}{
+		{
+			entry: "00400000-02e00000 r-xp 00000000 00:00 0",
+			want: &Mapping{
+				Start: 0x400000,
+				Limit: 0x2e00000,
+			},
+		},
+		{
+			entry: "02e00000-02e8a000 r-xp 02a00000 00:00 15953927    /foo/bin",
+			want: &Mapping{
+				Start:  0x2e00000,
+				Limit:  0x2e8a000,
+				Offset: 0x2a00000,
+				File:   "/foo/bin",
+			},
+		},
+		{
+			entry: "02e00000-02e8a000 r-xp 000000 00:00 15953927    [vdso]",
+			want: &Mapping{
+				Start: 0x2e00000,
+				Limit: 0x2e8a000,
+				File:  "[vdso]",
+			},
+		},
+		{
+			entry: "  02e00000-02e8a000: /foo/bin (@2a00000)",
+			want: &Mapping{
+				Start:  0x2e00000,
+				Limit:  0x2e8a000,
+				Offset: 0x2a00000,
+				File:   "/foo/bin",
+			},
+		},
+		{
+			entry: "  02e00000-02e8a000: /foo/bin",
+			want: &Mapping{
+				Start: 0x2e00000,
+				Limit: 0x2e8a000,
+				File:  "/foo/bin",
+			},
+		},
+		{
+			entry: "  02e00000-02e8a000: [vdso]",
+			want: &Mapping{
+				Start: 0x2e00000,
+				Limit: 0x2e8a000,
+				File:  "[vdso]",
+			},
+		},
+		{entry: "0xff6810563000 0xff6810565000 r-xp abc_exe 87c4d547f895cfd6a370e08dc5c5ee7bd4199d5b",
+			want: &Mapping{
+				Start:   0xff6810563000,
+				Limit:   0xff6810565000,
+				File:    "abc_exe",
+				BuildID: "87c4d547f895cfd6a370e08dc5c5ee7bd4199d5b",
+			},
+		},
+		{entry: "7f5e5435e000-7f5e5455e000 --xp 00002000 00:00 1531        myprogram",
+			want: &Mapping{
+				Start:  0x7f5e5435e000,
+				Limit:  0x7f5e5455e000,
+				Offset: 0x2000,
+				File:   "myprogram",
+			},
+		},
+		{entry: "7f7472710000-7f7472722000 r-xp 00000000 fc:00 790190      /usr/lib/libfantastic-1.2.so",
+			want: &Mapping{
+				Start: 0x7f7472710000,
+				Limit: 0x7f7472722000,
+				File:  "/usr/lib/libfantastic-1.2.so",
+			},
+		},
+		{entry: "7f47a542f000-7f47a5447000: /lib/libpthread-2.15.so",
+			want: &Mapping{
+				Start: 0x7f47a542f000,
+				Limit: 0x7f47a5447000,
+				File:  "/lib/libpthread-2.15.so",
+			},
+		},
+		{entry: "0x40000-0x80000 /path/to/binary      (@FF00)            abc123456",
+			want: &Mapping{
+				Start:   0x40000,
+				Limit:   0x80000,
+				File:    "/path/to/binary",
+				Offset:  0xFF00,
+				BuildID: "abc123456",
+			},
+		},
+		{entry: "7f5e5435e000-7f5e5455e000 ---p 00002000 00:00 1531        myprogram",
+			want: nil,
+		},
+	} {
+		got, err := parseMappingEntry(test.entry)
+		if err != nil {
+			t.Error(err)
+		}
+		if !reflect.DeepEqual(test.want, got) {
+			t.Errorf("%s want=%v got=%v", test.entry, test.want, got)
+		}
+	}
+}

profile/profile_test.go

@@ -19,7 +19,6 @@ import (
 	"fmt"
 	"io/ioutil"
 	"path/filepath"
-	"reflect"
 	"regexp"
 	"strings"
 	"testing"
@@ -361,71 +360,6 @@ func checkAggregation(prof *Profile, a *aggTest) error {
 	return nil
 }
 
-func TestParseMappingEntry(t *testing.T) {
-	for _, test := range []*struct {
-		entry string
-		want  *Mapping
-	}{
-		{
-			entry: "00400000-02e00000 r-xp 00000000 00:00 0",
-			want: &Mapping{
-				Start: 0x400000,
-				Limit: 0x2e00000,
-			},
-		},
-		{
-			entry: "02e00000-02e8a000 r-xp 02a00000 00:00 15953927    /foo/bin",
-			want: &Mapping{
-				Start:  0x2e00000,
-				Limit:  0x2e8a000,
-				Offset: 0x2a00000,
-				File:   "/foo/bin",
-			},
-		},
-		{
-			entry: "02e00000-02e8a000 r-xp 000000 00:00 15953927    [vdso]",
-			want: &Mapping{
-				Start: 0x2e00000,
-				Limit: 0x2e8a000,
-				File:  "[vdso]",
-			},
-		},
-		{
-			entry: "  02e00000-02e8a000: /foo/bin (@2a00000)",
-			want: &Mapping{
-				Start:  0x2e00000,
-				Limit:  0x2e8a000,
-				Offset: 0x2a00000,
-				File:   "/foo/bin",
-			},
-		},
-		{
-			entry: "  02e00000-02e8a000: /foo/bin",
-			want: &Mapping{
-				Start: 0x2e00000,
-				Limit: 0x2e8a000,
-				File:  "/foo/bin",
-			},
-		},
-		{
-			entry: "  02e00000-02e8a000: [vdso]",
-			want: &Mapping{
-				Start: 0x2e00000,
-				Limit: 0x2e8a000,
-				File:  "[vdso]",
-			},
-		},
-	} {
-		got, err := parseMappingEntry(test.entry)
-		if err != nil {
-			t.Error(err)
-		}
-		if !reflect.DeepEqual(test.want, got) {
-			t.Errorf("%s want=%v got=%v", test.entry, test.want, got)
-		}
-	}
-}
-
 // Test merge leaves the main binary in place.
 func TestMergeMain(t *testing.T) {
 	prof := testProfile.Copy()