Fix size of capability relocations for linker-defined symbols #768
Conversation
This shows that when in hybrid, we don't treat capability relocations against the linker-syntesized or linker-script-defined section symbols as having a size. It also shows that we don't set the sizes correctly for linker-script synthesized scripts even in purecap outputs. This test is adapted from the Morello LLVM test checking the same property for Morello. Co-authored-by: Amilendra Kodithuwakku <[email protected]>
This adapts the patch from Morello LLVM to use the section size for start symbols even in hybrid mode. Co-authored-by: Jessica Clarke <[email protected]>
This adapts the patch from Morello LLVM to use the section size for start symbols even in hybrid mode. This only partially fixes the start-stop-symbols.s test since the isSectionStartSymbol value is lost prior to writing the relocations. Co-authored-by: Jessica Clarke <[email protected]>
Symbol::mergeProperties() was dropping the isSectionStartSymbol value, so linker-script defined symbols never had this set. To fix this issue we could also have moved the assignment of isSectionStartSymbol down after the call to mergeProperties inside LinkerScript::addSymbol(), but performing the check inside mergeProperties seems more future-proof in case this code is refactored in upstream changes.
|
Do we actually care about this edge case in hybrid? Besides, if st_size stays as 0, how you get the capability will lead to different bounds, which isn't particularly sensible; a run-time linker isn't going to get this right, for example. Given we've never seen a need for this on CHERI-RISC-V and it's likely a result of Arm caring too much about hybrid, my inclination would be to drop this, and only pull in the minimal set of changes needed to support Morello in the ways it's actually used. |
|
Our compiler support for this isn't even coherent; it's an addrspacecast in a global's initialiser, but an addrspacecast inside a function gives you a CSetAddr DDC, not a bounded and permission-restricted capability. |
I'd be tempted to set st_size as soon as we encounter any indication of CHERI support in any of the input files? That way we don't break the existing upstream tests but set something sensible for hybrid? |
Just because there are no CHERI relocations doesn't mean it's not being used in a hybrid context. Hybrid just does not work fully and never will. Can at least prioritise purecap (and the bits of hybrid that CheriBSD needs) and come back to this contentious case some time later, if you believe we should be caring about it (which I don't)? |
That's fair. The other option would be to just always set st_size to Nonzero. I don't see this causing any problems but when I tried to upstream that many years ago I recall some pushback. But having it downstream should be fine IMO and there really shouldn't be many tests that are affected. |
2 participants
See individual commits for change descriptions