[3.x] Changing the class a GDScript file extends crashes the engine.

[Rubonnek]

Godot version

3.x branch at 0a7c6c0

System information

Arch Linux

Issue description

Changing the class a GDScript file extends crashes the engine.

Steps to reproduce

1. Create a new project with a single Node2D
2. Add the following script to the node:

extends Node2D

func _ready() -> void:
	pass

3. Close the script.
4. Open the file with an external editor and have the external editor connect an LSP client to Godot. (Vim ALE was used in this case).
5. Change the class the script extends to Reference, and save the file.
6. Godot should crash with an illegal instruction due to CRASH_BAD_INDEX triggered by ExtendGDScriptParser::parse_documentation

Here is the backtrace:

(gdb) bt
#0  0x0000000000b1edef in CowData<String>::get (p_index=1, this=0x7fff115990b0) at ./core/cowdata.h:156
#1  Vector<String>::operator[] (p_index=1, this=0x7fff115990a8) at ./core/vector.h:87
#2  ExtendGDScriptParser::parse_documentation (this=0x7fff11598f90, p_line=0, p_docs_down=true) at modules/gdscript/language_server/gdscript_extend_parser.cpp:412
#3  0x0000000000b18061 in ExtendGDScriptParser::parse_class_symbol (this=0x7fff11598f90, p_class=0x7fff1158bd10, r_symbol=...) at modules/gdscript/language_server/gdscript_extend_parser.cpp:159
#4  0x0000000000b170c1 in ExtendGDScriptParser::update_symbols (this=0x7fff11598f90) at modules/gdscript/language_server/gdscript_extend_parser.cpp:87
#5  0x0000000000b2976e in ExtendGDScriptParser::parse (this=0x7fff11598f90, p_code=..., p_path=...) at modules/gdscript/language_server/gdscript_extend_parser.cpp:788
#6  0x0000000000716abe in GDScriptWorkspace::parse_script (this=0xc7d9a70, p_path=..., p_content=...) at modules/gdscript/language_server/gdscript_workspace.cpp:420
#7  0x00000000007123a6 in GDScriptWorkspace::reload_all_workspace_scripts (this=0xc7d9a70) at modules/gdscript/language_server/gdscript_workspace.cpp:205
#8  0x000000000071605a in GDScriptWorkspace::initialize (this=0xc7d9a70) at modules/gdscript/language_server/gdscript_workspace.cpp:393
#9  0x00000000006e27c2 in GDScriptLanguageProtocol::initialize (this=0x8f0cdd8, p_params=...) at modules/gdscript/language_server/gdscript_language_protocol.cpp:203
#10 0x00000000006f2f31 in MethodBind1R<Dictionary, Dictionary const&>::call (this=0xc7d6030, p_object=0x8f0cdd8, p_args=0x7fff1d7f9750, p_arg_count=1, r_error=...) at ./core/method_bind.gen.inc:961
#11 0x000000000474b606 in Object::call (this=0x8f0cdd8, p_method=..., p_args=0x7fff1d7f9750, p_argcount=1, r_error=...) at core/object.cpp:918
#12 0x000000000474ae2c in Object::callv (this=0x8f0cdd8, p_method=..., p_args=...) at core/object.cpp:827
#13 0x000000000074257e in JSONRPC::process_action (this=0x8f0cdd8, p_action=..., p_process_arr_elements=true) at modules/jsonrpc/jsonrpc.cpp:129
#14 0x0000000000742f83 in JSONRPC::process_string (this=0x8f0cdd8, p_input=...) at modules/jsonrpc/jsonrpc.cpp:165
#15 0x00000000006e056a in GDScriptLanguageProtocol::process_message (this=0x8f0cdd8, p_text=...) at modules/gdscript/language_server/gdscript_language_protocol.cpp:141
#16 0x00000000006dfc20 in GDScriptLanguageProtocol::LSPeer::handle_data (this=0x7fff100011d0) at modules/gdscript/language_server/gdscript_language_protocol.cpp:96
#17 0x00000000006e4214 in GDScriptLanguageProtocol::poll (this=0x8f0cdd8) at modules/gdscript/language_server/gdscript_language_protocol.cpp:242
#18 0x00000000006f4f82 in GDScriptLanguageServer::thread_main (p_userdata=0x8f0cbb0) at modules/gdscript/language_server/gdscript_language_server.cpp:81
#19 0x000000000490e967 in Thread::callback (p_self=0x8f0cf30, p_settings=..., p_callback=0x6f4f5a <GDScriptLanguageServer::thread_main(void*)>, p_userdata=0x8f0cbb0) at core/os/thread.cpp:79
#20 0x000000000490f8af in std::__invoke_impl<void, void (*)(Thread*, Thread::Settings const&, void (*)(void*), void*), Thread*, Thread::Settings, void (*)(void*), void*> (__f=@0xd138448: 0x490e8ca <Thread::callback(Thread*, Thread::Settings const&, void (*)(void*), void*)>) at /usr/include/c++/11.1.0/bits/invoke.h:61
#21 0x000000000490f732 in std::__invoke<void (*)(Thread*, Thread::Settings const&, void (*)(void*), void*), Thread*, Thread::Settings, void (*)(void*), void*> (__fn=@0xd138448: 0x490e8ca <Thread::callback(Thread*, Thread::Settings const&, void (*)(void*), void*)>) at /usr/include/c++/11.1.0/bits/invoke.h:96
#22 0x000000000490f5ed in std::thread::_Invoker<std::tuple<void (*)(Thread*, Thread::Settings const&, void (*)(void*), void*), Thread*, Thread::Settings, void (*)(void*), void*> >::_M_invoke<0ul, 1ul, 2ul, 3ul, 4ul> (this=0xd138428) at /usr/include/c++/11.1.0/bits/std_thread.h:253
#23 0x000000000490f552 in std::thread::_Invoker<std::tuple<void (*)(Thread*, Thread::Settings const&, void (*)(void*), void*), Thread*, Thread::Settings, void (*)(void*), void*> >::operator() (this=0xd138428) at /usr/include/c++/11.1.0/bits/std_thread.h:260
#24 0x000000000490f536 in std::thread::_State_impl<std::thread::_Invoker<std::tuple<void (*)(Thread*, Thread::Settings const&, void (*)(void*), void*), Thread*, Thread::Settings, void (*)(void*), void*> > >::_M_run (this=0xd138420) at /usr/include/c++/11.1.0/bits/std_thread.h:211
#25 0x0000000004e2a304 in execute_native_thread_routine ()
#26 0x00007ffff7d68259 in start_thread () from /usr/lib/libpthread.so.0
#27 0x00007ffff7b465e3 in clone () from /usr/lib/libc.so.6

