LibraryImportGenerator tests failing on big-endian target (ref pointer type mismatch)

[uweigand]

On s390x, the LibraryImportGenerator test cases have been disabled a while back: https://github.com/dotnet/runtime/pull/60833/files
I've just tried running them again to check current status, and it looks like the original issue has been resolved, so the tests actually build and execute now. However, there are a number of test failures, all related to functions using a ref pointer to retrieve an output. One typical instance is:

    LibraryImportGenerator.IntegrationTests.BooleanTests.ValidateByteBoolReturns(value: 1, expected: True) [FAIL]
      Assert.Equal() Failure
      Expected: True
      Actual:   False
      Stack Trace:
        /home/uweigand/runtime/src/libraries/System.Runtime.InteropServices/tests/LibraryImportGenerator.Tests/BooleanTests.cs(108,0): at LibraryImportGenerator.IntegrationTests.BooleanTests.ValidateByteBoolReturns(UInt32 value, Boolean expected)
        /home/uweigand/runtime/src/mono/System.Private.CoreLib/src/System/Reflection/MethodInvoker.Mono.cs(33,0): at System.Reflection.MethodInvoker.InterpretedInvoke(Object obj, Span`1 args, BindingFlags invokeAttr)

which fails in this line:

            NativeExportsNE.ReturnUIntAsByteBool_Ref(value, ref result);
            Assert.Equal(expected, result);

The called function is declared using LibraryImport here:

        [LibraryImport(NativeExportsNE_Binary, EntryPoint = "winbool_return_as_refuint")]
        public static partial void ReturnUIntAsByteBool_Ref(uint input, [MarshalAs(UnmanagedType.U1)] ref bool res);

and the automatically generated stub is built making the assumption that the called native routine uses an "uint8_t *" to implement the ref output (as indicated by the MarshalAs attribute):

  .method public hidebysig static void  ReturnUIntAsByteBool_Ref(uint32 input,
                                                                 bool&  marshal( unsigned int8) res) cil managed
[...]
    .locals (uint8 V_0)
[...]
    IL_000c:  ldarg.0
    IL_000d:  ldloca.s   V_0
    IL_000f:  conv.u
    IL_0010:  call       void LibraryImportGenerator.IntegrationTests.NativeExportsNE::'<ReturnUIntAsByteBool_Ref>g____PInvoke|18_0'(uint32,
                                                                                                                                     uint8*)
    IL_0015:  nop
    IL_0016:  nop
    IL_0017:  ldarg.1
    IL_0018:  ldloc.0
    IL_0019:  ldc.i4.0
    IL_001a:  cgt.un
    IL_001c:  stind.i1

However, the method actually being called is created from this definition:

        [UnmanagedCallersOnly(EntryPoint = "winbool_return_as_refuint")]
        public static void ReturnUIntAsRefUInt(uint input, uint* res)
        {
            *res = input;
        }

via a DNNE-generated native stub:

// Computed from NativeExports.Booleans.ReturnUIntAsRefUInt
static void (DNNE_CALLTYPE* winbool_return_as_refuint_ptr)(uint32_t input, uint32_t* res);
DNNE_API void DNNE_CALLTYPE winbool_return_as_refuint(uint32_t input, uint32_t* res)
{
    if (winbool_return_as_refuint_ptr == NULL)
    {
        const char_t* methodName = DNNE_STR("ReturnUIntAsRefUInt");
        winbool_return_as_refuint_ptr = get_fast_callable_managed_function(t3_name, methodName);
    }
    winbool_return_as_refuint_ptr(input, res);
}

Now, this called native routine will write its output to a "uint32_t *". This does not match the expectation of the caller. On a little-endian system, this may not be noticable in this case since the only effect is that a few unused bytes on the stack are clobbered. But on a big-endian system, the caller now actually reads the wrong byte of the result, which is why we see False as return value.

It appears to me that this is simply a bug in the test case. Is this correct, or is there something that should have a done some automatic type conversion / marshalling anywhere?

CC @AaronRobinsonMSFT @jkoritzinsky

[dotnet-issue-labeler]

I couldn't figure out the best area label to add to this issue. If you have write-permissions please help me learn by adding exactly one area label.

[ghost]

Tagging subscribers to this area: @dotnet/interop-contrib
See info in area-owners.md if you want to be subscribed.

Issue Details

---

On s390x, the LibraryImportGenerator test cases have been disabled a while back: https://github.com/dotnet/runtime/pull/60833/files
I've just tried running them again to check current status, and it looks like the original issue has been resolved, so the tests actually build and execute now. However, there are a number of test failures, all related to functions using a ref pointer to retrieve an output. One typical instance is:

    LibraryImportGenerator.IntegrationTests.BooleanTests.ValidateByteBoolReturns(value: 1, expected: True) [FAIL]
      Assert.Equal() Failure
      Expected: True
      Actual:   False
      Stack Trace:
        /home/uweigand/runtime/src/libraries/System.Runtime.InteropServices/tests/LibraryImportGenerator.Tests/BooleanTests.cs(108,0): at LibraryImportGenerator.IntegrationTests.BooleanTests.ValidateByteBoolReturns(UInt32 value, Boolean expected)
        /home/uweigand/runtime/src/mono/System.Private.CoreLib/src/System/Reflection/MethodInvoker.Mono.cs(33,0): at System.Reflection.MethodInvoker.InterpretedInvoke(Object obj, Span`1 args, BindingFlags invokeAttr)

which fails in this line:

            NativeExportsNE.ReturnUIntAsByteBool_Ref(value, ref result);
            Assert.Equal(expected, result);

The called function is declared using LibraryImport here:

        [LibraryImport(NativeExportsNE_Binary, EntryPoint = "winbool_return_as_refuint")]
        public static partial void ReturnUIntAsByteBool_Ref(uint input, [MarshalAs(UnmanagedType.U1)] ref bool res);

