Yet another Major JIT Optimizer BUG

[Guest]

Try this in Release mode of .Net 1.1 SP1:

class Bug2
{
class C : IDisposable
{
public C() {}
void IDisposable.Dispose() {}
}

bool A()
{
return true;
}

public Bug2()
{
if (A())
{
Console.WriteLine("A Is True!");
using (new C()) {}
}
else
{
Console.WriteLine("A Is False!");
}
}
}

 

[Scott Coonce]

Um, I have .Net 1.1 SP1... what's the error you get?

copied your code into a console project and added the following main():
[STAThread]
static void Main(string[] args) {
Bug2 bg = new Bug2();
Console.ReadLine();
}

the output was:
A Is True!
C.Dispose();

(I changed "void IDisposable.Dispose() {}" to
"void IDisposable.Dispose() { Console.WriteLine("C.Dispose();"); }"
)

Otherwise it remained unchanged... what's the problem?
Scott

 

[Guest]

If i try it in release build (/o+),
i get
A Is True!
A Is False!

Um, I have .Net 1.1 SP1... what's the error you get?

copied your code into a console project and added the following main():
[STAThread]
static void Main(string[] args) {
Bug2 bg = new Bug2();
Console.ReadLine();
}

the output was:
A Is True!
C.Dispose();

(I changed "void IDisposable.Dispose() {}" to
"void IDisposable.Dispose() { Console.WriteLine("C.Dispose();"); }"
)

Otherwise it remained unchanged... what's the problem?
Scott

Try this in Release mode of .Net 1.1 SP1:

class Bug2
{
class C : IDisposable
{
public C() {}
void IDisposable.Dispose() {}
}

bool A()
{
return true;
}

public Bug2()
{
if (A())
{
Console.WriteLine("A Is True!");
using (new C()) {}
}
else
{
Console.WriteLine("A Is False!");
}
}
}

 

[Guest]

Hi Boaz,

If i try it in release build (/o+),
i get
A Is True!
A Is False!

[...]

Scary. You seem to have a knack for finding JIT bugs ...

Kind regards,

 

[Scott Coonce]

Whoops! Sorry, I overlooked the "Console.WriteLine("A Is False")" in the
else clause. Once I add this, I too get the A Is True/A Is False combo in
Release mode, but _not_ Debug.

I've attached my code file to make sure we test using the exact same code,
and so anyone else can reproduce the problem.

Try this: right click the project in the solution explorer -> properties.
Choose "Configuration-->Release" in combo box
Select "Configuration Properties-->Build"
set "Optimize Code" to false

.... and the problem disappears. I don't know why, though.

Scott

If i try it in release build (/o+),
i get
A Is True!
A Is False!

Um, I have .Net 1.1 SP1... what's the error you get?

copied your code into a console project and added the following main():
[STAThread]
static void Main(string[] args) {
Bug2 bg = new Bug2();
Console.ReadLine();
}

the output was:
A Is True!
C.Dispose();

(I changed "void IDisposable.Dispose() {}" to
"void IDisposable.Dispose() { Console.WriteLine("C.Dispose();"); }"
)

Otherwise it remained unchanged... what's the problem?
Scott

Try this in Release mode of .Net 1.1 SP1:

class Bug2
{
class C : IDisposable
{
public C() {}
void IDisposable.Dispose() {}
}

bool A()
{
return true;
}

public Bug2()
{
if (A())
{
Console.WriteLine("A Is True!");
using (new C()) {}
}
else
{
Console.WriteLine("A Is False!");
}
}
}

 

[Scott Coonce]

