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

Wine: Properly Store and Restore TF, DF, and AF before clear #4310

Draft
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

MelodicAlbuild
Copy link

In it's current implementation, FEX discards TF, DF, and AF when throwing guest exceptions. These bits are restored by Windows after an exception, and therefore needs to be saved by FEX before clear.

This PR is a Draft while I work on the Wine Patch and testing the implementation with Wine.
This is not the final version of these changes as I plan on making it a bit cleaner and more performant

@bylaws
Copy link
Collaborator

bylaws commented Jan 29, 2025

TF is already handled here so that part can be dropped. Is the an application you have ran into depending on af/df being restored?

(You'd also wanna just move things into ReconstructThreadState where other flags are already handled)

@bylaws
Copy link
Collaborator

bylaws commented Jan 29, 2025

I.e. this patch should really only be a thrre line change, setting

with a comment this is specific behaviour for a downstream wine patch and adding to
static constexpr uint32_t ECValidEFlagsMask {(1U << FEXCore::X86State::RFLAG_OF_RAW_LOC) | (1U << FEXCore::X86State::RFLAG_CF_RAW_LOC) |

@MelodicAlbuild
Copy link
Author

Didn't realize TF had already been handled, I saw the TODO and figured there hadn't been any implementation yet.

Currently running into the issue when running Photoshop on top of Wine, specifically when using long chain actions and an action fails. The AF flag specifically is used here Unsure why and is cleared when the action failed, then the application crashes after the fail is caught because it tries to resume, but AF is now 0, and it fails out.

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.

2 participants