RFR: 8315038: Capstone disassembler stops when it sees a bad instruction

[Jorn Vernee]

On Fri, 15 Sep 2023 14:22:48 GMT, Andrew Haley <aph at openjdk.org> wrote:

> At present, the Capstone disassembler stops whenever it encounters an undefined instruction. We really need it not to do that, because we use undefined instructions in JIT-generated code for many things.
> 
> The fix is described here:
>  https://www.capstone-engine.org/skipdata.html

Marked as reviewed by jvernee (Reviewer).

Some thoughts: that linked page claims this is only available on the `next` branch, but I see this option is available in the 5.0.1 release from 3 weeks ago (https://github.com/capstone-engine/capstone/releases/tag/5.0.1). That is good.

The page also explains that this option works by skipping bytes until capstone finds something that it can disassemble, which might not be correct. If there is an issue with the approach proposed by this patch in the future, we might want to look at the `CS_OPT_SKIPDATA_SETUP`, which seems to allow specifying a custom callback that figures out the number of bytes to skip. (Do you have any thoughts about that?). Either way, this patch seems like an improvement over the status quo, so I think we can move forward with it.

src/utils/hsdis/capstone/hsdis-capstone.c line 153:

> 151:   cs_option(cs_handle, CS_OPT_SYNTAX, ops.intel_syntax ? CS_OPT_SYNTAX_INTEL : CS_OPT_SYNTAX_ATT);
> 152: 
> 153:   // Turn on SKIPDATA mode

The comment is self-evident.

Suggestion:

  // Turn on SKIPDATA mode to skip broken instructions, which are ubiquitous in JIT-generated code.

-------------

PR Review: https://git.openjdk.org/jdk/pull/15763#pullrequestreview-1629157901
PR Comment: https://git.openjdk.org/jdk/pull/15763#issuecomment-1721417858
PR Review Comment: https://git.openjdk.org/jdk/pull/15763#discussion_r1327417993