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

Jump to bottom

cmd/link/internal/arm: off-by-one error in trampoline phase call reachability calculation #59034

Closed
thanm opened this issue Mar 14, 2023 · 6 comments
Closed

cmd/link/internal/arm: off-by-one error in trampoline phase call reachability calculation #59034

thanm opened this issue Mar 14, 2023 · 6 comments
Assignees
@thanm
Labels
FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done.
Milestone
Go1.21

Comments

@thanm
Copy link
Contributor

thanm commented Mar 14, 2023

[Note: splitting this issue off from #58425, since although it was reported there and is related, this is actually a separate problem in the Go linker]

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

$ go version
go version go1.20.2 linux/amd64

Does this issue reproduce with the latest release?

Yes. Also an issue for 1.19.

What operating system and processor architecture are you using (go env)?

Problem is with 32-bit arm.

What did you do?

Download and build grafana agent for ARM (here I'm doing this from a linux/amd64 machine with a cross-compiler installed):

$ git clone https://github.com/grafana/agent.git
$ cd agent
$ git checkout 2eb10ae840faf565f8eb063a79a8f1d826fb9483
$ cat t.sh
#!/bin/sh
set -x
set -e
rm -f grafana-agent.exe
rm -rf tmpdir
mkdir tmpdir
GOOS=linux \
  CC=/usr/bin/arm-linux-gnueabihf-gcc \
  GOARCH=arm \
  GOARM=7 \
  CGO_ENABLED=1 \
  CGO_CFLAGS=" -mlong-calls" \
  go build  -ldflags="-v -tmpdir=tmpdir" -tags "netgo " \
    -x -o /tmp/grafana-agent.exe  ./cmd/grafana-agent
$

What did you expect to see?

Clean build.

What did you see instead?

tmpdir/go.o: in function `k8s.io/api/storage/v1.(*VolumeError).Unmarshal':
/ssd2/go1/pkg/mod/k8s.io/api@v0.25.4/storage/v1/generated.pb.go:5158:(.text+0x207bc74): relocation truncated to fit: R_ARM_CALL against `runtime.morestack_noctxt'

I looked at the relocation in question, and from my calculations the calculated displacement is exactly at the lower limit of the relocation in question (e.g. the minimum value represented by a signed 24-bit quantity, or -0x800000). Source of the relocation (this is from objdump -dr):

207bc70: mov r3, lr
207bc74: bl 0x207bc74 <k8s.io/api/storage/v1.(*VolumeError).Unmarshal+0x6cc> @ imm = #-8
0207bc74: R_ARM_CALL runtime.morestack_noctxt
and target of the relocation is at 0x0007bc74 with offset of 1740, so this computation here yields exactly -0x800000.

Bottom line is that this is a disagreement between the Go linker's calculation of what will reach and the external linker's calculation.

For this line here in the Go linker

if t > 0x7fffff || t < -0x800000 || ...

if I change t < -0x800000 to t <= -0x800000 the program links ok.

So although the symptom is the same as , this looks like a slightly different bug.
@thanm thanm self-assigned this Mar 14, 2023
@thanm thanm added the NeedsFix The path to resolution is known, but the work has not been done. label Mar 14, 2023
@thanm thanm added this to the Go1.21 milestone Mar 14, 2023
@gopherbot
Copy link

Change https://go.dev/cl/475957 mentions this issue: cmd/link/internal/arm: fix off-by-1 in trampoline reachability computation

@thanm
Copy link
Contributor Author

thanm commented Mar 15, 2023

@gopherbot please open back-ports

@thanm
Copy link
Contributor Author

thanm commented Mar 15, 2023

@gopherbot please open backport issues

This was referenced Mar 15, 2023
Closed
Closed
@gopherbot
Copy link

Backport issue(s) opened: #59058 (for 1.19), #59059 (for 1.20).

Remember to create the cherry-pick CL(s) as soon as the patch is submitted to master, according to https://go.dev/wiki/MinorReleases.

@gopherbot
Copy link

Change https://go.dev/cl/476935 mentions this issue: cmd/link/internal/arm: fix off-by-1 in trampoline reachability computation

@gopherbot
Copy link

Change https://go.dev/cl/476936 mentions this issue: cmd/link/internal/arm: fix off-by-1 in trampoline reachability computation

gopherbot pushed a commit that referenced this issue Mar 22, 2023
@thanm @heschi
[release-branch.go1.20] cmd/link/internal/arm: fix off-by-1 in trampo…
b52a696 
…line reachability computation

Tweak the code in trampoline generation that determines if a given
call branch will reach, changing the lower limit guard from "x <
-0x800000" to "x <= -0x800000". This is to resolve linking failures
when the computed displacement is exactly -0x800000, which results in
errors of the form

  .../ld.gold: internal error in arm_branch_common, at ../../gold/arm.cc:4079

when using the Gold linker, and

  ...:(.text+0x...): relocation truncated to fit: R_ARM_CALL against `runtime.morestack_noctxt'

when using the bfd linker.

Fixes #59059.
Updates #59034.
Updates #58425.

Change-Id: I8a76986b38727df1b961654824c2af23f06b9fcf
Reviewed-on: https://go-review.googlesource.com/c/go/+/475957
Run-TryBot: Than McIntosh <thanm@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Cherry Mui <cherryyz@google.com>
(cherry picked from commit f26bf20)
Reviewed-on: https://go-review.googlesource.com/c/go/+/476936
gopherbot pushed a commit that referenced this issue Mar 22, 2023
@thanm @heschi
[release-branch.go1.19] cmd/link/internal/arm: fix off-by-1 in trampo…
826fa3e 
…line reachability computation

Tweak the code in trampoline generation that determines if a given
call branch will reach, changing the lower limit guard from "x <
-0x800000" to "x <= -0x800000". This is to resolve linking failures
when the computed displacement is exactly -0x800000, which results in
errors of the form

  .../ld.gold: internal error in arm_branch_common, at ../../gold/arm.cc:4079

when using the Gold linker, and

  ...:(.text+0x...): relocation truncated to fit: R_ARM_CALL against `runtime.morestack_noctxt'

when using the bfd linker.

Fixes #59058.
Updates #59034.
Updates #58425.

Change-Id: I8a76986b38727df1b961654824c2af23f06b9fcf
Reviewed-on: https://go-review.googlesource.com/c/go/+/475957
Run-TryBot: Than McIntosh <thanm@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Cherry Mui <cherryyz@google.com>
(cherry picked from commit f26bf20)
Reviewed-on: https://go-review.googlesource.com/c/go/+/476935
@golang golang locked and limited conversation to collaborators Mar 15, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees

@thanm thanm

Labels
FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done.
Projects
None yet
Milestone
Go1.21
Development

No branches or pull requests
2 participants
@gopherbot @thanm