search

golang / go

The Go programming language
https://go.dev
BSD 3-Clause "New" or "Revised" License
123.73k stars 17.63k forks source link

cmd/ld: runtime.goarm is incorrectly loaded as uint32, should be uint8 #6952

Closed gopherbot closed 9 years ago

gopherbot commented 10 years ago

by rounin.urashima:

What steps will reproduce the problem?

1. hg clone -u release https://code.google.com/p/go
2. cd go/src
3. ./all.bash

What is the expected output?

ALL TESTS PASSED

---

What do you see instead?

# Building C bootstrap tool.
cmd/dist

# Building compilers and Go bootstrap tool for host, linux/arm.
lib9
libbio
libmach
liblink
misc/pprof
cmd/addr2line
cmd/nm
cmd/objdump
cmd/pack
cmd/prof
cmd/cc
cmd/gc
.
.
.
net/http/httptest
net/http/httputil
net/http/pprof
net/mail
net/rpc
net/rpc/jsonrpc
net/smtp
os/user
# os/user
SIGSEGV: segmentation violation
PC=0x4a124

runtime.atomicload64(0x0, 0x0)
        ~/go/src/pkg/runtime/atomic_arm.c:144 +0x44
sysmon()
        ~/go/src/pkg/runtime/proc.c:2366 +0x70
runtime.mstart()
        ~/go/src/pkg/runtime/proc.c:597 +0x94

goroutine 1 [running]:
        goroutine running on other thread; stack unavailable

trap    0xe
error   0x17
oldmask 0x0
r0      0x0
r1      0x1
r2      0xb6e17f68
r3      0x0
r4      0x0
r5      0x0
r6      0x1
r7      0x8e
r8      0x30c298
r9      0x10610000
r10     0x106011e0
fp      0x310928
ip      0xb6e17ffd
sp      0xb6e17f60
lr      0x6e60c
pc      0x4a124
cpsr    0x60000010
fault   0x0
runtime/debug
runtime/race
testing
testing/iotest
testing/quick
text/scanner
rounin@LKG:~/go/src$ go version
go version go1.2 linux/arm
rounin@LKG:~/go/src$ uname -a
Linux LKG 3.2.0-4-ixp4xx #1 Debian 3.2.41-2 armv5tel GNU/Linux
rounin@LKG:~/go/src$ date
Sat Dec 14 10:48:01 GMT 2013

Which compiler are you using (5g, 6g, 8g, gccgo)?

Which operating system are you using?
Debian

Which version are you using?  (run 'go version')
go version go1.2 linux/arm

Please provide any additional information below.

This is a Debian Linux system running on an LINKSYS NSLU2 ARM device

Linux LKG 3.2.0-4-ixp4xx #1 Debian 3.2.41-2 armv5tel GNU/Linux

more /etc/issue
Debian GNU/Linux 7.0 \n \l
gopherbot commented 10 years ago

Comment 1 by rounin.urashima:

I tried to compile again with the release, and get this message:
     3  rounin@LKG:~/go/src$ hg identify
     4  87dea3f5ebe7 (release-branch.go1.2) go1.2/release
     5  rounin@LKG:~/go/src$ ./all.bash
     6  # Building C bootstrap tool.
     7  cmd/dist
     8  
     9  # Building compilers and Go bootstrap tool for host, linux/arm.
    10  lib9
    11  libbio
    12  libmach
    13  misc/pprof
    14  cmd/addr2line
    15  cmd/nm
    16  cmd/objdump
    17  cmd/pack
    18  cmd/prof
    19  cmd/cc
    20  cmd/gc
    21  cmd/5l
    22  cmd/5a
    23  cmd/5c
    24  cmd/5g
    25  pkg/runtime
    26  pkg/errors
    27  pkg/sync/atomic
    28  pkg/sync
    29  pkg/io
    30  pkg/unicode
    31  pkg/unicode/utf8
    32  pkg/unicode/utf16
