Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
70 changes: 13 additions & 57 deletions .github/workflows/ci.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,10 @@ on:
pull_request:
schedule:
- cron: '0 2 * * *' # Run every day, at 2AM UTC.

permissions:
contents: read

env:
GOPATH: ${{ github.workspace }}
WORKING_DIR: ./src/github.com/google/pprof/
Expand All @@ -18,7 +22,7 @@ jobs:
strategy:
fail-fast: false
matrix:
go: ['1.24', '1.25', 'tip']
go: ['1.25', '1.26']
# Supported macOS versions can be found in
# https://github.com/actions/virtual-environments#available-environments.
os: ['macos-14', 'macos-15']
Expand Down Expand Up @@ -48,43 +52,19 @@ jobs:

steps:
- name: Checkout the repo
uses: actions/checkout@9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0 # v7.0.0
uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1
with:
path: ${{ env.WORKING_DIR }}

- name: Update Go version using setup-go
uses: actions/setup-go@924ae3a1cded613372ab5595356fb5720e22ba16 # v6.5.0
if: matrix.go != 'tip'
uses: actions/setup-go@b7ad1dad31e06c5925ef5d2fc7ad053ef454303e # v7.0.0
with:
# Include cache directives to allow proper caching. Without them, we
# get setup-go "Restore cache failed" warnings.
go-version: ${{ matrix.go }}
cache: true
cache-dependency-path: '**/go.sum'

- name: Install Go bootstrap compiler
uses: actions/setup-go@924ae3a1cded613372ab5595356fb5720e22ba16 # v6.5.0
if: matrix.go == 'tip'
with:
# Bootstrapping go tip requires 1.24
# Include cache directives to allow proper caching. Without them, we
# get setup-go "Restore cache failed" warnings.
go-version: 1.24
cache: true
cache-dependency-path: '**/go.sum'

- name: Update Go version manually
if: matrix.go == 'tip'
working-directory: ${{ github.workspace }}
run: |
git clone https://go.googlesource.com/go $HOME/gotip
cd $HOME/gotip/src
./make.bash
echo "GOROOT=$HOME/gotip" >> $GITHUB_ENV
echo "RUN_STATICCHECK=false" >> $GITHUB_ENV
echo "RUN_GOLANGCI_LINTER=false" >> $GITHUB_ENV
echo "$HOME/gotip/bin:$PATH" >> $GITHUB_PATH

- name: Set up Xcode
uses: maxim-lobanov/setup-xcode@ed7a3b1fda3918c0306d1b724322adc0b8cc0a90 # v1.7.0
with:
Expand Down Expand Up @@ -120,47 +100,23 @@ jobs:
strategy:
fail-fast: false
matrix:
go: ['1.24', '1.25', 'tip']
go: ['1.25', '1.26']
os: ['ubuntu-24.04', 'ubuntu-22.04']
steps:
- name: Checkout the repo
uses: actions/checkout@9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0 # v7.0.0
uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1
with:
path: ${{ env.WORKING_DIR }}

- name: Update Go version using setup-go
uses: actions/setup-go@924ae3a1cded613372ab5595356fb5720e22ba16 # v6.5.0
if: matrix.go != 'tip'
uses: actions/setup-go@b7ad1dad31e06c5925ef5d2fc7ad053ef454303e # v7.0.0
with:
# Include cache directives to allow proper caching. Without them, we
# get setup-go "Restore cache failed" warnings.
go-version: ${{ matrix.go }}
cache: true
cache-dependency-path: '**/go.sum'

- name: Install Go bootstrap compiler
uses: actions/setup-go@924ae3a1cded613372ab5595356fb5720e22ba16 # v6.5.0
if: matrix.go == 'tip'
with:
# Bootstrapping go tip requires 1.24
# Include cache directives to allow proper caching. Without them, we
# get setup-go "Restore cache failed" warnings.
go-version: 1.24
cache: true
cache-dependency-path: '**/go.sum'

- name: Update Go version manually
if: matrix.go == 'tip'
working-directory: ${{ github.workspace }}
run: |
git clone https://go.googlesource.com/go $HOME/gotip
cd $HOME/gotip/src
./make.bash
echo "GOROOT=$HOME/gotip" >> $GITHUB_ENV
echo "RUN_STATICCHECK=false" >> $GITHUB_ENV
echo "RUN_GOLANGCI_LINTER=false" >> $GITHUB_ENV
echo "$HOME/gotip/bin" >> $GITHUB_PATH

- name: Check chrome for browser tests
run: |
google-chrome --version
Expand Down Expand Up @@ -214,15 +170,15 @@ jobs:
strategy:
fail-fast: false
matrix:
go: ['1.24', '1.25']
go: ['1.25', '1.26']
steps:
- name: Checkout the repo
uses: actions/checkout@9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0 # v7.0.0
uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1
with:
path: ${{ env.WORKING_DIR }}

- name: Update Go version using setup-go
uses: actions/setup-go@924ae3a1cded613372ab5595356fb5720e22ba16 # v6.5.0
uses: actions/setup-go@b7ad1dad31e06c5925ef5d2fc7ad053ef454303e # v7.0.0
with:
# Include cache directives to allow proper caching. Without them, we
# get setup-go "Restore cache failed" warnings.
Expand Down
4 changes: 1 addition & 3 deletions browsertests/go.mod
Original file line number Diff line number Diff line change
@@ -1,8 +1,6 @@
module github.com/google/pprof/browsertests

