Land hardening option and win32 ExecutableAllocator randomization

RESOLVED FIXED in mozilla13

Status

()

Core
JavaScript Engine
RESOLVED FIXED
6 years ago
2 years ago

People

(Reporter: cdleary, Assigned: cdleary)

Tracking

(Blocks: 1 bug)

unspecified
mozilla13
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox11-)

Details

Attachments

(1 attachment)

Separate bug for landing data/resolution, based on patch 2 in bug 677272 (which I should have split into different bugs to begin with).

It's been rebased, pushed to try:

https://hg.mozilla.org/try/pushloghtml?changeset=a19f0237eb77
https://hg.mozilla.org/integration/mozilla-inbound/rev/ecc57a1b8b89
https://hg.mozilla.org/integration/mozilla-inbound/rev/1bf4c1a6412b
Target Milestone: --- → mozilla11
https://hg.mozilla.org/mozilla-central/rev/ecc57a1b8b89
https://hg.mozilla.org/mozilla-central/rev/1bf4c1a6412b
Status: ASSIGNED → RESOLVED
Last Resolved: 6 years ago
Resolution: --- → FIXED
ehr 1bf4c1a6412b has been backed out
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Yeah, sorry about that -- I thought I had added a backout cset comment, but apparently not. Have to debug the frequent WinXP reftest failure caused by the win32 randomization patch today.
Status: REOPENED → ASSIGNED
https://hg.mozilla.org/integration/mozilla-inbound/rev/6ea9d4ad53a0 (backout)

Need to debug failing Windows XP reftest.
Target Milestone: mozilla11 → ---
tracking-firefox11: --- → +
Take 2, with WinXP workaround:

https://hg.mozilla.org/integration/mozilla-inbound/rev/9e93f190f64c

It looked good on try, hopefully it sticks.
Target Milestone: --- → mozilla13
dvander gave post-push r+ on the workaround, because I felt like I could use a sanity check. Thanks, David!
https://hg.mozilla.org/mozilla-central/rev/9e93f190f64c
Status: ASSIGNED → RESOLVED
Last Resolved: 6 years ago5 years ago
Resolution: --- → FIXED

Updated

5 years ago
Depends on: 728623
Why do we need the separate RandomizeIsBrokenImpl and RandomizeIsBroken?
It's not clear why we need to track this for FF11. Please re-nominate if you disagree with this bug being untracked for FF11.
tracking-firefox11: + → -
(In reply to Boris Zbarsky (:bz) from comment #9)
> Why do we need the separate RandomizeIsBrokenImpl and RandomizeIsBroken?

I was trying to get the compiler intrinsic guards for |static type val = expr()| to avoid calling RandomizeIsBroken twice if you race to create runtimes from different threads. Kind of a hack, but I think it should work out okay. I can file a followup bug if you think that's too heinous.
I thinks it's OK for that, assuming the compiler handles such a race correctly, but then you need a nice comment explaining that's what's going on!
(In reply to Boris Zbarsky (:bz) from comment #12)
> I thinks it's OK for that, assuming the compiler handles such a race
> correctly, but then you need a nice comment explaining that's what's going
> on!

Yeah, mea culpa. Will pastebin r? you on IRC in a little while for a comment patch. :-)
Created attachment 600451 [details] [diff] [review]
Add explanatory comment.
Attachment #600451 - Flags: review?(bzbarsky)
Comment on attachment 600451 [details] [diff] [review]
Add explanatory comment.

r=me
Attachment #600451 - Flags: review?(bzbarsky) → review+
Though are you sure that all our compilers do threadsafety stuff here?
https://hg.mozilla.org/integration/mozilla-inbound/rev/941481edc290
(In reply to Boris Zbarsky (:bz) from comment #16)
> Though are you sure that all our compilers do threadsafety stuff here?

I'm not totally sure about MSVC, but, even if they do race, they're writing the same atomic-sized value (int) to the static, so you should get consistency anyway. Kind of hacky, but I'm not sure we even really support this mutliple runtimes racing for creation on multiple threads use case anyway, it's kind of a best-effort approach on my part for a simple thread-safe memoization.
https://hg.mozilla.org/mozilla-central/rev/941481edc290
Depends on: 960416
Depends on: 1232907
You need to log in before you can comment on or make changes to this bug.