Merge pull request #70 from microsoft/user/trgibeau/bugFix3

Fix issue with same module names

Author: trgibeau

src/ProfileExplorer.Profiling.Tests/Integration/FunctionProfilerEndToEndTests.cs

@@ -75,7 +75,7 @@ public void FullPipeline_PdbAndDll_ProducesAnnotatedAssembly() {
     // We'll test the lower-level components directly since we have local files.
     var ipResolver = new Profiling.IpResolver();
     ipResolver.AddImage(TestDataHelper.MsoModuleName, moduleBase, 0x1000000);
-    ipResolver.SetFunctions(TestDataHelper.MsoModuleName, allFunctions);
+    ipResolver.SetFunctions(moduleBase, allFunctions);
 
     var aggregator = new Profiling.SampleAggregator(ipResolver);
     aggregator.AddSamples(samples);
@@ -183,7 +183,7 @@ public void MultipleModules_IndependentProfiles() {
 
     var ipResolver = new Profiling.IpResolver();
     ipResolver.AddImage(TestDataHelper.MsoModuleName, moduleBase, 0x1000000);
-    ipResolver.SetFunctions(TestDataHelper.MsoModuleName, functions);
+    ipResolver.SetFunctions(moduleBase, functions);
 
     var aggregator = new Profiling.SampleAggregator(ipResolver);
     aggregator.AddSamples([
@@ -290,7 +290,7 @@ private static (ProfileFunctionId FuncId, FunctionProfileData? Profile, int DllT
 
     var ipResolver = new Profiling.IpResolver();
     ipResolver.AddImage(TestDataHelper.MsoModuleName, LocalBinaryModuleBase, 0x1000000);
-    ipResolver.SetFunctions(TestDataHelper.MsoModuleName, functions);
+    ipResolver.SetFunctions(LocalBinaryModuleBase, functions);
 
     var aggregator = new Profiling.SampleAggregator(ipResolver);
     var samples = new List<IProfileSample>();

src/ProfileExplorer.Profiling.Tests/Integration/IpResolverRealProviderTests.cs

@@ -48,7 +48,7 @@ private static IpResolver BuildResolver(PdbSymbolProvider provider, List<Functio
     int size = (int)Math.Min(maxEndRva + 0x10000, int.MaxValue);
     var resolver = new IpResolver();
     resolver.AddImage(TestDataHelper.MsoModuleName, ModuleBase, size);
-    resolver.SetFunctions(TestDataHelper.MsoModuleName, functions, withProvider ? provider : null);
+    resolver.SetFunctions(ModuleBase, functions, withProvider ? provider : null);
     return resolver;
   }
 

src/ProfileExplorer.Profiling.Tests/Unit/AggregatorSemanticsCharacterizationTests.cs

@@ -53,7 +53,7 @@ public void CallerInstructionAttribution_LibraryVsCore_Matches() {
     // ---- LIBRARY: real SampleAggregator over a leaf-first stack [leaf, caller-return] ----
     var ipResolver = new IpResolver();
     ipResolver.AddImage(Module, ModuleBase, ModuleSize);
-    ipResolver.SetFunctions(Module, new List<FunctionDebugInfo> {
+    ipResolver.SetFunctions(ModuleBase, new List<FunctionDebugInfo> {
       new FunctionDebugInfo("Leaf", LeafRva, 0x100),
       new FunctionDebugInfo("Caller", CallerRva, 0x100)
     });

src/ProfileExplorer.Profiling.Tests/Unit/CallTreeBuilderTests.cs

@@ -18,10 +18,10 @@ public class CallTreeBuilderTests {
       resolver.AddImage(module, moduleBase, moduleSize);
     }
 
-    var grouped = functions.GroupBy(f => f.module);
+    var grouped = functions.GroupBy(f => (f.module, f.moduleBase));
     foreach (var group in grouped) {
       var funcList = group.Select(f => new FunctionDebugInfo(f.funcName, f.funcRva, f.funcSize)).ToList();
-      resolver.SetFunctions(group.Key, funcList);
+      resolver.SetFunctions(group.Key.moduleBase, funcList);
     }
 
     return (resolver, new CallTreeBuilder(resolver));

src/ProfileExplorer.Profiling.Tests/Unit/CounterAggregatorTests.cs

@@ -22,7 +22,7 @@ private class TestCounterEvent : IPerformanceCounterEvent {
   private (IpResolver resolver, CounterAggregator aggregator) Setup() {
     var resolver = new IpResolver();
     resolver.AddImage("test.dll", 0x1000, 0x10000);
-    resolver.SetFunctions("test.dll", [
+    resolver.SetFunctions(0x1000, [
       new FunctionDebugInfo("HotFunc", 0x100, 0x50)
     ]);
     return (resolver, new CounterAggregator(resolver));

src/ProfileExplorer.Profiling.Tests/Unit/FunctionProfilerInjectionTests.cs

@@ -42,7 +42,7 @@ public void AddResolvedFunctions_ResolvesAndAggregates_WithoutSymbolDownload() {
     using var profiler = new FunctionProfiler(options, new NoSymbolLocator());
 
     profiler.AddImages(SyntheticSampleBuilder.CreateImages((module, baseAddr, size)));
-    profiler.AddResolvedFunctions(module, new List<FunctionDebugInfo> {
+    profiler.AddResolvedFunctions(module, baseAddr, new List<FunctionDebugInfo> {
       new("Main", 0x1000, 0x800),
       new("Foo", 0x2000, 0x800),
       new("Bar", 0x3000, 0x800)
@@ -77,14 +77,116 @@ public void AddResolvedFunctions_ResolvesAndAggregates_WithoutSymbolDownload() {
   }
 
   [TestMethod]
-  public void AddResolvedFunctions_NullArguments_Throw() {
+  public void AddResolvedFunctions_InvalidArguments_Throw() {
+    const string module = "app.dll";
+    const long baseAddr = 0x140000000;
+    const int size = 0x100000;
     var options = new ProfilerOptions { SymbolPaths = new[] { "srv*https://symbols.invalid" } };
     using var profiler = new FunctionProfiler(options, new NoSymbolLocator());
+    profiler.AddImages(SyntheticSampleBuilder.CreateImages((module, baseAddr, size)));
 
-    Assert.ThrowsException<ArgumentException>(() =>
-      profiler.AddResolvedFunctions("", new List<FunctionDebugInfo>()));
+    // Null function list.
     Assert.ThrowsException<ArgumentNullException>(() =>
-      profiler.AddResolvedFunctions("m.dll", null!));
+      profiler.AddResolvedFunctions(module, baseAddr, null!));
+    // Empty module name.
+    Assert.ThrowsException<ArgumentException>(() =>
+      profiler.AddResolvedFunctions("", baseAddr, new List<FunctionDebugInfo>()));
+    // Base address was never registered via AddImages.
+    Assert.ThrowsException<ArgumentException>(() =>
+      profiler.AddResolvedFunctions(module, 0xDEADBEEF, new List<FunctionDebugInfo>()));
+    // Module name does not match the image registered at that base (wrong-module guard).
+    Assert.ThrowsException<ArgumentException>(() =>
+      profiler.AddResolvedFunctions("wrong.dll", baseAddr, new List<FunctionDebugInfo>()));
+  }
+
+  [TestMethod]
+  public void AddResolvedFunctions_ModuleNameCaseMismatch_Throws() {
+    const string module = "App.DLL";
+    const long baseAddr = 0x140000000;
+    const int size = 0x100000;
+    var options = new ProfilerOptions {
+      SymbolPaths = new[] { "srv*https://symbols.invalid" },
+      IncludeManagedCode = false,
+      IncludePerformanceCounters = false
+    };
+    using var profiler = new FunctionProfiler(options, new NoSymbolLocator());
+    profiler.AddImages(SyntheticSampleBuilder.CreateImages((module, baseAddr, size)));
+
+    // Module identity is case-sensitive (Ordinal) throughout the library: a case-only difference denotes
+    // a different binary (the WinUI vs UWP xaml pair), so the wrong-module guard rejects it.
+    Assert.ThrowsException<ArgumentException>(() =>
+      profiler.AddResolvedFunctions("app.dll", baseAddr,
+        new List<FunctionDebugInfo> { new("Foo", 0x1000, 0x800) }));
+  }
+
+  [TestMethod]
+  public void AddImages_DuplicateSameImage_IsIdempotent() {
+    const string module = "app.dll";
+    const long baseAddr = 0x140000000;
+    const int size = 0x100000;
+    var options = new ProfilerOptions {
+      SymbolPaths = new[] { "srv*https://symbols.invalid" },
+      IncludeManagedCode = false,
+      IncludePerformanceCounters = false
+    };
+    using var profiler = new FunctionProfiler(options, new NoSymbolLocator());
+
+    // The same image reported twice (e.g. an ImageDCStart rundown followed by its ImageLoad) is
+    // deduped, matching old Core — resolution still works normally afterward.
+    profiler.AddImages(SyntheticSampleBuilder.CreateImages((module, baseAddr, size)));
+    profiler.AddImages(SyntheticSampleBuilder.CreateImages((module, baseAddr, size)));
+
+    profiler.AddResolvedFunctions(module, baseAddr,
+      new List<FunctionDebugInfo> { new("Foo", 0x1000, 0x800) });
+    profiler.AddSamples(new IProfileSample[] {
+      new SyntheticSample(baseAddr + 0x1000 + 0x10, TimeSpan.FromMilliseconds(5), 1, 1, module, baseAddr)
+    });
+
+    var report = profiler.GetReport();
+    Assert.AreEqual(TimeSpan.FromMilliseconds(5),
+      report.Functions[new ProfileFunctionId(module, "Foo")].ExclusiveWeight);
+  }
+
+  [TestMethod]
+  public void AddImages_SameBaseDifferentImage_ThrowsByDefault() {
+    const long baseAddr = 0x140000000;
+    const int size = 0x100000;
+    var options = new ProfilerOptions {
+      SymbolPaths = new[] { "srv*https://symbols.invalid" },
+      IncludeManagedCode = false,
+      IncludePerformanceCounters = false
+    };
+    using var profiler = new FunctionProfiler(options, new NoSymbolLocator());
+
+    // Two DIFFERENT binaries at the same base means the caller didn't de-dupe per sampling window;
+    // silently keeping one would mis-attribute the other's samples, so this throws by default.
+    profiler.AddImages(SyntheticSampleBuilder.CreateImages(("first.dll", baseAddr, size)));
+    Assert.ThrowsException<InvalidOperationException>(() =>
+      profiler.AddImages(SyntheticSampleBuilder.CreateImages(("second.dll", baseAddr, size))));
+  }
+
+  [TestMethod]
+  public void AddImages_SameBaseDifferentImage_LenientMode_LastWins() {
+    const long baseAddr = 0x140000000;
+    const int size = 0x100000;
+    var options = new ProfilerOptions {
+      SymbolPaths = new[] { "srv*https://symbols.invalid" },
+      IncludeManagedCode = false,
+      IncludePerformanceCounters = false,
+      ThrowOnImageBaseCollision = false
+    };
+    using var profiler = new FunctionProfiler(options, new NoSymbolLocator());
+
+    // Opt-in lenient mode: the base-keyed resolver can hold only one, so the latest registration wins
+    // (warned, non-fatal). Attaching functions to the displaced image is then rejected by the guard.
+    profiler.AddImages(SyntheticSampleBuilder.CreateImages(("first.dll", baseAddr, size)));
+    profiler.AddImages(SyntheticSampleBuilder.CreateImages(("second.dll", baseAddr, size)));
+
+    profiler.AddResolvedFunctions("second.dll", baseAddr,
+      new List<FunctionDebugInfo> { new("Bar", 0x1000, 0x800) });
+    Assert.ThrowsException<ArgumentException>(() =>
+      profiler.AddResolvedFunctions("first.dll", baseAddr,
+        new List<FunctionDebugInfo> { new("Foo", 0x1000, 0x800) }));
   }
 
   [TestMethod]
@@ -101,7 +203,7 @@ public void AddSamples_WithInstancePath_FocusesOnMatchingStacks() {
 
     using var profiler = new FunctionProfiler(options, new NoSymbolLocator());
     profiler.AddImages(SyntheticSampleBuilder.CreateImages((module, baseAddr, size)));
-    profiler.AddResolvedFunctions(module, new List<FunctionDebugInfo> {
+    profiler.AddResolvedFunctions(module, baseAddr, new List<FunctionDebugInfo> {
       new("Main", 0x1000, 0x800),
       new("Foo", 0x2000, 0x800),
       new("Bar", 0x3000, 0x800),

src/ProfileExplorer.Profiling.Tests/Unit/IpResolverProviderTests.cs

@@ -60,7 +60,7 @@ private static IpResolver CreateResolver(ISymbolDebugInfo? provider,
     resolver.AddImage("mod.dll", ModuleBase, ModuleSize);
     // The contiguous list would resolve SampleIp to listFuncName; a registered provider must win.
     var functions = new List<FunctionDebugInfo> { new(listFuncName, FuncRva, FuncSize) };
-    resolver.SetFunctions("mod.dll", functions, provider);
+    resolver.SetFunctions(ModuleBase, functions, provider);
     return resolver;
   }
 
@@ -145,7 +145,7 @@ public void SplitChunkAddress_MissingFromContiguousList_ResolvesToParentViaProvi
     // Parent's primary chunk is the only entry in the contiguous list; the cold chunk is absent.
     var withProvider = new IpResolver();
     withProvider.AddImage("mod.dll", ModuleBase, ModuleSize);
-    withProvider.SetFunctions("mod.dll", new List<FunctionDebugInfo> { parentFunc }, stub);
+    withProvider.SetFunctions(ModuleBase, new List<FunctionDebugInfo> { parentFunc }, stub);
 
     var resolved = withProvider.Resolve(ColdChunkIp);
     Assert.IsNotNull(resolved);
@@ -157,7 +157,7 @@ public void SplitChunkAddress_MissingFromContiguousList_ResolvesToParentViaProvi
     // the exact regression the provider path fixes.
     var listOnly = new IpResolver();
     listOnly.AddImage("mod.dll", ModuleBase, ModuleSize);
-    listOnly.SetFunctions("mod.dll", new List<FunctionDebugInfo> { parentFunc });
+    listOnly.SetFunctions(ModuleBase, new List<FunctionDebugInfo> { parentFunc });
 
     var listOnlyResolved = listOnly.Resolve(ColdChunkIp);
     Assert.IsNotNull(listOnlyResolved);

src/ProfileExplorer.Profiling.Tests/Unit/SampleAggregatorTests.cs

@@ -18,7 +18,7 @@ private IpResolver CreateResolverWithFunction(string module, long moduleBase, in
     var functions = new List<FunctionDebugInfo> {
       new(funcName, funcRva, funcSize)
     };
-    resolver.SetFunctions(module, functions);
+    resolver.SetFunctions(moduleBase, functions);
     return resolver;
   }
 
@@ -54,7 +54,7 @@ public void MultipleSamples_SameFunction_AggregatesWeight() {
   public void MultipleSamples_DifferentFunctions_SeparateProfiles() {
     var resolver = new IpResolver();
     resolver.AddImage("test.dll", 0x1000, 0x10000);
-    resolver.SetFunctions("test.dll", [
+    resolver.SetFunctions(0x1000, [
       new FunctionDebugInfo("Foo", 0x100, 0x50),
       new FunctionDebugInfo("Bar", 0x200, 0x50),
       new FunctionDebugInfo("Baz", 0x300, 0x50)
@@ -72,6 +72,29 @@ public void MultipleSamples_DifferentFunctions_SeparateProfiles() {
     Assert.AreEqual(3, profiles.Count);
   }
 
+  [TestMethod]
+  public void SameFileName_DifferentBases_ResolveIndependently() {
+    // Two different binaries both named "foo.dll" at different bases, with DIFFERENT function layouts.
+    // Each base's samples must resolve to that binary's OWN function (symbols are keyed by base), not
+    // both resolve through whichever function list registered last for the shared name.
+    var resolver = new IpResolver();
+    resolver.AddImage("foo.dll", 0x10000, 0x1000);
+    resolver.AddImage("foo.dll", 0x20000, 0x1000);
+    resolver.SetFunctions(0x10000, [new FunctionDebugInfo("Alpha", 0x100, 0x50)]);
+    resolver.SetFunctions(0x20000, [new FunctionDebugInfo("Beta", 0x100, 0x50)]);
+
+    var aggregator = new SampleAggregator(resolver);
+    var w = TimeSpan.FromMilliseconds(1);
+    aggregator.AddSamples([
+      new SyntheticSample(0x10110, w, 1, 1, "foo.dll", 0x10000), // base 0x10000 -> Alpha
+      new SyntheticSample(0x20110, w, 1, 1, "foo.dll", 0x20000)  // base 0x20000 -> Beta
+    ]);
+
+    var names = aggregator.Build().Select(p => p.Key.FunctionName).OrderBy(n => n).ToArray();
+    CollectionAssert.AreEqual(new[] { "Alpha", "Beta" }, names,
+      "Each same-named binary's samples must resolve to its own function, not merge.");
+  }
+
   [TestMethod]
   public void InstructionWeights_AggregatesPerOffset() {
     var resolver = CreateResolverWithFunction("test.dll", 0x1000, 0x10000, "Foo", 0x100, 0x50);
@@ -198,7 +221,7 @@ public void UserImagelessLeaf_ResolvedCallersStillCreditedInclusive() {
     // receive inclusive weight (the leaf's `continue` used to drop the whole caller walk).
     var resolver = new IpResolver();
     resolver.AddImage("test.dll", 0x1000, 0x10000);
-    resolver.SetFunctions("test.dll", [new FunctionDebugInfo("Bar", 0x200, 0x50)]);
+    resolver.SetFunctions(0x1000, [new FunctionDebugInfo("Bar", 0x200, 0x50)]);
 
     var aggregator = new SampleAggregator(resolver);
     var weight = TimeSpan.FromMilliseconds(1);
@@ -227,7 +250,7 @@ public void KernelImagelessLeaf_ResolvedCallersStillCreditedInclusive() {
     // get inclusive weight. Only the leaf is dropped — not the whole stack.
     var resolver = new IpResolver();
     resolver.AddImage("test.dll", 0x1000, 0x10000);
-    resolver.SetFunctions("test.dll", [new FunctionDebugInfo("Bar", 0x200, 0x50)]);
+    resolver.SetFunctions(0x1000, [new FunctionDebugInfo("Bar", 0x200, 0x50)]);
 
     var aggregator = new SampleAggregator(resolver);
     var weight = TimeSpan.FromMilliseconds(1);
@@ -251,7 +274,7 @@ public void KernelImagelessLeaf_ResolvedCallersStillCreditedInclusive() {
   public void PercentCalculation_RelativeToTotalWeight() {
     var resolver = new IpResolver();
     resolver.AddImage("test.dll", 0x1000, 0x10000);
-    resolver.SetFunctions("test.dll", [
+    resolver.SetFunctions(0x1000, [
       new FunctionDebugInfo("Foo", 0x100, 0x50),
       new FunctionDebugInfo("Bar", 0x200, 0x50)
     ]);