Bug 56247 - Enumerable.OrderByDescending behaves differently on LLVM FullAOT
Summary: Enumerable.OrderByDescending behaves differently on LLVM FullAOT
Status: RESOLVED FIXED
Alias: None
Product: Runtime
Classification: Mono
Component: JIT (show other bugs)
Version: master
Hardware: PC Mac OS
: --- normal
Target Milestone: ---
Assignee: Bugzilla
URL:
Depends on:
Blocks:
 
Reported: 2017-05-11 20:07 UTC by Andi McClure
Modified: 2017-05-15 11:35 UTC (History)
3 users (show)

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


Attachments
Test program (1.60 KB, text/plain)
2017-05-11 20:07 UTC, Andi McClure
Details


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 Andi McClure 2017-05-11 20:07:00 UTC
Created attachment 22098 [details]
Test program

The Enumerable.OrderByDescending method is defined in the documentation to be a stable sort. It is possible to construct a test case where OrderByDescending behaves as a stable sort for all of (1) Normal JIT usage (2) LLVM JIT usage and (3) Normal FullAOT, but acts as an unstable sort (in fact produces consistently-wrong results for a stable sort) for LLVM FullAOT.

This seems to indicate either a problem with codegen or a problem with how LLVM FullAOT uses Linq (OrderByDescending is in System.Linq).

Example:

The following assumes Mac OS X, and assumes `export PATH=$PATH:/Library/Frameworks/Mono.framework/Versions/Current/bin` was run prior to executing the tests. Test 4 is the test which fails. "program.cs" does a simple custom sort on an array of objects, such that the sort should not result in changing the order of the list items.

--- Test 1, normal JIT, successful

Download program.cs (attached) into empty directory

mcs program.cs
mono program.exe

Result: "Success? True"

--- Test 2, LLVM JIT, successful

Download program.cs into empty directory

mcs --llvm program.cs
mono --llvm program.exe

Result: "Success? True"

--- Test 3, FullAOT, successful

Download program.cs into empty directory

mcs program.cs
cp /Library/Frameworks/Mono.framework/Versions/Current/lib/mono/4.5/mscorlib.dll .
cp /Library/Frameworks/Mono.framework/Versions/Current/lib/mono/4.5/System.dll .
cp /Library/Frameworks/Mono.framework/Versions/Current/lib/mono/4.5/System.Core.dll .
cp /Library/Frameworks/Mono.framework/Versions/Current/lib/mono/4.5/Mono.Security.dll .
cp /Library/Frameworks/Mono.framework/Versions/Current/lib/mono/4.5/System.Xml.dll .
cp /Library/Frameworks/Mono.framework/Versions/Current/lib/mono/4.5/System.Configuration.dll .
cp /Library/Frameworks/Mono.framework/Versions/Current/lib/mono/4.5/System.Security.dll .
cp /Library/Frameworks/Mono.framework/Versions/Current/lib/mono/4.5/Mono.Posix.dll .
MONO_PATH=. mono --aot=full mscorlib.dll
mono --aot=full System.dll
mono --aot=full System.Core.dll
mono --aot=full Mono.Security.dll
mono --aot=full System.Xml.dll
mono --aot=full System.Configuration.dll
mono --aot=full System.Security.dll
mono --aot=full Mono.Posix.dll
mono --aot=full program.exe
MONO_PATH=. mono --full-aot program.exe

Result: "Success? True"

-- Test 4, LLVM FullAOT, failure

mcs program.cs
cp /Library/Frameworks/Mono.framework/Versions/Current/lib/mono/4.5/mscorlib.dll .
cp /Library/Frameworks/Mono.framework/Versions/Current/lib/mono/4.5/System.dll .
cp /Library/Frameworks/Mono.framework/Versions/Current/lib/mono/4.5/System.Core.dll .
cp /Library/Frameworks/Mono.framework/Versions/Current/lib/mono/4.5/Mono.Security.dll .
cp /Library/Frameworks/Mono.framework/Versions/Current/lib/mono/4.5/System.Xml.dll .
cp /Library/Frameworks/Mono.framework/Versions/Current/lib/mono/4.5/System.Configuration.dll .
cp /Library/Frameworks/Mono.framework/Versions/Current/lib/mono/4.5/System.Security.dll .
cp /Library/Frameworks/Mono.framework/Versions/Current/lib/mono/4.5/Mono.Posix.dll .
MONO_PATH=. mono --llvm --aot=full mscorlib.dll
mono --llvm --aot=full System.dll
mono --llvm --aot=full System.Core.dll
mono --llvm --aot=full Mono.Security.dll
mono --llvm --aot=full System.Xml.dll
mono --llvm --aot=full System.Configuration.dll
mono --llvm --aot=full System.Security.dll
mono --llvm --aot=full Mono.Posix.dll
mono --llvm --aot=full program.exe
MONO_PATH=. mono --full-aot program.exe

Result: items 0 and 1 get swapped.
Comment 1 Andi McClure 2017-05-11 20:38:56 UTC
Update: if you change the Val4 value from bool to int, the problem goes away even on LLVM+FullAOT. Example:

diff -r a7349082afe8 -r f18c4b57dc10 program.cs
--- a/program.cs	Thu May 11 15:57:23 2017 -0400
+++ b/program.cs	Thu May 11 16:33:39 2017 -0400
@@ -8,7 +8,7 @@
     {
         class Entity2
         {
-            public Entity2(int id, bool val1, bool val2, bool val3, bool val4)
+            public Entity2(int id, bool val1, bool val2, bool val3, int val4)
             {
                 Id = id;
                 Val1 = val1;
@@ -21,17 +21,17 @@
             public bool Val1;
             public bool Val2;
             public bool Val3;
-            public bool Val4;
+            public int Val4;
         }
 
         public static int test_0_OrderByDescending2()
         {
             List<Entity2> EntityList = new List<Entity2>()
             {
-                new Entity2(0, true, false, false, true),
-                new Entity2(1, true, false, false, true),
-                new Entity2(863, false, false, false, false),
-                new Entity2(864, false, false, false, false),
+                new Entity2(0, true, false, false, 1),
+                new Entity2(1, true, false, false, 1),
+                new Entity2(863, false, false, false, 0),
+                new Entity2(864, false, false, false, 0),
             };
 
             var SortedEntityList = EntityList.OrderByDescending(
Comment 2 Zoltan Varga 2017-05-12 00:25:39 UTC
https://github.com/mono/mono/pull/4841
Comment 3 Zoltan Varga 2017-05-15 11:35:58 UTC
-> FIXED.