godotengine / godot

Godot Engine – Multi-platform 2D and 3D game engine
https://godotengine.org
MIT License
90.78k stars 21.13k forks source link

Some projects with missing gdscript files cause editor crash in GDScriptParserRef #73555

Closed spellcard199 closed 1 year ago

spellcard199 commented 1 year ago

Godot version

4.0 dev (e9c7b8d2246bd0797af100808419c994fa43a9d2)

System information

Linux Fedora, Vulkan API 1.3.205 - Forward+ - Using Vulkan Device 1: NVIDIA - NVIDIA GeForce 940MX

Issue description

Expected behavior: if some gdscript files in a project are missing Godot should show an error, not crash.

Backtrace:

================================================================
handle_crash: Program crashed with signal 11
Engine version: Godot Engine v4.0.rc.custom_build (e9c7b8d2246bd0797af100808419c994fa43a9d2)
Dumping the backtrace. Please include this when reporting the bug to the project developer.
[1] /lib64/libc.so.6(+0x54dc0) [0x7f54aeb70dc0] (??:0)
[2] GDScriptParserRef::raise_status(GDScriptParserRef::Status) (/media/Data/godot/modules/gdscript/gdscript_cache.cpp:61)
[3] GDScriptAnalyzer::resolve_datatype(GDScriptParser::TypeNode*) (/media/Data/godot/modules/gdscript/gdscript_analyzer.cpp:628)
[4] GDScriptAnalyzer::resolve_assignable(GDScriptParser::AssignableNode*, char const*) (/media/Data/godot/modules/gdscript/gdscript_analyzer.cpp:1661)
[5] GDScriptAnalyzer::resolve_parameter(GDScriptParser::ParameterNode*) (/media/Data/godot/modules/gdscript/gdscript_analyzer.cpp:1784)
[6] GDScriptAnalyzer::resolve_function_signature(GDScriptParser::FunctionNode*, GDScriptParser::Node const*, bool) (/media/Data/godot/modules/gdscript/gdscript_analyzer.cpp:1451)
[7] GDScriptAnalyzer::resolve_class_member(GDScriptParser::ClassNode*, int, GDScriptParser::Node const*) (/media/Data/godot/modules/gdscript/gdscript_analyzer.cpp:957)
[8] GDScriptAnalyzer::resolve_class_interface(GDScriptParser::ClassNode*, GDScriptParser::Node const*) (/media/Data/godot/modules/gdscript/gdscript_analyzer.cpp:1064)
[9] GDScriptAnalyzer::resolve_class_interface(GDScriptParser::ClassNode*, bool) (/media/Data/godot/modules/gdscript/gdscript_analyzer.cpp:1072)
[10] GDScriptAnalyzer::resolve_interface() (/media/Data/godot/modules/gdscript/gdscript_analyzer.cpp:4996)
[11] GDScriptAnalyzer::analyze() (/media/Data/godot/modules/gdscript/gdscript_analyzer.cpp:5031)
[12] GDScript::reload(bool) (/media/Data/godot/modules/gdscript/gdscript.cpp:874)
[13] GDScriptCache::get_full_script(String const&, Error&, String const&, bool) (/media/Data/godot/modules/gdscript/gdscript_cache.cpp:301)
[14] ResourceFormatLoaderGDScript::load(String const&, String const&, Error*, bool, float*, ResourceFormatLoader::CacheMode) (/media/Data/godot/modules/gdscript/gdscript.cpp:2641)
[15] ResourceLoader::_load(String const&, String const&, String const&, ResourceFormatLoader::CacheMode, Error*, bool, float*) (/media/Data/godot/core/io/resource_loader.cpp:213)
[16] ResourceLoader::_thread_load_function(void*) (/media/Data/godot/core/io/resource_loader.cpp:240)
[17] ResourceLoader::load(String const&, String const&, ResourceFormatLoader::CacheMode, Error*) (/media/Data/godot/core/io/resource_loader.cpp:568)
[18] EditorFileSystem::_update_script_classes() (/media/Data/godot/editor/editor_file_system.cpp:1580)
[19] EditorFileSystem::_update_pending_script_classes() (/media/Data/godot/editor/editor_file_system.cpp:1611)
[20] EditorFileSystem::update_file(String const&) (/media/Data/godot/editor/editor_file_system.cpp:1720)
[21] EditorNode::_resource_saved(Ref<Resource>, String const&) (??:?)
[22] ResourceSaver::save(Ref<Resource> const&, String const&, unsigned int) (/media/Data/godot/core/io/resource_saver.cpp:146)
[23] ResourceImporterCSVTranslation::import(String const&, String const&, HashMap<StringName, Variant, HashMapHasherDefault, HashMapComparatorDefault<StringName>, DefaultTypedAllocator<HashMapElement<StringName, Variant> > > const&, List<String, DefaultAllocator>*, List<String, DefaultAllocator>*, Variant*) (/media/Data/godot/editor/import/resource_importer_csv_translation.cpp:134)
[24] EditorFileSystem::_reimport_file(String const&, HashMap<StringName, Variant, HashMapHasherDefault, HashMapComparatorDefault<StringName>, DefaultTypedAllocator<HashMapElement<StringName, Variant> > > const&, String const&, Variant*) (/media/Data/godot/editor/editor_file_system.cpp:2029)
[25] EditorFileSystem::reimport_files(Vector<String> const&) (/media/Data/godot/editor/editor_file_system.cpp:2296)
[26] EditorFileSystem::_update_scan_actions() (/media/Data/godot/editor/editor_file_system.cpp:689)
[27] EditorFileSystem::_notification(int) (/media/Data/godot/editor/editor_file_system.cpp:1279)
[28] EditorFileSystem::_notificationv(int, bool) (/media/Data/godot/editor/editor_file_system.h:146)
[29] Object::notification(int, bool) (/media/Data/godot/core/object/object.cpp:790)
[30] SceneTree::_notify_group_pause(StringName const&, int) (/media/Data/godot/scene/main/scene_tree.cpp:874)
[31] SceneTree::process(double) (/media/Data/godot/scene/main/scene_tree.cpp:466)
[32] Main::iteration() (/media/Data/godot/main/main.cpp:3158)
[33] OS_LinuxBSD::run() (/media/Data/godot/platform/linuxbsd/os_linuxbsd.cpp:880)
[34] ./bin/godot.linuxbsd.editor.dev.x86_64.llvm(main+0x1f6) [0x21cf7e6] (/media/Data/godot/platform/linuxbsd/godot_linuxbsd.cpp:73)
[35] /lib64/libc.so.6(+0x3feb0) [0x7f54aeb5beb0] (??:0)
[36] /lib64/libc.so.6(__libc_start_main+0x80) [0x7f54aeb5bf60] (??:0)
[37] ./bin/godot.linuxbsd.editor.dev.x86_64.llvm(_start+0x25) [0x21cf525] (??:?)
-- END OF BACKTRACE --
================================================================

