From bd9c7dad6482b64760ef98ee4573950f592ff6b6 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?G=C3=BCnther=20Foidl?= <gue@korporal.at>
Date: Thu, 28 Dec 2017 16:59:43 +0100
Subject: [PATCH 1/3] Queue RCE

With value types the effect is not so big, because there is still one (manual) check for bounds.
For reference types one bounds-check can be saved, so there is a win.

In MoveNext the conditional is with if, so the JIT can produce better code.
---
 .../src/System/Collections/Generic/Queue.cs   | 32 ++++++++++++-------
 1 file changed, 21 insertions(+), 11 deletions(-)

diff --git a/src/System.Collections/src/System/Collections/Generic/Queue.cs b/src/System.Collections/src/System/Collections/Generic/Queue.cs
index d7f0e502ce7b..fb77944ed7cd 100644
--- a/src/System.Collections/src/System/Collections/Generic/Queue.cs
+++ b/src/System.Collections/src/System/Collections/Generic/Queue.cs
@@ -234,15 +234,18 @@ IEnumerator IEnumerable.GetEnumerator()
         // InvalidOperationException.
         public T Dequeue()
         {
-            if (_size == 0)
+            int head = _head;
+            T[] array = _array;
+            
+            if (_size == 0 || (uint)head >= (uint)array.Length)
             {
                 ThrowForEmptyQueue();
             }
 
-            T removed = _array[_head];
+            T removed = array[head];
             if (RuntimeHelpers.IsReferenceOrContainsReferences<T>())
             {
-                _array[_head] = default(T);
+                array[head] = default;
             }
             MoveNext(ref _head);
             _size--;
@@ -252,16 +255,19 @@ public T Dequeue()
 
         public bool TryDequeue(out T result)
         {
-            if (_size == 0)
+            int head = _head;
+            T[] array = _array;
+
+            if (_size == 0 || (uint)head >= (uint)array.Length)
             {
-            	result = default(T);
+            	result = default;
             	return false;
             }
 
-            result = _array[_head];
+            result = array[head];
             if (RuntimeHelpers.IsReferenceOrContainsReferences<T>())
             {
-                _array[_head] = default(T);
+                array[head] = default;
             }
             MoveNext(ref _head);
             _size--;
@@ -367,10 +373,14 @@ private void SetCapacity(int capacity)
         // Increments the index wrapping it if necessary.
         private void MoveNext(ref int index)
         {
-            // It is tempting to use the remainder operator here but it is actually much slower 
-            // than a simple comparison and a rarely taken branch.   
-            int tmp = index + 1;
-            index = (tmp == _array.Length) ? 0 : tmp;
+            // It is tempting to use the remainder operator here but it is actually much slower
+            // than a simple comparison and a rarely taken branch.
+            // JIT produces better code than with ternary operator ?:
+            index++;
+            if (index == _array.Length)
+            {
+                index = 0;
+            }
         }
 
         private void ThrowForEmptyQueue()

From 2964c4daecab58970e8c9ef49cb0ad5847382a40 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?G=C3=BCnther=20Foidl?= <gue@korporal.at>
Date: Thu, 28 Dec 2017 19:10:40 +0100
Subject: [PATCH 2/3] MoveNext uses temp-Variable for register access instead
 of memory access

---
 .../src/System/Collections/Generic/Queue.cs                | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/src/System.Collections/src/System/Collections/Generic/Queue.cs b/src/System.Collections/src/System/Collections/Generic/Queue.cs
index fb77944ed7cd..1995b575bbd8 100644
--- a/src/System.Collections/src/System/Collections/Generic/Queue.cs
+++ b/src/System.Collections/src/System/Collections/Generic/Queue.cs
@@ -376,11 +376,12 @@ private void MoveNext(ref int index)
             // It is tempting to use the remainder operator here but it is actually much slower
             // than a simple comparison and a rarely taken branch.
             // JIT produces better code than with ternary operator ?:
-            index++;
-            if (index == _array.Length)
+            int tmp = index + 1;
+            if (tmp == _array.Length)
             {
-                index = 0;
+                tmp = 0;
             }
+            index = tmp;
         }
 
         private void ThrowForEmptyQueue()

From 244839c9a56c24fb37bad3ad7d8fd6d1bcbcdeb4 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?G=C3=BCnther=20Foidl?= <gue@korporal.at>
Date: Mon, 29 Jan 2018 21:40:34 +0100
Subject: [PATCH 3/3] Addressed PR feedback

* https://github.com/dotnet/corefx/pull/26087#pullrequestreview-92229686
* https://github.com/dotnet/corefx/pull/26087#pullrequestreview-92229910
---
 .../src/System/Collections/Generic/Queue.cs                   | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/System.Collections/src/System/Collections/Generic/Queue.cs b/src/System.Collections/src/System/Collections/Generic/Queue.cs
index 1995b575bbd8..4c36fa5e0878 100644
--- a/src/System.Collections/src/System/Collections/Generic/Queue.cs
+++ b/src/System.Collections/src/System/Collections/Generic/Queue.cs
@@ -237,7 +237,7 @@ public T Dequeue()
             int head = _head;
             T[] array = _array;
             
-            if (_size == 0 || (uint)head >= (uint)array.Length)
+            if (_size == 0)
             {
                 ThrowForEmptyQueue();
             }
@@ -258,7 +258,7 @@ public bool TryDequeue(out T result)
             int head = _head;
             T[] array = _array;
 
-            if (_size == 0 || (uint)head >= (uint)array.Length)
+            if (_size == 0)
             {
             	result = default;
             	return false;