Bug 2011 - Guid.Equals returns true when it shouldn't
Summary: Guid.Equals returns true when it shouldn't
Status: RESOLVED FIXED
Alias: None
Product: Compilers
Classification: Mono
Component: C# ()
Version: unspecified
Hardware: PC Linux
: --- normal
Target Milestone: ---
Assignee: Marek Safar
URL:
Depends on:
Blocks:
 
Reported: 2011-11-11 12:20 UTC by Neale Ferguson
Modified: 2011-11-16 09:20 UTC (History)
3 users (show)

Tags:
Is this bug a regression?: ---
Last known good build:

Notice (2018-05-24): bugzilla.xamarin.com is now in read-only mode.

Please join us on Visual Studio Developer Community and in the Xamarin and Mono organizations on GitHub to continue tracking issues. Bugzilla will remain available for reference in read-only mode. We will continue to work on open Bugzilla bugs, copy them to the new locations as needed for follow-up, and add the new items under Related Links.

Our sincere thanks to everyone who has contributed on this bug tracker over the years. Thanks also for your understanding as we make these adjustments and improvements for the future.


Please create a new report on GitHub or Developer Community with your current version information, steps to reproduce, and relevant error messages or log files if you are hitting an issue that looks similar to this resolved bug and you do not yet see a matching new report.

Related Links:
Status:
RESOLVED FIXED

Description Neale Ferguson 2011-11-11 12:20:46 UTC
The following program when run on Windows using .NET and Mono (x86) returns "Ok", but "Bad" when run on Linux (x86_64 and s390x) with mono:

using System;
using System.Reflection;
using System.Collections.Generic;
using System.Xml.Serialization;
using System.IO;

namespace fail22
{
	class MainClass
	{
		public struct DC
		{
			private readonly Guid m_Id;

			public DC(Guid Id)
			{
				m_Id = Id;
			}

			public Guid Id
			{
				get { return m_Id; }
			}


		}

		public static void Main (string[] args)
		{
			Guid Id = Guid.NewGuid();
			DC dc = new DC(Id);
			Console.WriteLine("id: {0} default: {1}",Id,default(Guid));
			if (dc.Id.Equals(default(Guid)))
				Console.WriteLine("Bad");
			else
				Console.WriteLine("Ok");
		}
	}
} 

Using --trace shows: 

[  2] . . ENTER: System.Guid:Equals (System.Guid) ip: 0x200006adc86 sp: 0x3ffffad4a28 - this:[value:0x3ffffad4b20:3298185339774e85], [VALUETYPE:00,00,00,00,00,00,00,00,00,00,00,00,00,00,00,00,], 
[  3] . . . ENTER: System.Guid:CompareTo (System.Guid) ip: 0x200009e800e sp: 0x3ffffad48f0 - this:[value:0x3ffffad4b20:3298185339774e85], [VALUETYPE:00,00,00,00,00,00,00,00,00,00,00,00,00,00,00,00,],  
[  3] . . . LEAVE: System.Guid:CompareTo (System.Guid)[INT4:0] ip: 0x200009e851c[  2] . . LEAVE: System.Guid:Equals (System.Guid)[TRUE:1] ip: 0x200009e804e

If I add the line:

Console.WriteLine("CompareTo: {0}",dc.Id.CompareTo(default(Guid)));

I get "1" on Windows and "0" on Linux. Two questions:

1. Although the trace doesn't show this routine being called is the logic incomplete for the case where x == y? I do note that it is only called in the instance where x != y but is it possible to be used by something else in the future that doesn't have that restriction?

                private static int Compare (int x, int y)
                {
                        if (x < y) {
                                return -1;
                        }
                        else {
                                return 1;
                        }
                }

