Added files to enable fuzz testing and fixed bug found with fuzzing (#150)
* Added files to enable fuzz testing
* Added test for crashes found with fuzzing
* fixed divide-by-zero error in profile/legacy_java_profile.go found with fuzzing
When pruning unsimplified names, don't trim "(anonymous namespace)". (#140)
* When pruning unsimplified names, don't trim "(anonymous namespace)".
Fixes #139.
* Fix copy/paste. Add test.
* Check for braces with a RE. Add more complex test cases. And operator().
* Fix last-minute debug.
* Construct bracketRx more cleanly
* spelling
* Redundant nil check
* While we're removing things...
Add support for terabyte and petabyte memory units. (#138)
* Add support for terabyte and petabyte memory units.
This fixes #137 (but not #136, this will come separately once we decide
what we want to do if anything).
* Add a test.
* Update copyright year
Create a fake mapping for profile.proto profiles (#135)
* Create a fake mapping for profile.proto profiles
If a profile has mappings but no profiles, pprof may be unable to
symbolize it offline, as it uses the mappings to keep track of which
locations need symbolization.
This fixes #120.
Added the test, verified it fails on Mac with Go 1.7 before the fix, and
passes with the fix. The test is done by augmenting the existing test
for handling https+insecure:// schema in URLs. This is a bit vague but I
figured that this test needed an updated anyway since as we moved it
recently we stopped exercising the symbolization as part of the test
which was its original intention in fixing #94. Can split the tests if
things do look too ugly.
* Fix the test to include the failed regex matching error in the message.
There is an issue where decreasing nodefraction (which should produce
more nodes) ended up reducing the number of displayed nodes. This is
because we have an upper limit on number of nodes to display and the
number of node-lets counts against that limit. This can cause a problem
if a node with large number of node-lets shows up near the front of
the list of nodes (e.g., std::allocator_traits::allocate in a heapz
profile had 102 node-lets, which caused all subsequent nodes to be
dropped).
Fixed by limiting the count of charged node-lets per node to
maxNodelets (4), which is the maximum number of node-lets we
display per node.
* Align file names in weblist disassembly
The disassembly displayed by weblist attempts to align file:line
information after each instruction, but two things interfere with this
alignment: 1) the instruction text often has tabs in it, which defeats
the length-based alignment and generally interacts poorly with HTML
rendering and 2) the instruction text often has special HTML
characters like < and > in it, but we pad the string after escaping
it, so the length is again not representative of how it will display.
For example, the following shows what this looks like for disassembly
that contains < and >:
. . 41c634: cmp $0x200,%rdx mgcsweepbuf.go:130
. . 41c63b: jae 41c64f <runtime.(*gcSweepBuf).pop+0x4f> mgcsweepbuf.go:130
. . 41c63d: mov (%rax,%rdx,8),%rcx mgcsweepbuf.go:130
. . 41c64f: callq 424800 <runtime.panicindex> mgcsweepbuf.go:130
. . 41c654: ud2 mgcsweepbuf.go:130
Fix these problems by replacing tab characters with spaces and padding
the string before escaping it. After this, the file names are properly
aligned:
. . 41c634: cmp $0x200,%rdx mgcsweepbuf.go:130
. . 41c63b: jae 41c64f <runtime.(*gcSweepBuf).pop+0x4f> mgcsweepbuf.go:130
. . 41c63d: mov (%rax,%rdx,8),%rcx mgcsweepbuf.go:130
. . 41c64f: callq 424800 <runtime.panicindex> mgcsweepbuf.go:130
. . 41c654: ud2 mgcsweepbuf.go:130
* Separate discontiguous assembly blocks
Currently, weblist's disassembled output for each line simply lists
all instructions that are marked with that line number. This leads to
confusing disassembly because a lot of things look like straight-line
control flow when they aren't. For example, index panics look like
they always happen:
. . 41c62c: test %al,(%rax)
. . 41c62e: and $0x1ff,%edx
. . 41c634: cmp $0x200,%rdx
. . 41c63b: jae 41c64f <runtime.(*gcSweepBuf).pop+0x4f>
. . 41c63d: mov (%rax,%rdx,8),%rcx
. . 41c64f: callq 424800 <runtime.panicindex>
. . 41c654: ud2
In reality, 41c64f is at the end of the function, but it looks like we
call it immediately after a successful index operation.
Fix this by adding a vertical ellipses to separate blocks of assembly
that have other instructions between them. With this change, the above
looks like:
. . 41c62c: test %al,(%rax)
. . 41c62e: and $0x1ff,%edx
. . 41c634: cmp $0x200,%rdx
. . 41c63b: jae 41c64f <runtime.(*gcSweepBuf).pop+0x4f>
. . 41c63d: mov (%rax,%rdx,8),%rcx
⋮
. . 41c64f: callq 424800 <runtime.panicindex>
. . 41c654: ud2
Use a more regular mechanism to name sources during driver tests and
recognize the naming during fetching to avoid saving the test profiles
into $HOME/pprof.
Also move the https+insecure testing into fetch_test, to focus it on
fetching the profile.
This fixes #107
Measure coverage of tests using codecov.io. (#123)
* Measure coverage of tests using codecov.io.
* Add codecov badge to README.md
* Recurse into packages when testing with coverage.
* Push testing kind of commands down to test.sh
internal/report: change the format of tags command output
This change also includes:
- Instead of printing and sorting based on the first value in the
internal value slice, use the sample type user selected.
- Use the unit and print numbers in human friendly version.
- Add a flag '-update' to internal/driver/driver_test to update the
golden files.
TODO: apply to other tests that use other golden files.
Handle shared libraries with nonzero program headers. (#119)
Generalize GetBase for shared libraries to handle cases when both process
mapping offset and program header addresses are non-zero.
Based on process mapping information, a sample at a virtual address x maps to
a file offset fx = x - map_start + map_offset.
The program header for the segment with the .text section may have non-zero
mapping information. Thus, a file offset fx maps to a symbol address
sx = fx - ph_offset + ph_virt_addr.
Thus, sx = x - map_start + map_offset - ph_offset + ph_virt_addr, and the base
address for symbolization is map_start - map_offset + ph_offset - ph_virt_addr.
Recognize (again) https+insecure scheme for symbolization POSTs. (#121)
* Recognize (again) https+insecure scheme for symbolization POSTs.
This reverts the revert of #111 relaxing the test to make it pass as the
issue reported in #111 is caused by unrelated #120.
Added a test, verified the test failed before the fix.
This fixes #94.
* Fix the staticcheck failure.
*: golint, go tool vet, unsued, gosimple, staticcheck (#113)
* `git ls-files -- '*.go' | xargs gofmt -s -w`
* driver: unexport InternalOptions
This was an exported method that returned an internal type, so it's
unlikely that anyone depends on its presence.
* internal/binutils: remove newline in error string
golint reported: error strings should not be capitalized or end with
punctuation or a newline
* internal/driver: s/buildId/buildID/ per golint
* internal/elfexec: remove punctuation in error string
golint reported: error strings should not be capitalized or end with
punctuation or a newline
* internal/graph: correct comment
Found with golint.
* internal/graph: add method comment
golint reported: exported method Edge.WeightValue should have comment
or be unexported
* internal/report: remove newline in error string
golint reported: error strings should not be capitalized or end with
punctuation or a newline
* *: remove unused code
Found with honnef.co/go/tools/cmd/unused:
internal/binutils/disasm_test.go:137:8: const objdump is unused (U1000)
internal/driver/driver_test.go:353:6: type testProfile is unused (U1000)
internal/graph/graph.go:838:6: func countEdges is unused (U1000)
profile/proto.go:148:6: func encodeStringOpt is unused (U1000)
* *: simply code
Found with honnef.co/go/tools/cmd/gosimple:
internal/driver/commands.go:471:24: should omit comparison to bool constant, can be simplified to !b (S1002)
internal/driver/driver.go:163:5: should omit comparison to bool constant, can be simplified to !trim (S1002)
internal/driver/driver.go:168:5: should omit comparison to bool constant, can be simplified to !focus (S1002)
internal/driver/driver.go:172:5: should omit comparison to bool constant, can be simplified to !tagfocus (S1002)
internal/driver/driver.go:176:5: should omit comparison to bool constant, can be simplified to !hide (S1002)
internal/graph/dotgraph.go:213:34: should use make(map[string][]*Tag) instead (S1019)
internal/report/report.go:1061:3: should replace loop with label = append(label, rpt.options.ProfileLabels...) (S1011)
profile/proto.go:157:5: should omit comparison to bool constant, can be simplified to !x (S1002)
* *: correct mistakes
Found with honnef.co/go/tools/cmd/staticcheck:
driver/driver.go:280:20: infinite recursive call (SA5007)
internal/driver/driver_focus.go:54:11: this value of err is never used (SA4006)
profile/proto.go:74:32: x | 0 always equals x (SA4016)
* Add linters to .travis.yml
Go 1.6 currently fails the tests with build error
`profile/legacy_profile_test.go:309: t.Run undefined (type *testing.T has no field or method Run)`
Alternatively, we can fix it but I am not sure there is a strong reason
to support Go 1.6 or older.
Also add build status badge in the README.md file.
Remove filenames when showing function (default) granularity
Previously we were showing filenames together with function names
when generating reports at function granularity. This was an attempt
to provide more information to the user, but it clutters the output.
Also, in cases where a function is associated to multiple files
(eg template instantiations in C++), keeping the filename may prevent
us from combining nodes from the same function.
Will stop keeping filenames at the default function granularity, and will
remove the "functionnameonly" option which was previously providing this
functionality.
Do not honor call_tree option when generating non-visual reports.
There is a check to only honor the call_tree option when generating non-visual
reports, but it wasn't being checked in all appropriate places, causing a
mismatch that triggered a panic.
In changeset f90721db3d, fetch.go was changed to handle
the correct home directory environment variable on Plan 9
and Windows, but the tests were still using the $HOME
environment variable.
We change the tests to use the homeEnv function instead
of the $HOME environment variable.
Fixes #100.