Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

cmd/link: unaligned loads generated for rodata on mips64 #42071

Closed
4a6f656c opened this issue Oct 19, 2020 · 11 comments
Closed

cmd/link: unaligned loads generated for rodata on mips64 #42071

4a6f656c opened this issue Oct 19, 2020 · 11 comments
Milestone

Comments

@4a6f656c
Copy link
Contributor

@4a6f656c 4a6f656c commented Oct 19, 2020

The mips64 assembler deals with the load of constants that exceed 32 bits by placing them in rodata and loading from memory:

https://github.com/golang/go/blob/master/src/cmd/internal/obj/mips/obj0.go#L91

However, there does not appear to be any guarantee that the symbol added to rodata (via ctxt.Int64Sym) will have 64 bit alignment, even though the resulting code is an

On OpenBSD/octeon this results in the following:

$ cat hello.go
package main

func main() {
        println("hello world")
}
$ go build -o hello hello.go 
$ egdb hello
...
(gdb) run
Starting program: hello 

Program received signal SIGBUS, Bus error.
0x000000000003be68 in runtime/internal/sys.Len64 (x=1, n=<optimized out>) at /home/joel/src/go/src/runtime/internal/sys/intrinsics_common.go:49
49              if x >= 1<<32 {
(gdb) disassemble
...
   0x000000000003be60 <+184>:   lui     s7,0xa
   0x000000000003be64 <+188>:   daddu   s7,s7,gp
=> 0x000000000003be68 <+192>:   ld      a6,-25052(s7)
   0x000000000003be6c <+196>:   sltu    a7,a2,a6
   0x000000000003be70 <+200>:   bnez    a7,0x3bf2c <runtime.(*pallocBits).summarize+388>

The instruction triggering the SIGBUS is an ld of 0xa0000 - 25052 = 0x99e24.

This presumably also happens on linux/mips64, however I would guess that the kernel deals with the unaligned access fault and performs multiple separate 32 bit loads (which will have a performance impact).

There are probably a few solutions, including laying out the RODATA section in descending symbol size order, or adding padding for alignment.

@4a6f656c
Copy link
Contributor Author

@4a6f656c 4a6f656c commented Oct 19, 2020

@cagedmantis cagedmantis added this to the Backlog milestone Oct 19, 2020
@cherrymui
Copy link
Contributor

@cherrymui cherrymui commented Oct 20, 2020

https://tip.golang.org/src/cmd/internal/obj/objfile.go#L309 sets data symbol's alignment. Int64Sym is content-addressable, so it should have alignment set to 8.

ld a6,-25052(s7)

I think it is quite rare for the toolchain to generate a negative offset. Maybe this offset is not calculated correctly.

Do you know what symbol it is loading? Do you know its address, say, in gdb or using the nm command?

@4a6f656c
Copy link
Contributor Author

@4a6f656c 4a6f656c commented Oct 20, 2020

https://tip.golang.org/src/cmd/internal/obj/objfile.go#L309 sets data symbol's alignment. Int64Sym is content-addressable, so it should have alignment set to 8.

Interesting... there goes that theory :)

ld a6,-25052(s7)

I think it is quite rare for the toolchain to generate a negative offset. Maybe this offset is not calculated correctly.

Do you know what symbol it is loading? Do you know its address, say, in gdb or using the nm command?

The symbol is for the 1<<32 constant, which should be a symbol named $i64.0000000100000000. Strangely, I'm not seeing this symbol in the symbol list.

In a different program (with imports), the same code disassembles to:

=> 0x0000000000041788 <+184>:   lui     s7,0x11
   0x000000000004178c <+188>:   daddu   s7,s7,gp
   0x0000000000041790 <+192>:   ld      a6,14744(s7)

And the 0x100000000 value is correctly loaded from this address (into register a6):

...
                    a4               a5               a6               a7
 R8   03f79d71b4ca8b09 0000000000181fc0 0000000100000000 0000000000000000 

The symbol name is not present in the symbol table in this case either, however it does exist just past the end of the named symbols in .rodata.

@cherrymui
Copy link
Contributor

@cherrymui cherrymui commented Oct 20, 2020

