Re: gawk 3.1.6: rand.awk test case alignment problem

[Aharon Robbins]

This is now applied. It'll be showing up in CVS shortly.

See my earlier note about the dfa.c patch, which isn't correct,
but I've added the correct fix and it will be in CVS shortly also.

While I agree with Andy that POSIX doesn't require integer alignment,
I'd just as soon do something that doesn't break other systems than
try to fight GCC / architecture problems.

Thanks!

Arnold

> Date: Thu, 13 Mar 2008 14:53:56 +0000
> From: Duncan Moore <address@hidden>
> Subject: gawk 3.1.6: rand.awk test case alignment problem
> To: address@hidden
>
> Hi
>
> I've sent this bug report to gnu.utils.bug twice recently (Feb 11, Jan 2), 
> but now think this was the wrong address. There's been no response, so I'm 
> now sending it here as well. My apologies if you've seen it before.
>
>
> I've compiled gawk 3.1.6 on an ARM 710 platform using gcc 4.1.1, and running
> the random number test case gawk-3.1.6/test/rand.awk:
>
> BEGIN {
>  srand(2)
>  for (i = 0; i < 19; i++)
>   printf "%3d ", (1 + int(100 * rand()))
>      print ""
> }
>
> with 'gawk -f rand.awk', gives:
>
>  30   5  39  81  93  31  71  45  82  43  99  97 100  52  86  91  68  65  60
>
> As I understand it, this is wrong, and the output should be identical on
> every platform, since the RNG is built in to gawk. This is what should be
> output:
>
>  62  67  88   6  35  77   3  68  30  96  90  26  35   8  88  93  49  53  37
>
> The reason for the error, is that char array 'state' in builtin.c is not
> word aligned, and it is cast to an int*. This causes problems on
> architectures that require ints to be word aligned. There is a comment in
> random.c that this causes bus errors on the Sparc platform, but the code
> does not appear to have been fixed. Other platforms could be affected too.
>
> A fix for this (which will not affect other platforms) is:
>
> diff -u -p gawk-3.1.6/builtin.c gawk-3.1.6.ro2/builtin.c
> --- gawk-3.1.6/builtin.c 2007-09-30 19:57:05 +0000
> +++ gawk-3.1.6.ro2/builtin.c 2008-01-21 19:38:12 +0000
> @@ -2027,14 +2027,16 @@ do_cos(NODE *tree)
>  /* do_rand --- do the rand function */
>
>  static int firstrand = TRUE;
> -static char state[256];
> +#define SIZEOF_STATE 256
> +static uint32_t istate[SIZEOF_STATE/sizeof(uint32_t)];
> +static char *const state = (char *const) istate;
>
>  /* ARGSUSED */
>  NODE *
>  do_rand(NODE *tree ATTRIBUTE_UNUSED)
>  {
>   if (firstrand) {
> -  (void) initstate((unsigned) 1, state, sizeof state);
> +  (void) initstate((unsigned) 1, state, SIZEOF_STATE);
>    /* don't need to srandom(1), initstate() does it for us. */
>    firstrand = FALSE;
>    setstate(state);
> @@ -2057,7 +2059,7 @@ do_srand(NODE *tree)
>   long ret = save_seed; /* SVR4 awk srand returns previous seed */
>
>   if (firstrand) {
> -  (void) initstate((unsigned) 1, state, sizeof state);
> +  (void) initstate((unsigned) 1, state, SIZEOF_STATE);
>    /* don't need to srandom(1), we're changing the seed below */
>    firstrand = FALSE;
>    (void) setstate(state);
>
> The rand.awk test case then gives the same results as on other platforms.
>
>
> As an aside, when trying to locate the problem I used -Wextra which gives
> this warning (amongst others):
>
> dfa.c: At top level:
> dfa.c:762: warning: missing initializer
> dfa.c:762: warning: (near initialization for 'prednames[12].pred')
>
> This turned out to be a red-herring. I think the following change may better
> reflect what the compiler actually produces, and what the programmer
> intended. (It's some time now since I looked at this, but I think that the
> search of the array can't run off the end.)
>
> --- gawk-3.1.6/dfa.c            2007-09-03 03:30:12 +0000
> +++ gawk-3.1.6.ro2/dfa.c        2008-01-02 14:22:54 +0000
> @@ -759,7 +759,6 @@ static struct {
>    { ":graph:]", is_graph },
>    { ":cntrl:]", is_cntrl },
>    { ":blank:]", is_blank },
> -  { 0 }
>  };
>
> Regards
>
> Duncan Moore