Skip to content

Commit

Permalink
Check variable type in ValidateNoReferencesToReferences instead of da…
Browse files Browse the repository at this point in the history
…taflow-tracked value and only validate on changes to locals (dotnet/linker#2983)

Commit migrated from dotnet/linker@8695133
  • Loading branch information
jtschuster authored Aug 23, 2022
1 parent 887f22a commit 647ae93
Show file tree
Hide file tree
Showing 5 changed files with 20 additions and 11 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,8 @@

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;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,8 @@

namespace ILLink.Shared.TrimAnalysis
{
public partial record LocalVariableReferenceValue (VariableDefinition LocalDefinition) : ReferenceValue
public partial record LocalVariableReferenceValue (VariableDefinition LocalDefinition)
: ReferenceValue (LocalDefinition.VariableType)
{
public override SingleValue DeepCopy ()
{
Expand Down
18 changes: 11 additions & 7 deletions src/tools/illink/src/linker/Linker.Dataflow/MethodBodyScanner.cs
Original file line number Diff line number Diff line change
Expand Up @@ -186,12 +186,12 @@ static void ValidateNoReferenceToReference (LocalVariableStore locals, MethodDef
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)));
}
}
}
Expand Down Expand Up @@ -289,7 +289,6 @@ protected virtual void Scan (MethodBody methodBody, ref InterproceduralState int

ReturnValue = new ();
foreach (Instruction operation in methodBody.Instructions) {
ValidateNoReferenceToReference (locals, methodBody.Method, operation.Offset);
int curBasicBlock = blockIterator.MoveNext (operation);

if (knownStacks.ContainsKey (operation.Offset)) {
Expand Down Expand Up @@ -415,6 +414,7 @@ protected virtual void Scan (MethodBody methodBody, ref InterproceduralState int
case Code.Ldloca:
case Code.Ldloca_S:
ScanLdloc (operation, currentStack, methodBody, locals);
ValidateNoReferenceToReference (locals, methodBody.Method, operation.Offset);
break;

case Code.Ldstr: {
Expand Down Expand Up @@ -559,6 +559,7 @@ protected virtual void Scan (MethodBody methodBody, ref InterproceduralState int
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:
Expand All @@ -578,6 +579,7 @@ protected virtual void Scan (MethodBody methodBody, ref InterproceduralState int
case Code.Stloc_2:
case Code.Stloc_3:
ScanStloc (operation, currentStack, methodBody, locals, curBasicBlock);
ValidateNoReferenceToReference (locals, methodBody.Method, operation.Offset);
break;

case Code.Constrained:
Expand Down Expand Up @@ -619,6 +621,7 @@ protected virtual void Scan (MethodBody methodBody, ref InterproceduralState int
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:
Expand Down Expand Up @@ -656,6 +659,7 @@ protected virtual void Scan (MethodBody methodBody, ref InterproceduralState int
// 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;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 ()
{
Expand Down
Original file line number Diff line number Diff line change
@@ -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 { }
}

0 comments on commit 647ae93

Please sign in to comment.