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

Add R_RISCV_ALIGN_DOWN #392

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

palmer-dabbelt
Copy link
Contributor

We're in the process of stumbling into another round of these bugs related to insufficient alignment bytes. The idea of adding another alignment reloation that rounds down has come up a few times, but I don't remember if anyone's actually written the spec. So here's one.

We're in the process of stumbling into another round of these bugs
related to insufficient alignment bytes.  The idea of adding another
alignment reloation that rounds down has come up a few times, but I
don't remember if anyone's actually written the spec.  So here's one.

Signed-off-by: Palmer Dabbelt <palmer@rivosinc.com>
@jrtc27
Copy link
Collaborator

jrtc27 commented Jul 18, 2023

This needs justification within this forum (I at least have not been aware of any of the discussions that led to this proposal)

@palmer-dabbelt
Copy link
Contributor Author

@kito-cheng
Copy link
Collaborator

That remind me that we have similar issue for R_RISCV_RELAX: #116, and @jrtc27 has proposed we can point a dummy ISA symbol by the R_RISCV_RELAX, and I think...that could also use similar mechanism here? and IIRC @ptomsich has mention RISC-V not implement max alignment for the .align directive, so I guess...we could resolve together instead of add two more relocation (one for R_RISCV_ALIGN_DOWN and one for R_RISCV_ALIGN_MAX) .

c.c. @Nelson1225 @MaskRay

@MaskRay
Copy link
Collaborator

MaskRay commented Jul 22, 2023

Thanks for tagging me. To address issues like riscv-collab/riscv-gnu-toolchain#445 , I agree that discerning RVC vs non-RVC R_RISCV_RELAX is important, and we can use @jrtc27's idea to use the symbol index part of the r_info field (say, symbol_index!=0 means non-RVC). This scheme is compatible with existing linkers.

I haven't thought much regarding R_RISCV_ALIGN. Right now I just know that the two fields in the syntax .align [abs-expr[, abs-expr[, abs-expr]]] are not representable, but it seems that people are fine with the limitation. A new relocation type doesn't seem to be needed if we just try to resolve riscv-collab/riscv-gnu-toolchain#445?

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

Successfully merging this pull request may close these issues.

4 participants