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

boger at us dot ibm.com changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |boger at us dot ibm.com

--- Comment #24 from boger at us dot ibm.com ---
In my opinion, the way the decrement of pc is done in libbacktrace is
incorrect.  The code in libbacktrace looks similar to code in libgcc, where the
pc type is _Unwind_Ptr.  Since the libgcc has been around for quite a while I
assume it is correct, and I see no reason why the code to do the pc decrement
in libbacktrace should be different from what's in libgcc.  

I made this change to libbacktrace/backtrace.c and simple.c:

 if (!ip_before_insn)
    pc = (uintptr_t)((_Unwind_Ptr)pc-1);

And then in pprof.go, added this so that the pc value did not get decremented
again for ppc64 & ppc64le:

  tracepc := pc
                        // Back up to call instruction.
                        if i > 0 && pc > f.Entry() && !wasPanic {
                                if runtime.GOARCH == "386" || runtime.GOARCH ==
"amd64" {
                                        tracepc--
                                } else if runtime.GOARCH == "s390" ||
runtime.GOARCH == "s390x" {
                                        // only works if function was called
                                        // with the brasl instruction (or a
                                        // different 6-byte instruction).
                                        tracepc -= 6
                                } else if runtime.GOARCH != "ppc64" &&
runtime.GOARCH != "ppc64le" {
                                        tracepc -= 4 // arm, etc
                                }
                        }

After making this change the regression went away and no further failures
occurred on ppc64 and ppc64le.  I am not familiar with other platforms (i.e.,
other supported GOARCHes) to understand how they might be affected by this. 
However, since the code that appears in libgcc has existed and been used for
quite some time, that code should be right and decrementing the pc in
libbacktrace in the same way as is done in libgcc should be correct for all
platforms that use it.  Then that leads to the question of what is done in
pprof.go, which looks like a hack to undo what was done incorrectly in
libbacktrace at least for some platforms.

I will continue to do further testing, but would be curious to know if this
change to libbacktrace would fix s390, assuming the special check in pprof.go
would be removed?  And then I don't think you should have to make changes in
other locations, once libbacktrace has been corrected.

Reply via email to