Manually randomize the memory addresses for regions of JIT code in the Large ...

VirtualAlloc on Windows 7 does not currently provide sufficient randomization to protect JIT code from being aligned in large regions at a predictable location.

This patch manually randomizes the allocation address for PAGE_EXECUTE_READWRITE regions between kAllocationRandomAddressMin and kAllocationRandomAddressMax.


BUG=none
TEST=allocate lots of javascript code and check for contiguous allocations

Committed: http://code.google.com/p/v8/source/detail?r=5169

[Søren Thygesen Gjesse, 2010-07-30 08:08:14]

http://codereview.chromium.org/2832095/diff/6001/7001
File src/platform-win32.cc (right):

http://codereview.chromium.org/2832095/diff/6001/7001#newcode846
src/platform-win32.cc:846: // TODO(pmehta@chromium.org): Port to x64 and Linux
Please create an issue on code.google.com/p/v8 and change this TODO to
TODO(issue number).

http://codereview.chromium.org/2832095/diff/6001/7001#newcode846
src/platform-win32.cc:846: // TODO(pmehta@chromium.org): Port to x64 and Linux
Please add a comment that for executable pages we try to randomize the
allocation address.

http://codereview.chromium.org/2832095/diff/6001/7001#newcode846
src/platform-win32.cc:846: // TODO(pmehta@chromium.org): Port to x64 and Linux
To support x64 will it not just be a matter of having the constant 0x4FFF0000 in
a intptr_t constant with a different constant on x64 controlled by
V8_HOST_ARCH_64_BIT, see globals.h. If not please use a #ifdef
V8_HOST_ARCH_64_BIT to use the original code for x64.

