[9fans] compiler bug?

[9fans] compiler bug?

[Steve Simon]

[-- Attachment #1: Type: text/plain, Size: 364 bytes --]

Whilst porting some code from the net I came across
the attached, rather obscure, code.

run as:

	larch% 8c -D 'STATIC=static' t.c && 8l t.8 && 8.out
	78780000
	larch% 8c -D 'STATIC=' t.c && 8l t.8 && 8.out
	0

I think it is tickling a bug in 8c, though
I may be just showing my lack of knowledge of
the C standard...

anyone any thoughts?

-Steve

[-- Attachment #2: t.c --]
[-- Type: text/plain, Size: 479 bytes --]

#include <u.h>
#include <libc.h>

static uint getdword_n(void *mem, int n)
{
    STATIC uint tmp = 0;

    switch (n)
    {
    case 3:
        ((uchar*)&tmp)[1] = ((uchar*)mem)[2];
    case 2:
        ((uchar*)&tmp)[2] = ((uchar*)mem)[1];
    case 1:
        ((uchar*)&tmp)[3] = ((uchar*)mem)[0];
    default:
        break;
    }

    return tmp;
}

void
main(void)
{
	uint x;

	char buf[] = "xxxx";

	x = getdword_n(buf, 2);
	print("%x\n", x);
}

Re: [9fans] compiler bug?

[erik quanstrom]

On Wed Nov 27 11:04:46 EST 2013, steve@quintile.net wrote:

> Whilst porting some code from the net I came across
> the attached, rather obscure, code.
>
> run as:
>
> 	larch% 8c -D 'STATIC=static' t.c && 8l t.8 && 8.out
> 	78780000
> 	larch% 8c -D 'STATIC=' t.c && 8l t.8 && 8.out
> 	0
>
> I think it is tickling a bug in 8c, though
> I may be just showing my lack of knowledge of
> the C standard...
>
> anyone any thoughts?

this code is not correct.  it breaks the anti-aliasing rules.  the
compiler is allowed to assume that tmp is not modified.
recently had trouble with floating point in stdio for the same
reasons.

to work around this, use a union, or better yet getle(buf, 3)
(or be as the case may be),

since there's not enough context to sort out if the original is
supposed to be big or little endian, here's a union version:

- erik
------
#include <u.h>
#include <libc.h>

static uint
getdword_n(void *vmem, int n)
{
	uchar *mem;
	union {
		uint	i;
		uchar	u[sizeof(uint)];
	} tmp;

	mem = vmem;
	tmp.i = 0;
	switch (n){
	case 3:
		tmp.u[1] = mem[2];
	case 2:
		tmp.u[2] = mem[1];
	case 1:
		tmp.u[3] = mem[0];
	}
	return tmp.i;
}

void
main(void)
{
	uint x;

	char buf[] = "xxxx";

	x = getdword_n(buf, 2);
	print("%x\n", x);
}

Re: [9fans] compiler bug?

[Skip Tavakkolian]

[-- Attachment #1: Type: text/plain, Size: 1847 bytes --]

why is mips different (erik's version)?

supermic% ./8.out
78780000
mikro% ./v.out
7878
rpi% ./5.out
78780000



On Wed, Nov 27, 2013 at 8:31 AM, erik quanstrom <quanstro@labs.coraid.com>wrote:

> On Wed Nov 27 11:04:46 EST 2013, steve@quintile.net wrote:
>
> > Whilst porting some code from the net I came across
> > the attached, rather obscure, code.
> >
> > run as:
> >
> >       larch% 8c -D 'STATIC=static' t.c && 8l t.8 && 8.out
> >       78780000
> >       larch% 8c -D 'STATIC=' t.c && 8l t.8 && 8.out
> >       0
> >
> > I think it is tickling a bug in 8c, though
> > I may be just showing my lack of knowledge of
> > the C standard...
> >
> > anyone any thoughts?
>
> this code is not correct.  it breaks the anti-aliasing rules.  the
> compiler is allowed to assume that tmp is not modified.
> recently had trouble with floating point in stdio for the same
> reasons.
>
> to work around this, use a union, or better yet getle(buf, 3)
> (or be as the case may be),
>
> since there's not enough context to sort out if the original is
> supposed to be big or little endian, here's a union version:
>
> - erik
> ------
> #include <u.h>
> #include <libc.h>
>
> static uint
> getdword_n(void *vmem, int n)
> {
>         uchar *mem;
>         union {
>                 uint    i;
>                 uchar   u[sizeof(uint)];
>         } tmp;
>
>         mem = vmem;
>         tmp.i = 0;
>         switch (n){
>         case 3:
>                 tmp.u[1] = mem[2];
>         case 2:
>                 tmp.u[2] = mem[1];
>         case 1:
>                 tmp.u[3] = mem[0];
>         }
>         return tmp.i;
> }
>
> void
> main(void)
> {
>         uint x;
>
>         char buf[] = "xxxx";
>
>         x = getdword_n(buf, 2);
>         print("%x\n", x);
> }
>
>

[-- Attachment #2: Type: text/html, Size: 2609 bytes --]

Re: [9fans] compiler bug?

[erik quanstrom]

On Wed Nov 27 13:07:31 EST 2013, skip.tavakkolian@gmail.com wrote:

> why is mips different (erik's version)?
>
> supermic% ./8.out
> 78780000
> mikro% ./v.out
> 7878
> rpi% ./5.out
> 78780000
>

because it's big endian.  what you're doing there is
putting bytes in specific positions.  in this case you have
bytes (in hex):

	0 0 78 78

on a little endian machine this is 0x78780000 and on a
big-endian machine this is 0x00007878.

- erik

Re: [9fans] compiler bug?

[Skip Tavakkolian]

[-- Attachment #1: Type: text/plain, Size: 754 bytes --]

the Go to C switch got me :) (no breaks!); couldn't see why more than one
byte was getting the 'x' value! changing buf to "1234" made it more obvious.


On Wed, Nov 27, 2013 at 10:38 AM, erik quanstrom
<quanstro@labs.coraid.com>wrote:

> On Wed Nov 27 13:07:31 EST 2013, skip.tavakkolian@gmail.com wrote:
>
> > why is mips different (erik's version)?
> >
> > supermic% ./8.out
> > 78780000
> > mikro% ./v.out
> > 7878
> > rpi% ./5.out
> > 78780000
> >
>
> because it's big endian.  what you're doing there is
> putting bytes in specific positions.  in this case you have
> bytes (in hex):
>
>         0 0 78 78
>
> on a little endian machine this is 0x78780000 and on a
> big-endian machine this is 0x00007878.
>
> - erik
>

[-- Attachment #2: Type: text/html, Size: 1288 bytes --]

Re: [9fans] compiler bug?

[Bakul Shah]

On Wed, 27 Nov 2013 11:31:26 EST erik quanstrom <quanstro@labs.coraid.com> wrote:
> On Wed Nov 27 11:04:46 EST 2013, steve@quintile.net wrote:
>
> > Whilst porting some code from the net I came across
> > the attached, rather obscure, code.
> >
> > run as:
> >
> > 	larch% 8c -D 'STATIC=static' t.c && 8l t.8 && 8.out
> > 	78780000
> > 	larch% 8c -D 'STATIC=' t.c && 8l t.8 && 8.out
> > 	0
> >
> > I think it is tickling a bug in 8c, though
> > I may be just showing my lack of knowledge of
> > the C standard...
> >
> > anyone any thoughts?
>
> this code is not correct.  it breaks the anti-aliasing rules.  the
> compiler is allowed to assume that tmp is not modified.
> recently had trouble with floating point in stdio for the same
> reasons.

I think this is a compiler bug.  What happens when you
declare a union but still do, for example,

	(((uchar*)&tmp.i)[0] = ((uchar*)mem)[3]

etc.? I bet it works.

> to work around this, use a union, or better yet getle(buf, 3)
> (or be as the case may be),
>
> since there's not enough context to sort out if the original is
> supposed to be big or little endian, here's a union version:
>
> - erik
> ------
> #include <u.h>
> #include <libc.h>
>
> static uint
> getdword_n(void *vmem, int n)
> {
> 	uchar *mem;
> 	union {
> 		uint	i;
> 		uchar	u[sizeof(uint)];
> 	} tmp;
>
> 	mem = vmem;
> 	tmp.i = 0;
> 	switch (n){
> 	case 3:
> 		tmp.u[1] = mem[2];
> 	case 2:
> 		tmp.u[2] = mem[1];
> 	case 1:
> 		tmp.u[3] = mem[0];
> 	}
> 	return tmp.i;
> }
>
> void
> main(void)
> {
> 	uint x;
>
> 	char buf[] = "xxxx";
>
> 	x = getdword_n(buf, 2);
> 	print("%x\n", x);
> }
>

Re: [9fans] compiler bug?

[erik quanstrom]

> I think this is a compiler bug.  What happens when you
> declare a union but still do, for example,
>
> 	(((uchar*)&tmp.i)[0] = ((uchar*)mem)[3]
>
> etc.? I bet it works.

the union itself explicitly declares aliasing, not which
member you use.

- erik

Re: [9fans] compiler bug?

[Charles Forsyth]

[-- Attachment #1: Type: text/plain, Size: 538 bytes --]

On 27 November 2013 21:18, erik quanstrom <quanstro@labs.coraid.com> wrote:

> the union itself explicitly declares aliasing, not which
> member you use.
>

actually, even a union won't help you here:
"When a value is stored in a member of an object of union type, the bytes
of the object
representation that do not correspond to that member but do correspond to
other members
take unspecified values."

Really, the way to do this particular operation portably is to write it
portably, and extract and rearrange the bytes.

[-- Attachment #2: Type: text/html, Size: 1137 bytes --]

Re: [9fans] compiler bug?

[erik quanstrom]

> actually, even a union won't help you here: "When a value is stored in
> a member of an object of union type, the bytes of the object
> representation that do not correspond to that member but do correspond
> to other members take unspecified values."

in the example, don't all the bytes of i correspond to a byte to u,
and thus can't take on unspecified values?

or have i read that incorrectly?  if i have, then FPdbleword is wrong,
and all the code that uses it needs fixing.

> Really, the way to do this particular operation portably is to write
> it portably, and extract and rearrange the bytes.

this certainly is the winning way to do this.  clearer and there's
no argument as to correctness.

- erik

Re: [9fans] compiler bug?

[Charles Forsyth]

[-- Attachment #1: Type: text/plain, Size: 269 bytes --]

On 27 November 2013 21:35, erik quanstrom <quanstro@labs.coraid.com> wrote:

> in the example, don't all the bytes of i correspond to a byte to u,
> and thus can't take on unspecified values?
>

you're right, I was using an overly strict meaning of "correspond"

[-- Attachment #2: Type: text/html, Size: 628 bytes --]

Re: [9fans] compiler bug?

[Steve Simon]

> this code is not correct.  it breaks the anti-aliasing rules.

Yep, I see.

Its pretty crufty code, the original had #ifdefs for big and little endian.

I will see if I can push some nicer code fix upstream.

Thanks for the analysis.

-Steve