Consider MVID when testing EC cache validity

Our EvaluationContext cache invalidation computation was consuming the
method token and version, but not the identity of the declaring assembly.
This lead to a strange bug where we would reuse the cache across assembly
boundaries if consecutive breakpoint happened to have the same method
token (each in its own assembly).

I believe this bug existed before I revised the cache invalidation
computation and we were just getting lucky - we used to check the spans
of all containing scopes, which are unlikely to match exactly across
assemblies.

src/ExpressionEvaluator/CSharp/Source/ExpressionCompiler/EvaluationContext.cs

@@ -129,7 +129,7 @@ internal sealed class EvaluationContext : EvaluationContextBase
                 var previousContext = previous.EvaluationContext;
                 if (previousContext != null &&
                     previousContext.MethodContextReuseConstraints.HasValue &&
-                    previousContext.MethodContextReuseConstraints.GetValueOrDefault().AreSatisfied(methodToken, methodVersion, ilOffset))
+                    previousContext.MethodContextReuseConstraints.GetValueOrDefault().AreSatisfied(moduleVersionId, methodToken, methodVersion, ilOffset))
                 {
                     return previousContext;
                 }
@@ -144,7 +144,7 @@ internal sealed class EvaluationContext : EvaluationContextBase
             var allScopes = ArrayBuilder<ISymUnmanagedScope>.GetInstance();
             var containingScopes = ArrayBuilder<ISymUnmanagedScope>.GetInstance();
             typedSymReader.GetScopes(methodToken, methodVersion, ilOffset, IsLocalScopeEndInclusive, allScopes, containingScopes);
-            var methodContextReuseConstraints = allScopes.GetReuseConstraints(methodToken, methodVersion, ilOffset, IsLocalScopeEndInclusive);
+            var methodContextReuseConstraints = allScopes.GetReuseConstraints(moduleVersionId, methodToken, methodVersion, ilOffset, IsLocalScopeEndInclusive);
             allScopes.Free();
 
             var localNames = containingScopes.GetLocalNames();

src/ExpressionEvaluator/CSharp/Test/ExpressionCompiler/ExpressionCompilerTests.cs

@@ -374,7 +374,7 @@ static void G()
                 var constraints = previous.EvaluationContext.MethodContextReuseConstraints;
                 if (constraints.HasValue)
                 {
-                    Assert.Equal(scope == previousScope, constraints.GetValueOrDefault().AreSatisfied(methodToken, methodVersion, offset));
+                    Assert.Equal(scope == previousScope, constraints.GetValueOrDefault().AreSatisfied(moduleVersionId, methodToken, methodVersion, offset));
                 }
 
                 context = EvaluationContext.CreateMethodContext(previous, methodBlocks, symReader, moduleVersionId, methodToken, methodVersion, offset, localSignatureToken);
@@ -405,7 +405,7 @@ static void G()
             // Different references. No reuse.
             context = EvaluationContext.CreateMethodContext(previous, methodBlocks, symReader, moduleVersionId, methodToken, methodVersion, endOffset, localSignatureToken);
             Assert.NotEqual(context, previous.EvaluationContext);
-            Assert.True(previous.EvaluationContext.MethodContextReuseConstraints.Value.AreSatisfied(methodToken, methodVersion, endOffset));
+            Assert.True(previous.EvaluationContext.MethodContextReuseConstraints.Value.AreSatisfied(moduleVersionId, methodToken, methodVersion, endOffset));
             Assert.NotEqual(context.Compilation, previous.Compilation);
             previous = new CSharpMetadataContext(context);
 
@@ -413,7 +413,7 @@ static void G()
             GetContextState(runtime, "C.G", out methodBlocks, out moduleVersionId, out symReader, out methodToken, out localSignatureToken);
             context = EvaluationContext.CreateMethodContext(previous, methodBlocks, symReader, moduleVersionId, methodToken, methodVersion, ilOffset: 0, localSignatureToken: localSignatureToken);
             Assert.NotEqual(context, previous.EvaluationContext);
