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

liblink, cmd/ld: don't encode the instruction being relocated in Reloc.add #10050

Open
minux opened this issue Mar 2, 2015 · 5 comments
Open
Milestone

Comments

@minux
Copy link
Member

minux commented Mar 2, 2015

When I first implemented cgo for ARM, I made the wrong precedence to encode
the instruction itself in the relocation's addend field. It makes everything harder:

liblink must stuff the real addend into the instruction, put it in reloc.add, and then
in the linker, it must get the real addend out from the instruction, calculate the
real value, and stuff the value back into the instruction.

Additionally, it makes -S output unreadable.

rel 20+4 t=6 runtime.morestack_noctxt+9bfffffe
rel 40+4 t=6 runtime.duffzero+eb00005e

(Note: the 2nd line is calling into the middle of duffzero, but you can't tell
anything from the addend without some shifts and masks to figure out the
offset)
Compare this to the 6g -S output:

rel 16+4 t=4 runtime.morestack_noctxt+0
rel 37+4 t=4 runtime.duffzero+228

I suspect this is because in cmd/ld/lib.c, we always pass a zero val (the variable o)
to archreloc, instead, it really should pass the original value read from the symbol.

Should we correct this? so that archreloc is passed the real instrutions/data being
relocated in *val argument, and we place the real addend in Reloc.add field?

@minux minux added the repo-main label Mar 2, 2015
@minux minux self-assigned this Mar 2, 2015
@minux minux added this to the Go1.5Maybe milestone Mar 2, 2015
@mwhudson
Copy link
Contributor

mwhudson commented Mar 2, 2015

I don't know how seriously my input should be taken here, but this whole game confused the heck out of me when I did arm64 external linking. Passing the original value to archreloc would make a lot more sense.

@minux
Copy link
Member Author

minux commented Mar 2, 2015 via email

@rsc rsc removed the repo-main label Apr 14, 2015
@josharian
Copy link
Contributor

FWIW, this threw me for a while today.

@rsc
Copy link
Contributor

rsc commented Jun 29, 2015

Too late for Go 1.5.

@rsc rsc modified the milestones: Unplanned, Go1.5Maybe Jun 29, 2015
mwhudson added a commit to mwhudson/go that referenced this issue Aug 3, 2015
And clean up the mess on arm64 (the mess on arm is too confusing).

See issue golang#10050

Change-Id: I2ce813fe8646d4e818eb660612a7e4b2bb04de4c
mwhudson added a commit to mwhudson/go that referenced this issue Aug 3, 2015
And clean up the mess on arm64 (the mess on arm is too confusing).

See issue golang#10050

Change-Id: I2ce813fe8646d4e818eb660612a7e4b2bb04de4c
@gopherbot
Copy link

CL https://golang.org/cl/13884 mentions this issue.

mwhudson added a commit to mwhudson/go that referenced this issue Aug 27, 2015
And clean up the mess on arm64 (the mess on arm is too confusing).

See issue golang#10050

Change-Id: I2ce813fe8646d4e818eb660612a7e4b2bb04de4c
ianlancetaylor pushed a commit that referenced this issue Aug 31, 2015
And clean up the mess on arm64 (the mess on arm is too confusing).

See issue #10050

Change-Id: I2ce813fe8646d4e818eb660612a7e4b2bb04de4c
Reviewed-on: https://go-review.googlesource.com/13884
Reviewed-by: Ian Lance Taylor <iant@golang.org>
mwhudson added a commit to mwhudson/go that referenced this issue Sep 2, 2015
And clean up the mess on arm64 (the mess on arm is too confusing).

See issue golang#10050

Change-Id: I2ce813fe8646d4e818eb660612a7e4b2bb04de4c
@rsc rsc unassigned minux Jun 23, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants