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/internal/arm: missing relocations for SDYNIMPORT with trampolines #41121

Closed
4a6f656c opened this issue Aug 29, 2020 · 4 comments
Closed

cmd/link/internal/arm: missing relocations for SDYNIMPORT with trampolines #41121

4a6f656c opened this issue Aug 29, 2020 · 4 comments

Comments

@4a6f656c
Copy link
Contributor

@4a6f656c 4a6f656c commented Aug 29, 2020

Compiling the following code on openbsd/arm with Go -tip:

$ cat x.go
package main

import (
        _ "unsafe"
)

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

//go:cgo_import_dynamic _ _ "libc.so"

func getpid_trampoline()

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

On openbsd/arm results in a working binary:

$ go build -o b                               
$ ./b && echo $?
0
$ ldd ./b
./b:
        Start    End      Type  Open Ref GrpRef Name
        00010000 000e5000 exe   1    0   0      ./b
        6af65000 6b02f000 rlib  0    1   0      /usr/lib/libc.so.96.0
        78a24000 78a24000 ld.so 0    1   0      /usr/libexec/ld.so

$ objdump -R ./b    

./b:     file format elf32-littlearm

DYNAMIC RELOCATION RECORDS
OFFSET   TYPE              VALUE 
000d002c R_ARM_JUMP_SLOT   getpid

However, compiling with -ldflags=debugtramp=2 results in a non-working binary without relocations:

$ go build -ldflags=-debugtramp=2 -o b        
$ ./b && echo $?
panic: runtime error: invalid memory address or nil pointer dereference
[signal SIGSEGV: segmentation violation code=0x1 addr=0x0 pc=0x0]

goroutine 1 [running]:
main.getpid_trampoline(0x6505c, 0x418080, 0x0, 0x418080, 0x0, 0x0, 0x0, 0x0, 0x4000e0, 0x4247cc, ...)
        x.s:7 +0x14
main.main()
        x.go:18 +0x14
$ go build -ldflags=-debugtramp=2 -o b  
$ ldd ./b                                            
./b:
        Start    End      Type  Open Ref GrpRef Name
        00010000 000e5000 exe   1    0   0      ./b
        640a1000 6416b000 rlib  0    1   0      /usr/lib/libc.so.96.0
        6ad37000 6ad37000 ld.so 0    1   0      /usr/libexec/ld.so
$ objdump -R ./b 

./b:     file format elf32-littlearm

DYNAMIC RELOCATION RECORDS (none)

Given this is compiler specific, I would imagine this would be reproducable on linux/arm and freebsd/arm. I believe the issue is caused by the relocations being lost as part of the trampoline generation.

The same issue exists with Go 1.13.9, so it does not appear to be a recent linker regression.

@cherrymui
Copy link
Contributor

@cherrymui cherrymui commented Aug 31, 2020

You are right, it seems the trampoline doesn't have the dynamic relocations in internal linking mode. We could add that.

In general, cgo_import_dynamic is not user-facing, so it is not surprising that it only works on platforms that the toolchain or runtime actually uses it.

Loading

@cagedmantis cagedmantis added this to the Backlog milestone Aug 31, 2020
@4a6f656c
Copy link
Contributor Author

@4a6f656c 4a6f656c commented Sep 1, 2020

You are right, it seems the trampoline doesn't have the dynamic relocations in internal linking mode. We could add that.

Any pointers on what would be involved here?

In general, cgo_import_dynamic is not user-facing, so it is not surprising that it only works on platforms that the toolchain or runtime actually uses it.

Indeed, as I've been discovering :)

Loading

@cherrymui
Copy link
Contributor

@cherrymui cherrymui commented Sep 1, 2020

One possibility may be simply generating the relocation in gentramp if the target is SDYNIMPORT, even if we're internal linking https://tip.golang.org/src/cmd/link/internal/arm/asm.go#L448

Or maybe we should use gentrampdyn instead, if the target is SDYNIMPORT.

There may be other loose ends to tight up. Specifically, the code is all based on symbol's Value. For SDYNIMPORT symbols, its Value is 0. So maybe we need to special case SDYNIMPORT symbols in the Value-based calculations.

(I haven't tried it myself.)

Loading

@4a6f656c
Copy link
Contributor Author

@4a6f656c 4a6f656c commented May 11, 2021

This issue has been fixed, probably via https://golang.org/cl/314455.

Loading

@4a6f656c 4a6f656c closed this May 11, 2021
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
3 participants