Skip to content

cmd/link: SDYNIMPORT on arm results in incorrect assembly #40769

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.

Sign up for GitHub

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

Jump to bottom
Closed
4a6f656c opened this issue Aug 13, 2020 · 8 comments
Closed

cmd/link: SDYNIMPORT on arm results in incorrect assembly #40769

4a6f656c opened this issue Aug 13, 2020 · 8 comments
Labels
FrozenDueToAge NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Milestone
Unplanned

Comments

@4a6f656c
Copy link
Contributor

4a6f656c commented Aug 13, 2020 •
edited
Loading

What version of Go are you using (go version)?

$ go version
go version devel +7d7bd5abc7 Thu Aug 13 15:22:41 2020 +0000 openbsd/amd64

What did you do?

Building the following code for openbsd/arm (and presumably any ELF GOARCH=arm):

$ cat x.go
package main

import (
        _ "unsafe"
)

//go:cgo_import_dynamic libc_getpid getpid "libc.so"

//go:linkname libc_getpid libc_getpid

func trampoline()

func main() {
        trampoline()
}
$ cat x.s
TEXT ·trampoline(SB),0,$0
        CALL    libc_getpid(SB)
        RET

$ GOARCH=arm GOOS=openbsd go build

Results in a binary that decompiles to the following assembly:

...
0006a790 <main.main>:
   6a790:       e59a1008        ldr     r1, [sl, #8]
   6a794:       e15d0001        cmp     sp, r1
   6a798:       9a000002        bls     6a7a8 <main.main+0x18>
   6a79c:       e52de004        str     lr, [sp, #-4]!
   6a7a0:       eb000003        bl      6a7b4 <main.trampoline>
   6a7a4:       e49df004        ldr     pc, [sp], #4
   6a7a8:       e1a0300e        mov     r3, lr
   6a7ac:       ebfff097        bl      66a10 <runtime.morestack_noctxt>
   6a7b0:       eafffff6        b       6a790 <main.main>

0006a7b4 <main.trampoline>:
   6a7b4:       e59a1008        ldr     r1, [sl, #8]
   6a7b8:       e15d0001        cmp     sp, r1
   6a7bc:       9a000002        bls     6a7cc <main.trampoline+0x18>
   6a7c0:       e52de004        str     lr, [sp, #-4]!
   6a7c4:       00000019        andeq   r0, r0, r9, lsl r0
   6a7c8:       e49df004        ldr     pc, [sp], #4
   6a7cc:       e1a0300e        mov     r3, lr
   6a7d0:       ebfff08e        bl      66a10 <runtime.morestack_noctxt>
   6a7d4:       eafffff6        b       6a7b4 <main.trampoline>
Disassembly of section .plt:

0006a7d8 <.plt>:
   6a7d8:       e52de004        str     lr, [sp, #-4]!
   6a7dc:       e59fe004        ldr     lr, [pc, #4]    ; 6a7e8 <runtime.etext+0x10>
   6a7e0:       e08fe00e        add     lr, pc, lr
   6a7e4:       e5bef008        ldr     pc, [lr, #8]!
   6a7e8:       00065838        andeq   r5, r6, r8, lsr r8
   6a7ec:       e28fc600        add     ip, pc, #0      ; 0x0
   6a7f0:       e28cca65        add     ip, ip, #413696 ; 0x65000
   6a7f4:       e5bcf838        ldr     pc, [ip, #2104]!

In particular, note that the main.trampoline function contains an andeq instruction where a bl instruction would be expected. What I believe is happening is that the CALL is being replaced with a PLT and my current guess is that the actual instruction opcode is either not being included or is being being wiped in the process. I would think that the 00000019 value is specifying the offset to the PLT entry, however should have a 0xeb for the first byte in order to make this a bl instruction.
@andybons
Copy link
Member

@cherrymui @rsc @mdempsky @ianlancetaylor
@mwhudson @thanm

@andybons andybons added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Aug 13, 2020
@andybons andybons added this to the Unplanned milestone Aug 13, 2020
@mdempsky
Copy link
Contributor

Seems to reproduce with GOOS=linux too.

@cherrymui
Copy link
Member

Is this new on tip (with the merge of the new linker), or it was wrong before as well?

As cgo_import_dynamic is not user-facing, if it is not currently used by the runtime it is not surprising that it doesn't work.

Is it for ARM (32-bit) or ARM64? I saw both mentioned in the original post. (The disassembly looks like ARM32.)

@mdempsky
Copy link
Contributor

I'm reproducing with GOARCH=arm (and implicitly GOOS=linux).

I see similar andeq instructions in main.trampoline with Go 1.13 and Go 1.14.

@cherrymui
Copy link
Member

I think CL https://go-review.googlesource.com/c/go/+/248399 fixes it.

It is related to issue #19811, which I really hate...

@gopherbot
Copy link
Contributor

Change https://golang.org/cl/248399 mentions this issue: cmd/link: emit correct jump instruction on ARM for DYNIMPORT

@4a6f656c
Copy link
Contributor Author

Is this new on tip (with the merge of the new linker), or it was wrong before as well?

I should have mentioned that this is the same with 1.13.9, so I do not believe it is a regression.

As cgo_import_dynamic is not user-facing, if it is not currently used by the runtime it is not surprising that it doesn't work.

Is it for ARM (32-bit) or ARM64? I saw both mentioned in the original post. (The disassembly looks like ARM32.)

Apologies - that should have been ARM32 (GOARCH=arm) (fixed).

@4a6f656c
Copy link
Contributor Author

I think CL https://go-review.googlesource.com/c/go/+/248399 fixes it.

It does indeed, thanks (I was looking at the correct line of code, but failed to spot the obvious fix!)

The disassembly now gives:

000aa2bc <main.trampoline>:
   aa2bc:       e59a1008        ldr     r1, [sl, #8]
   aa2c0:       e15d0001        cmp     sp, r1
   aa2c4:       9a000002        bls     aa2d4 <main.trampoline+0x18>
   aa2c8:       e52de004        str     lr, [sp, #-4]!
   aa2cc:       eb000008        bl      aa2f4 <runtime.etext+0x14>
   aa2d0:       e49df004        ldr     pc, [sp], #4
   aa2d4:       e1a0300e        mov     r3, lr
   aa2d8:       ebff1025        bl      6e374 <runtime.morestack_noctxt>
   aa2dc:       eafffff6        b       aa2bc <main.trampoline>

And executes correctly.

It is related to issue #19811, which I really hate...

Indeed, that is quite ugly and error prone.

@golang golang locked and limited conversation to collaborators Aug 17, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
FrozenDueToAge NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Projects
None yet
Milestone
Unplanned
Development

No branches or pull requests
5 participants
@mdempsky @andybons @4a6f656c @gopherbot @cherrymui