cmd/compile: FuncForPC not returning correct function

[randall77]

package main

import (
	"reflect"
	"runtime"
)

func f(n int) int {
	return n % 2
}

func g(n int) int {
	return f(n)
}

func name(fn any) (res string) {
	return runtime.FuncForPC(uintptr(reflect.ValueOf(fn).Pointer())).Name()
}

func main() {
	println(name(f))
	println(name(g))
}

Prints

main.f
main.f

where it should print

main.f
main.g

This happens because the first instruction of g is an instruction inlined from f.

This came up recently because the scheduler change https://go-review.googlesource.com/c/go/+/270940 subtly changes the ordering, and line numbering, of assembly instructions at the start of functions.

[gopherbot]

Change https://go.dev/cl/465076 mentions this issue: cmd/compile: mark the initial memory with the function start position

[cherrymui]

FuncForPC is documented as "If pc represents multiple functions because of inlining, it returns the *Func describing the innermost function, but with an entry of the outermost function."

So if the PC of the first instruction is from an inlined function, returning the inner function seems reasonable?

[randall77]

Yes, it is reasonable. But people are using FuncForPC(reflect.ValueOf(fn).Pointer()) to get from a function closure to the name of the function it implements. It's unfortunate, but that's what people are doing, and marking the first instruction as from an inlined callee makes that pattern not work.
I think we can make sure the first instruction is not from anything inlined. For most cases, it's easy - it is only leaf functions calling inlined leaf functions where it matters. All others have a prologue with the right function name. So we just need to do something for this particular case. The obvious choices are to either lie about the function name for the first instruction, or add a nop before the body labeled with the right function name.

[thediveo]

Out of curiosity: what is the correct way to handle the result returned from FuncForPC in such inlining situations?

[randall77]

@thediveo You could use it for, e.g., a sampling profiler. Just take the result of FuncForPC and increment a counter somewhere based on the results.

I think the more relevant question here is, what is the correct way to handle the result returned from refelect.Pointer when passed a func type. Currently the answer to that is, "compare to 0" and not much else.

[thediveo]

@randall77 I probably didn't pose my question well: I wasn't asking for what I would need this information, as I've used it several times in the past in my own projects, such as a simplistic plugin-manager that derives package names from the caller's PC. What I was trying to ask is: is there a way to correctly detect the situation brought up in this issue in existing code and to properly handle it as to not take the inlined code as source, but the function where the inline code was put at the beginning?

[randall77]

@thediveo Not with FuncForPC. At least until/if this issue is fixed.

You can get the right answer, even in the presence of inlining, by using runtime.Callers + runtime.CallersFrames. It sounds like if you're using the caller's PC, that's the right strategy for you. And by that I mean, don't get the caller's PC some other way (runtime.Caller? Not sure how you're getting caller PCs) - it is provided implicitly in the result of runtime.Callers.

This issue is about getting the name of a function from a func that has not yet been called, which if I understand correctly is not your issue.

[gopherbot]

Change https://go.dev/cl/467977 mentions this issue: cmd/compile: ensure FuncForPC works on closures that start with NOPs