Closed Bug 1463596 Opened 4 years ago Closed 4 years ago

clang-cl fails TestDllInterceptor due to different page protection in nop space


(Core :: XPCOM, defect)

Not set



Tracking Status
firefox-esr52 --- unaffected
firefox-esr60 --- unaffected
firefox60 --- unaffected
firefox61 --- verified
firefox62 --- fixed


(Reporter: away, Assigned: bugzilla)



(Keywords: regression)


(2 files, 1 obsolete file)

This is bug 1201205 all over again. (In that patch, I left a comment describing the problem. It looks like the comment survived the interceptor refactor but the behavior didn't, oops.)

0:000> !address rotatepayload-5
Protect:                00000002          PAGE_READONLY

0:000> !address rotatepayload
Protect:                00000020          PAGE_EXECUTE_READ

aklotz: I'd normally just fix this myself, but I've been finding the new memory policy code to be extremely hard to understand due to all the indirection. If this is an easy one-liner for you, do you mind putting together a patch?
Flags: needinfo?(aklotz)
Component: General → XPCOM
Sure, I'll take it.
Assignee: nobody → aklotz
Flags: needinfo?(aklotz)
I had to fix a bug in the move constructor.
Attachment #8981265 - Attachment is obsolete: true
Attachment #8981265 - Flags: review?(davidp99)
Attachment #8981267 - Flags: review?(davidp99)
Wow, this turned out to be more involved than I had figured. Thanks Aaron!
Attachment #8981267 - Flags: review?(davidp99) → review+
Bug 1463596: Ensure that WritableTargetFunction correctly handles changing of protection attributes across regions that straddle page boundaries and have different initial protection attributes; r=handyman
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla62
Comment on attachment 8981267 [details] [diff] [review]
Ensure protection changes work on regions that straddle page boundaries w/ differing initial protection attributes (r2)

Approval Request Comment
[Feature/Bug causing the regression]: bug 1432653
[User impact if declined]: Hooks might not be set under certain conditions, affecting various behaviors across our code base
[Is this code covered by automated tests?]: Yes
[Has the fix been verified in Nightly?]: Yes
[Needs manual test from QE? If yes, steps to reproduce]: No
[List of other uplifts needed for the feature/fix]: None
[Is the change risky?]: Low
[Why is the change risky/not risky?]: It's a straightforward patch, it passes automated tests, and has been verified on Nightly.
[String changes made/needed]: None
Attachment #8981267 - Flags: approval-mozilla-beta?
Comment on attachment 8981267 [details] [diff] [review]
Ensure protection changes work on regions that straddle page boundaries w/ differing initial protection attributes (r2)

Not sure if SV was planning another round of TestDLLInterceptor testing during this Beta cycle or not, but they definitely should be now :).

Approved for 61.0b12.
Attachment #8981267 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
We did run another round of TestDLLInterceptor on Fx 61.0b12 and we found a new issue (bug 1467798) that is being addressed separately but did not encounter this issue here. Marking as verified on 61 based on our testing.
Flags: qe-verify+
You need to log in before you can comment on or make changes to this bug.