2. It appears that the test to determine whether Compare should be called is failing - e.g. it believes _a == value._a. A trace of the generated code confirms that it believes _a == 0 and value._a == 0 (and so on for b,c,d...):

                public int CompareTo (Guid value)
                {
                        if (_a != value._a) {
                                return Compare (_a, value._a);
                        }
                        else if (_b != value._b) {
                                return Compare (_b, value._b);
                        }
                        else if (_c != value._c) {
                                return Compare (_c, value._c);
                        }
                        else if (_d != value._d) {
                                return Compare (_d, value._d);
                        }
                        else if (_e != value._e) {
                                return Compare (_e, value._e);
                        }
                        else if (_f != value._f) {
                                return Compare (_f, value._f);
                        }
                        else if (_g != value._g) {
                                return Compare (_g, value._g);
                        }
                        else if (_h != value._h) {
                                return Compare (_h, value._h);
                        }
                        else if (_i != value._i) {
                                return Compare (_i, value._i);
                        }
                        else if (_j != value._j) {
                                return Compare (_j, value._j);
                        }
                        else if (_k != value._k) {
                                return Compare (_k, value._k);
                        }
                        return 0;
                }

Tracing the generated code shows that on entry:

-> 00000200009D48B0"  STMG    EB6EF0300024 >> 000003FFFFFE19D8     CC 2
GRG  2 =  000003FFFFFBDB90
GRG  3 =  000003FFFFFBDA48
d 0.30;base2 
V000003FFFFFBDB90  00000000 00000000 00000000 00000000 06 RC8458B90
V000003FFFFFBDBA0  1B8515E9 BC724F69 9EC1FF33 FAB8D675
V000003FFFFFBDBB0  00000000 00000000 00000000 00000000
d 0.30;base3
V000003FFFFFBDA48  00000000 00000000 00000000 00000000 06 RC8458A48
V000003FFFFFBDA58 to 000003FFFFFBDA77 suppressed line(s) same as above ....

R2 -> The Guid: 1b28a08f-4804-42dc-be21-3556ad0baaed
R3 -> default Guid

It then gets the contents of the Guid starting at 0x3ffffffbdb90 but the actual value appears to be at +0x10 from that location. For example, for "_a" - 

00000200009D48D0"  LG      E320F0A00004    000003FFFFA4E998     CC 2
G02=000003FFFFFBDB90 - Get address of Guid
00000200009D48D6"  LGF     E32020000014    ????????????????     CC 2
G02=0000000000000000 - Get the first word of the guid
00000200009D48DC"  LG      E330F0A80004    000003FFFFA4E9A0     CC 2
G03=000003FFFFFBDA48 - Get address of default Guid
00000200009D48E2"  LGF     E33030000014    ????????????????     CC 2
G03=0000000000000000 - Get it's first word
00000200009D48E8"  CR      1923        CC 0  - Compare values
00000200009D48EA"  BRZL    C0840000001D -> 00000200009D4924"    CC 0

This shows that _a is being loaded from *%r2+0 and value._a from *$r3+0. The comparison, of course, gives CC=0 as both values == 0. The comparisons then proceed down that address range. 

I note when the Guid object is instantiated we have code that looks like:

 vcall2 [System.Guid:NewGuid ()] [s390_r2 <- R22] clobbers: c
 add_imm R25 <- s390_r15 [232]
 add_imm R58 <- s390_r15 [264]
 s390_move [s390_r15 + 0xa0] <- [R58 + 0xa0]
 add_imm R26 <- s390_r15 [160]
 voidcall [fail22.MainClass/DC:.ctor (System.Guid)] [s390_r2 <- R25] [s390_r3 <- R26] clobbers: c
 i8const R31 <- [2151571040]
 call R30 <- [mono_object_new_ptrfree_box] [s390_r2 <- R31] clobbers: c
 add_imm R60 <- R30 [16]
 loadi8_membase R61 <- [s390_r15 + 0x108]
 storei8_membase_reg [R60] <- R61
 loadi8_membase R62 <- [s390_r15 + 0x110]
 storei8_membase_reg [R60 + 0x8] <- R62
 i8const R64 <- [0]
 storei8_membase_reg [s390_r15 + 0xf8] <- R64
 storei8_membase_reg [s390_r15 + 0x100] <- R64
 loadi8_membase R67 <- [s390_r15 + 0xf8]
 storei8_membase_reg [s390_r15 + 0x118] <- R67
 loadi8_membase R68 <- [s390_r15 + 0x100]
 storei8_membase_reg [s390_r15 + 0x120] <- R68
 i8const R35 <- [2151571040]
 call R34 <- [mono_object_new_ptrfree_box] [s390_r2 <- R35] clobbers: c
 add_imm R70 <- R34 [16]
 loadi8_membase R71 <- [s390_r15 + 0x118]
 storei8_membase_reg [R70] <- R71
 loadi8_membase R72 <- [s390_r15 + 0x120]
 storei8_membase_reg [R70 + 0x8] <- R72

