WindowsDllDetourPatcher uses the wrong jump target in the pJmp32 case

RESOLVED FIXED in mozilla30

Status

()

RESOLVED FIXED
5 years ago
5 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)

(Assignee)

Description

5 years ago
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?
(Assignee)

Comment 1

5 years ago
Created attachment 8375951 [details] [diff] [review]
pJmp32_fix
Assignee: nobody → dmajor
Attachment #8375951 - Flags: review?(mh+mozilla)
(Assignee)

Updated

5 years ago
Attachment #8375951 - Attachment is patch: true
(Assignee)

Comment 2

5 years ago
Created attachment 8375954 [details] [diff] [review]
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.
(Assignee)

Updated

5 years ago
Blocks: 681238, 951827

Comment 3

5 years ago
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+
(Assignee)

Comment 5

5 years ago
Created attachment 8378063 [details] [diff] [review]
pJmp32_fix

Updated patch title
Attachment #8375951 - Attachment is obsolete: true
Attachment #8375954 - Attachment is obsolete: true
Attachment #8378063 - Flags: review+
(Assignee)

Comment 6

5 years ago
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]
(Assignee)

Updated

5 years ago
Blocks: 985700
(Assignee)

Comment 9

5 years ago
Changed my mind about comment 6. I moved the test into its own bug. Resolving this based on comment 8.
Status: NEW → RESOLVED
Last Resolved: 5 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.