code review 1904044: runtime: add mmap of null page just in case

runtime: add mmap of null page just in case

[rsc, 2010-08-03 08:04:17]

Hello r (cc: golang-dev@googlegroups.com, robert.swiecki@gmail.com),

I'd like you to review this change.

[iant, 2010-08-03 08:34:25]

FYI

http://codereview.appspot.com/1904044/diff/2001/3003
File src/pkg/runtime/linux/mem.c (right):

http://codereview.appspot.com/1904044/diff/2001/3003#newcode50
src/pkg/runtime/linux/mem.c:50: runtime_mmap(nil, 4096, PROT_NONE,
MAP_ANON|MAP_PRIVATE, -1, 0);
Passing NULL for the first argument tells mmap to allocate some random page of
memory, which does little harm but is pointless.  In order to explicitly map the
page at address 0, you need to also pass MAP_FIXED in the flags.  That will then
reliably fail on Linux.

Also MAP_ANONYMOUS is the approved name, MAP_ANON is the old deprecated name.

[robert.swiecki, 2010-08-03 15:53:19]

After the fixes it'd be LGTM.

Also, I think you might want to use some PAGE_SIZE constant (should be defined
in sys/user.h), instead of 4096 (even on x86/x86-64 the page size might be of
different size, because of the HugeTLB feature).

[rsc1, 2010-08-03 20:13:45]

On 2010/08/03 15:53:19, robert.swiecki wrote:
> After the fixes it'd be LGTM.
> 
> Also, I think you might want to use some PAGE_SIZE constant (should be defined
> in sys/user.h), instead of 4096 (even on x86/x86-64 the page size might be of
> different size, because of the HugeTLB feature).

This isn't standard C so there's no sys/user.h.
4096 is what the compilers assume as the cutoff
(for larger offsets they emit checks before dereferencing),
so it's in sync with them.

[rsc, 2010-09-28 19:33:13]

PTAL

I shelved this change and forgot about it.
I've added the necessary changes to make
it compile and run correctly on FreeBSD, Linux, and OS X.
I still think it's overkill, but at least we get a better
error when mmap fails now.

Russ

[r, 2010-09-28 20:21:02]

LGTM

[iant, 2010-09-28 20:25:09]

LGTM

[rsc, 2010-09-29 00:30:05]

*** Submitted as http://code.google.com/p/go/source/detail?r=c55fd283bda6 ***

runtime: add mmap of null page just in case

R=r, iant, robert.swiecki, rsc1
CC=golang-dev
http://codereview.appspot.com/1904044