code review 884041: runtime: various arm fixes

runtime: various arm fixes
  * correct symbol table size
  * do not reorder functions in output
  * traceback
  * signal handling
  * use same code for go + defer
  * handle leaf functions in symbol table

[rsc, 2010-04-02 02:15:20]

Hello kaib (cc: golang-dev@googlegroups.com),

I'd like you to review this change.

[rsc, 2010-04-02 06:12:39]

Hello kaib (cc: golang-dev@googlegroups.com),

Please take another look.

[rsc, 2010-04-02 06:15:21]

> Please take another look.
>
> http://codereview.appspot.com/884041/show

Updated amd64 and 386 to support new interface
(mainly just adding getcallersp to asm.s and changing
the call to traceback).

Russ

[dpx, 2010-04-02 14:18:57]

Russ, thanks for all your work on this!

http://codereview.appspot.com/884041/diff/9001/4040
File src/pkg/runtime/symtab.c (right):

http://codereview.appspot.com/884041/diff/9001/4040#newcode18
src/pkg/runtime/symtab.c:18: #include "arch.h"
Did arch.h get left out of the CL?

[rsc, 2010-04-02 16:39:51]

> http://codereview.appspot.com/884041/diff/9001/4040#newcode18
> src/pkg/runtime/symtab.c:18: #include "arch.h"
> Did arch.h get left out of the CL?

Yes, fixed.  Thanks.

Russ

[rsc, 2010-04-02 23:57:10]

Uploaded a new copy.  The "failures" Dean was seeing are
now gone.  They were actually just spurious stack trace prints
(the malloc profiler was asking for a stack trace, and the
code got confused and printed it instead of recording it).

64bit.go and friends don't work because fmt.Init dies near
some floating point code.

Russ

[dpx, 2010-04-03 01:35:53]

http://codereview.appspot.com/884041/diff/21001/22031
File test/golden-arm.out (right):

http://codereview.appspot.com/884041/diff/21001/22031#newcode71
test/golden-arm.out:71: 
maybe change to:

Faulting address: xxx
PC=xxx

and then add something like:

s/^PC=0x[0-9a-f]*/PC=xxx/
s/^Faulting address: 0x[0-9a-f]*/Faulting address: xxx/

to the "clean up stack noise" section of run-arm

[kaib, 2010-04-05 10:34:38]

http://codereview.appspot.com/884041/diff/25001/26002
File src/cmd/5l/noop.c (right):

http://codereview.appspot.com/884041/diff/25001/26002#newcode402
src/cmd/5l/noop.c:402: p->from.offset = ((curtext->to.offset2 + 3) & ~3) + 4;
duh. the original looks like 64bit code that has been ported without thought.
i'm very surprised we didn't catch it earlier.

similar one a few lines down.

http://codereview.appspot.com/884041/diff/25001/26002#newcode452
src/cmd/5l/noop.c:452: p->from.offset = (curtext->to.offset2 & ~7) + 4;
can you fix this instance as well.

http://codereview.appspot.com/884041/diff/25001/26003
File src/cmd/5l/pass.c (right):

http://codereview.appspot.com/884041/diff/25001/26003#newcode235
src/cmd/5l/pass.c:235: if(q != P && q->as != ATEXT) {
heh, this would have taken me a while to find..

http://codereview.appspot.com/884041/diff/25001/26012
File src/pkg/runtime/arm/softfloat.c (right):

http://codereview.appspot.com/884041/diff/25001/26012#newcode7
src/pkg/runtime/arm/softfloat.c:7: // returns number of words that the fp
instruction is occupying
are you missing vlop.s from the cl? the modified lr was used verbatim as a
return address in _sfloat().

[rsc1, 2010-04-05 17:48:56]

http://codereview.appspot.com/884041/diff/25001/26002
File src/cmd/5l/noop.c (right):

http://codereview.appspot.com/884041/diff/25001/26002#newcode402
src/cmd/5l/noop.c:402: p->from.offset = ((curtext->to.offset2 + 3) & ~3) + 4;
On 2010/04/05 10:34:38, kaib wrote:
> duh. the original looks like 64bit code that has been ported without thought.
> i'm very surprised we didn't catch it earlier.
> 
> similar one a few lines down.

Done.

http://codereview.appspot.com/884041/diff/25001/26002#newcode452
src/cmd/5l/noop.c:452: p->from.offset = (curtext->to.offset2 & ~7) + 4;
On 2010/04/05 10:34:38, kaib wrote:
> can you fix this instance as well.

Done.

http://codereview.appspot.com/884041/diff/25001/26003
File src/cmd/5l/pass.c (right):

http://codereview.appspot.com/884041/diff/25001/26003#newcode235
src/cmd/5l/pass.c:235: if(q != P && q->as != ATEXT) {
On 2010/04/05 10:34:38, kaib wrote:
> heh, this would have taken me a while to find..

Me too.

http://codereview.appspot.com/884041/diff/25001/26012
File src/pkg/runtime/arm/softfloat.c (right):

http://codereview.appspot.com/884041/diff/25001/26012#newcode7
src/pkg/runtime/arm/softfloat.c:7: // returns number of words that the fp
instruction is occupying
On 2010/04/05 10:34:38, kaib wrote:
> are you missing vlop.s from the cl? the modified lr was used verbatim as a
> return address in _sfloat().

Changing isfltinstr to words is a bug fix by itself.
Note that the code below does lr += skip
where lr is a uint32* not a uint8*.

[kaib, 2010-04-05 19:28:29]

lgtm

http://codereview.appspot.com/884041/diff/25001/26012
File src/pkg/runtime/arm/softfloat.c (right):

http://codereview.appspot.com/884041/diff/25001/26012#newcode7
src/pkg/runtime/arm/softfloat.c:7: // returns number of words that the fp
instruction is occupying
On 2010/04/05 17:48:57, rsc1 wrote:
> On 2010/04/05 10:34:38, kaib wrote:
> > are you missing vlop.s from the cl? the modified lr was used verbatim as a
> > return address in _sfloat().
> 
> Changing isfltinstr to words is a bug fix by itself.
> Note that the code below does lr += skip
> where lr is a uint32* not a uint8*.

That totally makes sense. I could however swear that I checked the offsets
originally in the debugger and they came out right.. Weird.

[rsc, 2010-04-05 19:50:35]

submitted.  there will probably be more work
but this is a good checkpoint.

[rsc, 2010-04-05 19:51:13]

*** Submitted as http://code.google.com/p/go/source/detail?r=f5f628b2346a ***

runtime: various arm fixes
  * correct symbol table size
  * do not reorder functions in output
  * traceback
  * signal handling
  * use same code for go + defer
  * handle leaf functions in symbol table

R=kaib, dpx
CC=golang-dev
http://codereview.appspot.com/884041