Check variable type in ValidateNoReferencesToReferences instead of dataflow-tracked value and only validate on changes to locals (#2983)

5 files changed, 20 insertions, 11 deletions

diff --git a/src/linker/Linker.Dataflow/FieldReferenceValue.cs b/src/linker/Linker.Dataflow/FieldReferenceValue.cs
index de0f23541..37f7771c3 100644
--- a/src/linker/Linker.Dataflow/FieldReferenceValue.cs
+++ b/src/linker/Linker.Dataflow/FieldReferenceValue.cs
@@ -5,7 +5,8 @@ using Mono.Cecil;
 
 namespace ILLink.Shared.TrimAnalysis
 {
-	public partial record FieldReferenceValue (FieldDefinition FieldDefinition) : ReferenceValue
+	public partial record FieldReferenceValue (FieldDefinition FieldDefinition)
+		: ReferenceValue (FieldDefinition.FieldType)
 	{
 		public override SingleValue DeepCopy () => this;
 	}
diff --git a/src/linker/Linker.Dataflow/LocalVariableReferenceValue.cs b/src/linker/Linker.Dataflow/LocalVariableReferenceValue.cs
index 92365210a..2c405beb1 100644
--- a/src/linker/Linker.Dataflow/LocalVariableReferenceValue.cs
+++ b/src/linker/Linker.Dataflow/LocalVariableReferenceValue.cs
@@ -5,7 +5,8 @@ using Mono.Cecil.Cil;
 
 namespace ILLink.Shared.TrimAnalysis
 {
-	public partial record LocalVariableReferenceValue (VariableDefinition LocalDefinition) : ReferenceValue
+	public partial record LocalVariableReferenceValue (VariableDefinition LocalDefinition)
+		: ReferenceValue (LocalDefinition.VariableType)
 	{
 		public override SingleValue DeepCopy ()
 		{
diff --git a/src/linker/Linker.Dataflow/MethodBodyScanner.cs b/src/linker/Linker.Dataflow/MethodBodyScanner.cs
index 3233b917c..613d5e9ad 100644
--- a/src/linker/Linker.Dataflow/MethodBodyScanner.cs
+++ b/src/linker/Linker.Dataflow/MethodBodyScanner.cs
@@ -186,12 +186,12 @@ namespace Mono.Linker.Dataflow
 				MultiValue localValue = keyValuePair.Value.Value;
 				VariableDefinition localVariable = keyValuePair.Key;
 				foreach (var val in localValue) {
-					if (val is LocalVariableReferenceValue reference
-						&& locals[reference.LocalDefinition].Value.Any (v => v is ReferenceValue)) {
-						throw new LinkerFatalErrorException (MessageContainer.CreateCustomErrorMessage (
-								$"In method {method.FullName}, local variable {localVariable.Index} references variable {reference.LocalDefinition.Index} which is a reference.",
-								(int) DiagnosticId.LinkerUnexpectedError,
-								origin: new MessageOrigin (method, ilOffset)));
+					if (val is LocalVariableReferenceValue localReference && localReference.ReferencedType.IsByReference) {
+						string displayName = $"local variable V_{localReference.LocalDefinition.Index}";
+						throw new LinkerFatalErrorException (MessageContainer.CreateErrorMessage (
+							$"""In method {method.FullName}, local variable V_{localVariable.Index} references {displayName} of type {localReference.ReferencedType.GetDisplayName ()} which is a reference. Linker dataflow tracking has failed.""",
+							(int) DiagnosticId.LinkerUnexpectedError,
+							origin: new MessageOrigin (method, ilOffset)));
 					}
 				}
 			}
@@ -289,7 +289,6 @@ namespace Mono.Linker.Dataflow
 
 			ReturnValue = new ();
 			foreach (Instruction operation in methodBody.Instructions) {
-				ValidateNoReferenceToReference (locals, methodBody.Method, operation.Offset);
 				int curBasicBlock = blockIterator.MoveNext (operation);
 
 				if (knownStacks.ContainsKey (operation.Offset)) {
@@ -415,6 +414,7 @@ namespace Mono.Linker.Dataflow
 				case Code.Ldloca:
 				case Code.Ldloca_S:
 					ScanLdloc (operation, currentStack, methodBody, locals);
+					ValidateNoReferenceToReference (locals, methodBody.Method, operation.Offset);
 					break;
 
 				case Code.Ldstr: {
@@ -559,6 +559,7 @@ namespace Mono.Linker.Dataflow
 				case Code.Stind_Ref:
 				case Code.Stobj:
 					ScanIndirectStore (operation, currentStack, methodBody, locals, curBasicBlock, ref interproceduralState);
+					ValidateNoReferenceToReference (locals, methodBody.Method, operation.Offset);
 					break;
 
 				case Code.Initobj:
@@ -578,6 +579,7 @@ namespace Mono.Linker.Dataflow
 				case Code.Stloc_2:
 				case Code.Stloc_3:
 					ScanStloc (operation, currentStack, methodBody, locals, curBasicBlock);
+					ValidateNoReferenceToReference (locals, methodBody.Method, operation.Offset);
 					break;
 
 				case Code.Constrained:
@@ -619,6 +621,7 @@ namespace Mono.Linker.Dataflow
 				case Code.Newobj:
 					TrackNestedFunctionReference ((MethodReference) operation.Operand, ref interproceduralState);
 					HandleCall (methodBody, operation, currentStack, locals, ref interproceduralState, curBasicBlock);
+					ValidateNoReferenceToReference (locals, methodBody.Method, operation.Offset);
 					break;
 
 				case Code.Jmp:
@@ -656,6 +659,7 @@ namespace Mono.Linker.Dataflow
 							// If the return value is a reference, treat it as the value itself for now
 							//	We can handle ref return values better later
 							ReturnValue = MultiValueLattice.Meet (ReturnValue, DereferenceValue (retValue.Value, locals, ref interproceduralState));
+							ValidateNoReferenceToReference (locals, methodBody.Method, operation.Offset);
 						}
 						ClearStack (ref currentStack);
 						break;
diff --git a/src/linker/Linker.Dataflow/ParameterReferenceValue.cs b/src/linker/Linker.Dataflow/ParameterReferenceValue.cs
index 7f3804791..79bdfdeab 100644
--- a/src/linker/Linker.Dataflow/ParameterReferenceValue.cs
+++ b/src/linker/Linker.Dataflow/ParameterReferenceValue.cs
@@ -2,11 +2,13 @@
 // Licensed under the MIT license. See LICENSE file in the project root for full license information.
 using ILLink.Shared.DataFlow;
 using Mono.Cecil;
+using Mono.Linker;
 
 namespace ILLink.Shared.TrimAnalysis
 {
 	public partial record ParameterReferenceValue (MethodDefinition MethodDefinition, int ParameterIndex)
-: ReferenceValue
+		: ReferenceValue (MethodDefinition.HasImplicitThis () && ParameterIndex == 0 ? MethodDefinition.DeclaringType
+			: MethodDefinition.Parameters[MethodDefinition.HasImplicitThis () ? --ParameterIndex : ParameterIndex].ParameterType)
 	{
 		public override SingleValue DeepCopy ()
 		{
diff --git a/src/linker/Linker.Dataflow/ReferenceValue.cs b/src/linker/Linker.Dataflow/ReferenceValue.cs
index 4e43a95cd..2ea8d379b 100644
--- a/src/linker/Linker.Dataflow/ReferenceValue.cs
+++ b/src/linker/Linker.Dataflow/ReferenceValue.cs
@@ -1,11 +1,12 @@
 // Copyright (c) .NET Foundation and contributors. All rights reserved.
 // Licensed under the MIT license. See LICENSE file in the project root for full license information.
 using ILLink.Shared.DataFlow;
+using Mono.Cecil;
 
 namespace ILLink.Shared.TrimAnalysis
 {
 	/// <summary>
 	/// Acts as the base class for all values that represent a reference to another value. These should only be held in a ref type or on the stack as a result of a 'load address' instruction (e.g. ldloca).
 	/// </summary>
-	public abstract record ReferenceValue : SingleValue { }
+	public abstract record ReferenceValue (TypeReference ReferencedType) : SingleValue { }
 }