-
Notifications
You must be signed in to change notification settings - Fork 163
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
Document exception handling, add R_RISCV_32_PCREL description #110
base: master
Are you sure you want to change the base?
Document exception handling, add R_RISCV_32_PCREL description #110
Conversation
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 looks correct in general.
There is a potential issue here that this documents what gcc happens to do, but an unwinder should accept any valid encoding of the unwind info, even if it is different than what gcc emits. The variable and section names for the personality function support is what gcc emits, but should not necessarily be required. It isn't clear that any of this needs to be documented because this is a gcc standard for unwind info, and compilers should be allowed to emit anything that fits within that standard, even if it is different than what gcc currently emits.
The best reference I know of for gcc unwind info is
http://wiki.dwarfstd.org/index.php?title=Exception_Handling
in particular the blog postings on the Ian Taylor's airs.com site.
The missing reloc does need to be added to the docs.
Descriptor Entry applies to. Generally encoded as: | ||
|
||
``` | ||
DW_EH_PE_pcrel | DW_EH_PE_sdata4 |
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.
We don't have support for 64-bit DWARF in the gcc RISC-V port, so yes, this is always DW_EH_PE_sdata4 currently. I don't anticipate that we will ever need 64-bit DWARF. If someone does insist on adding support for that, then we will need a R_RISCV_64_PCREL relocation which does not exist yet. Most gcc ports don't bother to support 64-bit DWARF. It is only useful if you have binaries over 4GB in size, and if you do have binaries that big then you probably have other more important problems than unwind info.
the FDE applies to. In the first instance, a 32-bit signed offset | ||
is emitted from the current position in the FDE to the code address | ||
itself, and this generates a `R_RISCV_32_PCREL` relocation. In the | ||
second instance, a `R_RISCV_ADD32` and `R_RISCV_SUB32` pair is |
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.
Technically, the R_RISCV_ADD32 and R_RISCV_SUB32 relocations are only required when linker relaxation is enabled, since otherwise we can't know the size of the function until after linker relaxation. Unfortunately, the toolchain currently emits the relocs always. There is an open bug report somewhere asking us to fix this, so in the future these relocs may not be present when linker relaxation is disabled.
I've added a description to note that the registers used for passing values from the personality function through to the landing pad follow the normal integer calling convention. Since this is just an ABI doc, is there somewhere more relevant to put this kind of documentation (something like "notes for implementors"?). I wanted to note this down as part of getting exception handling generated by LLVM to match GCC, and whilst the generic docs and Ian Taylor's blog are enough for a general understanding, I was caught out by a couple of implementation details specific to RISC-V (linker errors from relocs in |
The presence of linker relaxation means anything involving a code range requires add/sub relocs so it can be computed after linker relaxation is done. In practice, only debug info and unwind info require code ranges, so only those use the add/sub relocs. But most people don't look at debug info and unwind info, so may be surprised by this. The missing docs for the R_RISCV_32_PCREL reloc is a clear mistake. Adding info about the landing pad seems reasonable. Maybe you could add a note indicating that this is one possible implementation strategy for the unwind info, and implementations aren't required to exactly follow this strategy, as long as they are compatible with it. |
Okay, I'll update this to just contain the R_RISCV_32_PCREL reloc documentation. I see there is another repository now riscv/riscv-toolchain-conventions which I'm guessing should contain the implementation details that aren't relevant to the ABI. |
The R_RISCV_32_PCREL change was added via #115 |
Did this patch make it to any of the official docs? |
This attempts to write down how exception handling using DWARF CFI extensions works with RISC-V. It's based on my understanding from trying to match the GCC behavior in LLVM.
I'm not sure whether this is appropriate for this document as I don't think the implementation details of exception handling are actually relevant to the ABI at all. Nonetheless, this doesn't seem to be written down anywhere.