"free(): invalid pointer" with AnsiStringMarshaller compiled with crossgen2

[gbalykov]

Description

If marshalling ilstub compilation is allowed in crossgen2, ansi string marshalling for string allocated in native code leads to "free(): invalid pointer".

Problem is related to out string argument marshalling, which tries to free native_pointer - sizeof(void*).

However, it seems that this problem might arise in SPC.dll even without any changes in crossgen2 because currently all such ilstubs are compiled, see GeneratesPInvoke:

            if (((EcmaMethod)method).Module.Equals(method.Context.SystemModule))
                return true;

Simple example to reproduce:

c# part:

using System;
using System.Runtime.InteropServices;

namespace ndirect
{
    class Program
    {
        [DllImport("tmp.so", EntryPoint = "test_func")]
        internal static extern int testFunc(out string value);

        static void Main(string[] args)
        {
            Console.WriteLine("Hello World!");

            string a;
            testFunc(out a);
            Console.WriteLine(a);
        }
    }
}

native part, compiled with gcc -shared -fPIC 1.c -o tmp.so

#include <malloc.h>
#include <string.h>

int test_func(char ** str)
{
  char * s = (char*) malloc(100);
  *str = s;
  strcpy(s, "my test string!");
  return strlen(s);
}

Output:

Hello World!
free(): invalid pointer
Aborted (core dumped)

Native code of compiled ilstub, which crashes:

10a10: 55                       push    rbp
                                UWOP_PUSH_NONVOL RBP(5)                                0

10a11: 41 57                    push    r15
                                UWOP_PUSH_NONVOL R15(15)                                0

10a13: 41 56                    push    r14
                                UWOP_PUSH_NONVOL R14(14)                                0

10a15: 41 55                    push    r13
                                UWOP_PUSH_NONVOL R13(13)                                0

10a17: 41 54                    push    r12
                                UWOP_PUSH_NONVOL R12(12)                                0

10a19: 53                       push    rbx
                                UWOP_PUSH_NONVOL RBX(3)                                0

10a1a: 48 83 ec 78              sub     rsp, 120
                                UWOP_ALLOC_SMALL 120                                0

10a1e: 48 8d ac 24 a0 00 00 00  lea     rbp, [rsp + 160]
10a26: 33 c0                    xor     eax, eax
10a28: 48 89 45 d0              mov     qword ptr [rbp - 48], rax
10a2c: 48 89 a5 60 ff ff ff     mov     qword ptr [rbp - 160], rsp
10a33: 48 89 7d c8              mov     qword ptr [rbp - 56], rdi
10a37: 48 8d 7d d0              lea     rdi, [rbp - 48]
10a3b: 48 89 7d c0              mov     qword ptr [rbp - 64], rdi
10a3f: 48 8d bd 68 ff ff ff     lea     rdi, [rbp - 152]
10a46: ff 15 ec 08 01 00        call    qword ptr [0x21338]   // PINVOKE_BEGIN (HELPER)
10a4c: 48 8b 05 25 09 01 00     mov     rax, qword ptr [0x21378] // int ndirect.Program.testFunc(ref string) (INDIRECT_PINVOKE_TARGET)
10a53: 48 8b 7d c0              mov     rdi, qword ptr [rbp - 64]
10a57: ff 10                    call    qword ptr [rax]
10a59: 8b d8                    mov     ebx, eax
10a5b: 48 8d bd 68 ff ff ff     lea     rdi, [rbp - 152]
10a62: ff 15 d8 08 01 00        call    qword ptr [0x21340]   // PINVOKE_END (HELPER)
10a68: 48 8b 7d d0              mov     rdi, qword ptr [rbp - 48]
10a6c: ff 15 e6 08 01 00        call    qword ptr [0x21358]   // string System.StubHelpers.AnsiBSTRMarshaler.ConvertToManaged(IntPtr) (METHOD_ENTRY_DEF_TOKEN)
                                RAX is live
10a72: 48 8b 7d c8              mov     rdi, qword ptr [rbp - 56]
                                RDI is live
