godotengine / godot-cpp

C++ bindings for the Godot script API
MIT License
1.74k stars 576 forks source link

The CharString destructor causes a crash in release exports only #805

Open Kyrio opened 2 years ago

Kyrio commented 2 years ago

Godot version

v4.0.alpha.custom_build [7355dfb] godot-cpp master [8ba1c05]

System information

Linux 64-bit (Solus), also reproduced on Windows 10 64-bit Export templates: Linux x64 release, Windows x64 release

Issue description

A crash occurs in my GDExtension exclusively in release exports. To obtain a stack trace I built my own export template with target=release debug_symbols=yes and the debug build of my gdextension. The attached project is equivalent to my setup and reproduces the issue.

Here is a dummy function that produces the crash:

int ExampleRequest::request_list(const String &url) {
    const char *c_url = url.utf8().get_data();
    return url.length();
}

And here is the stack trace:

#0  0x00007ffff7ad087b in pthread_kill () from /usr/lib/haswell/libc.so.6
#1  0x00007ffff7a7a7a6 in raise () from /usr/lib/haswell/libc.so.6
#2  0x00007ffff7a617f2 in abort () from /usr/lib/haswell/libc.so.6
#3  0x00007ffff7ac3540 in ?? () from /usr/lib/haswell/libc.so.6
#4  0x00007ffff7ada83a in ?? () from /usr/lib/haswell/libc.so.6
#5  0x00007ffff7adc944 in ?? () from /usr/lib/haswell/libc.so.6
#6  0x00007ffff7adf193 in free () from /usr/lib/haswell/libc.so.6
#7  0x00007fffe7a70b4b in godot::Memory::free_static (p_ptr=<optimized out>)
    at /home/kyrio/Projects/Bugs/CharString/extensions/godot-cpp/src/core/memory.cpp:46
#8  0x00007fffe7a78350 in godot::memdelete_arr<char const> (p_class=<optimized out>)
    at /home/kyrio/Projects/Bugs/CharString/extensions/godot-cpp/include/godot_cpp/core/memory.hpp:172
#9  0x00007fffe7a78029 in godot::CharString::~CharString (this=this@entry=0x7fffffffd820, 
    __in_chrg=<optimized out>)
    at /home/kyrio/Projects/Bugs/CharString/extensions/godot-cpp/src/variant/char_string.cpp:67
#10 0x00007fffe7a6daa2 in ExampleRequest::request_list (this=<optimized out>, url=...)
    at src/example_request.cpp:17

Steps to reproduce

  1. Build the gdextension using scons target=debug
  2. Open the Godot editor and export in release mode
  3. Start the build and notice it immediately crashing
  4. Start it with gdb or equivalent to obtain a stack trace (if your release template has debug symbols)

Minimal reproduction project

CharString.zip

Zylann commented 2 years ago

What happens if you change int ExampleRequest::request_list(const String &url) to use String url? (without the const and without the &)

Other note (but likely not cause of the crash), in the dummy function, c_url is a dangling pointer because the CharString owning it is allocated as a temporary, which will get destroyed after url.utf8().get_data(); is evaluated.

Kyrio commented 2 years ago

Sorry for not being able to answer. Since I opened this issue, many versions came out including beta1 and I figured it would be wise to reproduce it on the latest. But so far I haven't been able to use a GDExtension without the game or the editor crashing in beta1, so testing has been difficult 😕

I will update as soon as I get my project working.

Zylann commented 2 years ago

Actually it might be caused by https://github.com/godotengine/godot-cpp/issues/842

laingawbl commented 1 year ago

For the record, I have a function

inline std::string gd_to_std(const godot::String &src) {
  return std::string{src.ascii().get_data()};
}

that is also only causing crashes on release builds. Changing the signature to std::string(godot::String) (dropping ref and qualifiers) did not change this behaviour. I hope #842 fixes it!

If I instead use

inline std::string gd_to_std(const godot::String &src) {
  const auto buf = src.to_ascii_buffer();
  return std::string{buf.ptr(), buf.ptr() + buf.size()};
}

this sidesteps creating or destroying a CharString (as buf is a PackedByteArray) and works fine. I'm only dealing with ASCII strings so I don't care too much about to_ascii_buffer dropping characters.

(... uh, is there a better way to convert GD to stdlib strings...?)