Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[hot_reload] Fix unresolved token when new nested structs are used #76618

Merged
merged 2 commits into from
Oct 5, 2022

Conversation

lambdageek
Copy link
Member

When a hot reload delta contains a new nested struct that is used as a field in a newly-added class, the metadata delta adds the field of the enclosing class before it adds the fields of the nested struct. As a result, resolving the token of the field of the enclosing struct too early will make the runtime think that the nested struct has 0 fields and subsequent field lookups will fail.

Example:

Original Code:

public class C {
  public void ExistingMethod() { }

Delta code:

public class C {
  public void ExistingMethod() {
    new AddedClass();
  }
}

public class AddedClass {
  public NestedStruct Field1;
  public AddedClass() {
    Field1 = new NestedStruct { D = 1.0 };
  }
  public struct NestedStruct {
    public double D;
  }
}

Expected result:
Applying the hot reload delta succeeds

Actual result:

System.BadImageFormatException: Could not resolve field token 0x04000014

This fails with

```
       Stack Trace:

          Child exception:
            System.BadImageFormatException: Could not resolve field token 0x04000014
          File name: 'System.Reflection.Metadata.ApplyUpdate.Test.ReflectionAddNewType'
          /Users/alklig/work/dotnet-runtime/runtime/src/libraries/System.Runtime.Loader/tests/ApplyUpdate/System.Reflection.Metadata.ApplyUpdate.Test.ReflectionAddNewType/ReflectionAddNewType.cs(29,0):
          at
          System.Reflection.Metadata.ApplyUpdate.Test.ReflectionAddNewType.ZExistingClass.ExistingMethod()

```
@dotnet-issue-labeler dotnet-issue-labeler bot added the area-EnC-mono Hot Reload for WebAssembly, iOS/Android, etc label Oct 4, 2022
@ghost ghost assigned lambdageek Oct 4, 2022
@lambdageek
Copy link
Member Author

I'd like to backport this to .NET 7

@lambdageek
Copy link
Member Author

/backport to release/7.0

@github-actions
Copy link
Contributor

github-actions bot commented Oct 4, 2022

Started backporting to release/7.0: https://github.com/dotnet/runtime/actions/runs/3184163210

@lambdageek
Copy link
Member Author

/azp run runtime-wasm

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@lambdageek
Copy link
Member Author

Wasm debugger failure is #64188

@lambdageek lambdageek merged commit 246e6e0 into dotnet:main Oct 5, 2022
@ghost ghost locked as resolved and limited conversation to collaborators Nov 5, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-EnC-mono Hot Reload for WebAssembly, iOS/Android, etc
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants