code review 13334049: build: remove various uses of C undefined behavior

build: remove various uses of C undefined behavior

If you thought gcc -ansi -pedantic was pedantic, just wait
until you meet clang -fsanitize=undefined.

I think this addresses all the reported "errors", but we'll
need another run to be sure.

all.bash still passes.

Update issue 5764

Dave, can you please try again?

[rsc, 2013-09-09 16:00:29]

Hello golang-dev@googlegroups.com,

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

[bradfitz, 2013-09-09 16:37:03]

LGTM



On Mon, Sep 9, 2013 at 9:00 AM, <rsc@golang.org> wrote:

> Reviewers: golang-dev1,
>
> Message:
> Hello golang-dev@googlegroups.com,
>
> I'd like you to review this change to
> https://code.google.com/p/go/
>
>
> Description:
> build: remove various uses of C undefined behavior
>
> If you thought gcc -ansi -pedantic was pedantic, just wait
> until you meet clang -fsanitize=undefined.
>
> I think this addresses all the reported "errors", but we'll
> need another run to be sure.
>
> all.bash still passes.
>
> Update issue 5764
>
> Dave, can you please try again?
>
> Please review this at
https://codereview.appspot.**com/13334049/<https://codereview.appspot.com/133...
>
> Affected files (+29, -39 lines):
>   M include/bio.h
>   M src/cmd/cc/com.c
>   M src/cmd/cc/lex.c
>   M src/cmd/cc/lexbody
>   M src/cmd/gc/bv.c
>   M src/cmd/gc/go.h
>   M src/cmd/gc/md5.c
>   M src/cmd/gc/mparith2.c
>   M src/cmd/gc/subr.c
>   M src/cmd/ld/go.c
>   M src/cmd/ld/lib.c
>   M src/cmd/pack/ar.c
>   M src/libmach/5obj.c
>   M src/libmach/6obj.c
>   M src/libmach/8obj.c
>   M src/libmach/obj.c
>
>
> Index: include/bio.h
> ==============================**==============================**=======
> --- a/include/bio.h
> +++ b/include/bio.h
> @@ -79,7 +79,7 @@
>  #define        BGETLE2(bp)\
>         ((bp)->icount<=-2?((bp)->**icount+=2,((bp)->ebuf[(bp)->**
> icount-2])|((bp)->ebuf[(bp)->**icount-1]<<8)):Bgetle2((bp)))
>  #define        BGETLE4(bp)\
> -       ((bp)->icount<=-4?((bp)->**icount+=4,((bp)->ebuf[(bp)->**
> icount-4])|((bp)->ebuf[(bp)->**icount-3]<<8)|((bp)->ebuf[(bp)**
> ->icount-2]<<16)|((bp)->ebuf[(**bp)->icount-1]<<24)):Bgetle4((**bp)))
> +       (int)((bp)->icount<=-4?((bp)->**icount+=4,((bp)->ebuf[(bp)->**
> icount-4])|((bp)->ebuf[(bp)->**icount-3]<<8)|((bp)->ebuf[(bp)**
> ->icount-2]<<16)|((uint32)(bp)**->ebuf[(bp)->icount-1]<<24)):**
> Bgetle4((bp)))
>
>  /*
>   * These macros put 1-, 2-, and 4-byte integer values by writing the
> Index: src/cmd/cc/com.c
> ==============================**==============================**=======
> --- a/src/cmd/cc/com.c
> +++ b/src/cmd/cc/com.c
> @@ -1325,10 +1325,10 @@
>                 if(lt->width == 8)
>                         hi = big(0, ~0ULL);
>                 else
> -                       hi = big(0, (1LL<<(l->type->width*8))-1);
> +                       hi = big(0, (1ULL<<(l->type->width*8))-1);
>         }else{
> -               lo = big(~0ULL, -(1LL<<(l->type->width*8-1)));
> -               hi = big(0, (1LL<<(l->type->width*8-1))-1)**;
> +               lo = big(~0ULL, -(1ULL<<(l->type->width*8-1)))**;
> +               hi = big(0, (1ULL<<(l->type->width*8-1))-**1);
>         }
>
>         switch(op){
> Index: src/cmd/cc/lex.c
> ==============================**==============================**=======
> --- a/src/cmd/cc/lex.c
> +++ b/src/cmd/cc/lex.c
> @@ -1019,7 +1019,7 @@
>                         c += 10-'A';
>                 else
>                         goto bad;
> -               nn = n*16 + c;
> +               nn = (uvlong)n*16 + c;
>                 if(n < 0 && nn >= 0)
>                         goto bad;
>                 n = nn;
> Index: src/cmd/cc/lexbody
> ==============================**==============================**=======
> --- a/src/cmd/cc/lexbody
> +++ b/src/cmd/cc/lexbody
> @@ -224,7 +224,7 @@
>  lookup(void)
>  {
>         Sym *s;
> -       int32 h;
> +       uint32 h;
>         char *p;
>         int c, l;
>         char *r, *w;
> @@ -400,7 +400,7 @@
>                         if(c >= '0' && c <= '9') {
>                                 if(c > '7' && c1 == 3)
>                                         break;
> -                               yylval.lval <<= c1;
> +                               yylval.lval = (uvlong)yylval.lval << c1;
>                                 yylval.lval += c - '0';
>                                 c = GETC();
>                                 continue;
> @@ -410,7 +410,7 @@
>                         if(c >= 'A' && c <= 'F')
>                                 c += 'a' - 'A';
>                         if(c >= 'a' && c <= 'f') {
> -                               yylval.lval <<= c1;
> +                               yylval.lval = (uvlong)yylval.lval << c1;
>                                 yylval.lval += c - 'a' + 10;
>                                 c = GETC();
>                                 continue;
> @@ -770,6 +770,6 @@
>         f = 65536L;
>         fr = modf(fr*f, &ho);
>         ieee->l = ho;
> -       ieee->l <<= 16;
> +       ieee->l = (uint32)ieee->l << 16;
>         ieee->l |= (int32)(fr*f);
>  }
> Index: src/cmd/gc/bv.c
> ==============================**==============================**=======
> --- a/src/cmd/gc/bv.c
> +++ b/src/cmd/gc/bv.c
> @@ -41,7 +41,7 @@
>
>         if(i < 0 || i >= bv->n)
>                 fatal("bvset: index %d is out of bounds with length %d\n",
> i, bv->n);
> -       mask = 1 << (i % WORDBITS);
> +       mask = 1U << (i % WORDBITS);
>         bv->b[i / WORDBITS] |= mask;
>  }
>
> Index: src/cmd/gc/go.h
> ==============================**==============================**=======
> --- a/src/cmd/gc/go.h
> +++ b/src/cmd/gc/go.h
> @@ -78,7 +78,7 @@
>  struct Strlit
>  {
>         int32   len;
> -       char    s[3];   // variable
> +       char    s[1]; // variable
>  };
>
>  enum
> Index: src/cmd/gc/md5.c
> ==============================**==============================**=======
> --- a/src/cmd/gc/md5.c
> +++ b/src/cmd/gc/md5.c
> @@ -196,7 +196,7 @@
>
>                 for(i=0; i<16; i++) {
>                         j = i*4;
> -                       X[i] = p[j] | (p[j+1]<<8) | (p[j+2]<<16) |
> (p[j+3]<<24);
> +                       X[i] = p[j] | (p[j+1]<<8) | (p[j+2]<<16) |
> ((uint32)p[j+3]<<24);
>                 }
>
>                 // Round 1.
> Index: src/cmd/gc/mparith2.c
> ==============================**==============================**=======
> --- a/src/cmd/gc/mparith2.c
> +++ b/src/cmd/gc/mparith2.c
> @@ -565,11 +565,11 @@
>                 return 0;
>         }
>
> -       v = (vlong)a->a[0];
> -       v |= (vlong)a->a[1] << Mpscale;
> -       v |= (vlong)a->a[2] << (Mpscale+Mpscale);
> +       v = (uvlong)a->a[0];
> +       v |= (uvlong)a->a[1] << Mpscale;
> +       v |= (uvlong)a->a[2] << (Mpscale+Mpscale);
>         if(a->neg)
> -               v = -v;
> +               v = -(uvlong)v;
>         return v;
>  }
>
> @@ -586,7 +586,7 @@
>         x = c;
>         if(x < 0) {
>                 a->neg = 1;
> -               x = -x;
> +               x = -(uvlong)x;
>         }
>
>         a1 = &a->a[0];
> Index: src/cmd/gc/subr.c
> ==============================**==============================**=======
> --- a/src/cmd/gc/subr.c
> +++ b/src/cmd/gc/subr.c
> @@ -322,7 +322,7 @@
>  uint32
>  stringhash(char *p)
>  {
> -       int32 h;
> +       uint32 h;
>         int c;
>
>         h = 0;
> @@ -333,9 +333,9 @@
>                 h = h*PRIME1 + c;
>         }
>
> -       if(h < 0) {
> +       if((int32)h < 0) {
>                 h = -h;
> -               if(h < 0)
> +               if((int32)h < 0)
>                         h = 0;
>         }
>         return h;
> Index: src/cmd/ld/go.c
> ==============================**==============================**=======
> --- a/src/cmd/ld/go.c
> +++ b/src/cmd/ld/go.c
> @@ -37,13 +37,12 @@
>  static int
>  hashstr(char *name)
>  {
> -       int h;
> +       uint32 h;
>         char *cp;
>
>         h = 0;
>         for(cp = name; *cp; h += *cp++)
>                 h *= 1119;
> -       // not if(h < 0) h = ~h, because gcc 4.3 -O2 miscompiles it.
>         h &= 0xffffff;
>         return h;
>  }
> Index: src/cmd/ld/lib.c
> ==============================**==============================**=======
> --- a/src/cmd/ld/lib.c
> +++ b/src/cmd/ld/lib.c
> @@ -951,7 +951,7 @@
>  {
>         Sym *s;
>         char *p;
> -       int32 h;
> +       uint32 h;
>         int c;
>
>         h = v;
> @@ -1613,7 +1613,7 @@
>  uint32
>  le32(uchar *b)
>  {
> -       return b[0] | b[1]<<8 | b[2]<<16 | b[3]<<24;
> +       return b[0] | b[1]<<8 | b[2]<<16 | (uint32)b[3]<<24;
>  }
>
>  uint64
> @@ -1631,7 +1631,7 @@
>  uint32
>  be32(uchar *b)
>  {
> -       return b[0]<<24 | b[1]<<16 | b[2]<<8 | b[3];
> +       return (uint32)b[0]<<24 | b[1]<<16 | b[2]<<8 | b[3];
>  }
>
>  uint64
> Index: src/cmd/pack/ar.c
> ==============================**==============================**=======
> --- a/src/cmd/pack/ar.c
> +++ b/src/cmd/pack/ar.c
> @@ -937,21 +937,12 @@
>  int
>  hashstr(char *name)
>  {
> -       int h;
> +       uint32 h;
>         char *cp;
>
>         h = 0;
>         for(cp = name; *cp; h += *cp++)
>                 h *= 1119;
> -
> -       // the code used to say
> -       //      if(h < 0)
> -       //              h = ~h;
> -       // but on gcc 4.3 with -O2 on some systems,
> -       // the if(h < 0) gets compiled away as not possible.
> -       // use a mask instead, leaving plenty of bits but
> -       // definitely not the sign bit.
> -
>         return h & 0xfffffff;
>  }
>
> Index: src/libmach/5obj.c
> ==============================**==============================**=======
> --- a/src/libmach/5obj.c
> +++ b/src/libmach/5obj.c
> @@ -130,7 +130,7 @@
>                 BGETC(bp);
>                 break;
>         case D_CONST2:
> -               BGETLE4(bp);    // fall through
> +               Bgetle4(bp); // fall through
>         case D_OREG:
>         case D_CONST:
>         case D_BRANCH:
> Index: src/libmach/6obj.c
> ==============================**==============================**=======
> --- a/src/libmach/6obj.c
> +++ b/src/libmach/6obj.c
> @@ -134,7 +134,7 @@
>                         off = ((vlong)l << 32) | (off & 0xFFFFFFFF);
>                 }
>                 if(off < 0)
> -                       off = -off;
> +                       off = -(uvlong)off;
>         }
>         if(a.flags & T_SYM)
>                 a.sym = BGETC(bp);
> Index: src/libmach/8obj.c
> ==============================**==============================**=======
> --- a/src/libmach/8obj.c
> +++ b/src/libmach/8obj.c
> @@ -131,7 +131,7 @@
>                         off = -off;
>         }
>         if(a.flags & T_OFFSET2){
> -               BGETLE4(bp);
> +               Bgetle4(bp);
>         }
>         if(a.flags & T_SYM)
>                 a.sym = BGETC(bp);
> Index: src/libmach/obj.c
> ==============================**==============================**=======
> --- a/src/libmach/obj.c
> +++ b/src/libmach/obj.c
> @@ -244,7 +244,7 @@
>  static void
>  objlookup(int id, char *name, int type, uint sig)
>  {
> -       int32 h;
> +       uint32 h;
>         char *cp;
>         Sym *s;
>         Symtab *sp;
>
>
> --
>
> ---You received this message because you are subscribed to the Google
> Groups "golang-dev" group.
> To unsubscribe from this group and stop receiving emails from it, send an
> email to
golang-dev+unsubscribe@**googlegroups.com<golang-dev%2Bunsubscribe@googlegrou...
> .
> For more options, visit
https://groups.google.com/**groups/opt_out<https://groups.google.com/groups/o...
> .
>

[rsc, 2013-09-09 19:07:26]

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

build: remove various uses of C undefined behavior

If you thought gcc -ansi -pedantic was pedantic, just wait
until you meet clang -fsanitize=undefined.

I think this addresses all the reported "errors", but we'll
need another run to be sure.

all.bash still passes.

Update issue 5764

Dave, can you please try again?

R=golang-dev, bradfitz
CC=golang-dev
https://codereview.appspot.com/13334049