It is a content-addressable symbol, so it will be dedup'd by content if there is another symbol with a different name but the same content. Some of those symbols are not in the symbol table, like gcbits, strings.

In your first case (the one crashes), is the address in the RODATA section, just unaligned? Or it is completely off?

@4a6f656c
Copy link
Contributor Author

@4a6f656c 4a6f656c commented Oct 23, 2020

It is a content-addressable symbol, so it will be dedup'd by content if there is another symbol with a different name but the same content. Some of those symbols are not in the symbol table, like gcbits, strings.

Ah, interesting.

In your first case (the one crashes), is the address in the RODATA section, just unaligned? Or it is completely off?

It appears to be .rodata:

$ objdump -h hello | grep rodata 
  1 .rodata       0002c717  0000000000080000  0000000000080000  00070000  2**5

Which makes .rodata from 0x80000 to 0xf0000, with 0x99e24 being in that range. And in fact:

$ egdb ./hello
...
(gdb) run
...
Program received signal SIGBUS, Bus error.
0x000000000003be68 in runtime/internal/sys.Len64 (x=1, n=<optimized out>) at /home/joel/src/go/src/runtime/internal/sys/intrinsics_common.go:49
49              if x >= 1<<32 {
(gdb) disassemble
...
   0x000000000003be60 <+184>:   lui     s7,0xa
   0x000000000003be64 <+188>:   daddu   s7,s7,gp
=> 0x000000000003be68 <+192>:   ld      a6,-25052(s7)
...
(gdb) x/2x 0x99e24
0x99e24:        0x00000001      0x00000000

So not only is it readable from gdb, but it also contains the value that is expected (1 << 32).

I guess we're back to the original unaligned theory after all :)

@cherrymui
Copy link
Contributor

@cherrymui cherrymui commented Oct 23, 2020

Okay, thanks. Do you know what is the symbol immediately before this address? (e.g. p/a 0x99e24 in gdb.)

Also, is there a way I could build the binary myself? That would help me to find out why it is not aligned. Thanks.

@4a6f656c
Copy link
Contributor Author

@4a6f656c 4a6f656c commented Oct 24, 2020

Okay, thanks. Do you know what is the symbol immediately before this address? (e.g. p/a 0x99e24 in gdb.)

p/a is not helpful, but info symbol gives us some details:

(gdb) p/a 0x99e24
$1 = 0x99e24
(gdb) info symbol 0x99e24
func.* + 11 in section .rodata

And that's confirmed by objdump:

$ objdump -x hello | grep rodata.*func\\.\\*
0000000000099e19 l     O .rodata        0000000000000000 go.func.*

Also, is there a way I could build the binary myself? That would help me to find out why it is not aligned. Thanks.

I would expect that linux/mips64 would be the same, if you have access to such a system. Otherwise, this machine is running as the host-openbsd-mips64-joelsing builder, which you may be able to access using gomote. Failing that, send me an email with a public SSH key and I can give you access.

@gopherbot
Copy link

@gopherbot gopherbot commented Oct 24, 2020

Change https://golang.org/cl/264897 mentions this issue: cmd/link: preserve alignment for stackmap symbols

@cherrymui
Copy link
Contributor

@cherrymui cherrymui commented Oct 24, 2020

Thanks. go.func.* pointed out the problem. The CL above should fix this.

@4a6f656c
Copy link
Contributor Author

@4a6f656c 4a6f656c commented Oct 24, 2020

Excellent, thanks!

I'm currently rebuilding with that CL to confirm.

@4a6f656c
Copy link
Contributor Author

@4a6f656c 4a6f656c commented Oct 25, 2020

I've confirmed that this does fix the issue - the code now disassembles to:

   3be30:       3c17000a        lui     s7,0xa
   3be34:       02fcb82d        daddu   s7,s7,gp
   3be38:       deea9ff8        ld      a6,-24584(s7)

Where 0xa0000 - 24584 = 0x99ff8, which is obviously correctly aligned. The resulting binary now also runs correctly.

@gopherbot gopherbot closed this in 404899f Oct 26, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
4 participants
You can’t perform that action at this time.