On Fri, 15 Sep 2023 14:22:48 GMT, Andrew Haley <a...@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

Reply via email to