WindowsDllDetourPatcher uses the wrong jump target in the pJmp32 case

RESOLVED FIXED in mozilla30

Status

()

defect
RESOLVED FIXED
6 years ago
6 years ago

People

(Reporter: dmajor, Assigned: dmajor)

Tracking

(Blocks 1 bug)

Trunk
mozilla30
x86_64
Windows 7
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 2 obsolete attachments)

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?
Posted 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+
Posted 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: 6 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.