.
.
.
    62  pkg/text/template/parse
    63  pkg/text/template
    64  pkg/go/doc
    65  pkg/go/build
    66  cmd/go
    67  
    68  # Building packages and commands for linux/arm.
    69  runtime
    70  SIGILL: illegal instruction
    71  PC=0x3e7bec
    72  
    73  
    74  goroutine 1 [semacquire]:
    75  sync.runtime_Semacquire(0x108b6ff8)
    76          /home/rounin/go/src/pkg/runtime/sema.goc:199 +0x34
    77  sync.(*WaitGroup).Wait(0x108ffc40)
    78          /home/rounin/go/src/pkg/sync/waitgroup.go:127 +0x174
    79  main.(*builder).do(0x108fd800, 0x1076a5b0)
    80          /home/rounin/go/src/cmd/go/build.go:739 +0x418
    81  main.runInstall(0x3e2688, 0x1070a048, 0x1, 0x1)
    82          /home/rounin/go/src/cmd/go/build.go:332 +0x55c
    83  main.main()
    84          /home/rounin/go/src/cmd/go/main.go:161 +0x518
    85  
    86  goroutine 3 [syscall]:
    87  os/signal.loop()
    88          /home/rounin/go/src/pkg/os/signal/signal_unix.go:21 +0x24
    89  created by os/signal.init·1
    90          /home/rounin/go/src/pkg/os/signal/signal_unix.go:27 +0x48
    91  
    92  goroutine 5 [syscall]:
    93  syscall.Syscall6(0x72, 0x19ec, 0x10828638, 0x0, 0x108f43c0, ...)
    94          /home/rounin/go/src/pkg/syscall/asm_linux_arm.s:45 +0x8
    95  syscall.wait4(0x19ec, 0x10828638, 0x0, 0x108f43c0, 0xba63c, ...)
    96          /home/rounin/go/src/pkg/syscall/zsyscall_linux_arm.go:120 +0x6c
    97  syscall.Wait4(0x19ec, 0xb6e81794, 0x0, 0x108f43c0, 0x23, ...)
    98          /home/rounin/go/src/pkg/syscall/syscall_linux.go:222 +0x64
    99  os.(*Process).wait(0x1089b680, 0x1089b5a0, 0x0, 0x0)
   100          /home/rounin/go/src/pkg/os/exec_unix.go:22 +0xd0
   101  os.(*Process).Wait(0x1089b680, 0x3, 0x0, 0x0)
   102          /home/rounin/go/src/pkg/os/doc.go:43 +0x2c
   103  os/exec.(*Cmd).Wait(0x1090cb40, 0x0, 0x0)
   104          /home/rounin/go/src/pkg/os/exec/exec.go:311 +0x148
   105  os/exec.(*Cmd).Run(0x1090cb40, 0x0, 0x0)
   106          /home/rounin/go/src/pkg/os/exec/exec.go:233 +0x5c
   107  main.(*builder).runOut(0x108fd800, 0x107ba800, 0x1f, 0x201698, 0x7, ...)
   108          /home/rounin/go/src/cmd/go/build.go:1319 +0x250
   109  main.(*builder).run(0x108fd800, 0x107ba800, 0x1f, 0x201698, 0x7, ...)
   110          /home/rounin/go/src/cmd/go/build.go:1270 +0x7c
   111  main.gcToolchain.cc(0x108fd800, 0x107bc700, 0x10905600, 0x24, 0x10896db0, ...)
   112          /home/rounin/go/src/cmd/go/build.go:1637 +0x8ec
   113  main.gcToolchain.cc·i(0x1, 0x108fd800, 0x107bc700, 0x10905600, 0x24, ...)
   114          /home/rounin/go/src/cmd/go/bootstrap.go:1 +0x78
   115  main.(*builder).build(0x108fd800, 0x1076a8c0, 0x0, 0x0)
   116          /home/rounin/go/src/cmd/go/build.go:972 +0x2c4c
   117  main.func·005(0x1076a8c0)
   118          /home/rounin/go/src/cmd/go/build.go:673 +0x33c
   119  main.func·006()
   120          /home/rounin/go/src/cmd/go/build.go:730 +0x164
   121  created by main.(*builder).do
   122          /home/rounin/go/src/cmd/go/build.go:736 +0x3fc
   123  
   124  goroutine 20 [runnable]:
   125  os/exec.func·004(0x1089b580)
   126          /home/rounin/go/src/pkg/os/exec/exec.go:276
   127  created by os/exec.(*Cmd).Start
   128          /home/rounin/go/src/pkg/os/exec/exec.go:278 +0x7ec
   129  
   130  trap    0x6
   131  error   0x0
   132  oldmask 0x0
   133  r0      0x0
   134  r1      0x1
   135  r2      0x3e84c8
   136  r3      0x0
   137  r4      0x3e84c8
   138  r5      0x3e7bec
   139  r6      0x1
   140  r7      0xf0
   141  r8      0x3e7be8
   142  r9      0x10710000
   143  r10     0x107011e0
   144  fp      0x3ec580
   145  ip      0xb6d2cffd
   146  sp      0xb6e8bf60
   147  lr      0x7ff5c
   148  pc      0x3e7bec
   149  cpsr    0x60000010
   150  fault   0x0
   151  rounin@LKG:~/go/src$ hg identify
   152  87dea3f5ebe7 (release-branch.go1.2) go1.2/release
