-
Notifications
You must be signed in to change notification settings - Fork 183
Note Vendor Relocation Scheme is for Static Relocations #481
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
Conversation
The requirement for Vendor symbols to be `STB_LOCAL` was added to parallel the requirement that mapping symbols be local, and also resolved some issues with these symbols conflicting with user definitions (local symbols can have duplicate names). However, unlike mapping symbols, Vendor Symbols may appear in Dynamic Symbol Tables, because we may have dynamic vendor relocations. Dynamic symbol tables conventionally only hold GLOBAL and WEAK definitions. This does bring up the problem of how to deal with a dynamic symbol which has a name that matches a vendor symbol. This might become a QoI decision for the linker whether to emit an error gracefully.
|
For some history: Xqci only has static relocations, which is what was top of my mind when doing #458. Cheriot is adopting the vendor relocation scheme and has static and dynamic relocations, so they have hit this issue when implementing vendor relocs in LLD: llvm/llvm-project#168497 (comment) |
|
So I guess we have two choices that I can see:
We actually had a use case years ago for CHERI-MIPS where we wanted STB_LOCAL symbols in .dynsym so had a hack to convert them to STB_GLOBAL + STV_HIDDEN. That use case wasn't really all that legitimate, we should have represented it differently (i.e. with a different relocation type) rather than preserving the non-preemptible symbol for the run-time linker to inspect, but nevertheless something I'm familiar with and should have foreseen :( |
|
I will note that there is actually always one |
|
@MaskRay is unhappy about the need for vendor dynamic relocations. I understand some kind of cheriot-specific dynamic relocations are required, as they cannot reinterpret the standard dynamic relocations for all their needs. I also understand that CherIoT has only recently adopted vendor relocations, and CHERI will be standardised so not hit the same issue. In the case of static relocations, it is clear that it's good for a linker to be able to support different static relocations from different vendors, as they could be combined in one executable (especially when the extensions/instructions do not interfere with each other). With dynamic relocations, is that still the case? Will authors be writing loaders which support different vendor[ relocation]s in the same executable/library? I'm not sure they will, but I'm not entirely sure. Assuming only one dynamic relocation vendor will exist, does it make sense to use the This scheme is less flexible in one major way: vendors can allocate more vendor symbols for themselves today, so architectural extensions can require relocations from both "" and "2" (for example), which wouldn't work with a "single vendor" dynamic relocation model. This would also make disassemblers difficult again, as they would have to implement both the I think there is a neatness to the |
|
So, CHERIoT proper does not actually use dynamic relocations, but big CHERI does. Since CHERIoT builds on big CHERI, switching CHERIoT over to vendor-specific relocations necessitated doing the same for big CHERI's non-standard relocations, which created the need for dynamic vendor-specific relocations. Some day when an RVY ABI exists, assuming that un-namespaced relocations are allocated for it, I expect that we would be able to rebase on it and not require vendor-specific dynamic relocations. I don't have any insight on what a timeline for that would be, but perhaps @jrtc27 does. I will add that, while CHERIoT is not ABI-locked right now, we do want to achieve ABI stability in a reasonable timeframe, which could be an issue if the RVY ABI takes a long time to be standardized. |
|
The motivation behind #469 was to allow reserving standard relocations for CHERI without having to write the full ABI at the same time, and until someone (me or otherwise) actually gets round to reserving them CHERI will just be using un-namespaced custom relocations (i.e. how you used R_RISCV_CUSTOM<n> prior to R_RISCV_VENDOR's existence). |
Having an ABI is a requirement for ISA ratification, so there will be no RVY until there is an RVY ABI as well, and therefore you cannot assume the ISA will not change until then, so I don't see a world in which you can freeze CHERIoT and guarantee compatibility with the ratified ISA standard yet not have a standard RVY ABI. |
I'm speaking about CHERIoT v1, which is baked in silicon and doesn't use RVY encodings. As such its ABI stability doesn't depend on the RVY ISA spec. |
Thank you for the pointer on this, this looks like the missing ingredient. If we could go ahead and reserve some encodings for the RVY spec, we could completely obviate the issue regarding non-standard dynamic relocations. |
|
Given we seem to be avoiding dynamic relocs for Cheriot, do we want to just ban vendor dynamic relocs until we have a compelling reason for them? This might save implementation time too :) |
|
I'd be happy to defer figuring out what to do here until such time as someone actually needs a dynamic vendor relocation |
|
That's fine by me as well |
I agree with this, I guess my question relates to how/whether we get this sentiment into the document itself. To my mind, our choices are:
(I don't mind working on text for the latter) |
|
Probably worth doing the latter so we can be sure to revisit the issue if a vendor needs it |
|
(Updated text, PR title and description to reflect new direction) |
kito-cheng
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I’m happy with the current wording, and I also agree with deferring the opening of vendor relocations for dynamic relocations. The wording isn’t too strong to close the possibility in the future, while still accurately describing the current status.
I’d also like to wait for @jrtc27’s approval before merging, since she has been more involved in this discussion than I have. :P
riscv-elf.adoc
Outdated
| `R_RISCV_VENDOR` relocation is used by the linker to choose the correct | ||
| implementation for the associated nonstandard relocation. | ||
| implementation for the associated nonstandard relocation. This scheme for | ||
| nonstandard relocations is specifically designed for static relocations. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this reads a little ambiguously, and could be interpreted as "it's been designed primarily for static relocations but nothing's stopping you using it for dynamic ones" which is perhaps technically true but not the intended reading. Maybe something stronger like "This scheme only specifies how to encode static relocations; a future version of this specification may extend the scheme to support dynamic relocations."?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is better. I've added "vendor" so we're explicit about "static vendor relocations" and "dynamic vendor relocations", but otherwise the sentence is as you suggested.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh, i should be using "nonstandard", not "vendor".
Co-authored-by: Jessica Clarke <[email protected]>
jrtc27
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM now, but given I mostly wrote the latest addition others should proofread it and check they're happy with the wording :)
Explicitly note that the vendor relocation scheme is only aimed at static relocations at the moment.
This resolves details such as
STB_LOCALsymbols not usually appearing in dynamic symbol tables andR_RISCV_VENDORbeing defined as a static relocation.In future, we could extend this scheme to Dynamic Relocations, but that requires further consideration.
Co-authored-by: Jessica Clarke [email protected]