I'm not sure if this helps, but I've decided (for the first time) to dive
into the IL created for this. It seems that the probem is in the IL and not
the JIT (which i'd never understand anyway).

In trying to understand the IL code, I had to go to the msdn library to
figure it all out, so i copied a bit of the docs to make it easier. Note
near the bottom that a "goto" (whatever) is missing to jump over the
Console.WriteLine("A Is False!");

(I'm a veritable newbie at the IL thing, so I apologize if I'm stating the
obvious)

Scott



Here's the IL for Bug2.ctor():



..method public hidebysig specialname rtspecialname
instance void .ctor() cil managed
{
//
// The following is a Release build with
// Optimization = True.
//
// IL documentation comments taken from:
//
http://msdn.microsoft.com/library/d...mreflectionemitopcodesclassbrfalse_stopic.asp
//
//
// Code size 50 (0x32)
.maxstack 1
.locals init (class ConsoleTest.Bug2/C V_0)
IL_0000: ldarg.0

// Calls the method indicated by the
// passed method descriptor, return
// value pushed onto stack
IL_0001: call instance void [mscorlib]System.Object::.ctor()

// Loads the argument at index 0 onto the evaluation stack.
IL_0006: ldarg.0

// A() pushes "true" onto stack
IL_0007: call instance bool ConsoleTest.Bug2::A()

// Transfers control to a target
// instruction if value is false,
// a null reference, or zero.
IL_000c: brfalse.s IL_0027

IL_000e: ldstr "A Is True!"
IL_0013: call void [mscorlib]System.Console::WriteLine(string)

// pushing an object reference (type O) onto the evaluation stack.
IL_0018: newobj instance void ConsoleTest.Bug2/C::.ctor()

// Pops the current value from top of stack
// and stores it in a the local variable
// list at index 0.
// (object "C" stored at 0.)
IL_001d: stloc.0

// Loads the local variable at index 0
// onto the evaluation stack
// (this is object "C")
IL_001e: ldloc.0

// Transfers control to a target
// instruction if value is false,
// a null reference, or zero.
// (since value is "C", it should be non-null, ie. true.)
IL_001f: brfalse.s IL_0027

// Loads the local variable at index 0
// onto the evaluation stack
// (this is object "C")
IL_0021: ldloc.0

// Calls a late-bound method on an
// object, pushing the return value
// onto the evaluation stack.
IL_0022: callvirt instance void [mscorlib]System.IDisposable:ispose()

//
//-------------------------------------------------
// It seems that we're missing the "jump" over the
// "A Is False!" WriteLine
//-------------------------------------------------
//
IL_0027: ldstr "A Is False!"
IL_002c: call void [mscorlib]System.Console::WriteLine(string)
IL_0031: ret
} // end of method Bug2::.ctor

 

[Scott Coonce]

Two seconds after sending my previous post, I looked at the IL for an exe
with optimization set to _false_ on a release build.

Near the end of the method, is this little gem:


IL_002a: br.s IL_0036
IL_002c: ldstr "A Is False!"
IL_0031: call void [mscorlib]System.Console::WriteLine(string)
IL_0036: ret
} // end of method Bug2::.ctor

From the msdn docs, br.s means
"Unconditionally transfers control to a target instruction (short form)."

.... just what was missing from the Release w/optimization build.

Scott

 

[BravesCharm]

When viewing the debug build and release build in Lutz Reflector here
is the the what the functions look like for Bug2 constructor.

Debug build
public Bug2()
{
if (this.A())
{
Console.WriteLine("A Is True!");
using (Bug2.C c1 = new Bug2.C())
{
}
}
else
{
Console.WriteLine("A Is False!");
}
}
Release Build
public Bug2()
{
if (this.A())
{
Console.WriteLine("A Is True!");
Bug2.C c1 = new Bug2.C();
if (c1 != null)
{
((IDisposable) c1).Dispose();
}
}
Console.WriteLine("A Is False!");
}

It looks like it's a bug in the C# compiler and not the IL.

 

[Scott Coonce]

It looks like it's a bug in the C# compiler and not the IL.

Doesn't the c# compiler do c# code --> IL?
This is the first step, right?

Scott

 

[Stuart Carnie]

Nothing to do with the JIT / CLR. It's a C# compiler issue.

 

[BravesCharm]

Doesn't the c# compiler do c# code --> IL?

This is the first step, right?

The C# compiler produces the IL code that the CLR runs. The IL code
that the C# compiler created is not correct in release build for some
reasons.

 

