"free(): invalid pointer" with AnsiStringMarshaller compiled with crossgen2 #54820
Comments
dotnet-issue-labeler
bot
added
area-CodeGen-coreclr
jkotas
added
area-crossgen2-coreclr
bug
and removed
area-CodeGen-coreclr
|
cc @mangod9 This is the problem with AnsiBSTRMarshaler that have discussed this over email |
|
Hey @gbalykov , I am trying to repro this issue, but I am not getting the |
|
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+libsInvoked 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 --verboseCommits 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;
}
|
|
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 |
|
@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? |
|
Correct, |
|
Also /cc @MichalStrehovsky who was according to my recollection the original implementor of the Crossgen2 PInvoke marshallers. |
|
It doesn't look right to be calling into BStr marshalling at all. This should rather all call into CStrMarshaller. The generated IL should basically correspond to what ILCSTRMarshaler in ilmarshalers.cpp does. |
|
cc @AaronRobinsonMSFT This touches on the problem of creating marshalling helpers that can be used by all runtime, crossgen and source generators. |
ghost
added
the
in-pr
ghost
removed
the
in-pr
ghost
locked as resolved and limited conversation to collaborators
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 stringargument marshalling, which tries to freenative_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:
Simple example to reproduce:
c# part:
native part, compiled with
gcc -shared -fPIC 1.c -o tmp.soOutput:
Native code of compiled ilstub, which crashes:
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 subtractssizeof(void*)from ptr (Marshal.FreeBSTRin Marshal.Unix.cs actually performs subtraction). This itself happens becauseSystem.StubHelpers.AnsiBSTRMarshaler.ConvertToManagedfor 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
The text was updated successfully, but these errors were encountered: