code review 163550043: runtime: update comment for Callers

runtime: update comment for Callers

Attempt to clear up confusion about how to turn
the PCs reported by Callers into the file and line
number people actually want.

Fixes issue 7690.

[rsc, 2014-10-29 14:46:01]

Hello r (cc: golang-codereviews@googlegroups.com),

I'd like you to review this change to
https://code.google.com/p/go/

[Chris Hines, 2014-10-29 15:04:44]

On 2014/10/29 14:46:01, rsc wrote:
> Hello r (cc: mailto:golang-codereviews@googlegroups.com),
> 
> I'd like you to review this change to
> https://code.google.com/p/go/

The extra documentation is a big help.

Question regarding this portion:

     // As an exception to this rule, if pc[i-1] corresponds to the function
     // runtime.sigpanic, then pc[i] is the program counter of a faulting
     // instruction and should be used without any subtraction.

Is it safe to cache the pc for runtime.sigpanic in order more quickly check for
such entries in the data returned from runtime.Callers?

[rsc, 2014-10-29 15:11:38]

On Wed, Oct 29, 2014 at 11:04 AM, <chris.cs.guy@gmail.com> wrote:

> On 2014/10/29 14:46:01, rsc wrote:
>
>> Hello r (cc: mailto:golang-codereviews@googlegroups.com),
>>
>
>  I'd like you to review this change to
>> https://code.google.com/p/go/
>>
>
> The extra documentation is a big help.
>
> Question regarding this portion:
>
>      // As an exception to this rule, if pc[i-1] corresponds to the
> function
>      // runtime.sigpanic, then pc[i] is the program counter of a
> faulting
>      // instruction and should be used without any subtraction.
>
> Is it safe to cache the pc for runtime.sigpanic in order more quickly
> check for such entries in the data returned from runtime.Callers?
>

If you are processing the PCs in order, then you've already just done a
FuncForPC on pc[i-1]. You just need to check that f.Name() !=
"runtime.sigpanic". That should be cheap enough. You can cache the *Func
once you find it. I wouldn't cache the specific PC because there's not a
guarantee you'll always see the same spot inside the function. It might
work now, but it might stop working later.

Russ

[r, 2014-10-29 15:17:32]

LGTM ugly

[Chris Hines, 2014-10-29 15:26:11]

On 2014/10/29 15:11:38, rsc wrote:
> On Wed, Oct 29, 2014 at 11:04 AM, <mailto:chris.cs.guy@gmail.com> wrote:
> 
> > On 2014/10/29 14:46:01, rsc wrote:
> >
> >> Hello r (cc: mailto:golang-codereviews@googlegroups.com),
> >>
> >
> >  I'd like you to review this change to
> >> https://code.google.com/p/go/
> >>
> >
> > The extra documentation is a big help.
> >
> > Question regarding this portion:
> >
> >      // As an exception to this rule, if pc[i-1] corresponds to the
> > function
> >      // runtime.sigpanic, then pc[i] is the program counter of a
> > faulting
> >      // instruction and should be used without any subtraction.
> >
> > Is it safe to cache the pc for runtime.sigpanic in order more quickly
> > check for such entries in the data returned from runtime.Callers?
> >
> 
> If you are processing the PCs in order, then you've already just done a
> FuncForPC on pc[i-1]. You just need to check that f.Name() !=
> "runtime.sigpanic". That should be cheap enough. You can cache the *Func
> once you find it. I wouldn't cache the specific PC because there's not a
> guarantee you'll always see the same spot inside the function. It might
> work now, but it might stop working later.
> 
> Russ

LGTM

Thanks for the clarification.

Chris

[rsc, 2014-10-29 19:14:10]

*** Submitted as https://code.google.com/p/go/source/detail?r=47166db901ff ***

runtime: update comment for Callers

Attempt to clear up confusion about how to turn
the PCs reported by Callers into the file and line
number people actually want.

Fixes issue 7690.

LGTM=r, chris.cs.guy
R=r, chris.cs.guy
CC=golang-codereviews
https://codereview.appspot.com/163550043