10a76: 48 8b f0                 mov     rsi, rax
                                RSI is live
                                sp-56 is dead
10a79: ff 15 b1 08 01 00        call    qword ptr [0x21330]   // CHECKED_WRITE_BARRIER (HELPER)
                                RDI is dead
                                RAX is dead
                                RSI is dead
10a7f: 90                       nop
10a80: 48 8b 7d d0              mov     rdi, qword ptr [rbp - 48]
10a84: ff 15 d6 08 01 00        call    qword ptr [0x21360]   // void System.StubHelpers.AnsiBSTRMarshaler.ClearNative(IntPtr) (METHOD_ENTRY_DEF_TOKEN)
10a8a: 8b c3                    mov     eax, ebx
10a8c: 48 83 c4 78              add     rsp, 120
10a90: 5b                       pop     rbx
10a91: 41 5c                    pop     r12
10a93: 41 5d                    pop     r13
10a95: 41 5e                    pop     r14
10a97: 41 5f                    pop     r15
10a99: 5d                       pop     rbp
10a9a: c3                       ret

Patch to allow marshalling ilstub compilation:

commit 129e85a182b9ed8561b3428f647df8dcf93ae49d
Author: Gleb Balykov <g.balykov@samsung.com>
Date:   Mon Jun 28 12:53:58 2021 +0300

    Partially allow marshalling ILSTUB compilation for ndirect methods using crossgen2 if SPC.dll is in the same bubble
    
    AnsiStringMarshaller is disallowed because it incorrectly handles pointers returned from native code.

diff --git a/src/coreclr/tools/aot/ILCompiler.ReadyToRun/Compiler/ReadyToRunCompilationModuleGroupBase.cs b/src/coreclr/tools/aot/ILCompiler.ReadyToRun/Compiler/ReadyToRunCompilationModuleGroupBase.cs
index 92ef5346c77..eaa2821ac94 100644
--- a/src/coreclr/tools/aot/ILCompiler.ReadyToRun/Compiler/ReadyToRunCompilationModuleGroupBase.cs
+++ b/src/coreclr/tools/aot/ILCompiler.ReadyToRun/Compiler/ReadyToRunCompilationModuleGroupBase.cs
@@ -329,10 +329,8 @@ public sealed override bool GeneratesPInvoke(MethodDesc method)
         {
             // PInvokes depend on details of the core library, so for now only compile them if:
             //    1) We're compiling the core library module, or
-            //    2) We're compiling any module, and no marshalling is needed
-            //
-            // TODO Future: consider compiling PInvokes with complex marshalling in version bubble
-            // mode when the core library is included in the bubble.
+            //    2) We're compiling any module, and no marshalling is needed, or
+            //    3) We're compiling any module, and core library module is in the same bubble, and marhaller supports compilation
 
             Debug.Assert(method is EcmaMethod);
 
@@ -344,6 +342,9 @@ public sealed override bool GeneratesPInvoke(MethodDesc method)
             if (((EcmaMethod)method).Module.Equals(method.Context.SystemModule))
                 return true;
 
+            if (_versionBubbleModuleSet.Contains(method.Context.SystemModule))
+                return !Marshaller.IsMarshallingNotSupported(method);
+
             return !Marshaller.IsMarshallingRequired(method);
         }
 
diff --git a/src/coreclr/tools/aot/ILCompiler.ReadyToRun/Interop/IL/Marshaller.ReadyToRun.cs b/src/coreclr/tools/aot/ILCompiler.ReadyToRun/Interop/IL/Marshaller.ReadyToRun.cs
index 636c698e8d5..4bc3d1340e2 100644
--- a/src/coreclr/tools/aot/ILCompiler.ReadyToRun/Interop/IL/Marshaller.ReadyToRun.cs
+++ b/src/coreclr/tools/aot/ILCompiler.ReadyToRun/Interop/IL/Marshaller.ReadyToRun.cs
@@ -141,5 +141,37 @@ public static bool IsMarshallingRequired(MethodSignature methodSig, ParameterMet
 
             return false;
         }