[Wiktor Zychla [C# MVP]]

... just what was missing from the Release w/optimization build.

the compiler incorrecly emits a try-catch block for an empty block (for
using (...)) with optimizations turned on.
to see that try to add a code to the "using" block, for example:

public Bug2() {
if (A()) {
Console.WriteLine("A Is True!");
using (new C())
{
Console.WriteLine();
}
}
else {
Console.WriteLine("A Is False!");
}
}

....and it will run ok.

since I was sure this had been noticed long ago, I've spent two minuts and
here you are, reported over 3 years ago:
http://tinyurl.com/csjhd

I wonder if it still exists in the 2.0 version.

regards,
Wiktor Zychla

 

[Mattias Sjögren]

I wonder if it still exists in the 2.0 version.

No it's been fixed.

But if it's the same issue as reported three years ago it makes you
wonder why they haven't fixed it in a SP for v1.x.


Mattias

 

[Willy Denoyette [MVP]]

No it's been fixed.

But if it's the same issue as reported three years ago it makes you
wonder why they haven't fixed it in a SP for v1.x.


Mattias

Yes, it's still the same issue reported before and after v1.1 SP1 was
released, guess it wasn't too high on the JIT team's priority list.

Willy.

 

[Stuart Carnie]

It's nothing to to with the JIT (or the JIT team). It would be the C#
compiler team. As others have posted, looking at the IL shows it is
incorrectly generated when the /o+ flag is used, which comes from the C#
compiler. If you compiled the example code using the mono or Borland C#
compiler and ran the same program using the *Microsoft CLR* it would not
fail.

Cheers,

Stu

 

[Cowboy \(Gregory A. Beamer\)]

WARNING: Major sarcasm ahead!!!

MAJOR IT Optimizer BUG? Certainly a bug, not major.

First, let's add a line inside of the using statement:

using (new C())
{
string x = String.Empty;
}

Sure, my x=String.Empty statement is BS, but you did not give much to run
on. When I add the statement, however, life works again.

The point here is it is against proper programming practice to chain work to
your construtor. If you look at OO 101, you end up with the following
(albeit oversimplified)

1. Constructors set state
2. Properties hold state (or internal variables, if you want to get
technical on the private implementations)
3. Methods are used for behavior

As you are using the "using" statement, you should be USING the object, not
merely filling state. If this is true, the statement:

using (new C()) {}

is either a) garbage or b) bad programming practice, which gets us back to
garbage.

What is happening in your example is the optimizer is noticing nothing is
happening with the using statement (i.e., the object is NOT being used). If
one were to opt to the followign practice (chaining work to the
constructor), your bug could bite them:

class C : IDisposable
{
public C()
{
DoWorkHere();
}
void IDisposable.Dispose() {}

private void DoWorkHere()
{
string x = "using the constructor to do work";
}
}

But, the likelihood of actually encountering this bug in a well designed app
is effectively 0%.

Bug? Yes.
Major? Hardly.

--
Gregory A. Beamer
MVP; MCP: +I, SE, SD, DBA

***********************************************
Think Outside the Box!
***********************************************

 

[BravesCharm]

The compiler puts a try/finally block not a try/catch and when using
the "using" keyword it will use the try/finally block so it will make
sure it IDisposable interface gets called even if an exception is
thrown.

However, it's only putting this try/finally block in debug mode and not
release mode. It seems like when it forgets the try/finally it also
forgets the else too!

 

[Willy Denoyette [MVP]]

Agreed, sorry. Too heavily focused on the subject line I guess.

Willy.

 

[Wiktor Zychla [C# MVP]]

The compiler puts a try/finally block not a try/catch and when using
the "using" keyword it will use the try/finally block so it will make

try-finally, of course. a typo.

Wiktor

 

[Stuart Carnie]

Absolutely agree.

WARNING: Major sarcasm ahead!!!

MAJOR IT Optimizer BUG? Certainly a bug, not major.

First, let's add a line inside of the using statement:

using (new C())
{
string x = String.Empty;
}

Sure, my x=String.Empty statement is BS, but you did not give much to run
on. When I add the statement, however, life works again.

The point here is it is against proper programming practice to chain work
to your construtor. If you look at OO 101, you end up with the following
(albeit oversimplified)

1. Constructors set state
2. Properties hold state (or internal variables, if you want to get
technical on the private implementations)
3. Methods are used for behavior

As you are using the "using" statement, you should be USING the object,
not merely filling state. If this is true, the statement:

using (new C()) {}

is either a) garbage or b) bad programming practice, which gets us back to
garbage.

What is happening in your example is the optimizer is noticing nothing is
happening with the using statement (i.e., the object is NOT being used).
If one were to opt to the followign practice (chaining work to the
constructor), your bug could bite them:

class C : IDisposable
{
public C()
{
DoWorkHere();
}
void IDisposable.Dispose() {}

private void DoWorkHere()
{
string x = "using the constructor to do work";
}
}

But, the likelihood of actually encountering this bug in a well designed
app is effectively 0%.

Bug? Yes.
Major? Hardly.

--
Gregory A. Beamer
MVP; MCP: +I, SE, SD, DBA

***********************************************
Think Outside the Box!
***********************************************