davecheney commented 10 years ago

Comment 2:

Looks like our soft float detection code has gone missing in the linker changes. 
dfc@qnap:~/go/src$ gdb --args ../pkg/tool/linux_arm/go_bootstrap clean -i std
GNU gdb (GDB) 7.6 (Debian 7.6-5)
Copyright (C) 2013 Free Software Foundation, Inc.
License GPLv3+: GNU GPL version 3 or later <http://gnu.org/licenses/gpl.html>;
This is free software: you are free to change and redistribute it.
There is NO WARRANTY, to the extent permitted by law.  Type "show copying"
and "show warranty" for details.
This GDB was configured as "arm-linux-gnueabi".
For bug reporting instructions, please see:
<http://www.gnu.org/software/gdb/bugs/>;...
Reading symbols from /home/dfc/go/pkg/tool/linux_arm/go_bootstrap...done.
(gdb) r
Starting program: /home/dfc/go/pkg/tool/linux_arm/go_bootstrap clean -i std
Program received signal SIGBUS, Bus error.
runtime.asminit () at /home/dfc/go/src/pkg/runtime/asm_arm.s:97
97      WORD    $0xeef1ba10 // vmrs r11, fpscr
It is also possible that GOARM=5 is no longer getting to 5l

Labels changed: added release-go1.3, repo-main, arch-arm.

Owner changed to @davecheney.

Status changed to Accepted.

davecheney commented 10 years ago

Comment 3:

Owner changed to ---.

davecheney commented 10 years ago

Comment 4:

