runtime/pprof: lostProfileEvent stack breaks gentraceback guarantee

[prattmic]

gentraceback generates PCs which are usually following the CALL instruction. For those that aren't, it fixes up the PCs so that functions processing the output can unconditionally decrement the PC. See https://github.com/golang/go/blob/master/src/runtime/traceback.go#L343-L355.

When generating a fake "lost profile event" stack (https://github.com/golang/go/blob/master/src/runtime/pprof/proto.go#L325), addCPUData fails to meet this requirement, as lostProfileEvent-1 will land in the function preceeding lostProfileEvent.

#37447 exposes this bug as problematic. If the function preceeding lostProfileEvent contains inlined functions, then runtime_expandInlineFrames will panic when it tries to lookup the PC in the wrong function. Something like:

runtime: invalid pc-encoded table f=runtime/pprof.addMaxRSS pc=0x59145d targetpc=0x59145f tab=[0/0]0x0
        value=-1 until pc=0x59143c
        value=0 until pc=0x591443
        value=-1 until pc=0x59145d   
fatal error: invalid runtime symbol table

At HEAD, this isn't a problem because runtime/pprof.addMaxRSS precedes lostProfileEvent and contains no inlined calls, thus runtime_expandInlineFrames short-circuits. N.B., I modified addMaxRSS to add an inlined call for the panic above.

I'll need to double check if this is a problem for 1.14.1, but even if not, we probably want to patch 1.14.2, as subtle build order changes could break things.

cc @randall77 @heschik @hyangah

[gopherbot]

Change https://golang.org/cl/225661 mentions this issue: runtime/pprof: increment fake overflow record PC

[prattmic]

It looks like this could affect 1.14.1. lostProfileEvent is preceded by scaleMutexProfile, which inlines runtime.SetMaxProfileRate:

0000000000591680 <runtime/pprof.scaleMutexProfile>:
  591680:       48 8b 05 e1 0b 24 00    mov    0x240be1(%rip),%rax        # 7d2268 <runtime.mutexprofilerate>
  591687:       48 8b 4c 24 08          mov    0x8(%rsp),%rcx
  59168c:       48 0f af c8             imul   %rax,%rcx
  591690:       48 89 4c 24 18          mov    %rcx,0x18(%rsp)
  591695:       0f 57 c0                xorps  %xmm0,%xmm0
  591698:       f2 48 0f 2a c0          cvtsi2sd %rax,%xmm0
  59169d:       f2 0f 10 4c 24 10       movsd  0x10(%rsp),%xmm1
  5916a3:       f2 0f 59 c8             mulsd  %xmm0,%xmm1
  5916a7:       f2 0f 11 4c 24 20       movsd  %xmm1,0x20(%rsp)
  5916ad:       c3                      retq   
  5916ae:       cc                      int3   
  5916af:       cc                      int3   

00000000005916b0 <runtime/pprof.lostProfileEvent>:
  5916b0:       64 48 8b 0c 25 f8 ff    mov    %fs:0xfffffffffffffff8,%rcx
  5916b7:       ff ff 
  5916b9:       48 3b 61 10             cmp    0x10(%rcx),%rsp
  5916bd:       76 1a                   jbe    5916d9 <runtime/pprof.lostProfileEvent+0x29>
  5916bf:       48 83 ec 08             sub    $0x8,%rsp
  5916c3:       48 89 2c 24             mov    %rbp,(%rsp)
  5916c7:       48 8d 2c 24             lea    (%rsp),%rbp
  5916cb:       e8 e0 ff ff ff          callq  5916b0 <runtime/pprof.lostProfileEvent>
  5916d0:       48 8b 2c 24             mov    (%rsp),%rbp
  5916d4:       48 83 c4 08             add    $0x8,%rsp
  5916d8:       c3                      retq   
  5916d9:       e8 12 25 ed ff          callq  463bf0 <runtime.morestack_noctxt>
  5916de:       eb d0                   jmp    5916b0 <runtime/pprof.lostProfileEvent>

I'll send a cherry-pick request (though, per above, I think we want either way).

[prattmic]

@gopherbot, please open a backport issue for 1.14.

[gopherbot]

Backport issue(s) opened: #38118 (for 1.14).

Remember to create the cherry-pick CL(s) as soon as the patch is submitted to master, according to https://golang.org/wiki/MinorReleases.