C# Unsafe Bug

[JS]

I was writing some routines which could do bitwise boolean operations
on byte arrays, and I ran into what I think is a bug with C#'s unsafe
code. I am pasting a console application below. Can anyone give an
explanation, or has this type of problem been reported already?
Thanks.

using System;
using System.Collecti ons.Generic;
using System.Text;

namespace UnsafeBug
{
class Program
{
static void Main(string[] args)
{
byte[] dest = new byte[] { 0x01, 0xff };
byte[] sdata1 = new byte[] { 0x01, 0xff };
UnsafeOr(sdata1 , dest, dest, 7);

for (int ii = 0; ii < dest.Length; ii++)
{
Console.WriteLi ne("Byte {0} = 0x{1:X2}", ii + 1, dest[ii]);
}
Console.WriteLi ne("Hit Enter to quit..."); Console.ReadLin e();
}

// OR's together 2 byte arrays into 'dest' without touching the
first few bits of 'dest'.
// it is allowed for sources/destinations to be the same.
static void UnsafeOr(byte[] src1, byte[] src2, byte[] dest, int
firstBit)
{
byte mask = (byte)(0x80 >> firstBit);
int nbytes = Math.Min(Math.M in(src1.Length, src2.Length),
dest.Length);
unsafe
{
fixed (byte* s1data = &src1[0], s2data = &src2[0], ddata =
&dest[0])
{
byte* s1ptr = s1data;
byte* s2ptr = s2data;
byte* dptr = ddata;

#if false
// this version works
byte val = (byte)(*dptr & ~mask);
*dptr++ = (byte)(val | ((*s1ptr++ | *s2ptr++)&mask) );
#else
// this version does not work
*dptr++ = (byte)((*dptr & ~mask) | ((*s1ptr++ |
*s2ptr++)&mask) );
#endif
for (int ii = 1; ii < nbytes; ii++)
{
*dptr++ = (byte)(*s1ptr++ | *s2ptr++);
}
}
}
}
}
}

