[llvm] [BOLT] Do not reject irreversible branches during disassembly (PR #95572)

[Nathan Sidwell via llvm-commits]

urnathan wrote:

Thanks, that test fails with this original change (as expected).  Here's an update adding `isUnsupportedInstruction`, and hooking that into the disassembler's main loop -- not just inside the branch/call handling, so it can trigger on any instruction.  It preserves the behaviour of marking the target function of an unsupported branch as ignored too. I'm not sure if that's necessary, what was the intent there? Do X86 executables have inter-function loop and j.rcx instructions?

The X86_64 target marks the loop and j*rcx instructions as unsupported. This is the only overrider.

AFAICT, x86's `isDynamicBranch` check belongs in `isReversibleBranch` as the necessary annotation hasn't happened at the point the disassembler calls `isUnsupportedInstruction`. But, that check doesn't seem x86-specific, other arches could have self-modifying code, right?[*] So I moved that code to MCPlusBuilder's default `isReversibleBranch` implementation. I also added asserts there that we never see a non-conditional branch and never an unsupported instruction.

The Instrumentation pass change is a result of that assert, it doesn't seem to me that a special check is needed there.

The other change is in the X86 target code, and again I don't think it's needed.  We just reject indirect branches at that point -- but the placement of the comment 'Handle conditional branches and ignore indirect branches' confuses me.  I /think/ it's talking about the entire remainder of the loop, not just the if statment it's attached to.

The remaining call if isReversibleBranch in `BinaryFunction::fixBranches` appears to be a legitimate use.

[*] back in the pentium microarchitecture days, Intel had to figure out how close ahead to the PC existing self-modifying code wrote, btw

https://github.com/llvm/llvm-project/pull/95572