From a0d2f60d867c0991801f6bf08d6ebff3bd75bbd4 Mon Sep 17 00:00:00 2001 From: Larry Ewing Date: Tue, 9 Nov 2021 16:55:20 -0600 Subject: [PATCH 1/5] Correct the endian swapping and string implementation --- .../BrowserDebugProxy/MonoSDBHelper.cs | 108 ++++++------------ 1 file changed, 37 insertions(+), 71 deletions(-) diff --git a/src/mono/wasm/debugger/BrowserDebugProxy/MonoSDBHelper.cs b/src/mono/wasm/debugger/BrowserDebugProxy/MonoSDBHelper.cs index dee4b5ac0f5fde..ec742e8b362a4b 100644 --- a/src/mono/wasm/debugger/BrowserDebugProxy/MonoSDBHelper.cs +++ b/src/mono/wasm/debugger/BrowserDebugProxy/MonoSDBHelper.cs @@ -385,17 +385,17 @@ public MonoBinaryReader(Stream stream, bool hasError = false) : base(stream) HasError = hasError; } - internal static unsafe void PutBytesBE (byte *dest, byte *src, int count) + internal static void SwapForBE(Span dest, Span src) { - int i = 0; - - if (BitConverter.IsLittleEndian){ - dest += count; - for (; i < count; i++) - *(--dest) = *src++; - } else { - for (; i < count; i++) - *dest++ = *src++; + // SDB is big endian + if (BitConverter.IsLittleEndian) + { + for (int i = 0; i < src.Length; i++) + dest [src.Length - i - 1] = src [i]; + } + else + { + src.CopyTo(dest); } } @@ -409,105 +409,71 @@ public override string ReadString() } public unsafe long ReadLong() { - byte[] data = new byte[8]; - Read(data, 0, 8); + Span data = stackalloc byte[8]; + Read(data); long ret; - fixed (byte *src = &data[0]){ - PutBytesBE ((byte *) &ret, src, 8); - } - + SwapForBE(new Span(&ret, 8), data); return ret; } - public override unsafe sbyte ReadSByte() - { - byte[] data = new byte[4]; - Read(data, 0, 4); - int ret; - fixed (byte *src = &data[0]){ - PutBytesBE ((byte *) &ret, src, 4); - } - return (sbyte)ret; - } - - public unsafe byte ReadUByte() - { - byte[] data = new byte[4]; - Read(data, 0, 4); - - int ret; - fixed (byte *src = &data[0]){ - PutBytesBE ((byte *) &ret, src, 4); - } - return (byte)ret; - } + // SDB encodes these as 4 bytes + public override sbyte ReadSByte() => (sbyte)ReadInt32(); + public byte ReadUByte() => (byte)ReadUInt32(); + public ushort ReadUShort() => (ushort)ReadUInt32(); public override unsafe int ReadInt32() { - byte[] data = new byte[4]; - Read(data, 0, 4); + Span data = stackalloc byte[4]; + Read(data); + int ret; - fixed (byte *src = &data[0]){ - PutBytesBE ((byte *) &ret, src, 4); - } + SwapForBE(new Span(&ret, 4), data); return ret; } public override unsafe double ReadDouble() { - byte[] data = new byte[8]; - Read(data, 0, 8); + Span data = stackalloc byte[8]; + Read(data); double ret; - fixed (byte *src = &data[0]){ - PutBytesBE ((byte *) &ret, src, 8); - } + SwapForBE(new Span(&ret, 8), data); return ret; } public override unsafe uint ReadUInt32() { - byte[] data = new byte[4]; - Read(data, 0, 4); + Span data = stackalloc byte[4]; + Read(data); uint ret; - fixed (byte *src = &data[0]){ - PutBytesBE ((byte *) &ret, src, 4); - } + SwapForBE(new Span(&ret, 4), data); return ret; } - public unsafe ushort ReadUShort() - { - byte[] data = new byte[4]; - Read(data, 0, 4); - - uint ret; - fixed (byte *src = &data[0]){ - PutBytesBE ((byte *) &ret, src, 4); - } - return (ushort)ret; - } } internal class MonoBinaryWriter : BinaryWriter { public MonoBinaryWriter(Stream stream) : base(stream) {} + public void WriteString(string val) { + var bytes = Encoding.UTF8.GetBytes(val); Write(val.Length); Write(val.ToCharArray()); } - public void WriteLong(long val) + public unsafe void WriteLong(long val) { - Write((int)((val >> 32) & 0xffffffff)); - Write((int)((val >> 0) & 0xffffffff)); + long ret; + MonoBinaryReader.SwapForBE(new Span(&ret, sizeof(long)), new Span(&val, sizeof(long))); + base.Write(ret); } - public override void Write(int val) + public override unsafe void Write(int val) { - byte[] bytes = BitConverter.GetBytes(val); - Array.Reverse(bytes, 0, bytes.Length); - Write(bytes); + int ret; + MonoBinaryReader.SwapForBE(new Span(&ret, sizeof(int)), new Span(&val, sizeof(int))); + base.Write(ret); } public void WriteObj(DotnetObjectId objectId, MonoSDBHelper SdbHelper) { From 51a6551dcd71a5be90b57535acaba310e5965020 Mon Sep 17 00:00:00 2001 From: Larry Ewing Date: Tue, 9 Nov 2021 22:26:51 -0600 Subject: [PATCH 2/5] Rework the swaping reading logic --- .../BrowserDebugProxy/MonoSDBHelper.cs | 54 ++++++------------- 1 file changed, 15 insertions(+), 39 deletions(-) diff --git a/src/mono/wasm/debugger/BrowserDebugProxy/MonoSDBHelper.cs b/src/mono/wasm/debugger/BrowserDebugProxy/MonoSDBHelper.cs index ec742e8b362a4b..bc0945abd9b996 100644 --- a/src/mono/wasm/debugger/BrowserDebugProxy/MonoSDBHelper.cs +++ b/src/mono/wasm/debugger/BrowserDebugProxy/MonoSDBHelper.cs @@ -18,6 +18,7 @@ using Microsoft.CodeAnalysis.CSharp; using System.Reflection; using System.Text; +using System.Runtime.CompilerServices; namespace Microsoft.WebAssembly.Diagnostics { @@ -407,48 +408,25 @@ public override string ReadString() return new string(Encoding.UTF8.GetChars(value, 0, valueLen)); } - public unsafe long ReadLong() - { - Span data = stackalloc byte[8]; - Read(data); - - long ret; - SwapForBE(new Span(&ret, 8), data); - return ret; - } // SDB encodes these as 4 bytes public override sbyte ReadSByte() => (sbyte)ReadInt32(); public byte ReadUByte() => (byte)ReadUInt32(); public ushort ReadUShort() => (ushort)ReadUInt32(); + public override unsafe int ReadInt32() => Read(); - public override unsafe int ReadInt32() - { - Span data = stackalloc byte[4]; - Read(data); + public override unsafe double ReadDouble() => Read(); + public override unsafe uint ReadUInt32() => Read(); + public override unsafe float ReadSingle() => Read(); + public override unsafe ulong ReadUInt64() => Read(); + public override unsafe long ReadInt64() => Read(); - int ret; - SwapForBE(new Span(&ret, 4), data); - return ret; - } - - public override unsafe double ReadDouble() + protected unsafe T Read() where T : struct { - Span data = stackalloc byte[8]; + Span data = stackalloc byte[Unsafe.SizeOf()]; + T ret = default; Read(data); - - double ret; - SwapForBE(new Span(&ret, 8), data); - return ret; - } - - public override unsafe uint ReadUInt32() - { - Span data = stackalloc byte[4]; - Read(data); - - uint ret; - SwapForBE(new Span(&ret, 4), data); + SwapForBE(new Span(Unsafe.AsPointer(ref ret), data.Length), data); return ret; } } @@ -1710,7 +1688,7 @@ public async Task CreateJObjectForPtr(SessionId sessionId, ElementType { string type; string value; - long valueAddress = retDebuggerCmdReader.ReadLong(); + long valueAddress = retDebuggerCmdReader.ReadInt64(); var typeId = retDebuggerCmdReader.ReadInt32(); var className = ""; if (etype == ElementType.FnPtr) @@ -1921,7 +1899,7 @@ public async Task CreateJObjectForVariableValue(SessionId sessionId, Mo } case ElementType.R4: { - float value = BitConverter.Int32BitsToSingle(retDebuggerCmdReader.ReadInt32()); + float value = retDebuggerCmdReader.ReadSingle(); ret = CreateJObjectForNumber(value); break; } @@ -1933,15 +1911,13 @@ public async Task CreateJObjectForVariableValue(SessionId sessionId, Mo } case ElementType.I8: { - long value = retDebuggerCmdReader.ReadLong(); + long value = retDebuggerCmdReader.ReadInt64(); ret = CreateJObjectForNumber(value); break; } case ElementType.U8: { - ulong high = (ulong) retDebuggerCmdReader.ReadInt32(); - ulong low = (ulong) retDebuggerCmdReader.ReadInt32(); - var value = ((high << 32) | low); + ulong value = retDebuggerCmdReader.ReadUInt64(); ret = CreateJObjectForNumber(value); break; } From 600aa175f60baea1a70e389cca228faf8dd31ab0 Mon Sep 17 00:00:00 2001 From: Larry Ewing Date: Tue, 9 Nov 2021 22:35:50 -0600 Subject: [PATCH 3/5] Fix WriteString for real --- src/mono/wasm/debugger/BrowserDebugProxy/MonoSDBHelper.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/mono/wasm/debugger/BrowserDebugProxy/MonoSDBHelper.cs b/src/mono/wasm/debugger/BrowserDebugProxy/MonoSDBHelper.cs index bc0945abd9b996..64d56ab8476df7 100644 --- a/src/mono/wasm/debugger/BrowserDebugProxy/MonoSDBHelper.cs +++ b/src/mono/wasm/debugger/BrowserDebugProxy/MonoSDBHelper.cs @@ -438,8 +438,8 @@ public MonoBinaryWriter(Stream stream) : base(stream) {} public void WriteString(string val) { var bytes = Encoding.UTF8.GetBytes(val); - Write(val.Length); - Write(val.ToCharArray()); + Write(bytes.Length); + Write(bytes); } public unsafe void WriteLong(long val) { From 7e05e8ed51f5190ce7cecf97db2804116f119ba9 Mon Sep 17 00:00:00 2001 From: Larry Ewing Date: Tue, 9 Nov 2021 22:50:15 -0600 Subject: [PATCH 4/5] Rework writing swapping --- .../BrowserDebugProxy/MonoSDBHelper.cs | 23 +++++++++---------- 1 file changed, 11 insertions(+), 12 deletions(-) diff --git a/src/mono/wasm/debugger/BrowserDebugProxy/MonoSDBHelper.cs b/src/mono/wasm/debugger/BrowserDebugProxy/MonoSDBHelper.cs index 64d56ab8476df7..01490539520c48 100644 --- a/src/mono/wasm/debugger/BrowserDebugProxy/MonoSDBHelper.cs +++ b/src/mono/wasm/debugger/BrowserDebugProxy/MonoSDBHelper.cs @@ -441,18 +441,17 @@ public void WriteString(string val) Write(bytes.Length); Write(bytes); } - public unsafe void WriteLong(long val) - { - long ret; - MonoBinaryReader.SwapForBE(new Span(&ret, sizeof(long)), new Span(&val, sizeof(long))); - base.Write(ret); - } - public override unsafe void Write(int val) + + public override unsafe void Write(long val) => WriteType(val); + public override unsafe void Write(int val) => WriteType(val); + + protected unsafe void WriteType(T val) where T : struct { - int ret; - MonoBinaryReader.SwapForBE(new Span(&ret, sizeof(int)), new Span(&val, sizeof(int))); - base.Write(ret); + Span data = stackalloc byte[Unsafe.SizeOf()]; + MonoBinaryReader.SwapForBE(data, new Span(Unsafe.AsPointer(ref val), data.Length)); + base.Write(data); } + public void WriteObj(DotnetObjectId objectId, MonoSDBHelper SdbHelper) { if (objectId.Scheme == "object") @@ -1068,7 +1067,7 @@ public async Task SetBreakpoint(SessionId sessionId, int methodId, long il_ commandParamsWriter.Write((byte)1); commandParamsWriter.Write((byte)ModifierKind.LocationOnly); commandParamsWriter.Write(methodId); - commandParamsWriter.WriteLong(il_offset); + commandParamsWriter.Write(il_offset); var retDebuggerCmdReader = await SendDebuggerAgentCommand(sessionId, CmdEventRequest.Set, commandParams, token); return retDebuggerCmdReader.ReadInt32(); } @@ -1589,7 +1588,7 @@ public async Task GetPointerContent(SessionId sessionId, int pointerId, var ret = new List(); var commandParams = new MemoryStream(); var commandParamsWriter = new MonoBinaryWriter(commandParams); - commandParamsWriter.WriteLong(pointerValues[pointerId].address); + commandParamsWriter.Write(pointerValues[pointerId].address); commandParamsWriter.Write(pointerValues[pointerId].typeId); var retDebuggerCmdReader = await SendDebuggerAgentCommand(sessionId, CmdPointer.GetValue, commandParams, token); var varName = pointerValues[pointerId].varName; From aac76ede18a108f040319c22a6fd167dac42ca00 Mon Sep 17 00:00:00 2001 From: Larry Ewing Date: Wed, 10 Nov 2021 07:54:23 -0600 Subject: [PATCH 5/5] Use Span.Reverse --- .../BrowserDebugProxy/MonoSDBHelper.cs | 56 +++++++++---------- 1 file changed, 25 insertions(+), 31 deletions(-) diff --git a/src/mono/wasm/debugger/BrowserDebugProxy/MonoSDBHelper.cs b/src/mono/wasm/debugger/BrowserDebugProxy/MonoSDBHelper.cs index 01490539520c48..a6733de763a57c 100644 --- a/src/mono/wasm/debugger/BrowserDebugProxy/MonoSDBHelper.cs +++ b/src/mono/wasm/debugger/BrowserDebugProxy/MonoSDBHelper.cs @@ -386,20 +386,6 @@ public MonoBinaryReader(Stream stream, bool hasError = false) : base(stream) HasError = hasError; } - internal static void SwapForBE(Span dest, Span src) - { - // SDB is big endian - if (BitConverter.IsLittleEndian) - { - for (int i = 0; i < src.Length; i++) - dest [src.Length - i - 1] = src [i]; - } - else - { - src.CopyTo(dest); - } - } - public override string ReadString() { var valueLen = ReadInt32(); @@ -413,20 +399,24 @@ public override string ReadString() public override sbyte ReadSByte() => (sbyte)ReadInt32(); public byte ReadUByte() => (byte)ReadUInt32(); public ushort ReadUShort() => (ushort)ReadUInt32(); - public override unsafe int ReadInt32() => Read(); + public override int ReadInt32() => ReadBigEndian(); - public override unsafe double ReadDouble() => Read(); - public override unsafe uint ReadUInt32() => Read(); - public override unsafe float ReadSingle() => Read(); - public override unsafe ulong ReadUInt64() => Read(); - public override unsafe long ReadInt64() => Read(); + public override double ReadDouble() => ReadBigEndian(); + public override uint ReadUInt32() => ReadBigEndian(); + public override float ReadSingle() => ReadBigEndian(); + public override ulong ReadUInt64() => ReadBigEndian(); + public override long ReadInt64() => ReadBigEndian(); - protected unsafe T Read() where T : struct + protected unsafe T ReadBigEndian() where T : struct { Span data = stackalloc byte[Unsafe.SizeOf()]; T ret = default; Read(data); - SwapForBE(new Span(Unsafe.AsPointer(ref ret), data.Length), data); + if (BitConverter.IsLittleEndian) + { + data.Reverse(); + } + data.CopyTo(new Span(Unsafe.AsPointer(ref ret), data.Length)); return ret; } } @@ -435,20 +425,24 @@ internal class MonoBinaryWriter : BinaryWriter { public MonoBinaryWriter(Stream stream) : base(stream) {} - public void WriteString(string val) + public override void Write(string val) { var bytes = Encoding.UTF8.GetBytes(val); Write(bytes.Length); Write(bytes); } - public override unsafe void Write(long val) => WriteType(val); - public override unsafe void Write(int val) => WriteType(val); + public override void Write(long val) => WriteBigEndian(val); + public override void Write(int val) => WriteBigEndian(val); - protected unsafe void WriteType(T val) where T : struct + protected unsafe void WriteBigEndian(T val) where T : struct { Span data = stackalloc byte[Unsafe.SizeOf()]; - MonoBinaryReader.SwapForBE(data, new Span(Unsafe.AsPointer(ref val), data.Length)); + new Span(Unsafe.AsPointer(ref val), data.Length).CopyTo(data); + if (BitConverter.IsLittleEndian) + { + data.Reverse(); + } base.Write(data); } @@ -821,7 +815,7 @@ public async Task CreateString(SessionId sessionId, string value, Cancellat var retDebuggerCmdReader = await SendDebuggerAgentCommand(sessionId, CmdAppDomain.GetRootDomain, commandParams, token); var root_domain = retDebuggerCmdReader.ReadInt32(); commandParamsWriter.Write(root_domain); - commandParamsWriter.WriteString(value); + commandParamsWriter.Write(value); retDebuggerCmdReader = await SendDebuggerAgentCommand(sessionId, CmdAppDomain.CreateString, commandParams, token); return retDebuggerCmdReader.ReadInt32(); } @@ -932,7 +926,7 @@ public async Task GetAssemblyId(SessionId sessionId, string asm_name, Cance { var commandParams = new MemoryStream(); var commandParamsWriter = new MonoBinaryWriter(commandParams); - commandParamsWriter.WriteString(asm_name); + commandParamsWriter.Write(asm_name); var retDebuggerCmdReader = await SendDebuggerAgentCommand(sessionId, CmdVM.GetAssemblyByName, commandParams, token); return retDebuggerCmdReader.ReadInt32(); @@ -1452,7 +1446,7 @@ public async Task GetMethodIdByName(SessionId sessionId, int type_id, strin var commandParams = new MemoryStream(); var commandParamsWriter = new MonoBinaryWriter(commandParams); commandParamsWriter.Write((int)type_id); - commandParamsWriter.WriteString(method_name); + commandParamsWriter.Write(method_name); commandParamsWriter.Write((int)(BindingFlags.Public | BindingFlags.NonPublic | BindingFlags.Instance | BindingFlags.Static)); commandParamsWriter.Write((int)1); //case sensitive var retDebuggerCmdReader = await SendDebuggerAgentCommand(sessionId, CmdType.GetMethodsByNameFlags, commandParams, token); @@ -2198,7 +2192,7 @@ public async Task GetTypeByName(SessionId sessionId, string typeToSearch, C { var commandParams = new MemoryStream(); var commandParamsWriter = new MonoBinaryWriter(commandParams); - commandParamsWriter.WriteString(typeToSearch); + commandParamsWriter.Write(typeToSearch); var retDebuggerCmdReader = await SendDebuggerAgentCommand(sessionId, CmdVM.GetTypes, commandParams, token); var count = retDebuggerCmdReader.ReadInt32(); //count ret return retDebuggerCmdReader.ReadInt32();