Skip to content

Fix x86-64 OUT missing 16-bit opcode prefix#119

Merged
CensoredUsername merged 1 commit into
CensoredUsername:devfrom
scholzp:fix_x86_out_opcode_16bit_prefix
Dec 12, 2025
Merged

Fix x86-64 OUT missing 16-bit opcode prefix#119
CensoredUsername merged 1 commit into
CensoredUsername:devfrom
scholzp:fix_x86_out_opcode_16bit_prefix

Conversation

@scholzp

@scholzp scholzp commented Oct 16, 2025

Copy link
Copy Markdown
Contributor

We discovered a bug that leads to a missing 16-bit prefix for the x86 OUT instruction when using 16-bit registers. As far as I can tell, no other instructions miss the prefix in their 16-bit version. So far I've tested ADD, MOV, MUL, PUSH, and IN.

Below is a minimal reproducer:

use dynasmrt::{DynasmApi, dynasm};

fn main() {
    let mut ops = dynasmrt::x64::Assembler::new().unwrap();
    dynasm!(ops
        ; .arch x64
        ; out 0xA_i8, ax
    );
    let code = ops.finalize().unwrap();
    println!("Generated Code:");
    for byte in code.into_iter() {
        print!("{:#03x?} ", byte);
    }
    println!("");
    assert_eq!(vec![0x66_u8, 0xe7_u8, 0xa_u8], code.to_vec());
}

The assert fails with the current state of master. This PR adds changes that modify the opmap to generate the missing prefix.

I also wanted to add a test for this, but didn't know where to place it, nor where to start (as this would mean to test a lot of instructions and I don't know if I can just add them to one of the files in testing) and if this is even necessary. I would be happy to hear about your opinion on this!

Using 16-bit registers should emit the 16-bit prefix `0x66`.

Co-authored-by: Thomas Prescher <thomas.prescher@cyberus-technology.de>
@CensoredUsername

Copy link
Copy Markdown
Owner

Hey, thanks for the contribution. It looks good, I'll have to check why this wasn't caught in the test harnasses.

@scholzp

scholzp commented Nov 10, 2025

Copy link
Copy Markdown
Contributor Author

Hey, thanks for further investigating this issue. If there is anything I can do to help you push this over the finish line, please let me know!

@tpressure

Copy link
Copy Markdown
Contributor

@CensoredUsername is there anything we can do here to get this over the finish line?

@CensoredUsername

Copy link
Copy Markdown
Owner

Hey, sorry for the lack of progress. Life's been a bit in the way. I didn't find any specific pattern, it looks to me like it was just an oversight in the instruction encoding generation that this didn't make it in to the encoding list and test harnesses. I'll try to get this in a release ASAP.

@CensoredUsername CensoredUsername changed the base branch from master to dev December 12, 2025 23:19
@CensoredUsername CensoredUsername merged commit d6303e3 into CensoredUsername:dev Dec 12, 2025
1 check passed
@CensoredUsername

Copy link
Copy Markdown
Owner

I'm just adding this in right now, digging further has revealed that in the switch to the generated test cases a couple years ago any test for generic instructions at the end of the opmap (https://github.com/CensoredUsername/dynasm-rs/blob/master/plugin/src/arch/x64/gen_opmap.rs#L5170-L5571).

That's a bit more work to all add and y'all have waited long enough so I'll first just push a release with the fix.

@CensoredUsername

Copy link
Copy Markdown
Owner

Fixed in v4.0.2.

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.

3 participants