[Bug go/64999] s390x libgo test failure in TestMemoryProfiler

[ian at airs dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=64999

--- Comment #32 from Ian Lance Taylor <ian at airs dot com> ---
> I don't think it is a good idea to add code in multiple places to fix up the pc values after calling runtime.Callers.  That seems prone to error and confusing for future updates to the code.

Right, which is why I never suggested that.  I suggested changing
runtime.Callers itself to adjust the values that it gets from libbacktrace.


> - Add a wrapper function to the libgo code to call backtrace_full and then adjust the pc value based on the GOARCH value, understanding what backtrace_full might have done and what the GO code expects.  Then there should be no direct calls to backtrace_full in libgo, but only calls to this wrapper function.

Yes, that is exactly what I have been trying to say, but we don't need to
introduce a new function.  We already only call backtrace_full from a single
place in libgo: runtime_callers (which, to be clear, is not the same as
runtime.Callers).


> I think the pc mapping for inlined functions is a separate issue.  Inlining happens in gccgo and not gc and happens on all gcc compilers so the mapping of pc values to line numbers for inlined code is not an issue specific to gccgo and that is not the issue in this testcase.  Maybe it just needs to be documented so users understand that can happen or maybe inlining should be disabled by default for gccgo and then if users enable it they understand what could happen.

To be clear, libbacktrace can handle inlined functions just fine, and libgo
does the right thing for things like the stack traces dumped when a program
crashes.  It also does the right thing when handling the skip parameter to
runtime.Caller and runtime.Callers.  The problem arises when converting the
libbacktrace values into the single PC value expected by Go functions like
runtime.Callers.

I am not going to disable inlining by default for gccgo.  That would be a
performance killer.