1532470 - ARM64: nsWindowsDllInterceptor support for Milestone 4 (accessibility)

Status: RESOLVED FIXED

(Core :: mozglue, enhancement, P1)

Description

[(No longer employed by Mozilla) Aaron Klotz]

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

Comment 1

[(No longer employed by Mozilla) Aaron Klotz]

• 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.

Comment 2

[(No longer employed by Mozilla) Aaron Klotz]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=ecbeefca2ea6f9dc312849fdf71ee9c86c2f8b90

Comment 3

[(No longer employed by Mozilla) Aaron Klotz]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=a6fe2cf5d2505894563fe77efe6f589cf1838080

Comment 4

[(No longer employed by Mozilla) Aaron Klotz]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=5295f696e40be15e955a40aacbbf02256ce94e56

Comment 5

[(No longer employed by Mozilla) Aaron Klotz]

Attachment: Bug 1532470: Part 1 - Add ability to specify desired memory range when reserving memory; r=handyman!

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.

Comment 6

[(No longer employed by Mozilla) Aaron Klotz]

Attachment: Bug 1532470: 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!

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

Comment 7

[(No longer employed by Mozilla) Aaron Klotz]

Attachment: Bug 1532470: Part 3 - Modify trampolines to support trampoline pools and null trampolines; r=handyman!

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

Comment 8

[(No longer employed by Mozilla) Aaron Klotz]

Attachment: Bug 1532470: Part 4 - Add 4-byte patching to ARM64 interceptor; r=handyman!

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

Comment 9

[(No longer employed by Mozilla) Aaron Klotz]

Attachment: Bug 1532470: Part 5 - Update TestDllInterceptor to test new ARM64 capabilities; r=handyman!

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

Comment 10

[Pulsebot]

Pushed by [e-mail]:
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

Comment 11

[Bogdan Tara[:bogdan_tara | bogdant]]

Backed out 2 changesets (Bug 1532470) on aklotz's request

Backout link: https://hg.mozilla.org/integration/autoland/rev/89e9b4714a6335593f8dd0258b44e157eb5fac8a

Comment 12

[(No longer employed by Mozilla) Aaron Klotz]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=071e845f8513aed63da8c0bd9bc3d23daa563106

Comment 13

[Pulsebot]

Pushed by [e-mail]:
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

Comment 14

[Andrei Ciure[:aciure]]

https://hg.mozilla.org/mozilla-central/rev/7a4a079c4db0
https://hg.mozilla.org/mozilla-central/rev/c6720fe67519
https://hg.mozilla.org/mozilla-central/rev/198d9afc62cb
https://hg.mozilla.org/mozilla-central/rev/41f65bf1daf4
https://hg.mozilla.org/mozilla-central/rev/6ac0ea7a64e2

Comment 15

[BugBot [:suhaib / :marco/ :calixte]]

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

Comment 16

[(No longer employed by Mozilla) Aaron Klotz]

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.

Comment 17

[Ryan VanderMeulen [:RyanVM]]

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

Comment 18

[(No longer employed by Mozilla) Aaron Klotz]

Yeah, this needs to bake.