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

Crashing on some methods #11

Open
Jrius opened this issue May 14, 2018 · 4 comments
Open

Crashing on some methods #11

Jrius opened this issue May 14, 2018 · 4 comments
Assignees
Labels

Comments

@Jrius
Copy link
Contributor

Jrius commented May 14, 2018

Hello again :)

This time I'm having trouble with some functions crashing Unity, and having a hard time nailing down the cause... I was hoping you could help me figure it out.

This is the code I use (can be pasted directly into the sample .cpp file):

Transform t(go.GetTransform());
t.GetLocalToWorldMatrix();  // crash
Matrix4x4::GetIdentity();   // crash
t.GetPosition();            // ok
String layerName("Default");
LayerMask::NameToLayer(layerName); // crash

Oh, and the relevant NativeScriptTypes.json, if that helps:

{
    "Name": "UnityEngine.LayerMask",
    "Methods": [
        {
            "Name": "NameToLayer",
            "ParamTypes": [
                "System.String"
            ]
        }
    ]
},
{
    "Name": "UnityEngine.Transform",
    "Properties": [
        {
            "Name": "position",
            "Get": {},
            "Set": {
                "Exceptions": [
                    "System.NullReferenceException"
                ]
            }
        },
        {
            "Name": "worldToLocalMatrix",
            "Get": {}
        },
        {
            "Name": "localToWorldMatrix",
            "Get": {}
        }
    ]
}

I attached a debugger to the Unity Editor, and it doesn't say much (Access violation reading location 0x0, when calling Plugin::UnityEngineTransformPropertyGetLocalToWorldMatrix(Handle); in the generated bindings). Happens on both Unity 2017 and 2018, on both Debug and Release build, on Windows.

Those methods are pretty straightforward. GetPosition and GetLocalToWorldMatrix are almost identical when decompiling UnityEngine.dll - they are just calls to native code. I thought it might be because it's returning a Matrix4x4, but LayerMask::NameToLayer crashes too and it's only returning an int...

Out of curiosity, I tried moving those calls to a C# script, then call the C# script from the C++ code - the result was the same.

@jacksondunstan
Copy link
Owner

Hi @Jrius, I just tried this out using macOS in both Editor and Standalone and didn't run into any issues. Is this issue only happening for you on Windows?

@Jrius
Copy link
Contributor Author

Jrius commented May 19, 2018

I don't have a Mac to test with unfortunately... But I'm confident this is a Windows-only bug. Today I tried creating various similar methods in my C# game scripts, and found that methods or property getters crash when returning most value types. Wow, crazy I didn't notice sooner - but then my project is mostly about feeding data to Unity, and I rarely need to access some of the info I just gave it.
Not all value types are affected though. Int, float, double, Vector2 and Matrix4x4 crash, but not Vector3 or Vector4. I also tried a simple struct with an int and Vector2 field, which was returned correctly. I'm just glad it's not a problem with Unity's API itself.
I'm not sure why the bindings would fail on these, though... Especially when it comes to something as basic as an int. I'll see if I can find some more insight in the generated bindings.

@jacksondunstan
Copy link
Owner

jacksondunstan commented May 21, 2018

@Jrius, thanks for continuing to investigate this with me. I spent some time looking into the issue this weekend, including reproducing it on Windows 10 64-bit using your example code. I made a small example project to strip away as much of UnityNativeScripting as possible. I ended up with a single C++ file compiled into a CppLib.dll by a Visual Studio project:

struct TestStruct
{
	float f01; // 4 bytes  - OK
	float f02; // 8 bytes  - OK
	float f03; // 12 bytes - OK
	float f04; // 16 bytes - OK
	float f05; // 20 bytes - OK
	float f06; // 24 bytes - OK
	float f07; // 28 bytes - OK
	float f08; // 32 bytes - OK
	float f09; // 36 bytes - OK
	float f10; // 40 bytes - OK
	float f11; // 44 bytes - Crash
};

