https://gcc.gnu.org/bugzilla/show_bug.cgi?id=64999
--- Comment #60 from Dominik Vogt ---
Works on s390x.
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=64999
Ian Lance Taylor changed:
What|Removed |Added
Status|UNCONFIRMED |RESOLVED
Resolution|---
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=64999
--- Comment #58 from ian at gcc dot gnu.org ---
Author: ian
Date: Fri Apr 17 19:29:43 2015
New Revision: 222197
URL: https://gcc.gnu.org/viewcvs?rev=222197&root=gcc&view=rev
Log:
PR go/64999
PR go/65180
runtime: Adjust libbacktrace PC va
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=64999
--- Comment #57 from ian at gcc dot gnu.org ---
Author: ian
Date: Fri Apr 17 19:29:28 2015
New Revision: 222196
URL: https://gcc.gnu.org/viewcvs?rev=222196&root=gcc&view=rev
Log:
PR go/64999
PR go/65180
runtime: Adjust libbacktrace PC va
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=64999
--- Comment #56 from boger at us dot ibm.com ---
Here is a bit more detail. Now that I think I understand all the
considerations, I'm proposing this simple fix for gcc 5. Maybe longer term a
more thorough solution could be done but not sure it i
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=64999
--- Comment #55 from boger at us dot ibm.com ---
Created attachment 35344
--> https://gcc.gnu.org/bugzilla/attachment.cgi?id=35344&action=edit
Increment the pc in the callback routine for backtrace_full
Always increment the pc in the callback,
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=64999
--- Comment #54 from Ian Lance Taylor ---
I assume that it works on x86 because subtracting 1 from PC in libbacktrace,
and then subtracting 1 again in runtime/pprof/pprof.go, still gives you a PC
within the call instruction. On PPC, subtracting
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=64999
--- Comment #53 from boger at us dot ibm.com ---
I was taking the approach of only fixing what was known to be broken, and I was
not aware that this was broken on other platforms. Minimizing risk. I can
change it for all platforms but these test
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=64999
--- Comment #52 from Ian Lance Taylor ---
Why not just
pc++;
on all targets? Why the #ifdef? It seems to me that we should aim for
consistent results on all platforms.
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=64999
--- Comment #51 from boger at us dot ibm.com ---
Here is the change I made to go-callers.c. This patch along with my previous
changes to extern.go and pprof.go allows the test identified in this issue to
report the correct line number on ppc64le.
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=64999
--- Comment #46 from boger at us dot ibm.com ---
(In reply to Ian Lance Taylor from comment #45)
> If we change the PC returned by backtrace_full, and then use that changed PC
> to look up file/line information, we might get different results. Th
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=64999
--- Comment #50 from boger at us dot ibm.com ---
(In reply to Ian Lance Taylor from comment #49)
> libbacktrace returns the line number that you actually care about: the line
> number of the call instruction. There is no question that that is cor
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=64999
--- Comment #49 from Ian Lance Taylor ---
libbacktrace returns the line number that you actually care about: the line
number of the call instruction. There is no question that that is correct.
You say that it is a problem if the PC does not mat
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=64999
--- Comment #48 from boger at us dot ibm.com ---
(In reply to Ian Lance Taylor from comment #47)
> We have to separate backtrace_full and backtrace_simple, which are part of
> the libbacktrace library, from functions like runtime_callers which are
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=64999
--- Comment #47 from Ian Lance Taylor ---
We have to separate backtrace_full and backtrace_simple, which are part of the
libbacktrace library, from functions like runtime_callers which are part of
libgo. The libbacktrace library is used by sever
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=64999
--- Comment #45 from Ian Lance Taylor ---
If we change the PC returned by backtrace_full, and then use that changed PC to
look up file/line information, we might get different results. That seems
clear. My next question is: when does this matte
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=64999
--- Comment #44 from boger at us dot ibm.com ---
If we do the increment of the pc to fix it in the callback, here is how that
happens:
- backtrace_full gets the pc and decrements by 1
- backtrace_full calls backtrace_pcinfo to look up the file, fu
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=64999
--- Comment #43 from Ian Lance Taylor ---
I'm getting confused. I think I need to talk about one thing at a time.
You say that libbacktrace is returning incorrect line numbers. That obviously
needs to be fixed. When does that happen?
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=64999
--- Comment #42 from boger at us dot ibm.com ---
(In reply to Ian Lance Taylor from comment #41)
> I really don't want libbacktrace to be processor-dependent. That makes all
> uses of it more complex for no significant gain. Maybe we should c
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=64999
--- Comment #41 from Ian Lance Taylor ---
I really don't want libbacktrace to be processor-dependent. That makes all
uses of it more complex for no significant gain. Maybe we should change
libbacktrace, but definitely not by making it processor
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=64999
--- Comment #40 from boger at us dot ibm.com ---
Created attachment 34995
--> https://gcc.gnu.org/bugzilla/attachment.cgi?id=34995&action=edit
Proposed fix
Here is my proposed fix to correct the problem on Power, and mostly fix it for
s390/s390
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=64999
--- Comment #39 from Ian Lance Taylor ---
Unfortunately backtrace_simple does not handle inlined calls correctly, so it
won't work for a couple of very important cases: doing a stack backtrace on
panic, and skipping the right number of frames whe
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
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=64999
--- Comment #37 from Ian Lance Taylor ---
The PC returned by runtime.Caller is the same as that returned by
runtime.Callers: the instruction following the call.
Otherwise, yes, I agree.
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=64999
--- Comment #36 from boger at us dot ibm.com ---
I'd like to take a step back and clarify what the functions in question,
runtime_callers, runtime.Caller, and runtime.Callers should be returning: the
pc values for the call instruction, or the pc
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=64999
--- Comment #35 from Dominik Vogt ---
I'd like to bring back to attention the fact that the code that deducts six
from the pc (s390x) in pprof.go is broken regardless of what patches are made
to the runtime code. Determining the size of the call
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=64999
--- Comment #34 from Ian Lance Taylor ---
You're right, we could just change runtime_callers. Or, simpler still, just
change callback in libgo/runtime/go-callers.c. Good point.
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 er
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=64999
--- Comment #32 from Ian Lance Taylor ---
> 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.
Righ
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=64999
--- Comment #31 from Ian Lance Taylor ---
> Why is it important to be able to map a file:line to a single PC?
The problem is that the Go code in the runtime/pprof package assumes that it
can take a single PC value and map that to a meaningful fi
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=64999
--- Comment #30 from boger at us dot ibm.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.
>Fro
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=64999
--- Comment #29 from Dominik Vogt ---
(In reply to Ian Lance Taylor from comment #27)
> The problem is that we can't represent full file/line
> information using a single PC value, because a single PC value can't
> represent inlined functions.
W
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=64999
--- Comment #28 from boger at us dot ibm.com ---
I'm not concerned about the inline case. The user could build without inlining
if it was important.
To me it seems like you don't want libbacktrace to decrement the pc when it is
being called by
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=64999
--- Comment #27 from Ian Lance Taylor ---
The runtime.Callers function (and friends) are somewhat broken for gccgo no
matter what we do. The problem is that we can't represent full file/line
information using a single PC value, because a single
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=64999
--- Comment #26 from boger at us dot ibm.com ---
(In reply to Ian Lance Taylor from comment #25)
> To which code in libgcc are you referring? I don't see it.
>
Here is the code I was referring to, but I was wrong when I thought it fixed
the pro
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=64999
--- Comment #25 from Ian Lance Taylor ---
To which code in libgcc are you referring? I don't see it.
Our goal has to be for runtime.Callers to return the same sort of values in
gccgo as it does in gc. That means that your change to
go/runtime/
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
--- Com
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=64999
--- Comment #23 from Ian Lance Taylor ---
Yes, I do mean to change saveg in mprof.goc.
runtime_callers in general returns full file/line information, which is
required for full correctness when using gccgo. When it devolves back to a
plain PC,
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=64999
--- Comment #22 from Dominik Vogt ---
You mean the function saveg() code in mprof.goc?
I'm actually not sure how the above patch to runtime_callers() interacts with
all the other functions that call runtime_callers(). :-/
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=64999
--- Comment #21 from Ian Lance Taylor ---
Ah, thanks. So we should change that to
r->stk[i] = locstk[i].pc + 1;
too.
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=64999
--- Comment #20 from Dominik Vogt ---
runtime_MProf_Malloc() calls runtime_callers() without going through
runtime.Callers():
#0 runtime_callers (skip=skip@entry=1, locbuf=locbuf@entry=0xc2094734b8,
m=m@entry=32, keep_thunks=keep_thunks@en
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=64999
--- Comment #19 from Ian Lance Taylor ---
I see a call to runtime.Callers in Profile.Add in
libgo/go/runtime/pprof/pprof.go. If the PC values in question do not come from
there, where do they come from?
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=64999
--- Comment #18 from Dominik Vogt ---
Created attachment 34788
--> https://gcc.gnu.org/bugzilla/attachment.cgi?id=34788&action=edit
Different fix
How about this patch to the callback() function in go-callers.c to modify the
incoming pc. The a
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=64999
--- Comment #17 from Dominik Vogt ---
runtime.Callers is not called in this test case, so changing it won't help.
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=64999
--- Comment #16 from Ian Lance Taylor ---
Back in comment #9 I was trying to suggest that runtime.Callers should adjust
the PC that it returns to Go code. That seems simpler and should have the same
effect as your patch.
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=64999
--- Comment #15 from Dominik Vogt ---
P.S.: The patch only addresses s390 and s390x.
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=64999
--- Comment #14 from Dominik Vogt ---
Created attachment 34773
--> https://gcc.gnu.org/bugzilla/attachment.cgi?id=34773&action=edit
Experimental fix
The attached patch is a sketch of a possible fix (at the expense of duplicating
some code from
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=64999
--- Comment #13 from Dominik Vogt ---
I see.
But what bug or bugs do we have here then? Current symptoms are:
(1) A bogus call addres may be stored in the backtrace structure (i.e. next
instruction's address minus 1).
(2) The address from (1)
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=64999
--- Comment #12 from Ian Lance Taylor ---
I should add that for purposes of Go, it's not all that important that
libbacktrace does not yet handle sibling calls, because the Go compiler turns
on -fno-optimize-sibling-calls by default
(https://gcc.
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=64999
--- Comment #11 from Ian Lance Taylor ---
libbacktrace is all about stack backtraces. It is not about handling
exceptions.
libbacktrace handles inlined calls and hand written trampolines, assuming the
DWARF information is correct. libbacktrace
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=64999
--- Comment #10 from Dominik Vogt ---
As far as I understand, the code in libbacktrace was originally only intended
for handling exceptions, not for generating stack traces? For the former, the
code is fine. But given a function's return addres
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=64999
--- Comment #9 from Ian Lance Taylor ---
I think that libbacktrace is doing more or less the right thing. If we don't
subtract one from pc there, we have no way to convey signal handler frames
correctly. Also, the resulting PC value is in the i
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=64999
--- Comment #8 from Dominik Vogt ---
The cause for this bug is the interaction between the "--pc" and this code
snippet from printStackRecord() in pprof.go:
... if runtime.GOARCH == "s390" || runtime.GOARCH == "s390x" {
// only works if fu
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=64999
--- Comment #7 from Dominik Vogt ---
The "--pc" is definitely the cause of the bogus addresses. To me it seems that
these addresses are intended to identify the function and source line from the
dwarf info, not to be printed out as a real addres
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=64999
--- Comment #6 from Ian Lance Taylor ---
The odd addresses are most likely a symptom of the libbacktrace library. It
should probably be considered a bug. I'm guessing that it's because of the
--pc in the static function unwind in libbacktrace/b
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=64999
--- Comment #5 from Dominik Vogt ---
Just noticed that all function offsets in the stack traces are broken. On
s390x, only even offsets are valid, but all the numbers are odd, e.g.
pprof_test.allocatePersistent1K+0x23
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=64999
--- Comment #4 from Dominik Vogt ---
The stack traces in gdb look good:
line 64:
#0 __go_new (td=td@entry=0x8010c8e8 <__go_td_S1_xAN5_uint81024ee>,
size=size@entry=1024) at ../../../libgo/runtime/go-new.c:14
#1 0x8000c32c
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=64999
--- Comment #3 from Ian Lance Taylor ---
Of course it's also possible that there is some bug in libbacktrace. It's
worth checking what gdb reports.
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=64999
--- Comment #2 from Ian Lance Taylor ---
>From a cursory look the problem is that the regexp expects line 66 but the
actual output is line 65. Looking at the code line 66 seems correct. However
if the code is built with optimization it's easy t
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=64999
--- Comment #1 from Dominik Vogt ---
These entries looks screwed to me:
-- snip --
32: 1024 [32: 1024] @ ...
#0x8000c44b ...allocatePersistent1K ... mprof_test.go:43
#0x8000c595 ...TestMemoryProfiler ... mprof_test.go:65
...
0: 0 [1: 32
60 matches
Mail list logo