Edit: For the steps outlined above, here is the Vim ALE LSP interaction with the LSP server. Here is the full backtrace.

Minimal reproduction project

A brand new project will do.

[Rubonnek]

Caused by #53094.
CC @Razoric480

[Razoric480]

I'm unable to recreate this issue in Windows using VSCode. That's quite strange. I'll investigate the stacktrace and see if I can pull some clues out of it.

[akien-mga]

The crash likely means that lines[MAX(0, i + step)] can be indexing out of bounds (i.e. i + step can be bigger than lines.size()).

[Rubonnek]

The crash likely means that lines[MAX(0, i + step)] can be indexing out of bounds (i.e. i + step can be bigger than lines.size()).

Correct. In the backtrace above, MAX(0, i + step) returns 1, but lines.size() is 0.

[Rubonnek]

Tested with VSCode but in Linux instead, and I'm unable to reproduce this issue with it. There's something strange about how Vim ALE is interacting with the LSP server.

Dumping the full client-server interaction with ALE does not seem to be straightforward.

@Razoric480 is there an easy way to dump the interaction of the LSP client with the server through Godot?

[Rubonnek]

With #53094 reverted, the following messages also show up from time to time with Vim ALE.

ERROR: Index p_position.line = 1304 is out of bounds (lines.size() = 1).
   at: get_identifier_under_position (modules/gdscript/language_server/gdscript_extend_parser.cpp:482)
ERROR: Index p_position.line = 1304 is out of bounds (lines.size() = 1).
   at: get_identifier_under_position (modules/gdscript/language_server/gdscript_extend_parser.cpp:482)
ERROR: Index p_position.line = 1324 is out of bounds (lines.size() = 1).
   at: get_identifier_under_position (modules/gdscript/language_server/gdscript_extend_parser.cpp:482)
ERROR: Index p_position.line = 1324 is out of bounds (lines.size() = 1).
   at: get_identifier_under_position (modules/gdscript/language_server/gdscript_extend_parser.cpp:482)

