pprof icon indicating copy to clipboard operation
pprof copied to clipboard

internal/driver/fetch: searched files list looks weird

Open egonelbre opened this issue 3 years ago • 4 comments

Based on c488b8fa1db3fa467bf30beb5a1d6f4f10bb1b87.

I'm currently working through and trying to get ASLR disassembly support working in Go + Windows. One of the weird things I noticed is the line https://github.com/google/pprof/blob/c488b8fa1db3fa467bf30beb5a1d6f4f10bb1b87/internal/driver/fetch.go#L420.

The fileNames = append(fileNames, filepath.Join(path, m.File)) doesn't make sense when m.File is assumed to be absolute. Is that a typo and should it be fileNames = append(fileNames, m.File)? The comment does seems to indicate the latter. Or does it have some other purpose (e.g. compatibility in case the m.File is not absolute).

egonelbre avatar Jul 11 '22 15:07 egonelbre

Similarly I found that https://github.com/google/pprof/blob/c488b8fa1db3fa467bf30beb5a1d6f4f10bb1b87/internal/driver/fetch.go#L379 uses url.Parse to detect whether m.File is a URL, however that fails with Windows paths, because:

fmt.Println(url.Parse(`C:\example\test.exe`))
// Output: c:\example\test.exe, <nil>

Maybe it would probably make sense to use "has prefix http" or similar, rather than use url.Parse.

egonelbre avatar Jul 11 '22 15:07 egonelbre

There is a comment there:

// Try both the basename and the full path, to support the same directory
// structure as the perf symfs option.

aalexand avatar Jul 11 '22 16:07 aalexand

@aalexand I'm not quite following. If m.File = "C:\example\test.exe" then the filepath.Join(path, m.File) == "C:\UserData\egon\pprof\binaries\C:\example\test.exe", which is not a valid filepath.

egonelbre avatar Jul 11 '22 17:07 egonelbre

@egonelbre Ah, right - the volume name needs to be trimmed from m.File before appending it.

aalexand avatar Jul 11 '22 17:07 aalexand