Almost all `operator[](int64_t p_index)` can return null pointer reference

[NoctemCat]

Godot version

master

godot-cpp version

master

System information

linux

Issue description

Seems like godot-cpp relied on a native error logging before, but it was removed in godotengine/godot#66185 , and instead index operators now silently return nullptr. godot-cpp doesn't check for it and can silently dereference it, so the next time you access it, it will crash. I think this affects all arrays, both normal and packed. And also String... dictionary seems fine so far.

I guess this just needs a better error message and/or crash at this place specifically. As it's really unexpected to silently return invalid reference to nullptr, because this in itself won't cause a crash, but the next access to any of its member will.

Steps to reproduce

Use [] to access any element outside of valid range.

Minimal reproduction project

N/A

[dsnopek]

Is this different than using operator [] on the equivalent classes within Godot itself? If it is, then we should change to match the behavior within Godot. But if this already matches, then I think we should leave it as-is

[Ivorforce]

In Godot itself, using operator[] outside of bounds crashes the whole program.
Using operator[] on user-facing collections (like Array) uses ERR_FAIL_V(Variant()) (more or less).

[NoctemCat]

Yeah, I think crash on packed arrays will be fine... Well it crashes, but the problem is that it crashes in the wrong place with segfault instead of crashing with a nice error message that you get with CRASH_BAD_INDEX.

And when I say in the wrong place, it crashes when trying to access any member of that reference, and not when getting the reference itself.

I guess then String and Array should return fail? Accessible from C++ in engine Array also crash on [], I guess the error handling happens inside connections to the user-facing part. StringName forwards it to String, and it... when p_index == _cowdata.size() it returns a reference to constexpr char32_t _null = 0? Otherwise it also forwards it to cowdata that can crash.

Ah, right StringName is also missing this operator, I guess it can also forward it to String, or just not bother with it

[NoctemCat]

Huh... As always C++ is full of strange behaviours

godotengine/godot#21708

Hmm... return &self->ptrw()[p_index]; looks like gdextension interface for String doesn't take it into account on the engine side as the ref to 0 is explicitly handled by operator[] inside the String, but it tries to get it from the cowdata.

But yeah, I kind of forgot about strings ending on null, but the way null is stored outside of cowdata is strange