Note the last 5 instructions where the contents of the guid are stored at R34+16. The Console.WriteLine() method appears to cope with the layout of the Guid but the Equals/CompareTo appear to treat it differently. 

I will run the same traces on linux on x86_64 to see if it does similar things.

Neale
Comment 1 Neale Ferguson 2011-11-14 16:50:36 UTC
Continuing investigations show this code worked under 2.10.2 but has broken in the last few weeks. I will try and narrow down the point of breakage.
Comment 2 Neale Ferguson 2011-11-14 16:59:17 UTC
The code compiled with 2.10.2 works on runtime 2.11. The disassembled code for the working version is as follows:

    .method public static hidebysig                                 
           default void Main (string[] args)  cil managed           
    {                                                               
        // Method begins at RVA 0x2058                              
        .entrypoint                                                 
        // Code size 100 (0x64)                                     
        .maxstack 3                                                 
        .locals init (                                              
                valuetype [mscorlib]System.Guid V_0,                
                valuetype fail22.MainClass/DC   V_1,                
                valuetype [mscorlib]System.Guid V_2,                
                valuetype [mscorlib]System.Guid V_3,                
                valuetype [mscorlib]System.Guid V_4)                
        IL_0000:  call valuetype [mscorlib]System.Guid valuetype [mscorlib]System.Guid::NewGuid()
        IL_0005:  stloc.0 
        IL_0006:  ldloca.s 1                                        
        IL_0008:  ldloc.0  
        IL_0009:  call instance void valuetype fail22.MainClass/DC::'.ctor'(valuetype [mscorlib]System.Guid)
        IL_000e:  ldstr "id: {0} default: {1}" 
        IL_0013:  ldloc.0 
        IL_0014:  box [mscorlib]System.Guid
        IL_0019:  ldloca.s 2
        IL_001b:  initobj [mscorlib]System.Guid
        IL_0021:  ldloc.2 
        IL_0022:  box [mscorlib]System.Guid
        IL_0027:  call void class [mscorlib]System.Console::WriteLine(string, object, object)
        IL_002c:  ldloca.s 1
        IL_002e:  call instance valuetype [mscorlib]System.Guid valuetype fail22.MainClass/DC::get_Id()
        IL_0033:  stloc.3
        IL_0034:  ldloca.s 3
        IL_0036:  ldloca.s 4
        IL_0038:  initobj [mscorlib]System.Guid 
        IL_003e:  ldloc.s 4
        IL_0040:  call instance bool valuetype [mscorlib]System.Guid::Equals(valuetype [mscorlib]System.Guid)
        IL_0045:  brfalse IL_0059
        
        IL_004a:  ldstr "Bad"
        IL_004f:  call void class [mscorlib]System.Console::WriteLine(string)
        IL_0054:  br IL_0063
        
        IL_0059:  ldstr "Ok" 
        IL_005e:  call void class [mscorlib]System.Console::WriteLine(string)
        IL_0063:  ret 

For the failing case:

    .method public static hidebysig
           default void Main (string[] args)  cil managed
    {
        // Method begins at RVA 0x2058
        .entrypoint
        // Code size 99 (0x63)
        .maxstack 3
        .locals init (
                valuetype [mscorlib]System.Guid V_0,
                valuetype fail22.MainClass/DC   V_1,
                valuetype [mscorlib]System.Guid V_2)
        IL_0000:  call valuetype [mscorlib]System.Guid valuetype [mscorlib]System.Guid::NewGuid()
        IL_0005:  stloc.0
        IL_0006:  ldloca.s 1
        IL_0008:  ldloc.0
        IL_0009:  call instance void valuetype fail22.MainClass/DC::'.ctor'(valuetype [mscorlib]System.Guid)
        IL_000e:  ldstr "id: {0} default: {1}"
        IL_0013:  ldloc.0
        IL_0014:  box [mscorlib]System.Guid
        IL_0019:  ldloca.s 2
        IL_001b:  initobj [mscorlib]System.Guid
        IL_0021:  ldloc.2
        IL_0022:  box [mscorlib]System.Guid
        IL_0027:  call void class [mscorlib]System.Console::WriteLine(string, object, object)
        IL_002c:  ldloca.s 1
        IL_002e:  call instance valuetype [mscorlib]System.Guid valuetype fail22.MainClass/DC::get_Id()
        IL_0033:  stloc.2
        IL_0034:  ldloca.s 2
        IL_0036:  ldloca.s 2
        IL_0038:  initobj [mscorlib]System.Guid
        IL_003e:  ldloc.2
        IL_003f:  call instance bool valuetype [mscorlib]System.Guid::Equals(valuetype [mscorlib]System.Guid)
        IL_0044:  brfalse IL_0058

        IL_0049:  ldstr "Bad"
        IL_004e:  call void class [mscorlib]System.Console::WriteLine(string)
        IL_0053:  br IL_0062

        IL_0058:  ldstr "Ok"
        IL_005d:  call void class [mscorlib]System.Console::WriteLine(string)
        IL_0062:  ret
Comment 3 Neale Ferguson 2011-11-15 15:11:35 UTC
I attempted to use git bisect to try and locate the problem, but it appears to go back quite far. I had to stop as I started getting this message during build:

*** error: gettext infrastructure mismatch: using a Makefile.in.in from gettext version 0.17 but the autoconf macros are from gettext version 0.18

I don't really want to install old tools so I'll try and find the problem another way. Any hints?
Comment 4 Neale Ferguson 2011-11-15 15:37:03 UTC
Just taking another look at the compiled code I see this for the failing case:

        IL_0033:  stloc.2
        IL_0034:  ldloca.s 2
        IL_0036:  ldloca.s 2
        IL_0038:  initobj [mscorlib]System.Guid
        IL_003e:  ldloc.2
        IL_003f:  call instance bool valuetype
[mscorlib]System.Guid::Equals(valuetype [mscorlib]System.Guid)
        IL_0044:  brfalse IL_0058

Whereas the working case has:

        IL_0033:  stloc.3
        IL_0034:  ldloca.s 3
        IL_0036:  ldloca.s 4
        IL_0038:  initobj [mscorlib]System.Guid 
        IL_003e:  ldloc.s 4
        IL_0040:  call instance bool valuetype
[mscorlib]System.Guid::Equals(valuetype [mscorlib]System.Guid)

I'm not strong in this area but does that mean in the failing case we load the same value in parameter 1 and 2 and hence the Equals will report true?
Comment 5 Zoltan Varga 2011-11-15 18:09:58 UTC
Yes, this is an mcs bug/regression.
Comment 6 Neale Ferguson 2011-11-15 21:24:00 UTC
A client running some daily builds says "fails as of July 14 and worked as of july 8".
Comment 7 Marek Safar 2011-11-16 09:20:16 UTC
Fixed.