Seems like the crash happens at modules/gdscript/gdscript_cache.cpp#L61, in the first line of Error GDScriptParserRef::raise_status(Status p_new_status):

    Error GDScriptParserRef::raise_status(Status p_new_status) {
        ERR_FAIL_COND_V(parser == nullptr, ERR_INVALID_DATA);

I have an hypothesis on why it's happening, however this is my first time using gdb, so this is only a C++ beginner's speculation.

  1. At modules/gdscript/gdscript_analyzer.cpp#L627, get_parser_for(autoload.path) returns an invalid reference ref.
  2. In the following line ref->raise_status(GDScriptParserRef::INHERITANCE_SOLVED) is called:
    • At modules/gdscript/gdscript_cache.cpp#L61, the first line of Error GDScriptParserRef::raise_status(Status p_new_status) tries to access its parser field, but accessing a field of an invalid reference results in crash.

The reason why Ref<GDScriptParserRef> GDScriptAnalyzer::get_parser_for(autoload.path) returned an invalid reference may be because:

  1. At modules/gdscript/gdscript_analyzer.cpp#L4982, GDScriptAnalyzer::get_parser_for invokes GDScriptCache::get_parser.
  2. At modules/gdscript/gdscript_cache.cpp#L207, inside GDScriptCache::get_parser:

    1. ref is created:

      Ref<GDScriptParserRef> ref;
    2. In the code path where p_path doesn't exist, ref is never instantiated:

      if (!FileAccess::exists(p_path)) {
         r_error = ERR_FILE_NOT_FOUND;
         return ref;
      }

I tried adding the following line inside the if block and the crash stopped happening, but I didn't make a PR because I don't know if that's how ref is supposed to be used, nor if it's the right place to do it.

    if (!FileAccess::exists(p_path)) {
        ref.instantiate();
        r_error = ERR_FILE_NOT_FOUND;
        return ref;
    }

Steps to reproduce

I wasn't able to reduce it to a minimal project, but this is how I happened to trigger it:

  1. Clone project (the correct command would be git clone --recurse-submodules "https://github.com/V-Sekai/v-sekai-game.git", but we are trying to trigger the crash here)

    git clone "https://github.com/V-Sekai/v-sekai-game.git"
    cd "v-sekai-game"
    git checkout 90286e266e4ff8d6a35bdf0597593b11108425a4
  2. Open project in Godot editor.

  3. Click OK on warning about double precision causing loss of information.

  4. Wait for crash.

Minimal reproduction project

N/A

vnen commented 1 year ago

I believe this was fixed by #73679. Let me know if it still happens in RC4 or later

spellcard199 commented 1 year ago

I believe this was fixed by #73679. Let me know if it still happens in RC4 or later

I can confirm it's fixed in RC4. Thanks.