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

Invalid codegen with Unsafe.As on FullOpts when accessing a struct field through ulong->struct cast. #88950

Closed
neon-sunset opened this issue Jul 15, 2023 · 4 comments · Fixed by #88951
Assignees
Labels
area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI
Milestone

Comments

@neon-sunset
Copy link
Contributor

Description

Given:

struct Example
{
   object? Value;
   ulong Inner;

   struct ExampleInner { int Offset; int Length }
   
   public int Offset
   {
      get
      {
         var inner = Inner;
         return Unsafe.As<ulong, ExampleInner>(ref inner).Offset;
      }
   }
   
   public int Length
   {
      get
      {
         var inner = Inner;
         return Unsafe.As<ulong, ExampleInner>(ref inner).Length;
      }
   }
}

It appears that accessing ExampleInner fields stored as ulong produces invalid codegen on FullOpts but not on MinOpts or with Unsafe.BitCast: the produced assembly reads the Length field at the offset of 8 bytes instead of 12.

Reproduction Steps

  1. Create new console application
  2. Paste https://gist.github.com/neon-sunset/fed48339292c0cc1f153bf30ffb1ebca into Program.cs
  3. dotnet run -c release

Expected behavior

Output:

--- MinOpts
Offset: 1234
Length: 5678
--- As
Offset: 1234
Length: 5678
--- Bitcast
Offset: 1234
Length: 5678

Actual behavior

Output:

--- MinOpts
Offset: 1234
Length: 5678
--- As
Offset: 1234
Length: 1234 <--- Unexpected
--- Bitcast
Offset: 1234
Length: 5678

Regression?

Yes, does not reproduce on .NET 7 and .NET 6.

Known Workarounds

None, bitcast produces worse codegen.

Configuration

Reproduced on:

.NET SDK:
 Version:   8.0.100-preview.7.23364.31
 Commit:    378e8abf4f

Runtime Environment:
 OS Name:     Mac OS X
 OS Version:  14.0
 OS Platform: Darwin
 RID:         osx-arm64
 Base Path:   /usr/local/share/dotnet/sdk/8.0.100-preview.7.23364.31/
.NET SDK:
 Version:   8.0.100-preview.7.23363.19
 Commit:    bef7f10d1d

Runtime Environment:
 OS Name:     Windows
 OS Version:  10.0.22621
 OS Platform: Windows
 RID:         win10-x64
 Base Path:   C:\Program Files\dotnet\sdk\8.0.100-preview.7.23363.19\

Other information

No response

@dotnet-issue-labeler dotnet-issue-labeler bot added the area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI label Jul 15, 2023
@ghost ghost added the untriaged New issue has not been triaged by the area owner label Jul 15, 2023
@ghost
Copy link

ghost commented Jul 15, 2023

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

Issue Details

Description

Given:

struct Example
{
   object? Value;
   ulong Inner;

   struct ExampleInner { int Offset; int Length }
   
   public int Offset
   {
      get
      {
         var inner = Inner;
         return Unsafe.As<ulong, ExampleInner>(ref inner).Offset;
      }
   }
   
   public int Length
   {
      get
      {
         var inner = Inner;
         return Unsafe.As<ulong, ExampleInner>(ref inner).Length;
      }
   }
}

It appears that accessing ExampleInner fields stored as ulong produces invalid codegen on FullOpts but not on MinOpts or with Unsafe.BitCast: the produced assembly reads the Length field at the offset of 8 bytes instead of 12.

Reproduction Steps

  1. Create new console application
  2. Paste https://gist.github.com/neon-sunset/fed48339292c0cc1f153bf30ffb1ebca into Program.cs
  3. dotnet run -c release

Expected behavior

Output:

--- MinOpts
Offset: 1234
Length: 5678
--- As
Offset: 1234
Length: 5678
--- Bitcast
Offset: 1234
Length: 5678

Actual behavior

Output:

--- MinOpts
Offset: 1234
Length: 5678
--- As
Offset: 1234
Length: 1234 <--- Unexpected
--- Bitcast
Offset: 1234
Length: 5678

Regression?

Yes, does not reproduce on .NET 7 and .NET 6.

Known Workarounds

None, bitcast produces worse codegen.

Configuration

Reproduced on:

.NET SDK:
 Version:   8.0.100-preview.7.23364.31
 Commit:    378e8abf4f

Runtime Environment:
 OS Name:     Mac OS X
 OS Version:  14.0
 OS Platform: Darwin
 RID:         osx-arm64
 Base Path:   /usr/local/share/dotnet/sdk/8.0.100-preview.7.23364.31/
.NET SDK:
 Version:   8.0.100-preview.7.23363.19
 Commit:    bef7f10d1d

Runtime Environment:
 OS Name:     Windows
 OS Version:  10.0.22621
 OS Platform: Windows
 RID:         win10-x64
 Base Path:   C:\Program Files\dotnet\sdk\8.0.100-preview.7.23363.19\

Other information

No response

Author: neon-sunset
Assignees: -
Labels:

area-CodeGen-coreclr

Milestone: -

@neon-sunset neon-sunset changed the title Invalid codegen with Unsafe.As on FullOpts when accessing a struct field Invalid codegen with Unsafe.As on FullOpts when accessing a struct field through ulong->struct cast. Jul 15, 2023
@EgorBo
Copy link
Member

EgorBo commented Jul 15, 2023

hm.. interesting, sounds like FieldSeq is messed up in FIELDADDR

@EgorBo
Copy link
Member

EgorBo commented Jul 15, 2023

The bug seems to be in SelectLocalIndirTransform

@EgorBo EgorBo self-assigned this Jul 15, 2023
@EgorBo EgorBo added this to the 8.0.0 milestone Jul 15, 2023
@EgorBo EgorBo removed the untriaged New issue has not been triaged by the area owner label Jul 15, 2023
@ghost ghost added the in-pr There is an active PR which will close this issue when it is merged label Jul 15, 2023
@MichalPetryka
Copy link
Contributor

None, bitcast produces worse codegen.

That should be solved with #85562.

@ghost ghost removed the in-pr There is an active PR which will close this issue when it is merged label Jul 16, 2023
@ghost ghost locked as resolved and limited conversation to collaborators Aug 15, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants