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

Calling a GDNative C function with too many functions in it crashes debugger #11967

Closed
quendera opened this issue Oct 9, 2017 · 9 comments
Closed

Comments

@quendera
Copy link
Contributor

quendera commented Oct 9, 2017

Issue description:
Calling a function from GDNative C code with too many functions in it crashes the debugger.

I'm trying to feed some variables to a C function in a test package I'm developing. My C function takes char* arguments and therefore, when feeding said arguments from godot I need to convert them from 'godot_type' (as discussed here). It works just fine if I feed and convert 1/2 vars (you can check by using godot_print()), however, it crashes if I try to feed more than 2.

Steps to reproduce:
1.Get a C function and lib working that takes an array as argument.
2.Feed it an array from GDScript.
3.Perform operations on 1 of the vars in your array.
4.Recompile your lib and watch the magic happen.
5.Add same operations on other vars from your array.
6.Recompile your lib and watch godot debugger crash.

Example code:
GDSCRIPT

func _ready():
var gdn = GDNative.new()
gdn.library = load("res://lib/test.tres")

var a = "aaa"
var b = "bbb"
var c = "ccc"

gdn.initialize()
gdn.call_native("standard_varcall", "mytestfunction", [a,b,c])
gdn.terminate()

C that works

void GDN_EXPORT mytestfunction(void *data, godot_array *mytestarray) {

        const godot_int idx_a = 0;
        godot_variant var_a;
        var_a = godot_array_get(mytestarray, idx_a);
        godot_string gds_a = godot_variant_as_string(&var_a);
} 

C that crashes

void GDN_EXPORT mytestfunction(void *data, godot_array *mytestarray) {

        const godot_int idx_a = 0;
        godot_variant var_a;
        var_a = godot_array_get(mytestarray, idx_a);
        godot_string gds_a = godot_variant_as_string(&var_a);

        const godot_int idx_b = 1;
        godot_variant var_b;
        var_b = godot_array_get(mytestarray, idx_b);
        godot_string gds_b = godot_variant_as_string(&var_b);

        const godot_int idx_c = 2;
        godot_variant var_c;
        var_c = godot_array_get(mytestarray, idx_c);
        godot_string gds_c = godot_variant_as_string(&var_c);
} 

OS:
Running Ubuntu 16.04 x64 with clean Godot latest master compiled from source, latest godot-headers and cpp bindings.

@karroffel
Copy link
Contributor

You need to return a godot_variant from the C function. (also don't forget to call godot_XXX_destroy() so you don't leak memory).

If you don't return a godot_variant then it might still work but, for example, in a sysv conformant system the contents of the rax register are used as the return value. Because godot_variant is a complex struct type it doesn't get returned per se but a place to put the data is provided be the caller.

What that means is that the result value gets created from Godot, it is uninitialized. Godot calls your code and expects you to fill the struct with the return data, which you never do. So it could be a crash inside there because of uninitialized values, but that wouldn't explain why it happens when you have many arguments.

Can you include the stacktrace of the crash?

@bojidar-bg bojidar-bg added this to the 3.0 milestone Oct 9, 2017
@quendera
Copy link
Contributor Author

quendera commented Oct 10, 2017

Thanks for the reply and assistance.

Returning a godot_variant did not solve it (I had been using it in my real code but forgot to update in this mock one I posted). I'll go through the source more in depth to try and understand better.

I cannot find godot_"xx"_destroy() in the source, would it be possible if you pointed it to me?

Godot outputs no error to its output or debugger, the stacktrace is here: https://pastebin.com/fWmgG9bP

@karroffel
Copy link
Contributor

Sorry for the laaaaaaaate reply, for all types that need de-initialization there are _destroy functions, for example godot_string_destroy, godot_array_destroy, godot_dictionary_destroy and godot_variant_destroy.

This is only needed for types that use RAII in C++, other types don't have those functions, like godot_vector2/3 for example, they are just value types and don't need de-initialization.

I actually haven't tried to reproduce the error, I will put it on my TODO

@karroffel karroffel self-assigned this Nov 3, 2017
@akien-mga
Copy link
Member

We have now entered release freeze for Godot 3.0 and want to focus only on release critical issues for that milestone. Therefore, we're moving this issue to the 3.1 milestone, though a fix may be made available for a 3.0.x maintenance release after it has been tested in the master branch during 3.1 development. If you consider that this issue is critical enough to warrant blocking the 3.0 release until fixed, please comment so that we can assess it more in-depth.

@akien-mga akien-mga modified the milestones: 3.0, 3.1 Jan 6, 2018
@reduz
Copy link
Member

reduz commented Sep 5, 2018

@karroffel news?

@quendera
Copy link
Contributor Author

quendera commented Sep 5, 2018

Haven't touched GDNative in a while.

Would it help to try and see if the error persists on the current master/3.1-alpha?

@akien-mga
Copy link
Member

Would it help to try and see if the error persists on the current master/3.1-alpha?

Definitely, that would be very welcome.

@akien-mga
Copy link
Member

Can anyone still reproduce this issue? @karroffel @BastiaanOlij

@akien-mga akien-mga modified the milestones: 3.1, 3.2 Jan 19, 2019
@akien-mga
Copy link
Member

Closing as there are no updates.

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

No branches or pull requests

5 participants