History | Log In     View a printable version of the current page.  
Issue Details (XML | Word | Printable)

Key: BOO-705
Type: Bug Bug
Status: Resolved Resolved
Resolution: Fixed
Priority: Major Major
Assignee: Rodrigo B. de Oliveira
Reporter: Snaury
Votes: 0
Watchers: 0
Operations

If you were logged in you would be able to see more operations.
Boo

System.AccessViolation when using shl or shr

Created: 23/Mar/06 03:23 AM   Updated: 31/Mar/06 12:29 PM
Component/s: None
Affects Version/s: 0.7.6
Fix Version/s: 0.7.6

Time Tracking:
Not Specified

File Attachments: 1. File EmitAssembly.cs.diff (1 kb)

Environment: boo-0.7.6.2188/boo-0.7.6.2160, .net 2.0


 Description  « Hide
When you try to compile and run the following code:

v as uint = 1
v = v << 5

On .NET 2.0 exception System.AccessViolation is thrown. The strange part is when compiled with booc debug, the exception is not thrown (and the only difference with debug mode is in debug IL there are some nops, so I don't understand why it throws in debug mode only). Looking at IL I can see the following:

ldloc.0
conv.i8
ldc.i4 0x5
conv.i8
shl

When I tried to reproduce similar code in C#:

<uint or long or ulong> v = 1
<uint or long or ulong> n = 5
v = v << n

It says that shl cannot be used when both of the types are uint, long or ulong (however it works fine if left operand is whatever and right is int). I don't know it for sure, but maybe the right operand of shl/shr should always be int? Anyway, I think the right operand should be converted to i4, not anything else like i8.



 All   Comments   Work Log   Change History      Sort Order: Ascending order - Click to sort in descending order
Snaury - 29/Mar/06 07:33 AM
I'm not sure if anyone will pay attention at this bug because I forgot to specify component (and seems like can't do it after bug has been filed), but here's a possible fix. Diff taken against version 0.7.6.2160, but since function EmitBitwiseOperator is the same in the latest trunk I think that would be ok.

Does anyone know about any possible side effects of this change?
(OpCodes.Shl seems to be used for integer types only, so there shouldn't be any)


Rodrigo B. de Oliveira - 29/Mar/06 07:37 AM
The question is should we silently accept unsigned integers?

Snaury - 29/Mar/06 07:51 AM
Ehm... What do you mean 'silently accept'? Binary shift on uisigned integers/longs should be fine and supported. The current problem is that EmitBitwiseOperator determines resulting type and converts both, left and right operands to that type, which appears to be incorrect: right operator to shl and shr should be int (or could it be byte/short as well? I couldn't tempt csc into emitting anything other than i4 as right operand no matter how hard I tried). My fix practically just makes EmitBitwiseOperator convert right operand to i4 (and not i8, u8 and anything else), while still correctly converting left operand to ExpressionType.

Snaury - 29/Mar/06 07:54 AM
Btw, if you don't believe me, try to compile it in C#:

uint a = 3456; // or ulong or long
a = a << 5;

and then use ildasm to see IL yourself.
C# generates correct code for this.


Snaury - 31/Mar/06 02:43 AM
Hi, I tried some tests with << and >> on various types, and it seems that with my patch nothing is changed much except that this bug gets fixed. For example uint << object produces an error, and uint << bool works the same way as in 0.7.6.2188 (didn't try latest trunk yet). However, while testing I stumbled upon something strange. The code:

uint a = 3456
a = a << a
print a

It prints 3456, both before and after my patch (and looking at IL it seems to be correct, so maybe it is .NET bug after all). And this code:

uint a = 3456
a = a << 3456
print a

It prints 0 with debug after my patch, prints 3456 with -debug after my patch, and prints 3456 when compiled with debug before my patch (can't test it with -debug before my patch because it throws). That's very strange, indeed, but still it works the same way as before my patch, except that it doesn't throw. Looking at IL produced by C# I can see that it often ands value of right operand with bitmask (which I'm not sure is the good thing, as 3456 << 33 in C# becomes 6912, not really what might be intended!).

Still don't understand what did you mean by silently accepting unsigned integers. Is there anything else I'd need to increase chance this bug gets fixed in trunk?


Rodrigo B. de Oliveira - 31/Mar/06 04:22 AM
I meant 'should boo behave like c# for the program below'?

using System;

class Program
{
public static void Main()

{ uint a = 3; uint b = 2; uint c = a << b; Console.WriteLine(c); }

}

shift.cs(9,12): error CS0019: Operator '<<' cannot be applied to operands of type 'uint' and 'uint'


Snaury - 31/Mar/06 04:51 AM
Hmm, given the fact that the following works in boo (and requires a cast in C#):

a as uint = 5
b as int = a
print b

I can say that definitely it should accept it silently.


Rodrigo B. de Oliveira - 31/Mar/06 12:29 PM
Thanks for the report and fix!