-            Assert.False(previous.EvaluationContext.MethodContextReuseConstraints.Value.AreSatisfied(methodToken, methodVersion, 0));
+            Assert.False(previous.EvaluationContext.MethodContextReuseConstraints.Value.AreSatisfied(moduleVersionId, methodToken, methodVersion, 0));
             Assert.Equal(context.Compilation, previous.Compilation);
 
             // No EvaluationContext. Should reuse Compilation

src/ExpressionEvaluator/CSharp/Test/ExpressionCompiler/MethodContextReuseConstraintsTests.cs

 // Copyright (c) Microsoft.  All Rights Reserved.  Licensed under the Apache License, Version 2.0.  See License.txt in the project root for license information.
 
+using System;
 using Microsoft.CodeAnalysis.ExpressionEvaluator;
 using Xunit;
 
@@ -10,33 +11,37 @@ public class MethodContextReuseConstraintsTests : ExpressionCompilerTestBase
         [Fact]
         public void AreSatisfied()
         {
+            var moduleVersionId = Guid.NewGuid();
             const int methodToken = 0x06000001;
             const int methodVersion = 1;
             const uint startOffset = 1;
             const uint endOffsetExclusive = 3;
 
             var constraints = MethodContextReuseConstraints.CreateTestInstance(
+                moduleVersionId,
                 methodToken, 
                 methodVersion, 
                 startOffset, 
                 endOffsetExclusive);
 
-            Assert.True(constraints.AreSatisfied(methodToken, methodVersion, (int)startOffset));
-            Assert.True(constraints.AreSatisfied(methodToken, methodVersion, (int)endOffsetExclusive - 1));
+            Assert.True(constraints.AreSatisfied(moduleVersionId, methodToken, methodVersion, (int)startOffset));
+            Assert.True(constraints.AreSatisfied(moduleVersionId, methodToken, methodVersion, (int)endOffsetExclusive - 1));
 
-            Assert.False(constraints.AreSatisfied(methodToken + 1, methodVersion, (int)startOffset));
-            Assert.False(constraints.AreSatisfied(methodToken, methodVersion + 1, (int)startOffset));
-            Assert.False(constraints.AreSatisfied(methodToken, methodVersion, (int)startOffset - 1));
-            Assert.False(constraints.AreSatisfied(methodToken, methodVersion, (int)endOffsetExclusive));
+            Assert.False(constraints.AreSatisfied(Guid.NewGuid(), methodToken, methodVersion, (int)startOffset));
+            Assert.False(constraints.AreSatisfied(moduleVersionId, methodToken + 1, methodVersion, (int)startOffset));
+            Assert.False(constraints.AreSatisfied(moduleVersionId, methodToken, methodVersion + 1, (int)startOffset));
+            Assert.False(constraints.AreSatisfied(moduleVersionId, methodToken, methodVersion, (int)startOffset - 1));
+            Assert.False(constraints.AreSatisfied(moduleVersionId, methodToken, methodVersion, (int)endOffsetExclusive));
         }
 
         [Fact]
         public void EndInclusive()
         {
+            var moduleVersionId = Guid.NewGuid();
             const int methodToken = 0x06000001;
             const int methodVersion = 1;
 
-            var builder = new MethodContextReuseConstraints.Builder(methodToken, methodVersion, ilOffset: 5, areRangesEndInclusive: true);
+            var builder = new MethodContextReuseConstraints.Builder(moduleVersionId, methodToken, methodVersion, ilOffset: 5, areRangesEndInclusive: true);
             Assert.True(builder.Build().HasExpectedSpan(0u, uint.MaxValue));
 
             builder.AddRange(1, 9);
@@ -55,10 +60,11 @@ public void EndInclusive()
         [Fact]
         public void EndExclusive()
         {
+            var moduleVersionId = Guid.NewGuid();
             const int methodToken = 0x06000001;
             const int methodVersion = 1;
 
-            var builder = new MethodContextReuseConstraints.Builder(methodToken, methodVersion, ilOffset: 5, areRangesEndInclusive: false);
+            var builder = new MethodContextReuseConstraints.Builder(moduleVersionId, methodToken, methodVersion, ilOffset: 5, areRangesEndInclusive: false);
             Assert.True(builder.Build().HasExpectedSpan(0u, uint.MaxValue));
 
             builder.AddRange(1, 9);
@@ -77,10 +83,11 @@ public void EndExclusive()
         [Fact]
         public void Cumulative()
         {
+            var moduleVersionId = Guid.NewGuid();
             const int methodToken = 0x06000001;
             const int methodVersion = 1;
 
-            var builder = new MethodContextReuseConstraints.Builder(methodToken, methodVersion, ilOffset: 5, areRangesEndInclusive: false);
+            var builder = new MethodContextReuseConstraints.Builder(moduleVersionId, methodToken, methodVersion, ilOffset: 5, areRangesEndInclusive: false);
             Assert.True(builder.Build().HasExpectedSpan(0u, uint.MaxValue));
 
             builder.AddRange(1, 10);

src/ExpressionEvaluator/Core/Source/ExpressionCompiler/MetadataUtilities.cs

@@ -296,9 +296,9 @@ internal static bool IsWindowsAssemblyIdentity(this AssemblyIdentity assemblyIde
             symMethod.GetAllScopes(allScopes, containingScopes, ilOffset, isScopeEndInclusive);
         }
 
-        internal static MethodContextReuseConstraints GetReuseConstraints(this ArrayBuilder<ISymUnmanagedScope> scopes, int methodToken, int methodVersion, int ilOffset, bool isEndInclusive)
+        internal static MethodContextReuseConstraints GetReuseConstraints(this ArrayBuilder<ISymUnmanagedScope> scopes, Guid moduleVersionId, int methodToken, int methodVersion, int ilOffset, bool isEndInclusive)
         {
-            var builder = new MethodContextReuseConstraints.Builder(methodToken, methodVersion, ilOffset, isEndInclusive);
+            var builder = new MethodContextReuseConstraints.Builder(moduleVersionId, methodToken, methodVersion, ilOffset, isEndInclusive);
             foreach (ISymUnmanagedScope scope in scopes)
             {
                 builder.AddRange((uint)scope.GetStartOffset(), (uint)scope.GetEndOffset());

src/ExpressionEvaluator/Core/Source/ExpressionCompiler/MethodContextReuseConstraints.cs

@@ -9,34 +9,38 @@ namespace Microsoft.CodeAnalysis.ExpressionEvaluator
 {
     internal struct MethodContextReuseConstraints
     {
+        private readonly Guid _moduleVersionId;
         private readonly int _methodToken;
         private readonly int _methodVersion;
         private readonly uint _startOffset;
         private readonly uint _endOffsetExclusive;
 
-        private MethodContextReuseConstraints(int methodToken, int methodVersion, uint startOffset, uint endOffsetExclusive)
+        private MethodContextReuseConstraints(Guid moduleVersionId, int methodToken, int methodVersion, uint startOffset, uint endOffsetExclusive)
         {
+            Debug.Assert(moduleVersionId != default(Guid));
             Debug.Assert(MetadataTokens.Handle(methodToken).Kind == HandleKind.MethodDefinition);
             Debug.Assert(methodVersion >= 1);
             Debug.Assert(startOffset <= endOffsetExclusive);
 
+            _moduleVersionId = moduleVersionId;
             _methodToken = methodToken;
             _methodVersion = methodVersion;
             _startOffset = startOffset;
             _endOffsetExclusive = endOffsetExclusive;
         }
 
-        public bool AreSatisfied(int methodToken, int methodVersion, int ilOffset)
+        public bool AreSatisfied(Guid moduleVersionId, int methodToken, int methodVersion, int ilOffset)
         {
-            return methodToken == _methodToken &&
+            return moduleVersionId == _moduleVersionId &&
+                methodToken == _methodToken &&
                 methodVersion == _methodVersion &&
                 ilOffset >= _startOffset &&
                 ilOffset < _endOffsetExclusive;
         }
 
-        internal static MethodContextReuseConstraints CreateTestInstance(int methodToken, int methodVersion, uint startOffset, uint endOffsetExclusive)
+        internal static MethodContextReuseConstraints CreateTestInstance(Guid moduleVersionId, int methodToken, int methodVersion, uint startOffset, uint endOffsetExclusive)
         {
-            return new MethodContextReuseConstraints(methodToken, methodVersion, startOffset, endOffsetExclusive);
+            return new MethodContextReuseConstraints(moduleVersionId, methodToken, methodVersion, startOffset, endOffsetExclusive);
         }
 
         internal bool HasExpectedSpan(uint startOffset, uint endOffsetExclusive)
@@ -46,11 +50,12 @@ internal bool HasExpectedSpan(uint startOffset, uint endOffsetExclusive)
 
         public override string ToString()
         {
-            return $"0x{_methodToken:x8}v{_methodVersion} [{_startOffset}, {_endOffsetExclusive})";
+            return $"0x{_methodToken:x8}v{_methodVersion} from {_moduleVersionId} [{_startOffset}, {_endOffsetExclusive})";
         }
 
         public class Builder
         {
+            private readonly Guid _moduleVersionId;
             private readonly int _methodToken;
             private readonly int _methodVersion;
             private readonly int _ilOffset;
@@ -59,12 +64,14 @@ public class Builder
             private uint _startOffset;
             private uint _endOffsetExclusive;
 
-            public Builder(int methodToken, int methodVersion, int ilOffset, bool areRangesEndInclusive)
+            public Builder(Guid moduleVersionId, int methodToken, int methodVersion, int ilOffset, bool areRangesEndInclusive)
             {
+                Debug.Assert(moduleVersionId != default(Guid));
                 Debug.Assert(MetadataTokens.Handle(methodToken).Kind == HandleKind.MethodDefinition);
                 Debug.Assert(methodVersion >= 1);
                 Debug.Assert(ilOffset >= 0);
 
+                _moduleVersionId = moduleVersionId;
                 _methodToken = methodToken;
                 _methodVersion = methodVersion;
                 _ilOffset = ilOffset;
@@ -76,6 +83,7 @@ public Builder(int methodToken, int methodVersion, int ilOffset, bool areRangesE
 
             public Builder(MethodContextReuseConstraints existingConstraints, int ilOffset, bool areRangesEndInclusive)
             {
+                _moduleVersionId = existingConstraints._moduleVersionId;
                 _methodToken = existingConstraints._methodToken;
                 _methodVersion = existingConstraints._methodVersion;
                 _ilOffset = ilOffset;
@@ -111,6 +119,7 @@ public void AddRange(uint startOffset, uint endOffset)
             public MethodContextReuseConstraints Build()
             {
                 return new MethodContextReuseConstraints(
+                    _moduleVersionId,
                     _methodToken,
                     _methodVersion,
                     _startOffset,

src/ExpressionEvaluator/VisualBasic/Source/ExpressionCompiler/EvaluationContext.vb

@@ -130,7 +130,7 @@ Namespace Microsoft.CodeAnalysis.VisualBasic.ExpressionEvaluator
                 Dim previousContext = previous.EvaluationContext
                 If previousContext IsNot Nothing AndAlso
                     previousContext.MethodContextReuseConstraints.HasValue AndAlso
-                    previousContext.MethodContextReuseConstraints.GetValueOrDefault().AreSatisfied(methodToken, methodVersion, ilOffset) Then
+                    previousContext.MethodContextReuseConstraints.GetValueOrDefault().AreSatisfied(moduleVersionId, methodToken, methodVersion, ilOffset) Then
                     Return previousContext
                 End If
                 compilation = previous.Compilation
@@ -142,7 +142,7 @@ Namespace Microsoft.CodeAnalysis.VisualBasic.ExpressionEvaluator
             Dim allScopes = ArrayBuilder(Of ISymUnmanagedScope).GetInstance()
             Dim containingScopes = ArrayBuilder(Of ISymUnmanagedScope).GetInstance()
             typedSymReader.GetScopes(methodToken, methodVersion, ilOffset, IsLocalScopeEndInclusive, allScopes, containingScopes)
-            Dim reuseConstraints = allScopes.GetReuseConstraints(methodToken, methodVersion, ilOffset, IsLocalScopeEndInclusive)
+            Dim reuseConstraints = allScopes.GetReuseConstraints(moduleVersionId, methodToken, methodVersion, ilOffset, IsLocalScopeEndInclusive)
             allScopes.Free()
 
             Dim methodHandle = CType(MetadataTokens.Handle(methodToken), MethodDefinitionHandle)

src/ExpressionEvaluator/VisualBasic/Test/ExpressionCompiler/ExpressionCompilerTests.vb

@@ -275,7 +275,7 @@ End Class"
                 Dim scope = scopes.GetInnermostScope(offset)
                 Dim constraints = previous.EvaluationContext.MethodContextReuseConstraints
                 If constraints.HasValue Then
-                    Assert.Equal(scope Is previousScope, constraints.GetValueOrDefault().AreSatisfied(methodToken, methodVersion, offset))
+                    Assert.Equal(scope Is previousScope, constraints.GetValueOrDefault().AreSatisfied(moduleVersionId, methodToken, methodVersion, offset))
                 End If
 
                 context = EvaluationContext.CreateMethodContext(previous, methodBlocks, MakeDummyLazyAssemblyReaders(), symReader, moduleVersionId, methodToken, methodVersion, offset, localSignatureToken)
@@ -307,7 +307,7 @@ End Class"
             ' Different references. No reuse.
             context = EvaluationContext.CreateMethodContext(previous, methodBlocks, MakeDummyLazyAssemblyReaders(), symReader, moduleVersionId, methodToken, methodVersion, endOffset - 1, localSignatureToken)
             Assert.NotEqual(context, previous.EvaluationContext)
-            Assert.True(previous.EvaluationContext.MethodContextReuseConstraints.Value.AreSatisfied(methodToken, methodVersion, endOffset - 1))
+            Assert.True(previous.EvaluationContext.MethodContextReuseConstraints.Value.AreSatisfied(moduleVersionId, methodToken, methodVersion, endOffset - 1))
             Assert.NotEqual(context.Compilation, previous.Compilation)
             previous = new VisualBasicMetadataContext(context)
 
@@ -315,7 +315,7 @@ End Class"
             GetContextState(runtime, "C.G", methodBlocks, moduleVersionId, symReader, methodToken, localSignatureToken)
             context = EvaluationContext.CreateMethodContext(previous, methodBlocks, MakeDummyLazyAssemblyReaders(), symReader, moduleVersionId, methodToken, methodVersion, ilOffset:=0, localSignatureToken:=localSignatureToken)
             Assert.NotEqual(context, previous.EvaluationContext)
-            Assert.False(previous.EvaluationContext.MethodContextReuseConstraints.Value.AreSatisfied(methodToken, methodVersion, 0))
+            Assert.False(previous.EvaluationContext.MethodContextReuseConstraints.Value.AreSatisfied(moduleVersionId, methodToken, methodVersion, 0))
             Assert.Equal(context.Compilation, previous.Compilation)
 
             ' No EvaluationContext. Should reuse Compilation