+
+        public static bool IsMarshallingNotSupported(MethodDesc targetMethod)
+        {
+            Debug.Assert(targetMethod.IsPInvoke);
+
+            if (targetMethod.IsUnmanagedCallersOnly)
+                return true;
+
+            PInvokeMetadata metadata = targetMethod.GetPInvokeMethodMetadata();
+            PInvokeFlags flags = metadata.Flags;
+
+            if (flags.SetLastError)
+                return true;
+
+            if (!flags.PreserveSig)
+                return true;
+
+            if (MarshalHelpers.ShouldCheckForPendingException(targetMethod.Context.Target, metadata))
+                return true;
+
+            var marshallers = GetMarshallersForMethod(targetMethod);
+            for (int i = 0; i < marshallers.Length; i++)
+            {
+                if (marshallers[i].GetType() == typeof(NotSupportedMarshaller))
+                    return true;
+            }
+
+            return false;
+        }
     }
 }

More detailed description

This problem happens inside System.StubHelpers.AnsiBSTRMarshaler.ClearNative, which tries to cleanup bstr buffer, and subtracts sizeof(void*) from ptr (Marshal.FreeBSTR in Marshal.Unix.cs actually performs subtraction). This itself happens because System.StubHelpers.AnsiBSTRMarshaler.ConvertToManaged for some reason ignores non-bstr buffers. However, pointer returned from native code is not bstr, but a simple buffer. So, incorrect pointer is passed to free.

cc @alpencolt @jkotas

[jkotas]

cc @mangod9 This is the problem with AnsiBSTRMarshaler that have discussed this over email

[mangod9]

Hey @gbalykov ,

I am trying to repro this issue, but I am not getting the AnsiBStrMarshaller stubs generated for the test_func method when I precompile your repro. Here is the generated code I am seeing with this commandline (perhaps I missing some argument?):
crossgen2.exe C:\temp\ansibstr\bin\Debug\net6.0\ansibstr.dll -r \git\runtime\artifacts\tests\coreclr\windows.x64.Debug\Tests\Core_Root\*.dll -o c:\temp\ansibstr.dll --inputbubble --targetos linux --targetarch x64 -O

int ndirect.Program.testFunc(ref string)
Handle: 0x06000001
Rid: 1
EntryPointRuntimeFunctionId: 0
Number of RuntimeFunctions: 2
Number of fixups: 1
    TableIndex 4, Offset 0000: int ndirect.Program.testFunc(ref string) (INDIRECT_PINVOKE_TARGET)

int ndirect.Program.testFunc(ref string)
Id: 0
StartAddress: 0x00010A20
Size: 139 bytes
UnwindRVA: 0x00010998
Version:            1
Flags:              0x03 EHANDLER UHANDLER
SizeOfProlog:       0x000E
CountOfUnwindCodes: 7
FrameRegister:      None
FrameOffset:        0x0
PersonalityRVA:     0x10B3C
UnwindCode[0]: CodeOffset 0x000E FrameOffset 0x0000 NextOffset 0x0 Op 120
UnwindCode[2]: CodeOffset 0x0009 FrameOffset 0x0000 NextOffset 0x0 Op R12(12)
UnwindCode[4]: CodeOffset 0x0005 FrameOffset 0x0000 NextOffset 0x0 Op R14(14)
UnwindCode[6]: CodeOffset 0x0001 FrameOffset 0x0000 NextOffset 0x0 Op RBP(5)

EH info @ 10B24, #clauses = 1
Flags 04 TryOff 0027 (RVA {(TryOffset + methodRva):X4}) TryEnd 0070 (RVA {(TryEnd + methodRva):X4}) HndOff 008B (RVA {(HandlerOffset + methodRva):X4}) HndEnd 00BE (RVA {(HandlerEnd + methodRva):X4}) ClsFlt 0000 FAULT

