code review 5978051: runtime: get per-process randome number from auxv for h...

runtime: get per-process random number from auxv for hash table
Decode AT_RANDOM, AT_HWCAP, and AT_PLATFORM.
This CL only make use of AT_RANDOM, but future CLs will make use of the others.

[minux1, 2012-04-15 18:03:09]

Hello golang-dev@googlegroups.com (cc: golang-dev@googlegroups.com),

I'd like you to review this change to
https://code.google.com/p/go/

[dave_cheney.net, 2012-04-15 22:34:54]

Hello,

Thank you for this CL. Previously you've mention that AT_* were newer kernel
features. Which kernel did they get added too? Specifically RHEL5 is a known
working platform, but uses 2.6.18, will this change continue to work there?

On 16/04/2012, at 4:03, minux.ma@gmail.com wrote:

> Reviewers: golang-dev_googlegroups.com,
> 
> Message:
> Hello golang-dev@googlegroups.com (cc: golang-dev@googlegroups.com),
> 
> I'd like you to review this change to
> https://code.google.com/p/go/
> 
> 
> Description:
> runtime: get per-process randome number from auxv for hash table
> Decode AT_RANDOM, AT_HWCAP, and AT_PLATFORM.
> This CL only make use of AT_RANDOM, but future CLs will make use of the
> others.
> 
> Please review this at http://codereview.appspot.com/5978051/
> 
> Affected files:
>  M src/pkg/runtime/asm_arm.s
>  M src/pkg/runtime/rt0_linux_arm.s
>  M src/pkg/runtime/signal_linux_arm.c
>  M src/pkg/runtime/sys_linux_arm.s
> 
> 
> Index: src/pkg/runtime/asm_arm.s
> ===================================================================
> --- a/src/pkg/runtime/asm_arm.s
> +++ b/src/pkg/runtime/asm_arm.s
> @@ -258,16 +258,6 @@
> TEXT runtime·emptyfunc(SB),0,$0
>    RET
> 
> -// int64 runtime·cputicks(), so really
> -// void runtime·cputicks(int64 *ticks)
> -// stubbed: return int64(0)
> -TEXT runtime·cputicks(SB),7,$0
> -    MOVW    0(FP), R1
> -    MOVW    $0, R0
> -    MOVW    R0, 0(R1)
> -    MOVW    R0, 4(R1)
> -    RET
> -
> TEXT runtime·abort(SB),7,$-4
>    MOVW    $0, R0
>    MOVW    (R0), R1
> Index: src/pkg/runtime/rt0_linux_arm.s
> ===================================================================
> --- a/src/pkg/runtime/rt0_linux_arm.s
> +++ b/src/pkg/runtime/rt0_linux_arm.s
> @@ -37,6 +37,10 @@
>    MOVW    $174, R7 // sys_sigaction
>    SWI    $0 // restore signal handler
>    ADD    $32, R13
> +
> +    SUB    $4, R13 // fake a stack frame for runtime·setup_auxv
> +    BL    runtime·setup_auxv(SB)
> +    ADD    $4, R13
>    B    _rt0_arm(SB)
> 
> TEXT bad_abi<>(SB),7,$-4
> Index: src/pkg/runtime/signal_linux_arm.c
> ===================================================================
> --- a/src/pkg/runtime/signal_linux_arm.c
> +++ b/src/pkg/runtime/signal_linux_arm.c
> @@ -138,3 +138,45 @@
>    sa.sa_handler = fn;
>    runtime·rt_sigaction(i, &sa, nil, 8);
> }
> +
> +#define AT_NULL        0
> +#define AT_HWCAP    16
> +#define AT_RANDOM    25
> +#define AT_PLATFORM    15
> +uint32 runtime·random_number;
> +uint32 runtime·hwcap;
> +uint8 runtime·arm_arch = 6; // we default to ARMv6
> +
> +#pragma textflag 7
> +void
> +runtime·setup_auxv(int32 argc, void *argv_list)
> +{
> +    byte **argv = &argv_list;
> +    byte **envp;
> +    uint32 *auxv;
> +    uint32 t;
> +
> +    // skip envp to get to ELF auxiliary vector.
> +    for(envp = &argv[argc+1]; *envp != nil; envp++)
> +        ;
> +    envp++;
> +    
> +    for(auxv=(uint32*)envp; auxv[0] != AT_NULL; auxv += 2) {
> +        switch(auxv[0]) {
> +        case AT_RANDOM: // kernel provided 16-byte worth of random data
> +            if(auxv[1])
> +                runtime·random_number = *(uint32 *)(4 + auxv[1]);
> +            break;
> +        case AT_PLATFORM: // v5l, v6l, v7l
> +            if(auxv[1]) {
> +                t = (*(uint8 *)(auxv[1]+1));
> +                if(t >= '5' && t <= '7')
> +                    runtime·arm_arch = t - '0';
> +            }
> +            break;
> +        case AT_HWCAP: // CPU capability bit flags
> +            runtime·hwcap = auxv[1];
> +            break;
> +        }
> +    }
> +}
> Index: src/pkg/runtime/sys_linux_arm.s
> ===================================================================
> --- a/src/pkg/runtime/sys_linux_arm.s
> +++ b/src/pkg/runtime/sys_linux_arm.s
> @@ -385,3 +385,14 @@
>    MOVW    $SYS_sched_yield, R7
>    SWI    $0
>    RET
> +
> +// int64 runtime·cputicks(), so really
> +// void runtime·cputicks(int64 *ticks)
> +// return the same random number that kernel provides,
> +// good enough for our use.
> +TEXT runtime·cputicks(SB),7,$0
> +    MOVW    0(FP), R1
> +    MOVW    runtime·random_number(SB), R0
> +    MOVW    R0, 0(R1)
> +    MOVW    R0, 4(R1)
> +    RET
> 
> 