and the automatically generated stub is built making the assumption that the called native routine uses an "uint8_t *" to implement the ref output (as indicated by the MarshalAs attribute):

  .method public hidebysig static void  ReturnUIntAsByteBool_Ref(uint32 input,
                                                                 bool&  marshal( unsigned int8) res) cil managed
[...]
    .locals (uint8 V_0)
[...]
    IL_000c:  ldarg.0
    IL_000d:  ldloca.s   V_0
    IL_000f:  conv.u
    IL_0010:  call       void LibraryImportGenerator.IntegrationTests.NativeExportsNE::'<ReturnUIntAsByteBool_Ref>g____PInvoke|18_0'(uint32,
                                                                                                                                     uint8*)
    IL_0015:  nop
    IL_0016:  nop
    IL_0017:  ldarg.1
    IL_0018:  ldloc.0
    IL_0019:  ldc.i4.0
    IL_001a:  cgt.un
    IL_001c:  stind.i1

However, the method actually being called is created from this definition:

        [UnmanagedCallersOnly(EntryPoint = "winbool_return_as_refuint")]
        public static void ReturnUIntAsRefUInt(uint input, uint* res)
        {
            *res = input;
        }

via a DNNE-generated native stub:

// Computed from NativeExports.Booleans.ReturnUIntAsRefUInt
static void (DNNE_CALLTYPE* winbool_return_as_refuint_ptr)(uint32_t input, uint32_t* res);
DNNE_API void DNNE_CALLTYPE winbool_return_as_refuint(uint32_t input, uint32_t* res)
{
    if (winbool_return_as_refuint_ptr == NULL)
    {
        const char_t* methodName = DNNE_STR("ReturnUIntAsRefUInt");
        winbool_return_as_refuint_ptr = get_fast_callable_managed_function(t3_name, methodName);
    }
    winbool_return_as_refuint_ptr(input, res);
}

Now, this called native routine will write its output to a "uint32_t *". This does not match the expectation of the caller. On a little-endian system, this may not be noticable in this case since the only effect is that a few unused bytes on the stack are clobbered. But on a big-endian system, the caller now actually reads the wrong byte of the result, which is why we see False as return value.

It appears to me that this is simply a bug in the test case. Is this correct, or is there something that should have a done some automatic type conversion / marshalling anywhere?

CC @AaronRobinsonMSFT @jkoritzinsky

Author:	uweigand
Assignees:	-
Labels:	area-System.Runtime.InteropServices, untriaged
Milestone:	-

[uweigand]

Hi @AaronRobinsonMSFT, thanks for the quick fix. With #73950 merged, the boolean tests now all pass. However, I'm still seeing failures in the character tests:

      <test name="LibraryImportGenerator.IntegrationTests.CharacterTests.ValidateUnicodeReturns(expected: 'A', value: 65)" type="LibraryImportGenerator.IntegrationTests.CharacterTests" method="ValidateUnicodeReturns" time="0.0351257" result="Fail">
        <failure exception-type="Xunit.Sdk.EqualException">
          <message><![CDATA[Assert.Equal() Failure\nExpected: 'A'\nActual:   '\\0']]></message>
          <stack-trace><![CDATA[   at LibraryImportGenerator.IntegrationTests.CharacterTests.ValidateUnicodeReturns(Char expected, UInt32 value) in /home/uweigand/runtime/src/libraries/System.Runtime.InteropServices/tests/LibraryImportGenerator.Tests/CharacterTests.cs:line 70

I think this problem is due to a similar mismatch here:

        [LibraryImport(NativeExportsNE_Binary, EntryPoint = "char_return_as_refuint", StringMarshalling = StringMarshalling.Utf16)]
        public static partial void ReturnUIntAsUnicode_Ref(uint input, ref char res);

vs.

        [UnmanagedCallersOnly(EntryPoint = "char_return_as_refuint")]
        public static void ReturnUIntAsRefUInt(uint input, uint* res)
        {
            *res = input;
        }

[AaronRobinsonMSFT]

@uweigand Boo. That is sad. We are in a bad situation here. Is there anyway you can help track all the failing cases down in this case? For me, visually auditing the entire test suite is going to be tedious and error prone. If you can comment out the failing tests and open an issue with all the ones failing, it would be easy for me, with endianness in mind, to simply fix them in bulk. Appreciate these issues being brought up.

[uweigand]

@AaronRobinsonMSFT it looks like char_return_as_refuint is the last remaining problem. Using the following hack all LibraryImportGenerator.Tests now pass for me:

diff --git a/src/libraries/System.Runtime.InteropServices/tests/TestAssets/NativeExports/Characters.cs b/src/libraries/System.Runtime.InteropServices/tests/TestAssets/NativeExports/Characters.cs
index ed4e18e22e1..64a4bca9fba 100644
--- a/src/libraries/System.Runtime.InteropServices/tests/TestAssets/NativeExports/Characters.cs
+++ b/src/libraries/System.Runtime.InteropServices/tests/TestAssets/NativeExports/Characters.cs
@@ -21,9 +21,9 @@ public static uint ReturnUIntAsUInt(uint input)
         }
 
         [UnmanagedCallersOnly(EntryPoint = "char_return_as_refuint")]
-        public static void ReturnUIntAsRefUInt(uint input, uint* res)
+        public static void ReturnUIntAsRefUInt(uint input, char* res)
         {
-            *res = input;
+            *res = (char)input;
         }
 
         [UnmanagedCallersOnly(EntryPoint = "char_reverse_buffer_ref")]

[uweigand]

Just to confirm that all LibraryImportGenerator.Tests now pass for me on mainline. Thanks!