Silk.NET.Assimp works only for x64 processes and crashes in x86 processes

[abenedik]

Summary

Silk.NET.Assimp works only for x64 processes and crashes in x86 processes. I assume that the calli IL method does not use Cdecl calling convention to call the native functions.

Steps to reproduce

• Download the new https://github.com/ab4d/Ab3d.DXEngine.Assimp project (assimp importer for Ab3d.DXEngine rendering engine)
• Open the Ab3d.DXEngine.Assimp.net50.sln solution
• Set target platform to x86 for both projects in the solution
• Start the Ab3d.DXEngine.Assimp.Samples project and load a file by clicking on "Load file" button
• 'Attempted to read or write protected memory. This is often an indication that other memory is corrupt.' exception is thrown when calling ImportFile.

Comments

As seen from the decompiled code the Silk.NET.Assimp is using calli IL method to call native assimp functions. The parameters are correctly passed to the native method but from the IL code I cannot see if calli is using StdCall or Cdecl (when creating the calli IL method it is possible to specify the calling convention - see https://docs.microsoft.com/en-us/dotnet/api/system.reflection.emit.ilgenerator.emitcalli?redirectedfrom=MSDN&view=net-5.0#overloads)

C# uses StdCall calling convention by default, but C++ uses Cdecl by default.

I did a quick test and in the x64 process the functions in the native assimp library can be called by using StdCall or Cdecl. But when using x86, then the functions can be only called by using Cdecl (StdCall produces an exception).

The following is the used test code (note that it uses VTable from Silk.NET.Assimp):
`[UnmanagedFunctionPointer(CallingConvention.Cdecl)]
private delegate Scene* ImportFileDelegate_cdecl(byte* fileName, uint flags);

[UnmanagedFunctionPointer(CallingConvention.StdCall)]
private delegate Scene* ImportFileDelegate_stdcall(byte* fileName, uint flags);

private unsafe void DirectAssimpMethodCallTest()
{
var assimp = Silk.NET.Assimp.Assimp.GetApi();
var importFilePtr = assimp.CurrentVTable.Load("aiImportFile");

byte* fileNamePtr = (byte*)SilkMarshal.StringToPtr(@"testModelFile");

//var functionPtr = Marshal.GetDelegateForFunctionPointer(importFilePtr, typeof(ImportFileDelegate));
var functionPtr_cdecl = Marshal.GetDelegateForFunctionPointer<ImportFileDelegate_cdecl>(importFilePtr);
var functionPtr_stdcall = Marshal.GetDelegateForFunctionPointer<ImportFileDelegate_stdcall>(importFilePtr);

var scene1 = functionPtr_cdecl(fileNamePtr, 0);
var scene2 = functionPtr_stdcall(fileNamePtr, 0);

}`

You can copy this code into the MainWindow.xaml.cs in Ab3d.DXEngine.Assimp.Samples project to test it.

Please check the code generator that is used for Silk.NET.Assimp and see if the calli method uses Cdecl call (I was not able to find that in the Silk.NET source).

Additional recommendation

The Silk.NET.Assimp uses a dependency on Ultz.Native.Assimp.

I do not like this because this forces the user to use the native assimp that is provided by the Ultz.Native.Assimp. In my opinion, the Silk.NET.Assimp should be only a wrapper without the native library and the application that uses the wrapper should decide which native library it would use - so the application should depend on Ultz.Native.Assimpis (or any other source of the native library).

What is more, the current version of Ultz.Native.Assimp does not work for .Net framework projects - in this case, the native library is not copied to the output folder (this works for .Net core and .Net 5.0 projects). So a .Net framework project that uses Silk.NET.Assimp also depends on Ultz.Native.Assimp despite this library does not work and the project needs to provide its own native library.

[HurricanKai]

Cdecl is our default calling convention and should be used everywhere (see here), I will verify this.
I would think that this is an issue with the native library used. Are you using the one provided in Ultz.Native.Assimp or a different one?

Regarding Ultz.Native.Assimp, we ship native libraries for all our bindings as it's usually just a hassle for users to retrieve them. What's the reason you want to use your own distribution?

[abenedik]

I used Assimp32.dll from the Ultz.Native.Assimp v5.0.1.2 (referenced by Silk.NET.Assimp), v5.0.1.3 (newer version) and a version that I have built by myself.

None of those versions work with Ultz.Native.Assimp in x86 process.

And ALL those native dlls work in x86 process with my older Assimp importer that can import 3D objects into WPF 3D objects (that assimp importer is using an old Assimp.Net library as a wrapper).

This can be tested by downloading the Ab3d.DXEngine samples project from https://github.com/ab4d/Ab3d.DXEngine.Wpf.Samples. Compile the project for x86 and replace the Assimp32.dll in the root folder with the Assimp32.dll from Ultz.Native.Assimp to test that the same native library works in another wrapper.

I understand from my own experience that getting compiled assimp dlls or compiling them is quite complicated for a usual .net developer. The problem with assimp library is that it does not have a well defined release cycle - the latest official release is from January 2020 (and this release was created a few years after the previous release). The library is constantly evolving and file importers are constantly being improved. Because the API is well defined newer versions of the library can be used by using the same API (as in the case of using a very old Assimp.Net). This adds new file importers and exporters and helps solve reading some of the files.

In the case of Ultz.Native.Assimp it is not documented which version of native assimp library the library is using. I assume that an official assimp from January 2020 is used. But maybe it is using a newer version compiled from a newer source (the Ultz.Native.Assimp has a new release a month ago).

[Perksey]

Can you produce a "minimal reproduction" for this? I don't have a 32-bit development PC so if there were something I could just hit dotnet run on that would be great.

[abenedik]

Hm, aren't the "Steps to reproduce" easy enough - you just download a solution from GitHub, open it in VS, change the Target Platform to x86 and click run (you can also change the target platform by opening the csproj file and change the value of PlatformTarget to x86; then you can run the sample project by using dotnet run).

You can start a 32-bit process (x86) on 64-bit windows OS (you do not need 32-bit windows).

[Perksey]

Ah I misread, I thought I'd have to copy and paste some code around. Will give this a go now.

[Perksey]

The issue where we're using an incorrect calling convention has been fixed. We are now using "Winapi" aka the platform default which is Stdcall on Windows. I am still getting this issue with your sample, so there might be something else wrong there (nasty binaries? maybe I just didn't copy my dev DLLs over correctly?) but the fix suggested in the issue has been implemented in #561 and will be in 2.7 releasing tomorrow

[abenedik]

I have tested version 2.7 and found out that my sample that comes with Ab3d.DXEngine.Assimp project still does not work when it is executed in x86 project.

I have used ildasm tool (can be started from "Developer Command prompt for VS") and found out that the calling conversion that is used by calli that calls the ImportFile method is correct.

After some additional investigation, I have found out that if I do not set Assimp logging callback (AttachLogStream), then importing files starts working. This means that ImportFile method works correctly and that the problem is in the logging callback (that is internally called by assimp when executing ImportFile method).

After using locally defined structs that are used by the AttachLogStream method, I found out that adding the following attribute to the LogStreamCallback delegate (defined in PfnLogStreamCallback.gen.cs) solves the issue: [UnmanagedFunctionPointer(CallingConvention.Cdecl)]

This attribute is probably also missing on other delegates that are defined in *Proc.gen.cs files in Silk.NET.Assimp/Structs/

[Perksey]

Aaahhhhh ok, that explains it. Will get a fix up later today.

[Perksey]

Good find by the way!