extern "C" __declspec(dllexport) void CppFunc(TestStruct(*fp)())
{
	TestStruct s = fp();
}

Then I made a Unity project that's empty except for one C# file:

using AOT;
using System;
using System.Runtime.InteropServices;
using UnityEngine;

public struct TestStruct
{
	public float f01; // 4 bytes  - OK
	public float f02; // 8 bytes  - OK
	public float f03; // 12 bytes - OK
	public float f04; // 16 bytes - OK
	public float f05; // 20 bytes - OK
	public float f06; // 24 bytes - OK
	public float f07; // 28 bytes - OK
	public float f08; // 32 bytes - OK
	public float f09; // 36 bytes - OK
	public float f10; // 40 bytes - OK
	public float f11; // 44 bytes - Crash
}

public delegate TestStruct DelegateType();

public class TestScript : MonoBehaviour
{
	[MonoPInvokeCallback(typeof(DelegateType))]
	public static TestStruct ReturnTestStruct()
	{
		return default(TestStruct);
	}

	[DllImport("CppLib")]
	static extern void CppFunc(IntPtr fp);

	void Start()
	{
		DelegateType del = new DelegateType(ReturnTestStruct);
		IntPtr fp = Marshal.GetFunctionPointerForDelegate(del);
		CppFunc(fp);
	}
}

Here's what this does:

  1. C# uses Win32's LoadLibrary to load the DLL
  2. C# creates a delegate for its CppMethod
  3. C# gets a function pointer for that delegate
  4. C# calls into C++'s CppFunction and passes the function pointer
  5. C++ calls the function pointer it was passed and sets the result to a local variable
  6. C# returns a large struct

This produces the same null dereference crash that you reported and I reproduced in UnityNativeScripting. The problem notably doesn't happen if any of the following are true:

  1. The struct is 40 bytes or less
  2. A primitive (e.g. int) is used instead of a struct (note: no primitive is larger than 40 bytes)
  3. The DLL is loaded with [DllImport] instead of LoadLibrary (note: [DllImport] is used in Windows builds)
  4. A C++ function returns the struct instead of a C# function returning the struct
  5. The struct is "returned" from C# via a ref parameter

So this seems like an edge case that should only occur with large structs like Matrix4x4, which is probably why I didn't catch it before. It looks like I'll need to re-work how large structs are returned to use a ref/pointer parameter instead of a basic return value to work around this issue. This can be handled transparently by mostly making changes to the code generator, so no game code should be affected. In the meantime, you should be able to work around the issue by exposing a C# helper function to C++. In the case of GetLocalToWorldMatrix, you'd write this C#:

public static class Workaround
{
	public static void TransformGetLocalToWorldMatrix(Transform t, ref Matrix4x4 m)
	{
		m = t.localToWorldMatrix;
	}
}

Then you'd expose it in the JSON config like this:

{
	"Types": [
		{
			"Name": "Workaround",
			"Methods": [
				{
					"Name": "TransformGetLocalToWorldMatrix",
					"ParamTypes": [
						"UnityEngine.Transform",
						"UnityEngine.Matrix4x4"
					]
				}
			]
		}
	]
}

Then you'd call it from C++ like this:

void Foo()
{
	GameObject go("test go");
	Transform t(go.GetTransform());
	
	Matrix4x4 m;
	Workaround::TransformGetLocalToWorldMatrix(t, &m);
}

Hopefully that unblocks you until I can commit a proper solution.

@jacksondunstan jacksondunstan self-assigned this May 21, 2018
@Jrius
Copy link
Contributor Author

Jrius commented May 25, 2018

Hey. Sorry for not answering sooner, I had one of those crazy week at work...
Anyway. Thanks for the extensive answer ! It's strange that returning ints doesn't crash for you, but at least we've narrowed down the issue for other structs. I'll keep track of anything that might explain it, maybe it has something to do with debug/release build and running in the editor vs running standalone...
Good idea about returning those objects using ref/out. For some reason I didn't think about that earlier, but I've tested and it works for my project. Great !

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants