Will panic soon when visit pprof web page

[fumeboy]

go1.19 amd64 macos

import (
    "net/http"
    _ "net/http/pprof"
)
func Test(t *testing.T)
    go func() {
        log.Println(http.ListenAndServe(":6060", nil))
    }()
        // call loaded module here

module is

package a

import "fmt"
import "time"

func main() (output int) {
    var m = map[string]int{}
    for i := 0; i < 100; i++ {
        m[fmt.Sprintf("%v", i)] = i
    }
    time.Sleep(time.Second / 1000)
    return m["1"]
}

after start, that is ok until I visit the pprof page http://127.0.0.1:6060/debug/pprof/heap?debug=1 then got a panic

[fumeboy]

func funcfile(f funcInfo, fileno int32) string {
    datap := f.datap
    if !f.valid() {
        return "?"
    }
    // Make sure the cu index and file offset are valid
    // println("debug", f.cuOffset, uint32(fileno), len(datap.cutab))
    if fileoff := datap.cutab[f.cuOffset+uint32(fileno)]; fileoff != ^uint32(0) {
        return gostringnocopy(&datap.filetab[fileoff])
    }
    // pcln section is corrupt.
    return "?"
}

panic at datap.cutab[f.cuOffset+uint32(fileno)] because fatal error: index out of range

[eh-steve]

(Fixed in my fork)

[fumeboy]

@eh-steve @pkujhd

While using pprof, I encountered another issue. When I extracted the function address from the symbol address map and forcefully cast it into a Go function, everything would work fine if I didn't refresh the pprof web page. However, if I refreshed the page a few times, it would panic after a few seconds.

Then, I tested it with eh-steve's repo and found that his program did not have this issue.

Upon comparing this repo with eh-steve's repo, I discovered that the difference was that eh-steve wrapped the function pointer with an interface, instead of directly using unsafe.Pointer for conversion.

below, I've listed two version snippets, where version 1 has error and version 2 works correctly.

version1 (bad): https://github.com/pkujhd/goloader/blob/master/examples/loader/loader.go#L91

// var fn = Fn[func() int](&module, "main")
// fn()

func Fn[T any](p *module, funcname string) T {
    const defaultpkgname = "<unlinkable>"

    fn := unsafe.Pointer(p.Symbols[defaultpkgname+"."+funcname])
    fnptr := (uintptr)(unsafe.Pointer(&fn))
    return *(*T)(unsafe.Pointer(&fnptr))
}

version2 (ok):

func Fn[T any](p *module, funcname string) T {
    const defaultpkgname = "<unlinkable>"

    fn_addr := unsafe.Pointer(p.Symbols[defaultpkgname+"."+funcname])

    var val interface{}
    {
        var fn T
        val = fn
        valp := (*[2]unsafe.Pointer)(unsafe.Pointer(&val))
        (*valp)[1] = unsafe.Pointer(&fn_addr)
    }

    return val.(T)
}

but I'm not clear about the underlying reason. If anyone knows, please let me know.

[eh-steve]

This issue is unrelated to the first one, likely to do with the unsafe.Pointer holding the function entrypoint being GC'd too early because of the cast to uintptr, whereas the interface{} will be treated correctly by the GC as it can be scanned properly. You'll probably find if you return the unsafe.Pointer too and keep a reference to it, it won't panic.

It's probably nothing to do with pprof, just that pprof is causing GCs to be triggered.

Also, in my fork, the correct *_type is already included into the SymbolsByPkg map[string]map[string]interface{} (via linker.buildExports(), so all the functions are guaranteed to have the correct type, whereas the second code you've posted is still unsafe as the type assertion will always succeed even if the function in the package isn't of the same type as that supplied by the host binary, since by definition the *_type will be that of the type supplied in the generic, not of whatever the function actually is.

[pkujhd]

This bug is the profiler to hold the export function's ptr.

Error caused by the profiler accessing invalid memory after module uninstallation.

The plugin module uses moduledata.ptab to store exported variables and functions. i consider using a similar solution to avoid issues, but it requires that the package name must be main and additional compilation parameters "-dynlink -complete -p packagename" must be added

On the branch of eh-steve, the compiler was modified to force it to export types

[eh-steve]

This bug is the profiler to hold the export function's ptr.

Error caused by the profiler accessing invalid memory after module uninstallation.

No it isn't? The first bug is due to the incorrect layout of pclntab and cutab (fixed on my branch), and the second bug is due to the GC of the value behind a uintptr funcptr that should have been an unsafe.Pointer or interface{}, not to do with any module unloading.

On the branch of eh-steve, the compiler was modified to force it to export types

My compiler patch makes it simpler to construct an interface{} from any exported symbols in any package regardless of whether it's main or not, and it prevents the GC from collecting funcptrs because they're stored in an interface{} with the correct type, but that isn't strictly required (if you pass around an unsafe.Pointer instead of uintptr, the second bug will probably not recur, even on your branch)

[pkujhd]

This bug is the profiler to hold the export function's ptr. Error caused by the profiler accessing invalid memory after module uninstallation.

No it isn't? The first bug is due to the incorrect layout of pclntab and cutab (fixed on my branch), and the second bug is due to the GC of the value behind a uintptr funcptr that should have been an unsafe.Pointer or interface{}, not to do with any module unloading.

On the branch of eh-steve, the compiler was modified to force it to export types

My compiler patch makes it simpler to construct an interface{} from any exported symbols in any package regardless of whether it's main or not, and it prevents the GC from collecting funcptrs because they're stored in an interface{} with the correct type, but that isn't strictly required (if you pass around an unsafe.Pointer instead of uintptr, the second bug will probably not recur, even on your branch)

the first panic with invalid PCFile are already fixed on commit https://github.com/pkujhd/goloader/commit/f7a07e8dd1eb5675f55e17f18d76660b86b9653f