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

[mono] Incorrect cast_class for IntPtr[] #97145

Closed
WhiteSmoogy opened this issue Jan 18, 2024 · 2 comments · Fixed by #103841
Closed

[mono] Incorrect cast_class for IntPtr[] #97145

WhiteSmoogy opened this issue Jan 18, 2024 · 2 comments · Fixed by #103841
Assignees
Labels
area-VM-meta-mono in-pr There is an active PR which will close this issue when it is merged
Milestone

Comments

@WhiteSmoogy
Copy link

WhiteSmoogy commented Jan 18, 2024

Description

mono repo has a intptr array Unit Test code:

public static int test_0_intptr_array_cast () {

This test case argues that an integer type array should be able to convert the pointer type array
It's produce difference value
.Net Framwork = 0
.Net Core = 5

It like there's something wrong with the .Net Framework

@dotnet-issue-labeler dotnet-issue-labeler bot added the needs-area-label An area label is needed to ensure this gets routed to the appropriate area owners label Jan 18, 2024
@ghost ghost added the untriaged New issue has not been triaged by the area owner label Jan 18, 2024
@WhiteSmoogy WhiteSmoogy changed the title test_0_intptr_array_cast output diff value on .NET Framwork and .NET Core test_0_intptr_array_cast are output different values on .NET Framwork and .NET Core Jan 18, 2024
@WhiteSmoogy WhiteSmoogy changed the title test_0_intptr_array_cast are output different values on .NET Framwork and .NET Core test_0_intptr_array_cast outputs different values on .NET Framwork and .NET Core Jan 18, 2024
@lambdageek lambdageek added area-VM-meta-mono question Answer questions and provide assistance, not an issue with source code or documentation. and removed needs-area-label An area label is needed to ensure this gets routed to the appropriate area owners labels Jan 18, 2024
@lambdageek lambdageek self-assigned this Jan 18, 2024
@jkotas jkotas changed the title test_0_intptr_array_cast outputs different values on .NET Framwork and .NET Core test_0_intptr_array_cast outputs different values on .NET Framework and .NET Core Jan 18, 2024
@lambdageek
Copy link
Member

lambdageek commented Jan 19, 2024

Looks like a bug in .NET 8 Mono.

Given this program:

    internal class Program
    {
        static void Main(string[] args)
        {
            Console.WriteLine($"Running on {System.Runtime.InteropServices.RuntimeInformation.FrameworkDescription}");
            Console.WriteLine($"IntPtr.Size = {IntPtr.Size}");
            object x0 = new long[1];
            object x1 = new ulong[1]; 
            Console.WriteLine($" x0 is IntPtr[] ? {x0 is IntPtr[]}");
            Console.WriteLine($" x1 is IntPtr[] ? {x1 is IntPtr[]}");
        }
    }

Here's output on various runtimes:

  1. .NET Framework 4.8 x64
  Running on .NET Framework 4.8.9181.0
  IntPtr.Size = 8
   x0 is IntPtr[] ? True
   x1 is IntPtr[] ? True
  1. .NET 8.0.1 CoreCLR arm64 or x64
    Running on .NET 8.0.1
    IntPtr.Size = 8
     x0 is IntPtr[] ? False
     x1 is IntPtr[] ? False
    
  2. .NET 8.0.1 Mono arm64
    Running on .NET 8.0.1
    IntPtr.Size = 8
     x0 is IntPtr[] ? True
     x1 is IntPtr[] ? True   
    
  3. Mono 6.12 arm64
    Running on Mono 6.12.0.188 (2020-02/ca8abcb6bc4 Thu Oct 13 14:26:22 EDT 2022)
    IntPtr.Size = 8
     x0 is IntPtr[] ? True
     x1 is IntPtr[] ? True
    

So modern Mono in .NET 8.0.1 is giving an incorrect answer. It should match CoreCLR 8.0.1

(Note: the tests in dotnet/runtime/src/mono/mono/mini/ don't run in CI, as far as I know).

@lambdageek
Copy link
Member

I think it's a problem in class_composite_fixup_cast_class:

static void
class_composite_fixup_cast_class (MonoClass *klass, gboolean for_ptr)
{
switch (m_class_get_byval_arg (m_class_get_cast_class (klass))->type) {
case MONO_TYPE_BOOLEAN:
if (!for_ptr)
break;
klass->cast_class = mono_defaults.byte_class;
break;
case MONO_TYPE_I1:
klass->cast_class = mono_defaults.byte_class;
break;
case MONO_TYPE_U2:
klass->cast_class = mono_defaults.int16_class;
break;
case MONO_TYPE_U4:
#if TARGET_SIZEOF_VOID_P == 4
case MONO_TYPE_I:
case MONO_TYPE_U:
#endif
klass->cast_class = mono_defaults.int32_class;
break;
case MONO_TYPE_U8:
#if TARGET_SIZEOF_VOID_P == 8
case MONO_TYPE_I:
case MONO_TYPE_U:
#endif
klass->cast_class = mono_defaults.int64_class;
break;
default:
break;
}
}

the code was introduced in mono/mono@2864540 and hasn't been updated for .NET 5+

it should probably set class->cast_cast = mono_defaults.int_class for MONO_TYPE_I and MONO_TYPE_U.
(although this code is kind of fragile, so there might be some cascading failures)

@lambdageek lambdageek changed the title test_0_intptr_array_cast outputs different values on .NET Framework and .NET Core [mono] Incorrect cast_class for IntPtr[] Jan 19, 2024
@lambdageek lambdageek removed question Answer questions and provide assistance, not an issue with source code or documentation. untriaged New issue has not been triaged by the area owner labels Jan 19, 2024
@lambdageek lambdageek added this to the 9.0.0 milestone Jan 19, 2024
steveisok added a commit to steveisok/runtime that referenced this issue Jun 21, 2024
This change fixes the behavior to be like modern .NET instead of .NET Framework.

Fixes dotnet#97145
@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 Jun 21, 2024
@github-actions github-actions bot locked and limited conversation to collaborators Jul 27, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-VM-meta-mono in-pr There is an active PR which will close this issue when it is merged
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants