This is the mail archive of the gcc-bugs@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

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


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

--- Comment #38 from boger at us dot ibm.com ---
I've spent some time thinking about a fix for this and made a few other
observations...

I've noticed that when runtime_callers calls backtrace_full, it locks the
thread using runtime_xadd and then looks up the line number and function name
information and returns it to runtime_callers.  However, in some cases, the
caller of runtime callers doesn't use the line and file information, as happens
with runtime.Callers and others.   And actually if the caller of
runtime_callers (or runtime.Caller) wants to back up the pc, then line number
information that was determined by backtrace_full might not be correct so has
to be looked up again.

That has me wondering if runtime_callers should be changed so that it calls
backtrace_simple instead of backtrace_full and returns just the pc values.  I
don't think backtrace_simple would require the thread to be locked since it
doesn't care about the backtrace_state; then the code in libbacktrace wouldn't
have to read in and save all the debug information as it does now.  And then it
would be up to the callers of runtime_callers to determine if the pc should be
backed up and determine the line information and file information after it has
the correct pc using FuncForPC.

And then to complete the fix for this, we could add a simple function BackupPC
which would be similar to what is in pprof.go to move the pc back to the
previous instruction based on the GOARCH value.  Unfortunately I am not
familiar with all that needs to be done for the s390 and s390x case but
whatever has to be done for that would be done in this function.  And then any
time the pc needs to be backed up it would be done by calling this function.

Summarizing the changes:
- runtime_callers would call backtrace_simple instead of backtrace_full,
returning an array of just the pc values for the stack
- backtrace_simple would not decrement the pc for Power and s390.  I'm assuming
it is OK to do the decrement for other platforms because they don't have an
issue with the way it is now?  This could be controlled by an #if defined
- a new function BackupPC would be added to take a pc value as input and return
the pc for the previous instruction, as described above
- callers of runtime_callers, runtime.Caller or runtime.Callers would call
BackupPC if that was needed, and then use FuncForPC to find the file and line
number information only if needed.

I have started to work on a patch for this but wanted to put this idea out
there.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]