Browse Source

Allow profiles with no samples

A previous commit tried to eliminate a panic when reading an empty
profile by stopping support for profiles with no samples. That is
unnecessary and it can obstruct some testing.

The panic was caused by the legacy parser returning a nil profile and
no error when processing an empty profile. Detect that situation and
generate a proper profile instead.

Tested by running 'pprof /dev/null'
Add testcase for profile parsing errors
Raul Silvera 8 years ago
parent
commit
7e5698fd75
3 changed files with 28 additions and 8 deletions
  1. 0
    4
      internal/driver/driver.go
  2. 10
    4
      profile/legacy_profile.go
  3. 18
    0
      profile/profile_test.go

+ 0
- 4
internal/driver/driver.go View File

107
 		}
107
 		}
108
 	}
108
 	}
109
 
109
 
110
-	if len(p.Sample) == 0 {
111
-		return fmt.Errorf("profile is empty")
112
-	}
113
-
114
 	if err := aggregate(p, vars); err != nil {
110
 	if err := aggregate(p, vars); err != nil {
115
 		return err
111
 		return err
116
 	}
112
 	}

+ 10
- 4
profile/legacy_profile.go View File

446
 func parseHeap(b []byte) (p *Profile, err error) {
446
 func parseHeap(b []byte) (p *Profile, err error) {
447
 	s := bufio.NewScanner(bytes.NewBuffer(b))
447
 	s := bufio.NewScanner(bytes.NewBuffer(b))
448
 	if !s.Scan() {
448
 	if !s.Scan() {
449
-		return nil, s.Err()
449
+		if err := s.Err(); err != nil {
450
+			return nil, err
451
+		}
452
+		return nil, errUnrecognized
450
 	}
453
 	}
451
 	p = &Profile{}
454
 	p = &Profile{}
452
 
455
 
660
 func parseContention(b []byte) (p *Profile, err error) {
663
 func parseContention(b []byte) (p *Profile, err error) {
661
 	s := bufio.NewScanner(bytes.NewBuffer(b))
664
 	s := bufio.NewScanner(bytes.NewBuffer(b))
662
 	if !s.Scan() {
665
 	if !s.Scan() {
663
-		return nil, s.Err()
666
+		if err := s.Err(); err != nil {
667
+			return nil, err
668
+		}
669
+		return nil, errUnrecognized
664
 	}
670
 	}
665
 	line := s.Text()
671
 	line := s.Text()
666
 
672
 
912
 
918
 
913
 		addrs = append(addrs, parseHexAddresses(line)...)
919
 		addrs = append(addrs, parseHexAddresses(line)...)
914
 	}
920
 	}
915
-	if s.Err() != nil {
916
-		return "", nil, s.Err()
921
+	if err := s.Err(); err != nil {
922
+		return "", nil, err
917
 	}
923
 	}
918
 	if sameAsPrevious {
924
 	if sameAsPrevious {
919
 		return line, nil, nil
925
 		return line, nil, nil

+ 18
- 0
profile/profile_test.go View File

21
 	"path/filepath"
21
 	"path/filepath"
22
 	"reflect"
22
 	"reflect"
23
 	"regexp"
23
 	"regexp"
24
+	"strings"
24
 	"testing"
25
 	"testing"
25
 
26
 
26
 	"github.com/google/pprof/internal/proftest"
27
 	"github.com/google/pprof/internal/proftest"
90
 	}
91
 	}
91
 }
92
 }
92
 
93
 
94
+func TestParseError(t *testing.T) {
95
+
96
+	testcases := []string{
97
+		"",
98
+		"garbage text",
99
+		"\x1f\x8b", // truncated gzip header
100
+		"\x1f\x8b\x08\x08\xbe\xe9\x20\x58\x00\x03\x65\x6d\x70\x74\x79\x00\x03\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00", // empty gzipped file
101
+	}
102
+
103
+	for i, input := range testcases {
104
+		_, err := Parse(strings.NewReader(input))
105
+		if err == nil {
106
+			t.Errorf("got nil, want error for input #%d", i)
107
+		}
108
+	}
109
+}
110
+
93
 // leaveTempfile leaves |b| in a temporary file on disk and returns the
111
 // leaveTempfile leaves |b| in a temporary file on disk and returns the
94
 // temp filename. This is useful to recover a profile when the test
112
 // temp filename. This is useful to recover a profile when the test
95
 // fails.
113
 // fails.