[Jon Skeet [C# MVP]]

JS <st********@ic. net> wrote:

I was writing some routines which could do bitwise boolean operations
on byte arrays, and I ran into what I think is a bug with C#'s unsafe
code. I am pasting a console application below. Can anyone give an
explanation, or has this type of problem been reported already?

<snip>

I don't believe this is a bug, and I don't believe it has anything to
do with unsafe code. Here's a smaller app which shows the same effect
without any unsafe code.

using System;

class Test
{
static void Main()
{
int[] x = {10};
int i=0;
x[i++] = i;
Console.WriteLi ne (x[0]);
}
}

That will print out 1, not 0 (which is what you'd have expected, I
believe).

What you're seeing is the postfix increment being executed before the
evaluation of the right hand side of the assignment operator.

This is entirely correct according to the specification.

--
Jon Skeet - <sk***@pobox.co m>
http://www.pobox.com/~skeet Blog: http://www.msmvps.com/jon.skeet
If replying to the group, please do not mail me too

[Stefan Simek]

JS wrote:

I was writing some routines which could do bitwise boolean operations
on byte arrays, and I ran into what I think is a bug with C#'s unsafe
code. I am pasting a console application below. Can anyone give an
explanation, or has this type of problem been reported already?
Thanks.

using System;
using System.Collecti ons.Generic;
using System.Text;

namespace UnsafeBug
{
class Program
{
static void Main(string[] args)
{
byte[] dest = new byte[] { 0x01, 0xff };
byte[] sdata1 = new byte[] { 0x01, 0xff };
UnsafeOr(sdata1 , dest, dest, 7);

for (int ii = 0; ii < dest.Length; ii++)
{
Console.WriteLi ne("Byte {0} = 0x{1:X2}", ii + 1, dest[ii]);
}
Console.WriteLi ne("Hit Enter to quit..."); Console.ReadLin e();
}

// OR's together 2 byte arrays into 'dest' without touching the
first few bits of 'dest'.
// it is allowed for sources/destinations to be the same.
static void UnsafeOr(byte[] src1, byte[] src2, byte[] dest, int
firstBit)
{
byte mask = (byte)(0x80 >> firstBit);
int nbytes = Math.Min(Math.M in(src1.Length, src2.Length),
dest.Length);
unsafe
{
fixed (byte* s1data = &src1[0], s2data = &src2[0], ddata =
&dest[0])
{
byte* s1ptr = s1data;
byte* s2ptr = s2data;
byte* dptr = ddata;

#if false
// this version works
byte val = (byte)(*dptr & ~mask);
*dptr++ = (byte)(val | ((*s1ptr++ | *s2ptr++)&mask) );
#else
// this version does not work
*dptr++ = (byte)((*dptr & ~mask) | ((*s1ptr++ |
*s2ptr++)&mask) );
#endif
for (int ii = 1; ii < nbytes; ii++)
{
*dptr++ = (byte)(*s1ptr++ | *s2ptr++);
}
}
}
}
}
}

The problem lies in the fact that left side of the assignment is
evaluated before the right side. First, *dptr++ is evaluated,
incrementing dptr but still resulting in the correct destination
address. When the right side is evaluated afterwards, the dptr is
already incremented so it points to a different location.

So you can either change your code to

*dptr = (byte)((byte)(* dptr++ & ~mask) | ((*s1ptr++ | *s2ptr++)&mask) );

or (maybe safer)

*dptr = (byte)((byte)(* dptr & ~mask) | ((*s1ptr++ | *s2ptr++)&mask) );
dptr++

HTH,
Stefan

[JS]

OK, I guess. It was my understanding that postfix was done after the
statement was executed. I suppose I should look at the language
specifications.

Thanks for the quick responses.

[Jon Skeet [C# MVP]]

JS <st********@ic. net> wrote:

OK, I guess. It was my understanding that postfix was done after the
statement was executed. I suppose I should look at the language
specifications.

Well, while I would never suggest that it's not a good idea to know the
language well, I think it would also be better to split your current
statement up into potentially several, to make it more readable. It
certainly wasn't obvious to me what it should be doing, partly due to
the number of side-effects involved.

I assume the reason for using unsafe code is performance - I would
benchmark various possible versions of the code (including safe code)
and record details of those benchmarks (source and result). You can
then make a trade-off between performance and readability (if, indeed,
the most readable code doesn't also perform the best).

(Also, shouldn't the mask be 0xff >> firstBit, not 0x80 >> firstBit?)

--
Jon Skeet - <sk***@pobox.co m>
http://www.pobox.com/~skeet Blog: http://www.msmvps.com/jon.skeet
If replying to the group, please do not mail me too

[JS]

Yes, it should have been 0xff>>firstBit. It got messed up when I
created the small project to demonstrate my 'problem'.

As you surmised we need to fully optimize this function. Our
applications sometimes call these image processing operations many
times, with images up to 8192x8192. If we don't optimize, it gets very
slow.

I will try benchmarking the performance using safe code, as you
suggest, although for these low-level functions readability is
secondary to speed.

[JS]

I have done a quick benchmark:

Average time to do simple binary image OR (1000x1000 pixels = 125000
bytes):
Unsafe code: 3.0 milliseconds
Safe code: 2.5 milliseconds

This is significant but not too significant. I will consider having 2
versions of each function, the default function being safe and an
unsafe version where it makes sense.

[Stefan Simek]

JS wrote:

I have done a quick benchmark:

Average time to do simple binary image OR (1000x1000 pixels = 125000
bytes):
Unsafe code: 3.0 milliseconds
Safe code: 2.5 milliseconds

This is significant but not too significant. I will consider having 2
versions of each function, the default function being safe and an
unsafe version where it makes sense.

The safe version seems to be faster in this case, so I think there is no
real need to use unsafe code at all ;)

[Jon Skeet [C# MVP]]

JS <st********@ic. net> wrote:

I have done a quick benchmark:

Average time to do simple binary image OR (1000x1000 pixels = 125000
bytes):
Unsafe code: 3.0 milliseconds
Safe code: 2.5 milliseconds

This is significant but not too significant. I will consider having 2
versions of each function, the default function being safe and an
unsafe version where it makes sense.

Have you got those the wrong way round? If the safe code is faster, why
not use that all the time?

--
Jon Skeet - <sk***@pobox.co m>
http://www.pobox.com/~skeet Blog: http://www.msmvps.com/jon.skeet
If replying to the group, please do not mail me too

[JS]

Sorry, Safe=3ms Unsafe=2.5ms