Debug Info
    Bounds:
    Native Offset: 0x0, Prolog, Source Types: StackEmpty
    Native Offset: 0x27, IL Offset: 0x0000, Source Types: StackEmpty
    Native Offset: 0x58, IL Offset: 0x0009, Source Types: StackEmpty
    Native Offset: 0x62, IL Offset: 0x0012, Source Types: StackEmpty
    Native Offset: 0x7A, IL Offset: 0x0021, Source Types: StackEmpty
    Native Offset: 0x7C, Epilog, Source Types: StackEmpty
    Native Offset: 0x8B, Prolog, Source Types: StackEmpty
    Native Offset: 0xA4, IL Offset: 0x001a, Source Types: StackEmpty
    Native Offset: 0xAE, IL Offset: 0x0020, Source Types: StackEmpty
    Native Offset: 0xAF, Epilog, Source Types: StackEmpty

    Variable Locations:
    Variable Number: 0
    Start Offset: 0x0
    End Offset: 0x27
    Loc Type: VLT_REG
    Register: RDI

    Variable Number: 0
    Start Offset: 0x27
    End Offset: 0x66
    Loc Type: VLT_STK
    Base Register: RBP
    Stack Offset: -56

    Variable Number: 1
    Start Offset: 0x58
    End Offset: 0x7A
    Loc Type: VLT_REG
    Register: RBX

    Variable Number: 2
    Start Offset: 0x58
    End Offset: 0x58
    Loc Type: VLT_REG
    Register: RBX

    Variable Number: 3
    Start Offset: 0x62
    End Offset: 0x66
    Loc Type: VLT_REG
    Register: RAX

10a20: 55                       push    rbp
                                UWOP_PUSH_NONVOL RBP(5)                                0

10a21: 41 57                    push    r15
                                UWOP_PUSH_NONVOL R15(15)                                0

10a23: 41 56                    push    r14
                                UWOP_PUSH_NONVOL R14(14)                                0

10a25: 41 55                    push    r13
                                UWOP_PUSH_NONVOL R13(13)                                0

10a27: 41 54                    push    r12
                                UWOP_PUSH_NONVOL R12(12)                                0

10a29: 53                       push    rbx
                                UWOP_PUSH_NONVOL RBX(3)                                0

10a2a: 48 83 ec 78              sub     rsp, 120
                                UWOP_ALLOC_SMALL 120                                0

10a2e: 48 8d ac 24 a0 00 00 00  lea     rbp, [rsp + 160]
10a36: 33 c0                    xor     eax, eax
10a38: 48 89 45 d0              mov     qword ptr [rbp - 48], rax
10a3c: 48 89 a5 60 ff ff ff     mov     qword ptr [rbp - 160], rsp
10a43: 48 89 7d c8              mov     qword ptr [rbp - 56], rdi
10a47: 48 8d 7d d0              lea     rdi, [rbp - 48]
10a4b: 48 89 7d c0              mov     qword ptr [rbp - 64], rdi
10a4f: 48 8d bd 68 ff ff ff     lea     rdi, [rbp - 152]
10a56: ff 15 dc 08 01 00        call    qword ptr [0x21338]   // PINVOKE_BEGIN (HELPER)
10a5c: 48 8b 05 15 09 01 00     mov     rax, qword ptr [0x21378] // int ndirect.Program.testFunc(ref string) (INDIRECT_PINVOKE_TARGET)
10a63: 48 8b 7d c0              mov     rdi, qword ptr [rbp - 64]
10a67: ff 10                    call    qword ptr [rax]
10a69: 8b d8                    mov     ebx, eax
10a6b: 48 8d bd 68 ff ff ff     lea     rdi, [rbp - 152]
10a72: ff 15 c8 08 01 00        call    qword ptr [0x21340]   // PINVOKE_END (HELPER)
10a78: 48 8b 7d d0              mov     rdi, qword ptr [rbp - 48]
10a7c: ff 15 d6 08 01 00        call    qword ptr [0x21358]   // System.Runtime.Intrinsics.Vector256`1<double> System.Runtime.Intrinsics.X86.Avx.RoundCurrentDirection(System.Runtime.Intrinsics.Vector256`1<double>) (METHOD_ENTRY_DEF_TOKEN)
                                RAX is live
