[BOLT][AArch64] Add tests for unsupported conditional tailcalls

[paschalis-mpeis]

Tests cover: cbz, cbnz, tbz, tbnz, b.eq, and b.ne

[maksfb]

Thanks for adding the test case! Looks good to me. Once the support is added, I think it will be simpler to have just once compilation, i.e. no need for macros.

[paschalis-mpeis]

Thanks – that's exactly the plan!

I used multiple macros in this reproducer commit to cover all 6 crash cases.
I'll add support in follow-up commits and run llvm-bolt on a single input to test them all at once.

[paschalis-mpeis]

Hey @maksfb ,

Sharing my thoughts to be sure we are aligned before I proceed with the below proposal.

The B or BL cases were simple because there was a 1-1 mapping between relocation types and instructions. We only needed a hardcoded opcode and the immediate value to patch (see BL example here).

Supporting conditional branches is slightly more involved. A single relocation type covers instructions that use the same number of bits for the immediate to be patched. So, CONDBR19 applies to both B.cond and CBZ/CBNZ. We also need to preserve some bits that are 'somewhat dynamic'. For example, the register in a CBZ instruction, the condition-code bits in B.cond, and more.

My plan is to read the original instruction encoding from the BinarySection Contents during relocation flushing, and pass it as an extra parameter to Relocation::encodeValue. Then, when encoding the patched instruction, preserve any of these bits I mention above as needed by extracting them from the original encoding.

What do you think? I can post a draft patch if it's unclear.

(cc: @yota9)

[maksfb]

Hi Paschalis, your suggestion makes complete sense to me. I'll add "basic" support for these instructions, so we don't crash when we encounter them in light mode. Which will be skipping the relocations and relying on target function entry point patching/stub functionality.

[maksfb]

I've created #140669 that adds a temporary support for CTC in lite mode.

Reading ARM manual, all conditional branches have the following statement in their description " provides a hint that this is not a subroutine call or return". Not sure what are the consequences if any when the instruction is used as a tail call. Regular branch B has the same comment attached to it.

[paschalis-mpeis]

Hey Maksim,

I don't think there are any consequences. Any branch that doesn't link to LR isn't primarily intended for calls.
Since it's only a hint, optimizations can still use it for tail calls.

All BR, B, B.cond, CBZ/CBNZ, and TBZ/TBNZ include this hint.
BL uses a similar note to hint that it's for calls. I suppose nothing prevents writing assembly that uses BL and then overwriting LR.

BLR wording is slightly stricter. I've raised this internally to see if we need to improve consistency there.

---

EDIT: Just to clarify for completeness (in case someone isn't following our context):
I am not suggesting to intentionally misuse branching, for example using B for calls and BL for non-calls.

If instructions are misused, features like our upcoming BRBE work, or PAC-BTI can be affected, in addition to performance implications.