Also group CFI jump tables according to sections

[PiJoules]

We have an embedded project that groups different functions into different custom sections. If I have function A in a custom section .ram_code that references another function B in the same section (through inline asm in my case), then the reference to B instead refers to the jumptable entry for B. This jump table is placed in .text.B. The linker script used for this project places .text very far away from .ram_code leading to relocation R_RISCV_JAL out of range.

I think the fix emitting jump tables per section in addition to per function type such that jump tables for functions in unique sections are in the same sections as those functions.

[PiJoules]

cc @ilovepi @pcc

[pcc]

That would make indirect calls more expensive because now every call would need to check for the function in both jump tables.

How do long range calls normally work in RISC-V? While I was looking at lld for an unrelated reason it didn't seem like there was anything like range extension thunks, maybe that needs to be implemented?

Also once #138366 lands (and a RISC-V implementation is added) we should usually be able to avoid these long range branches anyway.

[ilovepi]

I think we need to look at a more concrete example to discuss. Some things I'm wondering are if the inline assembly is getting updated correctly. The jal instruction typically has a +/- 1M range. If the call target could be further away we normally use a different instruction sequence and have the linker relax it if possible. Linker relaxation itself could also be the source of the problem, since the call target started in the same section especially on RISC-V where relaxation is much more involved. I'll admit I'm not enough of an expert on the rewriting rules in LLD to know for sure without looking. So I think we need to look at the precise order of both the CFI transforms, and inputs to linker relaxation to actually figure out a solution.

[pcc]

To begin with I would suggest looking at changing LowerTypeTests to emit the longer instruction sequence. Without this CFI won't work in binaries larger than 1M which would presumably be a violation of the code model. Relaxing may cause problems here, we can't use size relaxation on the jump table because that would break the arithmetic used to check calls. If there's a way to enable instruction relaxation without size relaxation I think that's what we should be using for the jump table section.

[ilovepi]

The compiler should be fairly agnostic about which sequence it uses for a call. jal is one sequence with a fixed range, but there are other sequences that can/should be chosen depending on known constraints. AFAIK, the selection mechanism is working appropriately, but maybe there is an issue as it pertains to some of the CFI related transforms? Another part of me wonders whether the inline ASM here has used jal when that is not guaranteed to be safe, but also if one of the transforms or linker relaxation has chosen to use jal due to a wrong assumption about what may happen under CFI or the normal linking process.

TBH, IDK if anyone has thought deeply about the interactions with CFI and RISCV linker relaxation. My understanding is that it does a lot of very complex rewriting, and unlike other targets dramatically changes code layout. I'm not sure that is actually a problem ATM, but w/o a fuller example I could imagine it to be. I think we need to look at something a bit more concrete to determine where the problem actually is: brittle ASM, generic compiler bug, something RISCV specific, something in the CFI implementation that assumes things it can't under RISCV, a bug in RISCV linker relaxation, ... some totally other thing?

My take is that we need to look at some examples of where things go wrong and plot our course from there.

[PiJoules]

Another idea could be extending the cfi_salt attr in #141846 to work with normal cfi-icall. Then I think you'd only need to check one jump table for that signature+salt pair, so functions in special named sections can also just add this attr.

[pcc]

That seems like an orthogonal issue. Also, there's no guarantee that an individual section won't be larger than 1MB so that wouldn't really be fixing the bug, it would just make it less noticeable for some users.

I went and looked at what the pass is doing. There doesn't seem like a problem with the relocations used by the jump table itself because it's emitting tail instructions, which according to some document that I found are valid long jump instructions. But someone should probably make sure that size relaxation is disabled for the jump table.

I read your bug report again. I noticed this:

(through inline asm in my case)

I think this is the actual bug: your inline asm is probably using jal. According to the document I linked above, jal should only be used for short calls, and call should be used for long calls. So if you want your asm to be compatible with CFI, it should be using call. I wouldn't expect it to make a difference to the quality of the generated code in the end because of size relaxation.

[ilovepi]

The reason I asked for a specific example is to rule out something weird in the inline ASM, as @pcc pointed out. I suspect that they use JAL directly instead of call, but there could be something wrong in the plumbing through to the linker for the correct target features in LTO(which I think we've fixed but 🤷) , or maybe its a long standing problem of module level inline asm not having the correct features set (#67698). It's hard to know without a specific example.

FWIW, I think relaxation should be fine for the target in question, since its the linker doing the relaxing, but there's always a Q when it comes to linker scripts.