[minux1, 2012-04-16 05:30:58]

On Mon, Apr 16, 2012 at 6:33 AM, Dave Cheney <dave@cheney.net> wrote:
>
> Thank you for this CL. Previously you've mention that AT_* were newer
> kernel features. Which kernel did they get added too? Specifically RHEL5 is
> a known working platform, but uses 2.6.18, will this change continue to
> work there?

AT_RANDOM is introduced in 2.6.29, but as the original code always use 0, I
think this is acceptable (I could
have read random bytes from /dev/random, but it might be overkill to read a
device file each time we create
a map). And, of course, we could do better by using some pseudo random
number generator for runtime.cputicks
and only use kernel provided random number as seed (I used fastrand1
algorithm for this now).
AT_PLATFORM is introduced in at least 2.6.11.
AT_HWCAP is introduced in at least 2.6.11 (
http://lxr.linux.no/#linux+v2.6.11/include/asm-arm/procinfo.h#L50)
(I've added this info in the comment)

[minux1, 2012-04-16 05:56:45]

On 2012/04/16 05:30:58, minux wrote:
> AT_RANDOM is introduced in 2.6.29, but as the original code always use 0, I
> think this is acceptable (I could have read random bytes from /dev/random, 
> but it might be overkill to read a device file each time we create a map).
Correction: we only call runtime.cputicks() once per M.

[dave_cheney.net, 2012-04-18 10:21:37]

Are there any parts of this CL that are applicable to linux/amd64 and 386 ? It
might improve the level of confidence with this addition if it can be tested
natively all the linux archs.

[minux1, 2012-04-18 10:27:58]

On Wed, Apr 18, 2012 at 6:21 PM, <dave@cheney.net> wrote:

> Are there any parts of this CL that are applicable to linux/amd64 and
> 386 ? It might improve the level of confidence with this addition if it

linux/386 already use auxv to get AT_SYSINFO (VDSO).

[dave_cheney.net, 2012-04-18 10:29:22]

Excellent, I didn't realise.

On Wed, Apr 18, 2012 at 8:27 PM, minux <minux.ma@gmail.com> wrote:
>
> On Wed, Apr 18, 2012 at 6:21 PM, <dave@cheney.net> wrote:
>>
>> Are there any parts of this CL that are applicable to linux/amd64 and
>> 386 ? It might improve the level of confidence with this addition if it
>
> linux/386 already use auxv to get AT_SYSINFO (VDSO).

[rsc, 2012-04-25 03:06:04]

http://codereview.appspot.com/5978051/diff/12007/src/pkg/runtime/signal_linux...
File src/pkg/runtime/signal_linux_arm.c (right):

http://codereview.appspot.com/5978051/diff/12007/src/pkg/runtime/signal_linux...
src/pkg/runtime/signal_linux_arm.c:168: runtime·randomNumber = *(uint32 *)(4 +
auxv[1]);
*(uint32*)(auxv[1]+4)

http://codereview.appspot.com/5978051/diff/12007/src/pkg/runtime/signal_linux...
src/pkg/runtime/signal_linux_arm.c:172: t = (*(uint8 *)(auxv[1]+1));
lots of parens here.  t = *(byte*)(auxv[1]+1)

http://codereview.appspot.com/5978051/diff/12007/src/pkg/runtime/sys_linux_arm.s
File src/pkg/runtime/sys_linux_arm.s (right):

http://codereview.appspot.com/5978051/diff/12007/src/pkg/runtime/sys_linux_ar...
src/pkg/runtime/sys_linux_arm.s:389: // int64 runtime·cputicks(), so really
Looks like this can go in signal_linux_arm.c?
I don't see any reason it needs to be written in assembly.

[minux1, 2012-04-25 08:03:59]

Please take another look.

[rsc, 2012-05-03 21:53:27]

LGTM

http://codereview.appspot.com/5978051/diff/19001/src/pkg/runtime/signal_linux...
File src/pkg/runtime/signal_linux_arm.c (right):

http://codereview.appspot.com/5978051/diff/19001/src/pkg/runtime/signal_linux...
src/pkg/runtime/signal_linux_arm.c:146: uint32 runtime·randomNumber;
Can you make this static?
It does not seem to be used outside this file.
What about hwcap?

http://codereview.appspot.com/5978051/diff/19001/src/pkg/runtime/signal_linux...
src/pkg/runtime/signal_linux_arm.c:148: uint8 runtime·armArch = 6; // we default
to ARMv6
There it is!

http://codereview.appspot.com/5978051/diff/19001/src/pkg/runtime/signal_linux...
src/pkg/runtime/signal_linux_arm.c:168: runtime·randomNumber = *(uint32
*)(auxv[1] + 4);
drop space in (uint32 *)

http://codereview.appspot.com/5978051/diff/19001/src/pkg/runtime/signal_linux...
src/pkg/runtime/signal_linux_arm.c:172: t = *(uint8 *)(auxv[1]+1);
drop space in (uint8 *)

[minux1, 2012-05-04 17:45:57]

On 2012/05/03 21:53:27, rsc wrote:
>
http://codereview.appspot.com/5978051/diff/19001/src/pkg/runtime/signal_linux...
> src/pkg/runtime/signal_linux_arm.c:146: uint32 runtime·randomNumber;
> Can you make this static?
Done.
> It does not seem to be used outside this file.
> What about hwcap?
It's unused now, but if we latter want to detect whether the machine has FPU
(VFP) or not,
we can make use of that.

>
http://codereview.appspot.com/5978051/diff/19001/src/pkg/runtime/signal_linux...
> src/pkg/runtime/signal_linux_arm.c:168: runtime·randomNumber = *(uint32
> *)(auxv[1] + 4);
> drop space in (uint32 *) 
>
http://codereview.appspot.com/5978051/diff/19001/src/pkg/runtime/signal_linux...
> src/pkg/runtime/signal_linux_arm.c:172: t = *(uint8 *)(auxv[1]+1);
> drop space in (uint8 *)
Both done.

[minux1, 2012-05-04 17:59:28]

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

runtime: get per-process random number from auxv for hash table
Decode AT_RANDOM, AT_HWCAP, and AT_PLATFORM.
This CL only make use of AT_RANDOM, but future CLs will make use of the others.

R=dave, rsc
CC=golang-dev
http://codereview.appspot.com/5978051