Skip to content

appModuleHandler.AppModule.appArchitecture: detect AMD64 apps in Windows 11 on ARM via kernel32.dll::GetProcessInformation#14404

Merged
seanbudd merged 13 commits into
nvaccess:masterfrom
josephsl:i14403win11GetProcessInformation
Dec 21, 2022
Merged

appModuleHandler.AppModule.appArchitecture: detect AMD64 apps in Windows 11 on ARM via kernel32.dll::GetProcessInformation#14404
seanbudd merged 13 commits into
nvaccess:masterfrom
josephsl:i14403win11GetProcessInformation

Conversation

@josephsl

@josephsl josephsl commented Nov 26, 2022

Copy link
Copy Markdown
Contributor

Link to issue number:

Closes #14403
Stems from #14397

Summary of the issue:

In Windows 11 on ARM, AMD64 apps are seen as ARM64 apps.

Description of user facing changes

Entering "focus.appModule.appArchitecture" in Python Console will say "AMD64" if focused on AMD64 application.

Description of development approach

In app module handler:

  1. Defined a process information structure with three attributes: process machine (unsigned short), res0 (double word), machine attributes (double word.
  2. Added 0xAA64 to represent ARM64 applications in app architecture property.
  3. Refined app architecture property in AppModule class to call kernel32.dll::GetProcessInformation on Windows 11 21H2 (build 22000) or later, with WoW64 detection reserved for Windows 10 and earlier.
  4. If GetProcessInformation fails (returns 0), report "unknown" as app architecture to prepare to handle cases where an app (perhaps hosted or an ARM64EC) may report wrong architecture information.
  5. Added type information to app architecture property (return type: str). This should inspire efforts to add type information to app module handler odule and classes (later).

Testing strategy:

Manual testing (ARM64 machine is ideal but also works on x64 machines):

  1. If not done so, upgrade to Windows 11 on a supported ARM-based computer (Surface Pro X, for example).
  2. Install and run an application known to be designed for AMD64 such as Notepad++ or Slack.
  3. While focused on the app, press Control+NVDA+Z to open Python Console and type "focus.appModule.appArchitecture" and observe the value (expected: AMD64).

Known issues with pull request:

None, although it might be possible that some x64 apps may continue to be seen as ARM64, in which case the development approach may need to be refined.

Change log entries:

Bug fixes (and changes for developers):

In Windows 11 on ARM, x64 apps are no longer identified as ARM64 applications. (#14403)

Code Review Checklist:

  • Pull Request description:
    • description is up to date
    • change log entries
  • Testing:
    • Unit tests
    • System (end to end) tests
    • Manual testing
  • API is compatible with existing add-ons.
  • Documentation:
    • User Documentation
    • Developer / Technical Documentation
    • Context sensitive help for GUI changes
  • UX of all users considered:
    • Speech
    • Braille
    • Low Vision
    • Different web browsers
    • Localization in other languages / culture than English
  • Security precautions taken.

…use when detecting various ap architectures in Windows 11. Re nvaccess#14403.

In Windows 11 on ARM, 64-bit x86 (AMD64) apps are identified as ARM64 when it should say AMD64, traced to a flaw in AppModule.appArchitecture property that relied on WoW64 detection. As part of resolving this, define a process machine info structure to be used in the refined app architecture property getter.
…tain app architecture in Windows 11. Re nvaccess#14403.

Rather than relying on WoW64 detection, call kernel32.dll::GetProcessInformation in Windows 11 (21H2 build 22000 and later) to obtain app architecture. This uses a constant defined in process information class enumeration to obtain machine info (9). The WoW64 method is used if this is Windows 10 or earlier.
@josephsl josephsl requested a review from a team as a code owner November 26, 2022 16:55
@josephsl josephsl requested a review from seanbudd November 26, 2022 16:55
@AppVeyorBot

Copy link
Copy Markdown

See test results for failed build of commit 1ab3d408f1

@rperez030

Copy link
Copy Markdown

Tested using a Windows on ARM VM. X86, AMD64 and ARM64 where reported correctly for Slack (X86), Remote Incident Manager (AMD64) and Visual Studio code (ARM64), however, when tested with 1Password 8 which should have been reported as AMD64, NVDA generated the following error:

Traceback (most recent call last):
File "", line 1, in
File "baseObject.pyc", line 26, in get
File "appModuleHandler.pyc", line 710, in _get_appArchitecture
KeyError: 0

NVDA 2022.3.2 incorrectly reports ARM64 for the same app.

@josephsl

josephsl commented Nov 27, 2022 via email

Copy link
Copy Markdown
Contributor Author

@rperez030

Copy link
Copy Markdown

I believe it is an AMD64, or maybe EC. I'm going by their website where they indicate that it requires Windows 11 64-bit or Windows 10 64-bit

@seanbudd seanbudd marked this pull request as draft November 27, 2022 22:59
@LeonarddeR

Copy link
Copy Markdown
Collaborator

I'm indeed pretty sure that 1Password is AMD64 only.

Comment thread source/appModuleHandler.py Outdated
processMachineInfo = _PROCESS_MACHINE_INFORMATION()
# Constant comes from PROCESS_INFORMATION_CLASS enumeration.
ProcessMachineTypeInfo = 9
ctypes.windll.kernel32.GetProcessInformation(

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Definitely should check the return value here. If false then we should report as unknown.
My hypothesis is that for 1Password this function fails due to process handle permissions.
However, if this is not the case, then at very least, using get or catching KeyError on the arch dictionary will obviously be necessary.

…n returning 0 (error). Re nvaccess#14403.

Comment from Mick Curran (NV Access): GetProcessInformation may say '0' (error) sometimes, therefore treat the app in question has targeting 'unknown' architecture.
Comment thread source/appModuleHandler.py
Comment thread source/appModuleHandler.py
@michaelDCurran michaelDCurran marked this pull request as ready for review December 13, 2022 01:12
@michaelDCurran

Copy link
Copy Markdown
Member

Having tested this on ARM64 now:
The appArchitecture of 1Password is failing because getProcessInformation returns 0, because the appModule's processHandle is actually 0. I.e. NVDA was unable to OpenProcess on 1Password.
However, the processHandle property of any NVDAObject from 1Password is valid. Oleacc's GetProcessHandleFromHwnd is working.
Therefore, it might be an idea to update an appModule's processHandle property if it is 0, with an NvDAObject's valid one.
I would suggest doing this in appModuleHandler.GetAppModuleForNVDAObject.
Btw, 1Password is in deed AMD64 on ARM64.

@michaelDCurran

Copy link
Copy Markdown
Member

It is also worth noting that in the 1password case, the appModule is also staying alive for ever, as passing a processHandle of 0 to WaitForSingleObject is returning a True value. (WAIT_FAIL). We should consider improving the isAlive property to only return True for WAIT_TIMEOUT, False for WAIT_0, and anything else should be logged as n error and probably return False.

@josephsl

Copy link
Copy Markdown
Contributor Author

Hi,

I'll incorporate Mick's suggestion this week after merging latest master commit on AMD64 support on Windows 11 ARM64.

Thanks.

…o set process handle to a valid one if it is 0. Re nvaccess#14403.

Comment from ick Curran (NV Access): GetProcessInformation fials for some apps because process handle is 0 but the handle reported by NVDA object is valid. Therefore, after obtaining app module based on object process Id, see if process handle for the app is 0, and if so, replace process handle with that of the one reported by NVDA object. Also added type information for getAppModuleFromNVDAObject function.
@josephsl josephsl force-pushed the i14403win11GetProcessInformation branch from f1b49c9 to 3deb7ed Compare December 20, 2022 13:59
…fined. Re nvaccess#14403.

When running unit tests, a placeholder NVDA object is created with process handle undefined. Therefore catch attribute error and move on.
@josephsl

Copy link
Copy Markdown
Contributor Author

Hi,

Perhaps a follow-up work: when running unit tests, process handle is undefined for placeholder NVDA object. For now NVDA will ignore it via attribute error.

Thanks.

@josephsl

Copy link
Copy Markdown
Contributor Author

Hi,

Pull request build: https://ci.appveyor.com/api/buildjobs/x6a2kw2muycblho7/artifacts/output%2Fnvda_snapshot_pr14404-27340%2C5a08f6a6.exe

While running this build, see if 1password is recognized as an AMD64 application under ARM64 (open NVDA Pyhton Console, type focus.appModule.appArchitecture).

Thanks.

Comment thread source/appModuleHandler.py Outdated
@seanbudd seanbudd merged commit d36588a into nvaccess:master Dec 21, 2022
@nvaccessAuto nvaccessAuto added this to the 2023.1 milestone Dec 21, 2022
@josephsl josephsl deleted the i14403win11GetProcessInformation branch December 21, 2022 11:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Windows 11 on ARM: "ARM64" is returned for app architecture for AMD64 applications

7 participants