http://codereview.chromium.org/2832095/diff/6001/7001#newcode847
src/platform-win32.cc:847: if (prot == PAGE_EXECUTE_READWRITE &&
I suggest you keep this condition on one line.

http://codereview.chromium.org/2832095/diff/6001/7001#newcode854
src/platform-win32.cc:854: if (mbase == NULL &&
Condition on one line.

[Valoo, 2010-07-30 22:29:48]

http://codereview.chromium.org/2832095/diff/6001/7001
File src/platform-win32.cc (right):

http://codereview.chromium.org/2832095/diff/6001/7001#newcode846
src/platform-win32.cc:846: // TODO(pmehta@chromium.org): Port to x64 and Linux
On 2010/07/30 08:08:14, Søren Gjesse wrote:
> Please create an issue on code.google.com/p/v8 and change this TODO to
> TODO(issue number).

Done.

http://codereview.chromium.org/2832095/diff/6001/7001#newcode846
src/platform-win32.cc:846: // TODO(pmehta@chromium.org): Port to x64 and Linux
On 2010/07/30 08:08:14, Søren Gjesse wrote:
> Please add a comment that for executable pages we try to randomize the
> allocation address.

Done.

http://codereview.chromium.org/2832095/diff/6001/7001#newcode846
src/platform-win32.cc:846: // TODO(pmehta@chromium.org): Port to x64 and Linux
On 2010/07/30 08:08:14, Søren Gjesse wrote:
> To support x64 will it not just be a matter of having the constant 0x4FFF0000
in
> a intptr_t constant with a different constant on x64 controlled by
> V8_HOST_ARCH_64_BIT, see globals.h. If not please use a #ifdef
> V8_HOST_ARCH_64_BIT to use the original code for x64.

Yup, your right.  I wasn't sure if platform-win32.cc was used for the x64 build.
 I defined kAllocationRandomAddressMin/Max in platform.h (is this the right
place for it? I didn't think that globals.h was since its OS/Architecture
dependent)

http://codereview.chromium.org/2832095/diff/6001/7001#newcode847
src/platform-win32.cc:847: if (prot == PAGE_EXECUTE_READWRITE &&
On 2010/07/30 08:08:14, Søren Gjesse wrote:
> I suggest you keep this condition on one line.

Done.

http://codereview.chromium.org/2832095/diff/6001/7001#newcode854
src/platform-win32.cc:854: if (mbase == NULL &&
On 2010/07/30 08:08:14, Søren Gjesse wrote:
> Condition on one line.

Done.

[Valoo, 2010-07-31 04:13:15]

http://codereview.chromium.org/2832095/diff/6001/7001
File src/platform-win32.cc (right):

http://codereview.chromium.org/2832095/diff/6001/7001#newcode846
src/platform-win32.cc:846: // TODO(pmehta@chromium.org): Port to x64 and Linux
Should this feature be wrapped in a #ifndef DEBUG so it doesn't affect debug
builds?

Also, some comparisons and benchmarks if your interested.

Randomized Chrome Address Space:
http://docs.google.com/a/chromium.org/leaf?id=0B9JqhY6Hu_kBZTc2ODA0ODctMThhYi...
Unmodified Chrome Address Space:
http://docs.google.com/a/chromium.org/leaf?id=0B9JqhY6Hu_kBOGNkNjE0NGUtNzFkNS...
V8 Benchmark Result Comparison:
http://docs.google.com/a/chromium.org/leaf?id=0B9JqhY6Hu_kBOTc1ZDBiOGEtOTFhMS...
V8 Benchmark Suite Comparison Data:
http://docs.google.com/a/chromium.org/leaf?id=0B9JqhY6Hu_kBM2Y5Y2RhM2ItMmFlMC...

On 2010/07/30 22:29:48, Valoo wrote:
> Yup, your right.  I wasn't sure if platform-win32.cc was used for the x64
build.
>  I defined kAllocationRandomAddressMin/Max in platform.h (is this the right
> place for it? I didn't think that globals.h was since its OS/Architecture
> dependent)
> > On 2010/07/30 08:08:14, Søren Gjesse wrote:
> > To support x64 will it not just be a matter of having the constant
0x4FFF0000
> in
> > a intptr_t constant with a different constant on x64 controlled by
> > V8_HOST_ARCH_64_BIT, see globals.h. If not please use a #ifdef
> > V8_HOST_ARCH_64_BIT to use the original code for x64.
>

[Søren Thygesen Gjesse, 2010-08-02 15:30:33]

Patch has been landed on bleeding edge.

I moved the constants into platform-win32.cc before committing, as they are
currently Windows specific.

Closing issue.

On 2010/07/31 04:13:15, Valoo wrote:
> http://codereview.chromium.org/2832095/diff/6001/7001
> File src/platform-win32.cc (right):
> 
> http://codereview.chromium.org/2832095/diff/6001/7001#newcode846
> src/platform-win32.cc:846: // TODO(pmehta@chromium.org): Port to x64 and Linux
> Should this feature be wrapped in a #ifndef DEBUG so it doesn't affect debug
> builds?
> 
> Also, some comparisons and benchmarks if your interested.
> 
> Randomized Chrome Address Space:
>
http://docs.google.com/a/chromium.org/leaf?id=0B9JqhY6Hu_kBZTc2ODA0ODctMThhYi...
> Unmodified Chrome Address Space:
>
http://docs.google.com/a/chromium.org/leaf?id=0B9JqhY6Hu_kBOGNkNjE0NGUtNzFkNS...
> V8 Benchmark Result Comparison:
>
http://docs.google.com/a/chromium.org/leaf?id=0B9JqhY6Hu_kBOTc1ZDBiOGEtOTFhMS...
> V8 Benchmark Suite Comparison Data:
>
http://docs.google.com/a/chromium.org/leaf?id=0B9JqhY6Hu_kBM2Y5Y2RhM2ItMmFlMC...
> 
> On 2010/07/30 22:29:48, Valoo wrote:
> > Yup, your right.  I wasn't sure if platform-win32.cc was used for the x64
> build.
> >  I defined kAllocationRandomAddressMin/Max in platform.h (is this the right
> > place for it? I didn't think that globals.h was since its OS/Architecture
> > dependent)
> > > On 2010/07/30 08:08:14, Søren Gjesse wrote:
> > > To support x64 will it not just be a matter of having the constant
> 0x4FFF0000
> > in
> > > a intptr_t constant with a different constant on x64 controlled by
> > > V8_HOST_ARCH_64_BIT, see globals.h. If not please use a #ifdef
> > > V8_HOST_ARCH_64_BIT to use the original code for x64.
> >