Closed Bug 1532470 Opened 3 years ago Closed 3 years ago

ARM64: nsWindowsDllInterceptor support for Milestone 4 (accessibility)

Categories

(Core :: mozglue, enhancement, P1)

Unspecified
Windows
enhancement

Tracking

()

RESOLVED FIXED
mozilla69
Tracking Status
firefox67 --- wontfix
firefox67.0.1 --- wontfix
firefox68 --- wontfix
firefox69 --- fixed

People

(Reporter: bugzilla, Assigned: bugzilla)

References

(Blocks 4 open bugs)

Details

Attachments

(5 files)

For M4 I have committed to having nsWindowsDllInterceptor working successfully for these functions:

  • tiptsf!ProcessCaretEvents
  • user32!SendMessageTimeoutW

(Less important, but also nice to have for M4)

  • Hooks set by PoisonIOInterposer on ntdll
Summary: ARM64: nsWindowsDllInterceptor support for Milestone 4 → ARM64: nsWindowsDllInterceptor support for Milestone 4 (accessibility)
Assignee: nobody → aklotz
Status: NEW → ASSIGNED
Priority: P3 → P1
  • We don't actually need to hook tiptsf!ProcessCaretEvents because that is unused on Windows 10.
  • user32!SendMessageTimeoutW is annoying because it is only 2 instructions long and there is another function immediately after it. In my current build of Windows 10's user32.dll, there are a few words of UDF prior to the function that could be used for patching, however those are not guaranteed to exist in other builds. I think we'll need to come up with something to do an 8-byte patch.
  • Syscall stubs will be easy; it looks like each one intentionally includes two UDF words, which gives us enough room to patch; I just need to modify the code to support that instruction sequence.
Depends on: 1547113
Depends on: 1546546
Depends on: 1552362

In order to support 4-byte patches on ARM64, we need to be able to reserve
trampoline space within +/- 128 MB of the beginning of a function.

These changes allow us to make such reservations using OS APIs when
available.

VMSharingPolicyShared needs to become much smarter. This patch modifies that
policy to track different VM reservations and reuse them whenever possible.

We add TrampolinePools to abstract away the differences between VM policies
with respect to the caller who is making the reservation.

Depends on D32190

A null trampoline is just a trampoline that is not backed by a VM reservation.
These are used for tracking the number of bytes that are needed to make a patch.

This patch also contains the changes needed to work with TrampolinePool.

Depends on D32191

This patch modifies arm64 so that detours are peformed via two passes:

  1. The first pass uses a null trampoline to count how many bytes are available
    for patching the original function.
  2. If we have >= 16 bytes to patch, we reuse existing trampoline space. If we
    have less than 16 bytes to patch, we reserve trampoline space within 128MB
    of the function, allowing for a 4 byte patch.
  3. Then we recurse, this time using a real trampoline.

Note that we still do a single-pass on x86(-64).

Depends on D32192

Update the tests for ARM64 to include additional functions that are now
supported via 4 byte patching.

We also convert the TEST macros to accept the DLL names as strings, as this
works better with clang-format.

Depends on D32193

Pushed by aklotz@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/54a7bf8f3092
Part 1 - Add ability to specify desired memory range when reserving memory; r=handyman
https://hg.mozilla.org/integration/autoland/rev/3693ec4875d8
Part 2 - Modify VM sharing policies to use trampoline pools and support the ability to specify a desired memory range when reserving address space; r=handyman
Flags: needinfo?(aklotz)
Flags: needinfo?(aklotz)
Pushed by aklotz@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/7a4a079c4db0
Part 1 - Add ability to specify desired memory range when reserving memory; r=handyman
https://hg.mozilla.org/integration/autoland/rev/c6720fe67519
Part 2 - Modify VM sharing policies to use trampoline pools and support the ability to specify a desired memory range when reserving address space; r=handyman
https://hg.mozilla.org/integration/autoland/rev/198d9afc62cb
Part 3 - Modify trampolines to support trampoline pools and null trampolines; r=handyman
https://hg.mozilla.org/integration/autoland/rev/41f65bf1daf4
Part 4 - Add 4-byte patching to ARM64 interceptor; r=handyman
https://hg.mozilla.org/integration/autoland/rev/6ac0ea7a64e2
Part 5 - Update TestDllInterceptor to test new ARM64 capabilities; r=handyman

Since the status are different for nightly and release, what's the status for beta?
For more information, please visit auto_nag documentation.

Setting fix-optional for now. Ideally we'd like this to get into 68, but we have to see how well it bakes on Nightly.

We're building 68 RC builds next week, safe to say this is a wontfix for 68 at this point?

Flags: needinfo?(aklotz)

Yeah, this needs to bake.

Flags: needinfo?(aklotz)
You need to log in before you can comment on or make changes to this bug.