[Rubonnek]

I added a print statement to line 113 of language_server/gdscript_language_protocol.cpp to print c_res:

godot/modules/gdscript/language_server/gdscript_language_protocol.cpp

Lines 104 to 113 in 0a7c6c0

	Error GDScriptLanguageProtocol::LSPeer::send_data() {
	int sent = 0;
	if (!res_queue.empty()) {
	CharString c_res = res_queue[0];
	if (res_sent < c_res.size()) {
	Error err = connection->put_partial_data((const uint8_t *)c_res.get_data() + res_sent, c_res.size() - res_sent - 1, sent);
	if (err != OK) {
	return err;
	}
	res_sent += sent;

And to line 141 in gdscript_language_protocol.cpp to print p_text:

godot/modules/gdscript/language_server/gdscript_language_protocol.cpp

Lines 140 to 141 in 0a7c6c0

	String GDScriptLanguageProtocol::process_message(const String &p_text) {
	String ret = process_string(p_text);

And this is the interaction -- Godot crashes sometime after the first GDScriptLanguageProtocol::process_message it seems:

GDScriptLanguageProtocol::process_message, {"method": "initialize", "jsonrpc": "2.0", "id": 1, "params": {"initializationOptions": {}, "rootUri": "project.godot", "capabilities": {"workspace": {"workspaceFolders": false, "configuration": false, "symbol": {"dynamicRegistration": false}, "applyEdit": false, "didChangeConfiguration": {"dynamicRegistration": false}}, "textDocument": {"documentSymbol": {"dynamicRegistration": false, "hierarchicalDocumentSymbolSupport": false}, "references": {"dynamicRegistration": false}, "publishDiagnostics": {"relatedInformation": true}, "rename": {"dynamicRegistration": false}, "completion": {"completionItem": {"snippetSupport": false, "commitCharactersSupport": false, "preselectSupport": false, "deprecatedSupport": false, "documentationFormat": ["plaintext"]}, "contextSupport": false, "dynamicRegistration": false}, "synchronization": {"didSave": true, "willSaveWaitUntil": false, "willSave": false, "dynamicRegistration": false}, "codeAction": {"dynamicRegistration": false}, "typeDefinition": {"dynamicRegistration": false}, "hover": {"dynamicRegistration": false, "contentFormat": ["plaintext"]}, "definition": {"dynamicRegistration": false, "linkSupport": false}}}, "rootPath": "project.godot", "processId": 7327}}
ERROR: FATAL: Index p_index = 1 is out of bounds (size() = 1).
   at: get (./core/cowdata.h:156)

Edit: Here's the full backtrace for the crash above which is reproduced in a different way. In one of my projects, I just open a specific file and Godot crashes instantly and that's the interaction above.

[Razoric480]

is there an easy way to dump the interaction of the LSP client with the server through Godot?

Not on Godot's side. The LSP is quite chatty. The VSCode client does it client-side when running through a debugger. Maybe I could add an option for debug-build-only on Godot.

"rootUri": "project.godot" ... "rootPath": "project.godot"

This seems strange. root_uri is used to build res:// paths throughout the workspace and when coming out of VSCode, is in the format file://, and root_path points to the project folder's location on the drive. If Vim ALE sends "project.godot" in both, I can well imagine that godot would crash.

Though that it doesn't crash all the time is also interesting, implying it's doing some translation that is keeping the server from noticing and tripping over it.

[Rubonnek]

Hmm... I'll find some time later today to properly fix my ALE configuration so it passes file://. My vimscript is quite rusty at the moment.

For completeness, here is the ALE LSP interaction using the original steps in the first post.

Here is the full backtrace for the crash of that interaction.

I also added these to the original post for convenience.

[Razoric480]

Doesn't seem to be the problem. I changed the code to

	String root_uri = "project.godot"; //p_params["rootUri"];
	String root = "project.godot"; //p_params["rootPath"];

and am encountering no crashes. Looks like the LSP detects the possibility of an invalid root/root_uri and just grabs the currently active project's root to configure the workspace.

I'll look over your logs and see if I can narrow it down.

[Razoric480]

I issued a fix that should at least prevent a crash, see #53261 if you could test it.

But I'd like the PR to also fix what you say in #53238 (comment)

[Rubonnek]

Nice. Will test and comment on the PR.

[akien-mga]

Fixed by #53261.