10a82: 48 8b 7d c8              mov     rdi, qword ptr [rbp - 56]
                                RDI is live
10a86: 48 8b f0                 mov     rsi, rax
                                RSI is live
                                sp-56 is dead
10a89: ff 15 a1 08 01 00        call    qword ptr [0x21330]   // CHECKED_WRITE_BARRIER (HELPER)
                                RDI is dead
                                RAX is dead
                                RSI is dead
10a8f: 90                       nop
10a90: 48 8b 7d d0              mov     rdi, qword ptr [rbp - 48]
10a94: ff 15 c6 08 01 00        call    qword ptr [0x21360]   // bool System.Reflection.TypeInfo+<get_DeclaredNestedTypes>d__22.MoveNext() (METHOD_ENTRY_DEF_TOKEN)
10a9a: 8b c3                    mov     eax, ebx
10a9c: 48 83 c4 78              add     rsp, 120
10aa0: 5b                       pop     rbx
10aa1: 41 5c                    pop     r12
10aa3: 41 5d                    pop     r13
10aa5: 41 5e                    pop     r14
10aa7: 41 5f                    pop     r15
10aa9: 5d                       pop     rbp
10aaa: c3                       ret

[gbalykov]

Forgot to mention that I'm seeing this on linux, both x64 and armel.

x64 build command:

./build.sh --arch x64 --runtimeConfiguration Release --librariesConfiguration Release --subset clr+libs

Invoked crossgen2 command (--inputbubble or -O do not have any effect):

./corerun ./crossgen2/crossgen2.dll -r:`pwd`/*.dll -o:`pwd`/ndirect/ndirect.ni.dll `pwd`/ndirect/ndirect.dll --inputbubble --verbose

Commits on which this reproduces: c139d00, 911640b. Then with R2RDump I obtained native code.

Actually, you can use PR #54847 with next small change, I've just verified that it reproduces crash:

diff --git a/src/coreclr/tools/aot/ILCompiler.ReadyToRun/Interop/IL/Marshaller.ReadyToRun.cs b/src/coreclr/tools/aot/ILCompiler.ReadyToRun/Interop/IL/Marshaller.ReadyToRun.cs
index e02bbbe8f2d..e4f218a1949 100644
--- a/src/coreclr/tools/aot/ILCompiler.ReadyToRun/Interop/IL/Marshaller.ReadyToRun.cs
+++ b/src/coreclr/tools/aot/ILCompiler.ReadyToRun/Interop/IL/Marshaller.ReadyToRun.cs
@@ -155,7 +155,8 @@ public static bool IsMarshallingNotSupported(MethodDesc targetMethod)
                 if (marshallers[i].GetType() == typeof(NotSupportedMarshaller)
                     // TODO: AnsiStringMarshaller can be allowed when it's logic is fixed,
                     // currently it leads to free(): invalid pointer
-                    || marshallers[i].GetType() == typeof(AnsiStringMarshaller))
+//                    || marshallers[i].GetType() == typeof(AnsiStringMarshaller))
+   )
                     return true;
             }
 

[mangod9]

hmm, yeah I do have your changes. I can get it to repro, except R2RDump seems to be incorrectly resolving methoddefs in my case hence I couldnt see the calls to AnsiBSTRMarshaler within the disassembly.

[trylek]

@jkotas - I believe you previously mentioned that on Linux we shouldn't be using AnsiBSTRMarshaler at all, does that mean there's some problem with the existing marshaller implementation - I don't see any Windows vs. Linux conditional logic there?

[trylek]

runtime/src/coreclr/tools/Common/TypeSystem/Interop/IL/Marshaller.cs

Line 1607 in 36e0432

var stringToAnsi =

[mangod9]

Correct, AnsiBSTRMarshaler is unconditionally used for all AnsiStrings which isnt right on Linux. Possibly just call into InteropServices here:

runtime/src/libraries/System.Private.CoreLib/src/System/Runtime/InteropServices/Marshal.Unix.cs

Line 7 in 57bfe47

namespace System.Runtime.InteropServices

?