Closed Bug 700822 Opened 13 years ago Closed 12 years ago

Land hardening option and win32 ExecutableAllocator randomization

Categories

(Core :: JavaScript Engine, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla13
Tracking Status
firefox11 - ---

People

(Reporter: cdleary, Assigned: cdleary)

References

Details

Attachments

(1 file)

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/mozilla-central/rev/ecc57a1b8b89
https://hg.mozilla.org/mozilla-central/rev/1bf4c1a6412b
Status: ASSIGNED → RESOLVED
Closed: 13 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 → ---
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
Closed: 13 years ago12 years ago
Resolution: --- → FIXED
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.
(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. :-)
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?
(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.
Depends on: 960416
Depends on: 1232907
You need to log in before you can comment on or make changes to this bug.