From 608fac6f22814e3eeb4609c80c0b5f08205e24ed Mon Sep 17 00:00:00 2001
From: Simon Ferquel <simon.ferquel@hotmail.fr>
Date: Thu, 22 Aug 2024 18:03:35 +0200
Subject: [PATCH] Fix WriteCompressedInt32 logic by using the same code as in
 System.Reflection.Metadata (#958)

* Fix WriteCompressedInt32 logic by using the same code as in System.Reflection.Metadata

Note: the implementation in SRM uses methods that are not available in netfx 4.0. To make the code work with all platforms supported by Mono.Cecil, I extracted the required methods.

* Cleanup whitespaces

---------

Co-authored-by: Jb Evain <jb@evain.net>
---
 Mono.Cecil.PE/ByteBuffer.cs              | 82 +++++++++++++++++++++---
 Test/Mono.Cecil.Tests/ByteBufferTests.cs | 16 +++++
 2 files changed, 88 insertions(+), 10 deletions(-)
 create mode 100644 Test/Mono.Cecil.Tests/ByteBufferTests.cs

diff --git a/Mono.Cecil.PE/ByteBuffer.cs b/Mono.Cecil.PE/ByteBuffer.cs
index 9a6567ce1..68668f53e 100644
--- a/Mono.Cecil.PE/ByteBuffer.cs
+++ b/Mono.Cecil.PE/ByteBuffer.cs
@@ -252,19 +252,81 @@ public void WriteCompressedUInt32 (uint value)
 
 		public void WriteCompressedInt32 (int value)
 		{
-			if (value >= 0) {
-				WriteCompressedUInt32 ((uint) (value << 1));
-				return;
+			// extracted from System.Reflection.Metadata
+			unchecked {
+				const int b6 = (1 << 6) - 1;
+				const int b13 = (1 << 13) - 1;
+				const int b28 = (1 << 28) - 1;
+
+				// 0xffffffff for negative value
+				// 0x00000000 for non-negative
+
+				int signMask = value >> 31;
+
+				if ((value & ~b6) == (signMask & ~b6)) {
+					int n = ((value & b6) << 1) | (signMask & 1);
+					WriteByte ((byte)n);
+				} else if ((value & ~b13) == (signMask & ~b13)) {
+					int n = ((value & b13) << 1) | (signMask & 1);
+					ushort val = (ushort)(0x8000 | n);
+					WriteUInt16 (BitConverter.IsLittleEndian ? ReverseEndianness (val) : val);
+				} else if ((value & ~b28) == (signMask & ~b28)) {
+					int n = ((value & b28) << 1) | (signMask & 1);
+					uint val = 0xc0000000 | (uint)n;
+					WriteUInt32 (BitConverter.IsLittleEndian ? ReverseEndianness (val) : val);
+				} else {
+					throw new ArgumentOutOfRangeException ("value", "valid range is -2^28 to 2^28 -1");
+				}
 			}
+		}
+
+		static uint ReverseEndianness (uint value)
+		{
+			// extracted from .net BCL
+
+			// This takes advantage of the fact that the JIT can detect
+			// ROL32 / ROR32 patterns and output the correct intrinsic.
+			//
+			// Input: value = [ ww xx yy zz ]
+			//
+			// First line generates : [ ww xx yy zz ]
+			//                      & [ 00 FF 00 FF ]
+			//                      = [ 00 xx 00 zz ]
+			//             ROR32(8) = [ zz 00 xx 00 ]
+			//
+			// Second line generates: [ ww xx yy zz ]
+			//                      & [ FF 00 FF 00 ]
+			//                      = [ ww 00 yy 00 ]
+			//             ROL32(8) = [ 00 yy 00 ww ]
+			//
+			//                (sum) = [ zz yy xx ww ]
+			//
+			// Testing shows that throughput increases if the AND
+			// is performed before the ROL / ROR.
+
+			return RotateRight (value & 0x00FF00FFu, 8) // xx zz
+				+ RotateLeft (value & 0xFF00FF00u, 8); // ww yy
+		}
+
+		// extracted from .net BCL
+		static uint RotateRight (uint value, int offset)
+			=> (value >> offset) | (value << (32 - offset));
+
+		static uint RotateLeft (uint value, int offset)
+			=> (value << offset) | (value >> (32 - offset));
+
+		static ushort ReverseEndianness (ushort value)
+		{
+			// extracted from .net BCL
 
-			if (value > -0x40)
-				value = 0x40 + value;
-			else if (value >= -0x2000)
-				value = 0x2000 + value;
-			else if (value >= -0x20000000)
-				value = 0x20000000 + value;
+			// Don't need to AND with 0xFF00 or 0x00FF since the final
+			// cast back to ushort will clear out all bits above [ 15 .. 00 ].
+			// This is normally implemented via "movzx eax, ax" on the return.
+			// Alternatively, the compiler could elide the movzx instruction
+			// entirely if it knows the caller is only going to access "ax"
+			// instead of "eax" / "rax" when the function returns.
 
-			WriteCompressedUInt32 ((uint) ((value << 1) | 1));
+			return (ushort)((value >> 8) + (value << 8));
 		}
 
 		public void WriteBytes (byte [] bytes)
diff --git a/Test/Mono.Cecil.Tests/ByteBufferTests.cs b/Test/Mono.Cecil.Tests/ByteBufferTests.cs
new file mode 100644
index 000000000..1ea94782f
--- /dev/null
+++ b/Test/Mono.Cecil.Tests/ByteBufferTests.cs
@@ -0,0 +1,16 @@
+using Mono.Cecil.PE;
+using NUnit.Framework;
+
+namespace Mono.Cecil.Tests {
+	public class ByteBufferTests {
+		[Test]
+		public void TestLargeIntegerCompressed ()
+		{
+			var testee = new ByteBuffer ();
+			testee.WriteCompressedInt32 (-9076);
+			testee.position = 0;
+			var result = testee.ReadCompressedInt32 ();
+			Assert.AreEqual (-9076, result);
+		}
+	}
+}