Skip to content

Conversation

@pmatos
Copy link
Collaborator

@pmatos pmatos commented Dec 16, 2025

Integrate Zydis as an optional dependency to enable x86/x86-64 guest instruction disassembly during JIT compilation.

Build with -DENABLE_ZYDIS=TRUE.
Use FEX_X86DISASSEMBLE=1 at runtime to output guest x86 instructions for each compiled block.

@pmatos
Copy link
Collaborator Author

pmatos commented Dec 16, 2025

I have been using his locally to help me understand some SMC code. I wonder if it makes sense upstream. I understand it adds a dependency which is unwelcome though.

Here's what the print out looks like when enabled: (some Crysis2 SMC code)

I 739765|739765 1314.297 Guest x86 Begin
I 739765|739765 1314.297 0x1fbe008: pushfd
I 739765|739765 1314.297 0x1fbe009: push eax
I 739765|739765 1314.297 0x1fbe00a: push ecx
I 739765|739765 1314.297 0x1fbe00b: push edx
I 739765|739765 1314.297 0x1fbe00c: push ebx
I 739765|739765 1314.297 0x1fbe00d: push ebp
I 739765|739765 1314.297 0x1fbe00e: push esi
I 739765|739765 1314.297 0x1fbe00f: push edi
I 739765|739765 1314.297 0x1fbe010: call 0x01FBE015
I 739765|739765 1314.297 0x1fbe015: pop edi
I 739765|739765 1314.297 0x1fbe016: add edi, 0x30
I 739765|739765 1314.297 0x1fbe019: mov edx, edi
I 739765|739765 1314.297 0x1fbe01b: mov ebp, 0x22A1E5C
I 739765|739765 1314.297 0x1fbe020: mov ecx, 0x2E8
I 739765|739765 1314.297 0x1fbe025: mov ebx, 0x09
I 739765|739765 1314.297 0x1fbe02a: mov esi, ebx
I 739765|739765 1314.297 0x1fbe02c: mov eax, ds:[ebp]
I 739765|739765 1314.297 0x1fbe030: xor [edi], al
I 739765|739765 1314.297 0x1fbe032: inc edi
I 739765|739765 1314.297 0x1fbe033: dec esi
I 739765|739765 1314.297 0x1fbe034: jnz 0x01FBE03A
I 739765|739765 1314.297 Guest x86 End

@bylaws
Copy link
Collaborator

bylaws commented Dec 16, 2025

If this is to be done it would want to walk the actual decoded blocks or instructions rather than just decode the whole range. I think this might be useful though, albeit I think gdb is pretty equally good here especially for SMC debugging etc

@pmatos
Copy link
Collaborator Author

pmatos commented Dec 16, 2025

If this is to be done it would want to walk the actual decoded blocks or instructions rather than just decode the whole range. I think this might be useful though, albeit I think gdb is pretty equally good here especially for SMC debugging etc

What do you mean by 'walk the actual decoded blocks' ?

@bylaws
Copy link
Collaborator

bylaws commented Dec 16, 2025

If this is to be done it would want to walk the actual decoded blocks or instructions rather than just decode the whole range. I think this might be useful though, albeit I think gdb is pretty equally good here especially for SMC debugging etc

What do you mean by 'walk the actual decoded blocks' ?

Like just printing in the
for (size_t i = 0; i < InstsInBlock; ++i) { loop.

@pmatos
Copy link
Collaborator Author

pmatos commented Dec 16, 2025

If this is to be done it would want to walk the actual decoded blocks or instructions rather than just decode the whole range. I think this might be useful though, albeit I think gdb is pretty equally good here especially for SMC debugging etc

What do you mean by 'walk the actual decoded blocks' ?

Like just printing in the for (size_t i = 0; i < InstsInBlock; ++i) { loop.

@bylaws is this related to SMC? That we might display code that's later modified and that we therefore are printing code that is not what's going to be executed and therefore is misleading?

@bylaws
Copy link
Collaborator

bylaws commented Dec 16, 2025

No - this is as a single decoded multiblock has no guarantee of being contiguous in memory and may contain padding bytes etc that would be jumped over, this current approach assumes fully linear code.

Copy link
Member

@neobrain neobrain left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Of course the moment I "start a review", I run out of things to comment on :)

Nice feature to have, thanks for implementing it!

Integrate Zydis as an optional dependency to enable x86/x86-64 guest
instruction disassembly during JIT compilation.

Build with -DENABLE_ZYDIS=TRUE.
Use FEX_X86DISASSEMBLE=1 at runtime to output guest x86 instructions
for each compiled block.
endif()

if (ENABLE_ZYDIS)
list (APPEND LIBS Zydis)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should use the namespaced variant: Zydis::Zydis or Zycore::Zycore (they are robust against typos and unintentionally undeclared build targets; "Zydis" may fall back to purely linking against libZydis.so without propagating include directories etc)

Comment on lines +320 to +324
find_package(Zydis QUIET)
if (Zydis_FOUND)
message(STATUS "Using system Zydis")
else()
message(STATUS "Using bundled Zydis")
Copy link
Member

@neobrain neobrain Dec 18, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unfortunately I realized Fedora does not actually ship a CMake config for Zydis, but only pkgconfig files. To add insult to injury, zycore uses a separate pkgconfig file, so this must be used instead:

Suggested change
find_package(Zydis QUIET)
if (Zydis_FOUND)
message(STATUS "Using system Zydis")
else()
message(STATUS "Using bundled Zydis")
pkg_search_module(Zydis QUIET IMPORTED_TARGET zydis)
pkg_search_module(Zycore QUIET IMPORTED_TARGET zycore)
if (TARGET PkgConfig::Zydis AND TARGET PkgConfig::Zycore AND NOT CMAKE_CROSSCOMPILING)
add_library(Zydis::Zydis ALIAS PkgConfig::Zydis)
add_library(Zycore::Zycore ALIAS PkgConfig::Zycore)
message(STATUS "Using system Zydis")
else()
message(STATUS "Using bundled Zydis")

None of the other distros that I use have that package to begin with so I don't know if CMake files are shipped anywhere. Might be interesting to check quickly on your systems, otherwise I wouldn't spend too much energy researching this.

One day we'll have nice things :(

"\tstats: Will print stats when disassembling the code"
]
},
"X86Disassemble": {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you also briefly update the description of the Disassemble option to distinguish from this option / make it clear it only covers generated arm code?

Comment on lines +598 to +600
LogMan::Msg::IFmt("{:#x}: {}", InstAddress, ZydisInst.text);
} else {
LogMan::Msg::IFmt("{:#x}: (decode failed, {} bytes)", InstAddress, DecodedInfo->InstSize);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
LogMan::Msg::IFmt("{:#x}: {}", InstAddress, ZydisInst.text);
} else {
LogMan::Msg::IFmt("{:#x}: (decode failed, {} bytes)", InstAddress, DecodedInfo->InstSize);
LogMan::Msg::IFmt(" {:#x}: {}", InstAddress, ZydisInst.text);
} else {
LogMan::Msg::IFmt(" {:#x}: (decode failed, {} bytes)", InstAddress, DecodedInfo->InstSize);

Copy link
Member

@neobrain neobrain left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(just highlighting there's items left to do here; luckily this doesn't block any other work)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants