Calling godot::String::ascii() or utf8() causes memory corruption if the string originated from GDScript.

[norton-corbett]

Godot version:
3.2-stable. Also tried with 3.2 built locally (23b553b)

OS/device including version:
Windows 10

Issue description:
Passing a String into a GDNative c++ plugin and then simply calling ascii() on it and doing nothing else seems to cause some kind of memory corruption (WinDbg will catch it https://docs.microsoft.com/en-us/windows-hardware/drivers/debugger/debugger-download-tools).

Steps to reproduce:

gdscript

var s = "test"

widget.test(s)

c++

class Widget: public godot::Node
{
	GODOT_CLASS(Widget, godot::Node);

public:

	static void _register_methods();
	void _init();

	// etc etc...

	void test(godot::String s)
	{
		s.ascii(); // Causes memory corruption (in CharString destructor?)
	}
};

utf8() has the same problem. Also happens if the string belongs to a godot::Dictionary. If the string was instantiated from GDNative then these functions seem fine.

The fun of this is that godot will probably crash somewhere completely unrelated, or might not crash at all. Running through WinDbg seems to catch the problem early but the stack trace it gives you will vary depending on what your project is doing and which build of Godot you use.

Example stack trace from WinDbg:

The few clues I picked up during my hair-pulling hell day lead me to think the problem lies somewhere in the CharString destructor but I've now found a workaround so I officially don't care.

Workarounds:

1. Call to_ascii() in GDScript and pass in a PoolByteArray instead. Then convert it back to a string in GDNative.
2. Or write your own std::string to_ascii(String s)

[akien-mga]

CC @bruvzg who's been working on this code recently with #40999.

[bruvzg]

It's probably GDNative C++ wrapper bug (something similar to the reason for TO_WCSTR in the #40999, CharString get freed before returning?) C++ wrapper ascii and utf8 code seems to be OK, so it should be issue with variant call.

With #40999, there are tests for String and GDNative C ascii and utf8, all passing, but these function were not changed much, and most likely were OK before.

[colugomusic]

Can confirm this is still an issue in 3.3.3 stable. A single call to String::utf8 will totally mess up the heap. For me it occurs regardless of where the String originated, and usually only in debug mode.

While digging I found that String::alloc_c_string also converts to utf8 and does not cause appear to cause any corruption, so I think there is a problem with godot-cpp's CharString class, though it seems to just be a thin wrapper around godot_char_string.

A workaround if you need String::utf8():

#include <GodotGlobal.hpp>

...

godot::String my_string;

...

const auto utf8_string = my_string.alloc_c_string();

// Do something with utf8_string

godot::api->godot_free(utf8_string);

[furrykef]

Is this still an issue in 3.5?

[colugomusic]

Probably yes, since the godot-cpp String implementation has not changed in any meaningful way since early 2021.

[colugomusic]

To be honest godot-cpp's CharString class is extremely poorly designed and error prone. I never actually looked at it properly before but I'm looking at it now and it is easy to see the problems.

String.hpp

class CharString {
	friend class String;

	godot_char_string _char_string;

public:
	~CharString();

	int length() const;
	const char *get_data() const;
};

...

class String {

...

CharString utf8() const;
CharString ascii(bool p_extended = false) const;

...

};

String.cpp

godot::CharString::~CharString() {
	godot::api->godot_char_string_destroy(&_char_string);
}

int godot::CharString::length() const {
	return godot::api->godot_char_string_length(&_char_string);
}

const char *godot::CharString::get_data() const {
	return godot::api->godot_char_string_get_data(&_char_string);
}

As you can see utf8() and ascii() return a CharString object by value. If you take a copy of that object you will get a double-free. If you call get_data() and pass the result to a function inline, e.g. foobar(string.ascii().get_data()) then you will get a use-after-free bug. This is probably the source of the reported problems.

[furrykef]

I'm honestly a little shocked that so far the developers haven't considered this a showstopper. If you ask me, "If you call this public API function, your code will crash" should stop any release dead in its tracks.

[minosvasilias]

+1 on this being extremely important to fix.

utf8().to_data() is an extremely common call when developing GDNative or iOS plugins, and some puzzling iOS crashes i have experienced in the past only made sense to me now, months later, as i encountered a more easily traceable occurrence of this and came across this issue.

If anyone else is dealing with this in iOS plugins, where the alloc_c_string method noted by @colugomusic above does not appear in the provided headers, take a look at the implementation of that method here (godot-cpp -> src/core/String.cpp) in order to replicate it in your environment. This does indeed appear to work around the problem from what i can tell.

[vaporvee]

@colugomusic

To be honest godot-cpp's CharString class is extremely poorly designed and error prone. I never actually looked at it properly before but I'm looking at it now and it is easy to see the problems.

String.hpp

class CharString {
	friend class String;

	godot_char_string _char_string;

public:
	~CharString();

	int length() const;
	const char *get_data() const;
};

...

class String {

...

CharString utf8() const;
CharString ascii(bool p_extended = false) const;

...

};

String.cpp

godot::CharString::~CharString() {
	godot::api->godot_char_string_destroy(&_char_string);
}

int godot::CharString::length() const {
	return godot::api->godot_char_string_length(&_char_string);
}

const char *godot::CharString::get_data() const {
	return godot::api->godot_char_string_get_data(&_char_string);
}

As you can see utf8() and ascii() return a CharString object by value. If you take a copy of that object you will get a double-free. If you call get_data() and pass the result to a function inline, e.g. foobar(string.ascii().get_data()) then you will get a use-after-free bug. This is probably the source of the reported problems.

hey do you maybe know how to pull this off in godot 4 gdextension?