972671 - WindowsDllDetourPatcher uses the wrong jump target in the pJmp32 case

Status: RESOLVED FIXED

(Core :: General, defect)

Description

[(Away)]

STR: Apply the test patch, run TestDllInterceptor.exe

Discovered while working on a fix for bug 951827. The code only needs to account for the difference between the old and new targets. Maybe this went unnoticed because jumps are frequently at offset 0?

Comment 1

[(Away)]

Attachment: pJmp32_fix

Comment 2

[(Away)]

Attachment: Patch with test (ugly, needs cleanup)

This is yucky, just a demonstration. I want to put it in an assembly file to get better control over the jump, and since this won't compile on win64 anyway.

Comment 3

[Benjamin Smedberg]

Comment on attachment 8375951 [details] [diff] [review]
pJmp32_fix

Makoto should review this.

Comment 4

[Makoto Kato [:m_kato]]

Comment on attachment 8375951 [details] [diff] [review]
pJmp32_fix

Review of attachment 8375951 [details] [diff] [review]:
-----------------------------------------------------------------

thanks.  Good

Comment 5

[(Away)]

Attachment: pJmp32_fix

Updated patch title

Comment 6

[(Away)]

I want to get this in because bug 951827 may make us hit this code more often. Will follow up with a cleaner version of the test code later.

Comment 7

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/795b23c31b4d

Comment 8

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/795b23c31b4d

Comment 9

[(Away)]

Changed my mind about comment 6. I moved the test into its own bug. Resolving this based on comment 8.