Fix ModuleLoadUnloadTraceData.ModuleID cast to be unchecked since its really a ulong...
authorJohan Lorensson <lateralusx.github@gmail.com>
Mon, 23 Oct 2023 23:46:55 +0000 (01:46 +0200)
committerGitHub <noreply@github.com>
Mon, 23 Oct 2023 23:46:55 +0000 (16:46 -0700)
The ModuleLoadUnload events ModuleID is typed as a uint64 in the
EventPipe manifest and emitted as a uint64 in the event payload.
However, the parsing logic in ModuleLoadUnloadTraceDataevent in trace
event:

https://github.com/microsoft/perfview/blob/a6c87911fe1aef8f59c9ce54aa4e16a1be6db91e/src/TraceEvent/Parsers/ClrEtwAll.cs.base#L4963

handles it as a long. Android devices running arm64 can use pointer
tagging meaning that high bits can be set in 64-bit addresses and since
module id is a memory address, it will be returned as a negative long
from ModuleLoadUnload event and since all diagnostic tooling is build
with overflow checking enabled by default, casting it to a ulong will
trigger and overflow exception when high bit is set.

Fix is to do an unchecked cast in this case since the value should be
threated as a ulong in first place.

Fixes https://github.com/dotnet/diagnostics/issues/4348.

src/Tools/dotnet-gcdump/DotNetHeapDump/DotNetHeapDumpGraphReader.cs

index c26488fcef958dcb7c5fffb7e3e59b409e69ebbf..328febb54d40714840475529f9fb19eea71ba7ff 100644 (file)
@@ -117,12 +117,13 @@ public class DotNetHeapDumpGraphReader
                 return;
             }
 
-            if (!m_moduleID2Name.ContainsKey((ulong)data.ModuleID))
+            ulong moduleID = unchecked((ulong)data.ModuleID);
+            if (!m_moduleID2Name.ContainsKey(moduleID))
             {
-                m_moduleID2Name[(ulong)data.ModuleID] = data.ModuleILPath;
+                m_moduleID2Name[moduleID] = data.ModuleILPath;
             }
 
-            m_log.WriteLine("Found Module {0} ID 0x{1:x}", data.ModuleILFileName, (ulong)data.ModuleID);
+            m_log.WriteLine("Found Module {0} ID 0x{1:x}", data.ModuleILFileName, moduleID);
         };
         source.Clr.AddCallbackForEvents(moduleCallback); // Get module events for clr provider
         // TODO should not be needed if we use CAPTURE_STATE when collecting.