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: app.runningUnderARM64Translation() always returning true on Windows ARM64 #39920

Merged
merged 1 commit into from Sep 20, 2023

Conversation

miniak
Copy link
Contributor

@miniak miniak commented Sep 19, 2023

Description of Change

This is the Chromium implementation added by https://chromium-review.googlesource.com/c/chromium/src/+/4415695
https://source.chromium.org/chromium/chromium/src/+/main:base/win/windows_version.cc;l=127-155

// Returns true if this is an x86/x64 process running on ARM64 through
// emulation.
// static
bool OSInfo::IsRunningEmulatedOnArm64() {
#if defined(ARCH_CPU_ARM64)
  // If we're running native ARM64 then we aren't running emulated.
  return false;
#else
  using IsWow64Process2Function = decltype(&IsWow64Process2);

  IsWow64Process2Function is_wow64_process2 =
      reinterpret_cast<IsWow64Process2Function>(::GetProcAddress(
          ::GetModuleHandleA("kernel32.dll"), "IsWow64Process2"));
  if (!is_wow64_process2) {
    return false;
  }
  USHORT process_machine;
  USHORT native_machine;
  bool retval = is_wow64_process2(::GetCurrentProcess(), &process_machine,
                                  &native_machine);
  if (!retval) {
    return false;
  }
  if (native_machine == IMAGE_FILE_MACHINE_ARM64) {
    return true;
  }
  return false;
#endif
}

However in Electron we are missing the part, which always returns false when compiled for ARM64.
So let's just call base::win::OSInfo::IsRunningEmulatedOnArm64();

This has always been broken since the API was added in #29168

Checklist

Release Notes

Notes: Fixed app.runningUnderARM64Translation() always returning true on Windows ARM64.

@electron-cation electron-cation bot added the new-pr 🌱 PR opened in the last 24 hours label Sep 19, 2023
@miniak miniak self-assigned this Sep 19, 2023
@miniak miniak added target/24-x-y PR should also be added to the "24-x-y" branch. target/25-x-y PR should also be added to the "25-x-y" branch. target/26-x-y PR should also be added to the "26-x-y" branch. target/27-x-y PR should also be added to the "27-x-y" branch. semver/minor backwards-compatible functionality labels Sep 19, 2023
@miniak miniak added semver/patch backwards-compatible bug fixes and removed semver/minor backwards-compatible functionality api-review/requested 🗳 labels Sep 19, 2023
@deepak1556
Copy link
Member

Can we just call into the upstream implementation instead ?

@miniak
Copy link
Contributor Author

miniak commented Sep 20, 2023

Can we just call into the upstream implementation instead ?

@deepak1556 done

@miniak miniak removed the target/24-x-y PR should also be added to the "24-x-y" branch. label Sep 20, 2023
@miniak miniak changed the title fix: app.runningUnderARM64Translation() always returning true on ARM64 fix: app.runningUnderARM64Translation() always returning true on Windows ARM64 Sep 20, 2023
@electron-cation electron-cation bot removed the new-pr 🌱 PR opened in the last 24 hours label Sep 20, 2023
@jkleinsc jkleinsc merged commit 6a8b706 into main Sep 20, 2023
21 checks passed
@jkleinsc jkleinsc deleted the miniak/fix-arm64-wow-api branch September 20, 2023 20:15
@release-clerk
Copy link

release-clerk bot commented Sep 20, 2023

Release Notes Persisted

Fixed app.runningUnderARM64Translation() always returning true on ARM64.

@trop
Copy link
Contributor

trop bot commented Sep 20, 2023

I have automatically backported this PR to "25-x-y", please check out #39930

@trop trop bot removed the target/25-x-y PR should also be added to the "25-x-y" branch. label Sep 20, 2023
@trop
Copy link
Contributor

trop bot commented Sep 20, 2023

I have automatically backported this PR to "27-x-y", please check out #39931

@trop
Copy link
Contributor

trop bot commented Sep 20, 2023

I have automatically backported this PR to "26-x-y", please check out #39932

@trop trop bot added in-flight/27-x-y in-flight/26-x-y merged/25-x-y PR was merged to the "25-x-y" branch. merged/27-x-y PR was merged to the "27-x-y" branch. merged/26-x-y PR was merged to the "26-x-y" branch. and removed target/27-x-y PR should also be added to the "27-x-y" branch. target/26-x-y PR should also be added to the "26-x-y" branch. in-flight/25-x-y in-flight/27-x-y in-flight/26-x-y labels Sep 20, 2023
MrHuangJser pushed a commit to MrHuangJser/electron that referenced this pull request Dec 11, 2023
…ndows ARM64 (electron#39920)

fix: app.runningUnderARM64Translation() always returning true on ARM64
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
merged/25-x-y PR was merged to the "25-x-y" branch. merged/26-x-y PR was merged to the "26-x-y" branch. merged/27-x-y PR was merged to the "27-x-y" branch. semver/patch backwards-compatible bug fixes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants