Closed Bug 972671 Opened 7 years ago Closed 7 years ago

WindowsDllDetourPatcher uses the wrong jump target in the pJmp32 case

Categories

(Core :: General, defect)

x86_64
Windows 7
defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla30

People

(Reporter: dmajor, Assigned: dmajor)

References

(Blocks 1 open bug)

Details

Attachments

(1 file, 2 obsolete files)

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?
Attached patch pJmp32_fix (obsolete) — Splinter Review
Assignee: nobody → dmajor
Attachment #8375951 - Flags: review?(mh+mozilla)
Attachment #8375951 - Attachment is patch: true
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.
Blocks: 681238, 951827
Comment on attachment 8375951 [details] [diff] [review]
pJmp32_fix

Makoto should review this.
Attachment #8375951 - Flags: review?(mh+mozilla) → review?(m_kato)
Comment on attachment 8375951 [details] [diff] [review]
pJmp32_fix

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

thanks.  Good
Attachment #8375951 - Flags: review?(m_kato) → review+
Attached patch pJmp32_fixSplinter Review
Updated patch title
Attachment #8375951 - Attachment is obsolete: true
Attachment #8375954 - Attachment is obsolete: true
Attachment #8378063 - Flags: review+
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.
Keywords: checkin-needed
Whiteboard: [leave open]
Blocks: 985700
Changed my mind about comment 6. I moved the test into its own bug. Resolving this based on comment 8.
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Whiteboard: [leave open]
Target Milestone: --- → mozilla30
You need to log in before you can comment on or make changes to this bug.