cDAC: fix inverted IncludeAvailableToProfilers check in Loader_1.GetModuleHandles

docs/design/datacontracts/Loader.md

@@ -326,7 +326,7 @@ IEnumerable<ModuleHandle> GetModuleHandles(TargetPointer appDomain, AssemblyIter
         }
 
         if ((assembly.NotifyFlags & ASSEMBLY_NOTIFYFLAGS_PROFILER_NOTIFIED) != 0 &&
-            !iterationFlags.HasFlag(AssemblyIterationFlags.IncludeAvailableToProfilers))
+            iterationFlags.HasFlag(AssemblyIterationFlags.IncludeAvailableToProfilers))
         {
             // The assembly has reached the state at which we would notify profilers,
             // and we're supposed to include such assemblies in the enumeration. So

src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/Contracts/Loader_1.cs

@@ -95,7 +95,7 @@ IEnumerable<ModuleHandle> ILoader.GetModuleHandles(TargetPointer appDomain, Asse
                 continue;
             }
 
-            if ((assembly.NotifyFlags & ASSEMBLY_NOTIFYFLAGS_PROFILER_NOTIFIED) != 0 && !iterationFlags.HasFlag(AssemblyIterationFlags.IncludeAvailableToProfilers))
+            if ((assembly.NotifyFlags & ASSEMBLY_NOTIFYFLAGS_PROFILER_NOTIFIED) != 0 && iterationFlags.HasFlag(AssemblyIterationFlags.IncludeAvailableToProfilers))
             {
                 // The assembly has reached the state at which we would notify profilers,
                 // and we're supposed to include such assemblies in the enumeration. So

src/native/managed/cdac/tests/UnitTests/LoaderTests.cs

@@ -859,6 +859,65 @@ public void GetILAddr_WebcilV1RvaToOffset(MockTarget.Architecture arch)
         Assert.Equal((TargetPointer)(imageBase + 0x2700u), contract.GetILAddr(peAssemblyAddr, 0x4500));
     }
 
+    public static IEnumerable<object[]> GetModuleHandles_ProfilerNotifiedData()
+    {
+        const AssemblyIterationFlags Loaded = AssemblyIterationFlags.IncludeLoaded | AssemblyIterationFlags.IncludeExecution;
+        const AssemblyIterationFlags AvailableToProfilers = AssemblyIterationFlags.IncludeAvailableToProfilers | AssemblyIterationFlags.IncludeExecution;
+        foreach (object[] archData in new MockTarget.StdArch())
+        {
+            var arch = (MockTarget.Architecture)archData[0];
+            // Profiler notified, still loading: not a loaded assembly
+            yield return [arch, true, false, Loaded, false];
+            // Profiler notified, still loading: available to profilers
+            yield return [arch, true, false, AvailableToProfilers, true];
+            // Profiler notified, loaded: a loaded assembly
+            yield return [arch, true, true, Loaded, true];
+            // Profiler notified, loaded: available to profilers
+            yield return [arch, true, true, AvailableToProfilers, true];
+            // Not notified, still loading: not available to profilers
+            yield return [arch, false, false, AvailableToProfilers, false];
+        }
+    }
+
+    [Theory]
+    [MemberData(nameof(GetModuleHandles_ProfilerNotifiedData))]
+    public void GetModuleHandles_ProfilerNotified(
+        MockTarget.Architecture arch,
+        bool profilerNotified,
+        bool isLoaded,
+        AssemblyIterationFlags iterationFlags,
+        bool expected)
+    {
+        const uint ProfilerNotified = 0x1; // ASSEMBLY_NOTIFYFLAGS_PROFILER_NOTIFIED
+        TargetPointer appDomain = default;
+        TargetPointer moduleAddress = default;
+
+        (ILoader contract, _) = CreateLoaderContractWithTarget(arch, (loader, targetBuilder) =>
+        {
+            MockLoaderModule module = loader.AddModule();
+            MockLoaderAssembly assembly = loader.GetAssembly(module);
+            assembly.NotifyFlags = profilerNotified ? ProfilerNotified : 0;
+            assembly.IsLoaded = isLoaded;
+            moduleAddress = module.Address;
+            appDomain = loader.AddAppDomain(module);
+            targetBuilder.AddTypes(new Dictionary<DataType, Target.TypeInfo>
+            {
+                [DataType.AppDomain] = TargetTestHelpers.CreateTypeInfo(loader.AppDomainLayout),
+                [DataType.ArrayListBase] = TargetTestHelpers.CreateTypeInfo(loader.ArrayListBaseLayout),
+                [DataType.ArrayListBlock] = TargetTestHelpers.CreateTypeInfo(loader.ArrayListBlockLayout),
+            });
+        });
+
+        List<TargetPointer> modules = [];
+        foreach (Contracts.ModuleHandle handle in contract.GetModuleHandles(appDomain, iterationFlags))
+        {
+            modules.Add(handle.Address);
+        }
+
+        TargetPointer[] expectedModules = expected ? [moduleAddress] : [];
+        Assert.Equal(expectedModules, modules);
+    }
+
     public static IEnumerable<object[]> IsModuleMappedData()
     {
         foreach (object[] archData in new MockTarget.StdArch())

src/native/managed/cdac/tests/UnitTests/MockDescriptors/MockDescriptors.Loader.cs

@@ -239,6 +239,18 @@ public ulong Module
         get => ReadPointerField(ModuleFieldName);
         set => WritePointerField(ModuleFieldName, value);
     }
+
+    public uint NotifyFlags
+    {
+        get => ReadUInt32Field(NotifyFlagsFieldName);
+        set => WriteUInt32Field(NotifyFlagsFieldName, value);
+    }
+
+    public bool IsLoaded
+    {
+        get => ReadByteField(IsLoadedFieldName) != 0;
+        set => WriteByteField(IsLoadedFieldName, value ? (byte)1 : (byte)0);
+    }
 }
 
 internal sealed class MockEEConfig : TypedView
