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

Unsafe accessors in Reflector don't work #834

Open
KirillOsenkov opened this issue Nov 8, 2024 · 7 comments
Open

Unsafe accessors in Reflector don't work #834

KirillOsenkov opened this issue Nov 8, 2024 · 7 comments

Comments

@KirillOsenkov
Copy link
Owner

People are reporting that on net8.0 Reflector doesn't work, fails to access fields.

@KirillOsenkov
Copy link
Owner Author

I temporarily disabled the net8.0 branch here:
1a82195

Unfortunately I don't have time to investigate now because I need to unblock users.

@filipnavara if you perhaps have time to investigate this at some point. But it's not urgent, I mitigated it for now.

@filipnavara
Copy link
Contributor

I'll put it on my backlog. Not sure when I will have time to have a look.

It worked in the local builds of the MSBuild Log Viewer, both in NativeAOT and CoreCLR, on macOS and Windows. I'd probably need more info or test case to see what is broken.

@KirillOsenkov
Copy link
Owner Author

there's absolutely no rush. I've asked for a repro.

@brettfo
Copy link

brettfo commented Nov 9, 2024

@KirillOsenkov @filipnavara I have a repro, but it's a bit involved. I tried to create a console app that shows the same behavior, but I couldn't, however this repro survives git clean -dxf on my box.

  1. Install .NET SDK 9.0rc1. I think I was able to repro it with rc2 as well, but definitely with rc1.
  2. Clone this repo at commit 92663fc.
  3. Clone https://github.com/dependabot/dependabot-core at this branch.
    1. There are necessary submodules, so you'll need to run git submodule update --init --recursive
  4. From the dependabot-core repo, edit the file nuget/helpers/lib/NuGetUpdater/NuGetUpdater.Core/NuGetUpdater.Core.csproj and remove the <PackageReference> for MSBuild.StructuredLogger and instead add a <ProjectReference> to src/StructuredLogger/StructuredLogger.csproj in this repo.
  5. From the dependabot-core repo open nuget/helpers/lib/NuGetUpdater/NuGetUpdater.sln in VS
    1. You may need to add StructuredLogger.csproj to the solution.
  6. In VS, open the file StructuredLogger -> Reflector.cs and set a breakpoint on line 55.
  7. Open the file NuGetUpdater.Core.Test -> Discover/DiscoveryWorkerTests.Project.cs and debug the first unit test.
  8. When you hit the breakpoint in Reflector.cs step and see the exception about the missing field message, even though the debugger shows it.

@filipnavara
Copy link
Contributor

Thanks. The detailed instructions are much appreciated. I will report back soon.

@KirillOsenkov
Copy link
Owner Author

may need to checkout the binlog viewer repo to 1fd0386 to revert my change where I disabled the 8.0 ifdef (1a82195)

@filipnavara
Copy link
Contributor

I can reproduce it locally. The gist is that the UnsafeAccessor fails when the field is volatile:

using System;
using System.Runtime.CompilerServices;

var a = new A();
Console.WriteLine(GetSetFoo(a));

[UnsafeAccessor(UnsafeAccessorKind.Field, Name = "foo")]
extern static ref string GetSetFoo(A args);

class A { private volatile string? foo = "Bar"; }

I'll check with the runtime team if this is expected behavior or bug.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants