Skip to content

Fix stack adjustment for x86 pop r16#8050

Merged
fuzyll merged 1 commit into
Vector35:devfrom
haileys:x86-pop-stack-adjust
Jul 1, 2026
Merged

Fix stack adjustment for x86 pop r16#8050
fuzyll merged 1 commit into
Vector35:devfrom
haileys:x86-pop-stack-adjust

Conversation

@haileys

@haileys haileys commented Apr 1, 2026

Copy link
Copy Markdown
Contributor

Related to #4028

pop r16 is lifted incorrectly and adjusts the stack by the wrong size:
image

This PR changes the lifting to always pop the memory operand size and insert a LLIL_LOW_PART if the destination register size is different:

image

Always pops operand size and then truncates to store in register
@CLAassistant

Copy link
Copy Markdown

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
You have signed the CLA already but the status is still pending? Let us recheck it.

@fuzyll fuzyll self-assigned this Jul 1, 2026

@fuzyll fuzyll left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Sorry for the delay on this.

I was a little confused when starting to review this because I didn't think x86-64 had an r16. I now understand that this is fixing lifting for things like segment registers where the destination is smaller than width of the stack pop. Not some weird general-purpose register thing. 😛

I verified that stuff like pop fs and pop gs appear improved while stuff like pop ax and pop word [rax] seem unaffected and the code itself seems like a clean change. I also verified that the change to push appears to not change anything and is just a refactor.

Thanks for the PR!

@fuzyll fuzyll merged commit 26fa100 into Vector35:dev Jul 1, 2026
1 check was pending
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