go 1.24.0

toolchain go1.24.9
go 1.25.0

// Use the version of pprof in this directory tree.
replace github.com/google/pprof => ../
Expand Down
4 changes: 1 addition & 3 deletions go.mod
Original file line number Diff line number Diff line change
@@ -1,8 +1,6 @@
module github.com/google/pprof

go 1.24.0

toolchain go1.24.9
go 1.25.0

require (
github.com/chzyer/readline v1.5.1
Expand Down
98 changes: 83 additions & 15 deletions internal/report/source.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ import (
"os"
"path/filepath"
"regexp"
"runtime"
"slices"
"sort"
"strconv"
Expand Down Expand Up @@ -1007,28 +1008,95 @@ func openSourceFile(path, searchPath, trim string) (*os.File, error) {
path = trimPath(path, trim, searchPath)
// If file is still absolute, require file to exist.
if filepath.IsAbs(path) {
f, err := os.Open(path)
return f, err
}
// Scan each component of the path.
for _, dir := range filepath.SplitList(searchPath) {
// Search up for every parent of each possible path.
for {
filename := filepath.Join(dir, path)
if f, err := os.Open(filename); err == nil {
return f, nil
}
parent := filepath.Dir(dir)
if parent == dir {
break
if f, err := tryOpenFile(path); err == nil {

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All this changes are round allowing to fallback later to other "handlers", for now we are adding the goroot handler, but in other PRs we will add more special handlers like gomodcache or vendor.

return f, nil
}
} else {
// Scan each component of the path.
for _, dir := range filepath.SplitList(searchPath) {
// Search up for every parent of each possible path.
for {
filename := filepath.Join(dir, path)
if f, err := tryOpenFile(filename); err == nil {
return f, nil
}
parent := filepath.Dir(dir)

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note to self: This is historical but I think we should remove using even parent path of the search paths. When I specify /foo/bar as the search path, I do not expect that the tool will attempt /foo/src/baz.cc paths or something like that. In particular, /foo/src can be a slow NFS / remote mounted directory and trying to look up those paths will slow down the scan.

if parent == dir {
break
}
dir = parent
}
dir = parent
}
}
// Fall back to looking for Go standard library sources under the local
// $GOROOT/src, since profiles from Go programs refer to standard library
// files under the GOROOT of the machine where the program was built.
if f, err := openGorootSourceFile(path, gorootSrc); err == nil {
return f, nil
}

return nil, fmt.Errorf("could not find file %s on path %s", path, searchPath)
}

// tryOpenFile opens the file at filename if it exists and is not a directory,
// which os.Open would also happily open.
func tryOpenFile(filename string) (*os.File, error) {

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fail fast when you try to open a directory.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Did you see cases where this practically happens? Asking because it seems unlikely that a source file path matches a directory name because source file names usually have an extension and directory names do not.

f, err := os.Open(filename)
if err != nil {
return nil, err
}
stat, err := f.Stat()
if err != nil {
f.Close()
return nil, err
}
if stat.IsDir() {
f.Close()
return nil, fmt.Errorf("%s is a directory", filename)
}
return f, nil
}

// gorootSrc is the directory holding the Go standard library sources, if
// known. runtime.GOROOT honors the $GOROOT environment variable and otherwise
// reports the GOROOT this binary was built with, which is the Go installation
// used to `go install` pprof in the common case.
var gorootSrc = func() string {
if goroot := runtime.GOROOT(); goroot != "" {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think pprof binary's runtime.GOROOT() can be different from the goroot of the profiled binary? Not sure we can guarantee that this is the right default.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the runtime.GOROOT() is used as the "fallback" in case the original reported file in the pprof hasn't been found. It is not "ideal", but is a reasonable fallback.

return filepath.Join(goroot, "src")
}
return ""
}()

// openGorootSourceFile tries to open a Go standard library source file under
// gorootSrc. Binaries built without -trimpath record standard library files
// under the build machine's GOROOT (e.g. /usr/local/go/src/runtime/proc.go),

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there a possibility we could instead 1) modify trimPath() below to trim the absolute path prefix; 2) add $GOROOT/src to the default search path? I assume that would be enough to locate the runtime files.

// which may not exist locally, so resolve the path components after a "/src/"
// component against gorootSrc. Binaries built with -trimpath record them
// relative to $GOROOT/src (e.g. runtime/proc.go), so resolve relative paths
// against gorootSrc directly.
func openGorootSourceFile(path, gorootSrc string) (*os.File, error) {

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is the special handling for goroot, only executed if the openSourceFile previous machinery doesn't find anything.

if gorootSrc == "" {
return nil, fmt.Errorf("GOROOT is not known")
}
if !filepath.IsAbs(path) {
if f, err := tryOpenFile(filepath.Join(gorootSrc, path)); err == nil {
return f, nil
}
}
sPath := filepath.ToSlash(path)
for {
i := strings.Index(sPath, "/src/")
if i == -1 {
return nil, fmt.Errorf("could not find file %s under %s", path, gorootSrc)
}
sPath = sPath[i+len("/src/"):]
if f, err := tryOpenFile(filepath.Join(gorootSrc, filepath.FromSlash(sPath))); err == nil {
return f, nil
}
}
}

// trimPath cleans up a path by removing prefixes that are commonly
// found on profiles plus configured prefixes.
// TODO(aalexand): Consider optimizing out the redundant work done in this
Expand Down
Loading