Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

"push r16" not correctly lifted in x86 (32 bits) #4028

Open
themaks opened this issue Mar 8, 2022 · 3 comments
Open

"push r16" not correctly lifted in x86 (32 bits) #4028

themaks opened this issue Mar 8, 2022 · 3 comments
Assignees
Labels
Arch: x86 Issues with the x86/x64 architecture plugin Component: Architecture Issue needs changes to an architecture plugin Dependency: XED Issue may be due to a bug in XED Effort: Low Issue should take < 1 week Impact: Low Issue is a papercut or has a good, supported workaround Lifting issues related to LLIL lifting State: Blocked (Dependency) Issue is blocked on the update of an external dependency Type: Bug Issue is a non-crashing bug with repro steps

Comments

@themaks
Copy link

themaks commented Mar 8, 2022

On x86 (32 bits mode), disassembling the opcodes 66 53 66 5B, binary ninja outputs the following disassembly:

00000000  6653               push    bx {var_4}
00000002  665b               pop     bx

which is correct; however, the lifted IL is not :

   0 @ 00000000  push(zx.d(bx))
   1 @ 00000002  bx = pop.w

Indeed, it should read instead :

   0 @ 00000000  push(bx)  // OperandSize == 16
   1 @ 00000002  bx = pop.w

The 0x66 prefix is correctly interpreted for the pop operation (since it operates on 16 bits), but not forthe push operation.

Binary Ninja Version: 3.0.3280-dev Personal, 8291c569
Platform: Windows 10 Version 2009

@plafosse plafosse transferred this issue from Vector35/arch-x86 Mar 1, 2023
@fuzyll fuzyll added Component: Architecture Issue needs changes to an architecture plugin Arch: x86 Issues with the x86/x64 architecture plugin labels Mar 6, 2023
@xusheng6 xusheng6 added the Dependency: XED Issue may be due to a bug in XED label Apr 7, 2023
@xusheng6
Copy link
Member

xusheng6 commented Apr 7, 2023

I can confirm this bug. The related code is here: https://github.com/Vector35/arch-x86/blob/master/il.cpp#L2819-L2834. For push bx instruction, stackAdjustment is 4 and opOneLen is 2. This is unexpected, since stackAdjustment should really be 2. Seems like a xed bug

@xusheng6 xusheng6 mentioned this issue Apr 8, 2023
@xusheng6 xusheng6 added Impact: Low Issue is a papercut or has a good, supported workaround Lifting issues related to LLIL lifting Effort: Low Issue should take < 1 week labels May 11, 2023
@xusheng6 xusheng6 added this to the Coruscant milestone May 30, 2023
@xusheng6 xusheng6 self-assigned this May 30, 2023
@xusheng6 xusheng6 added the Type: Bug Issue is a non-crashing bug with repro steps label Jul 4, 2023
@plafosse plafosse modified the milestones: Coruscant, Dorsai Aug 1, 2023
@plafosse plafosse modified the milestones: Dorsai, Elysium Jan 11, 2024
@xusheng6
Copy link
Member

Filed upstream bug report: intelxed/xed#319

@xusheng6 xusheng6 removed this from the Elysium milestone May 2, 2024
@xusheng6
Copy link
Member

xusheng6 commented May 2, 2024

I have removed the milestone for this issue because it depends on an upstream issue, so I cannot really do much for it

@xusheng6 xusheng6 added the State: Blocked (Dependency) Issue is blocked on the update of an external dependency label May 2, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Arch: x86 Issues with the x86/x64 architecture plugin Component: Architecture Issue needs changes to an architecture plugin Dependency: XED Issue may be due to a bug in XED Effort: Low Issue should take < 1 week Impact: Low Issue is a papercut or has a good, supported workaround Lifting issues related to LLIL lifting State: Blocked (Dependency) Issue is blocked on the update of an external dependency Type: Bug Issue is a non-crashing bug with repro steps
Projects
None yet
Development

No branches or pull requests

4 participants