code review 19910044: cmd/5l, runtime: fix divide for profiling tracebacks on ARM

cmd/5l, runtime: fix divide for profiling tracebacks on ARM

Two bugs:
1. The first iteration of the traceback always uses LR when provided,
which it is (only) during a profiling signal, but in fact LR is correct
only if the stack frame has not been allocated yet. Otherwise an
intervening call may have changed LR, and the saved copy in the stack
frame should be used. Fix in traceback_arm.c.

2. The division runtime call adds 8 bytes to the stack. In order to
keep the traceback routines happy, it must copy the saved LR into
the new 0(SP). Change

        SUB $8, SP

into

        MOVW    0(SP), R11 // r11 is temporary, for use by linker
        MOVW.W  R11, -8(SP)

to update SP and 0(SP) atomically, so that the traceback always
sees a saved LR at 0(SP).

Fixes issue 6681.

[rsc, 2013-10-31 18:08:54]

Hello golang-dev@googlegroups.com,

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

[r, 2013-10-31 18:14:50]

LGTM

[rsc, 2013-10-31 18:16:03]

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

cmd/5l, runtime: fix divide for profiling tracebacks on ARM

Two bugs:
1. The first iteration of the traceback always uses LR when provided,
which it is (only) during a profiling signal, but in fact LR is correct
only if the stack frame has not been allocated yet. Otherwise an
intervening call may have changed LR, and the saved copy in the stack
frame should be used. Fix in traceback_arm.c.

2. The division runtime call adds 8 bytes to the stack. In order to
keep the traceback routines happy, it must copy the saved LR into
the new 0(SP). Change

        SUB $8, SP

into

        MOVW    0(SP), R11 // r11 is temporary, for use by linker
        MOVW.W  R11, -8(SP)

to update SP and 0(SP) atomically, so that the traceback always
sees a saved LR at 0(SP).

Fixes issue 6681.

R=golang-dev, r
CC=golang-dev
https://codereview.appspot.com/19910044

[josharian, 2013-10-31 21:50:56]

On 2013/10/31 18:16:03, rsc wrote:
> *** Submitted as https://code.google.com/p/go/source/detail?r=9eb64f5ef3a6 ***
> 
> cmd/5l, runtime: fix divide for profiling tracebacks on ARM
> 
> Two bugs:
> 1. The first iteration of the traceback always uses LR when provided,
> which it is (only) during a profiling signal, but in fact LR is correct
> only if the stack frame has not been allocated yet. Otherwise an
> intervening call may have changed LR, and the saved copy in the stack
> frame should be used. Fix in traceback_arm.c.
> 
> 2. The division runtime call adds 8 bytes to the stack. In order to
> keep the traceback routines happy, it must copy the saved LR into
> the new 0(SP). Change
> 
>         SUB $8, SP
> 
> into
> 
>         MOVW    0(SP), R11 // r11 is temporary, for use by linker
>         MOVW.W  R11, -8(SP)
> 
> to update SP and 0(SP) atomically, so that the traceback always
> sees a saved LR at 0(SP).
> 
> Fixes issue 6681.
> 
> R=golang-dev, r
> CC=golang-dev
> https://codereview.appspot.com/19910044

LGTM

[josharian, 2013-10-31 21:51:08]

https://codereview.appspot.com/19910044/diff/50001/src/cmd/5l/noop.c
File src/cmd/5l/noop.c (right):

https://codereview.appspot.com/19910044/diff/50001/src/cmd/5l/noop.c#newcode486
src/cmd/5l/noop.c:486: /* SUB $8,SP */
Nit: Looks like this comment should be removed.