Ok, here is what I think is going on
Go 1.2:
(gdb) disassemble 
Dump of assembler code for function runtime.asminit:
   0x0008ae88 <+0>:   ldr r11, [pc, #28]  ; 0x8aeac <runtime.asminit+36>
   0x0008ae8c <+4>:   ldr r11, [r11]
=> 0x0008ae90 <+8>:    cmp r11, #5
   0x0008ae94 <+12>:  ble 0x8aea4 <runtime.asminit+28>
   0x0008ae98 <+16>:  vmrs    r11, fpscr
   0x0008ae9c <+20>:  bic r11, r11, #16777216 ; 0x1000000
   0x0008aea0 <+24>:  vmsr    fpscr, r11
   0x0008aea4 <+28>:  add pc, lr, #0
   0x0008aea8 <+32>:  b   0x8aea8 <runtime.asminit+32>
   0x0008aeac <+36>:  rsbeq   r8, r5, r12, lsr #18
End of assembler dump.
(gdb) p 0x8aeac
$1 = 569004
(gdb) p *0x8aeac
$2 = 6654252
(gdb) p **0x8aeac
$3 = 5
tip:
Dump of assembler code for function runtime.asminit:
   0x00066220 <+0>:   ldr r11, [pc, #28]  ; 0x66244 <runtime.asminit+36>
=> 0x00066224 <+4>:    ldr r11, [r11]
   0x00066228 <+8>:   cmp r11, #5
   0x0006622c <+12>:  ble 0x6623c <runtime.asminit+28>
   0x00066230 <+16>:  vmrs    r11, fpscr
   0x00066234 <+20>:  bic r11, r11, #16777216 ; 0x1000000
   0x00066238 <+24>:  vmsr    fpscr, r11
   0x0006623c <+28>:  add pc, lr, #0
   0x00066240 <+32>:  b   0x66240 <runtime.asminit+32>
   0x00066244 <+36>:  strhteq r0, [r4], -r11
End of assembler dump.
(gdb) p 0x66244
$1 = 418372
(gdb) p *0x66244
$2 = 2361275
(gdb) p **0x66244
$3 = 5
I believe that the sigbus is occuring when asminit tries to load the value at 2361275. 
gdb appears to be able to do this, but I think the program when running without the gdb
harness cannot.
ianlancetaylor commented 10 years ago

Comment 5:

The address in Go 1.2 is 6654252 == 0x65892c.  The address in tip is 2361275 ==
0x2407bb.  The code is loading a 32-bit value, which on ARM may only be loaded from an
aligned address.  So the bug is that for some reason on tip the value is not being
stored at an aligned address.
I'm not completely sure, but I think the right place to fix this is the floating point
constant code in obj5.c, obj6.c, obj8.c, and objfile.c.  It calls adduint{32,64} without
enforcing any sort of alignment.  But there doesn't seem to be any general alignment
support in liblink.
ianlancetaylor commented 10 years ago

Comment 6:

Owner changed to @rsc.

davecheney commented 10 years ago

Comment 8:

Thanks Ian, you are correct. Armv6 and above can load from unaligned addresses, armv5
cannot. 
As a slight modification to your diagnosis, it is not the floating point constant code,
but the actual injection of runtime.goarm which is 
https://code.google.com/p/go/source/browse/src/cmd/5l/obj.c#94
        // embed goarm to runtime.goarm
        s = linklookup(ctxt, "runtime.goarm", 0);
        s->type = SRODATA;
        adduint8(ctxt, s, ctxt->goarm);
hence, runtime.goarm is written as a uint8, not an uint32
this is further reinforced as
os_linux_arm.c
19:extern uint8  runtime·goarm; // set by 5l
but
TEXT runtime·asminit(SB),NOSPLIT,$0-0
        // disable runfast (flush-to-zero) mode of vfp if runtime.goarm > 5
        MOVW    runtime·goarm(SB), R11
is loading a word, not a byte, which is wrong (because padding) and also because it will
cause SIGBUS if runtime.goarm was not word aligned, which in previous versions of Go
turned out to be dumb luck.

Owner changed to @davecheney.

Status changed to Started.

davecheney commented 10 years ago

Comment 9:

Looking into this some more, that check was added in b489ac3f2e3e as part of the
netbsd/arm work
GLOBL runtime·goarm(SB), $4
TEXT runtime·asminit(SB),7,$0-0
        // disable runfast (flush-to-zero) mode of vfp if runtime.goarm > 5
        MOVW runtime·goarm(SB), R11
        CMP $5, R11
        BLE 4(PC)
        WORD $0xeef1ba10        // vmrs r11, fpscr
        BIC $(1<<24), R11
        WORD $0xeee1ba10        // vmsr fpscr, r11
        RET
runtime.goarm remained defined as a word (albeit set as a uint8 in 5l)  until
59ac8ae2bf33 which unified the definition of runtime.goarm to be a uint8. I should have
picked up this error at the time, but I did not. Also, the arm5 builder was having a lot
of problems at the time so we didn't see the failure there either.
The build prior to this change failed because the build ran overtime (possibly due to
swapping)
http://build.golang.org/log/54affd91a0e0f18f4f4886e7978ffd3fd7c022f1
the build after shows the failure we see now
http://build.golang.org/log/99a93b5715c6684fedcbe556cef3e2180a17f3a9
This patch fixes the alignment issue by loading runtime.goarm as a byte
https://golang.org/cl/46240043/
davecheney commented 10 years ago

Comment 10:

This issue was closed by revision d2fe44d56834b132f3759cef4bcd5dcc1bfeaa5.

Status changed to Fixed.

davecheney commented 10 years ago

Comment 11:

Issue #6953 has been merged into this issue.