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

["boger at us dot ibm.com" <gcc-bugzilla at gcc dot gnu dot org>]

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

--- Comment #33 from boger at us dot ibm.com ---
(In reply to Ian Lance Taylor from comment #32)
> > 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).
> 

In comments 21 & 23 it sounded there were plans to make changes in other
locations too.  I agree with what you just said here, that you could just make
a change in runtime_callers after it calls backtrace_full to adjust the pc and
then no other changes should be needed anywhere else.

Yes I realize runtime_callers and runtime.Callers are different and I was being
sloppy when I mentioned them.

> 
> > 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.