@@ -295,9 +307,13 @@ internal sealed class MockLoaderBuilder
     internal Layout<MockCGrowableSymbolStream> CGrowableSymbolStreamLayout { get; }
     internal Layout<MockModuleLookupMap> ModuleLookupMapLayout { get; }
     internal Layout DynamicILBlobTableLayout { get; }
+    internal Layout AppDomainLayout { get; }
+    internal Layout ArrayListBaseLayout { get; }
+    internal Layout ArrayListBlockLayout { get; }
 
     private readonly MockMemorySpace.BumpAllocator _allocator;
     private readonly Layout _dynamicILBlobTableHeaderLayout;
+    private readonly Dictionary<ulong, MockLoaderAssembly> _assemblies = new();
 
     public MockLoaderBuilder(MockMemorySpace.Builder builder)
         : this(builder, (DefaultAllocationRangeStart, DefaultAllocationRangeEnd))
@@ -333,6 +349,23 @@ public MockLoaderBuilder(MockMemorySpace.Builder builder, (ulong Start, ulong En
             builder.TargetTestHelpers.Arch,
             entryLayout.Size,
             [.. _dynamicILBlobTableHeaderLayout.Fields, .. entryLayout.Fields]);
+
+        // An ArrayListBase embeds its first block, and a block's elements start at ArrayStart.
+        // AssemblyList is last so that the first block's elements can follow the AppDomain.
+        ArrayListBlockLayout = new SequentialLayoutBuilder("ArrayListBlock", builder.TargetTestHelpers.Arch)
+            .AddPointerField("Next")
+            .AddUInt32Field("Size")
+            .AddPointerField("ArrayStart")
+            .Build();
+        ArrayListBaseLayout = new SequentialLayoutBuilder("ArrayListBase", builder.TargetTestHelpers.Arch)
+            .AddUInt32Field("Count")
+            .AddField("FirstBlock", ArrayListBlockLayout.Size)
+            .Build();
+        AppDomainLayout = new SequentialLayoutBuilder("AppDomain", builder.TargetTestHelpers.Arch)
+            .AddPointerField("RootAssembly")
+            .AddPointerField("FriendlyName")
+            .AddField("AssemblyList", ArrayListBaseLayout.Size)
+            .Build();
     }
 
     internal MockLoaderHeap AddLoaderHeap(ulong firstBlockAddress = 0)
@@ -384,9 +417,32 @@ internal MockLoaderModule AddModule(
         MockLoaderAssembly assembly = AssemblyLayout.Create(_allocator.Allocate((ulong)AssemblyLayout.Size, "Assembly"));
         assembly.Module = module.Address;
         module.Assembly = assembly.Address;
+        _assemblies[assembly.Address] = assembly;
         return module;
     }
 
+    internal MockLoaderAssembly GetAssembly(MockLoaderModule module) => _assemblies[module.Assembly];
+
+    // Adds an AppDomain whose assembly list holds the modules' assemblies, in order, in its first block.
+    internal ulong AddAppDomain(params MockLoaderModule[] modules)
+    {
+        TargetTestHelpers helpers = Builder.TargetTestHelpers;
+        int assemblyList = AppDomainLayout.GetField("AssemblyList").Offset;
+        int firstBlock = assemblyList + ArrayListBaseLayout.GetField("FirstBlock").Offset;
+        int arrayStart = firstBlock + ArrayListBlockLayout.GetField("ArrayStart").Offset;
+        int size = Math.Max(AppDomainLayout.Size, arrayStart + (modules.Length * helpers.PointerSize));
+        MockMemorySpace.HeapFragment appDomain = _allocator.Allocate((ulong)size, "AppDomain");
+
+        helpers.Write(appDomain.Data.AsSpan(assemblyList + ArrayListBaseLayout.GetField("Count").Offset), (uint)modules.Length);
+        helpers.Write(appDomain.Data.AsSpan(firstBlock + ArrayListBlockLayout.GetField("Size").Offset), (uint)modules.Length);
+        for (int i = 0; i < modules.Length; i++)
+        {
+            helpers.WritePointer(appDomain.Data.AsSpan(arrayStart + (i * helpers.PointerSize)), modules[i].Assembly);
+        }
+
+        return appDomain.Address;
+    }
+
     internal MockEEConfig AddEEConfig(uint modifiableAssemblies)
     {
         MockEEConfig config = EEConfigLayout.Create(_allocator.Allocate((ulong)EEConfigLayout.Size, "EEConfig"));