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

Fix default install location used when running under x64 emulation on Windows arm64. #68671

Merged
merged 1 commit into from
Apr 29, 2022

Conversation

elinor-fung
Copy link
Member

@elinor-fung elinor-fung commented Apr 28, 2022

IsWow64Process2 will return IMAGE_FILE_MACHINE_UNKNOWN as the process machine if the process is not a WoW64 process (x64 emulation is not, since it is 64-bit, not 32-bit running on 64-bit). The Windows implementation of pal::is_emulator_x64 was checking that the process machine was IMAGE_FILE_MACHINE_AMD64, so it would always return false.

This change makes us rely on our preprocessor define (TARGET_AMD64) to determine that the current process is x64 and then checks that the architecture of the host system (which IsWow64Process2 will set regardless of whether the process is Wow64 or not) is not IMAGE_FILE_MACHINE_AMD64. That is used as the indication that we are running (emulating) x64 on a non-x64 system.

Another option is the use the GetProcessInformation API to determine the process info. I have an implementation of that in elinor-fung@32b9908.

I put up the version that just relies on the compile-time define, as that seemed like the simpler option. If we needed to query process other than the current one or needed some generic is_emulating_X_on_Y, we'd have to do the full query, but I don't think it is necessary right now.

Addresses:

Validated manually running on Windows 11 ARM64 machine.

  • arm64 installed, x64 not installed
    • we report that no runtime is installed (instead of trying to load the arm64 hostfxr)
  • arm64 installed, x64 installed
    • we use the x64 runtime (same as without this change, since we have disabled multi-level lookup in 7.0)
  • arm64 installed, x64 at the default location, but not registered (so not using the installer)
    • we can find the x64 runtime (instead of trying to load the arm64 hostfxr)
  • arm64 not installed, x64 at the default location, but not registered (so not using the installer)
    • we can find the x64 runtime (instead of reporting that no runtime is installed)

@elinor-fung elinor-fung added this to the 7.0.0 milestone Apr 28, 2022
@ghost ghost assigned elinor-fung Apr 28, 2022
@ghost
Copy link

ghost commented Apr 28, 2022

Tagging subscribers to this area: @vitek-karas, @agocke, @VSadov
See info in area-owners.md if you want to be subscribed.

Issue Details

IsWow64Process2 will return IMAGE_FILE_MACHINE_UNKNOWN as the process machine if the process is not a WoW64 process (x64 emulation is not, since it is 64-bit, not 32-bit running on 64-bit). The Windows implementation of pal::is_emulator_x64 was checking that the process machine was IMAGE_FILE_MACHINE_AMD64, so it would always return false.

This change makes us rely on our preprocessor define (TARGET_AMD64) to determine that the current process is x64 and then checks that the architecture of the host system (which IsWow64Process2 will set regardless of whether the process is Wow64 or not) is not IMAGE_FILE_MACHINE_AMD64. That is used as the indication that we are running (emulating) x64 on a non-x64 system.

Another option is the use the GetProcessInformation API to determine the process info. I have an implementation of that in elinor-fung@32b9908.

I put up the version that just relies on the compile-time define, as that seemed like the simpler option. If we needed to query process other than the current one or needed some generic is_emulating_X_on_Y, we'd have to do the full query, but I don't think it is necessary right now.

Addresses:

Validated manually running on Windows 11 ARM64 machine.

  • arm64 installed, x64 not installed
    • we report that no runtime is installed (instead of trying to load the arm64 hostfxr)
  • arm64 installed, x64 at the default location, but not registered (so not using the installer)
    • we can find the x64 runtime (instead of trying to load the arm64 hostfxr)
  • arm64 not installed, x64 at the default location, but not registered (so not using the installer)
    • we can find the x64 runtime (instead of reporting that no runtime is installed)
Author: elinor-fung
Assignees: -
Labels:

area-Host

Milestone: 7.0.0

Copy link
Member

@vitek-karas vitek-karas left a comment

Choose a reason for hiding this comment

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

I think this is a good solution. Thanks a lot!

My only regret is that we can't really automatically test this - unless we want to add a x64 on arm64 leg - which might be a good idea, but it's definitely outside the scope of this change.

@elinor-fung
Copy link
Member Author

Yeah, the non-existent automated testing is rough. Ignoring that we have to add host test runs on arm64 in general (which is definitely a good idea), x64 on arm64 would require updating to / running on win11 and downloading artifacts from a build for a different arch (plus whatever host test code and infrastructure itself).

@elinor-fung elinor-fung merged commit b55ae3d into dotnet:main Apr 29, 2022
@elinor-fung elinor-fung deleted the is-emulating-x64 branch April 29, 2022 16:31
elinor-fung added a commit to elinor-fung/runtime that referenced this pull request Apr 29, 2022
@vitek-karas
Copy link
Member

Honestly even though it's relatively complicated to setup x64 on arm64 testing, given our current woes with M1 machines in that setup (the entire dotnet-ef fiasco), it is probably worth it - but that would have to happen in the SDK/installer repo, since it's not just about host.

@elinor-fung
Copy link
Member Author

Oh, yeah, I'd definitely agree it is worth it - especially (or at least) in an e2e situation like sdk/installer.

carlossanlop pushed a commit that referenced this pull request May 4, 2022
…st (#68583)

* Use x64 directory if running x64 process on arm64 apphost

Backport of #59890

* Fix is_emulating_x64 on Windows (#68671)

Co-authored-by: Mateo Torres-Ruiz <mateoatr@users.noreply.github.com>
@ghost ghost locked as resolved and limited conversation to collaborators May 30, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants