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

[NativeAOT] DiagnosticMethodInfo.Create(Delegate) does not work with closed delegates over instance methods on value types #108688

Closed
SingleAccretion opened this issue Oct 8, 2024 · 1 comment · Fixed by #110782
Labels
area-NativeAOT-coreclr bug in-pr There is an active PR which will close this issue when it is merged
Milestone

Comments

@SingleAccretion
Copy link
Contributor

SingleAccretion commented Oct 8, 2024

Reproduction (compile, publish, run):

using System;
using System.Diagnostics;

class Program
{
    static void Main()
    {
        DelStruct s;
        ClosedDel del = s.InstanceMethod;
        var dmi = DiagnosticMethodInfo.Create(del);
        Console.WriteLine(dmi is null);
    }

    delegate void ClosedDel();

    struct DelStruct
    {
        public void InstanceMethod() { }
    }
}

Expected result:

False

Actual result:

True

The problem is that code here may pass an unboxing thunk address to the stack trace logic, which the latter doesn't understand - stack trace data records only unboxed entrypoints.

It probably makes sense to normalize the entrypoints on this path to their unboxed form (the reflection-based lookup will understand both).

@dotnet-policy-service dotnet-policy-service bot added the untriaged New issue has not been triaged by the area owner label Oct 8, 2024
Copy link
Contributor

Tagging subscribers to this area: @agocke, @MichalStrehovsky, @jkotas
See info in area-owners.md if you want to be subscribed.

@MichalStrehovsky MichalStrehovsky added this to the 10.0.0 milestone Oct 9, 2024
@MichalStrehovsky MichalStrehovsky removed the untriaged New issue has not been triaged by the area owner label Oct 9, 2024
MichalStrehovsky added a commit to MichalStrehovsky/runtime that referenced this issue Dec 17, 2024
Fixes dotnet#108688.

We actually have test coverage for this here:

https://github.com/dotnet/runtime/blob/ce9dd2ad46a4842f5cf0f03c4a30b4d29bd0b4cc/src/libraries/System.Diagnostics.StackTrace/tests/DiagnosticMethodInfoTests.cs#L137-L147

But hitting the bug requires not considering the method to be target of reflection. We root libraries tests due to xUnit. Reflection can deal with both boxed and unboxed entrypoints, stack trace metadata can't.
@dotnet-policy-service dotnet-policy-service bot added the in-pr There is an active PR which will close this issue when it is merged label Dec 17, 2024
@jkotas jkotas closed this as completed in 78ede32 Dec 17, 2024
@github-actions github-actions bot locked and limited conversation to collaborators Jan 17, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-NativeAOT-coreclr bug in-pr There is an active PR which will close this issue when it is merged
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

2 participants