Blender imports will always start within `.godot/imported` folder because we first convert the .blend file to .gltf, store it in `.godot/imported` and run the import from there, so on-disk resources linked from .blend files end up with duplicate textures.
This fixes UBSAN errors reported by running our testsuite, importing the
TPS demo, and running the TPS demo. I have tried, wherever possible, to
fix issues related to reported issues but not directly reported by UBSAN
because thse code paths just happened to not have been exercised in
these cases.
These fixes apply only to errors reported, and caused by, core/
The following things have been changed:
* Make sure there are no implicit sign changing casts in core.
* Explicitly type enums that are part of a public API such that users of
the API cannot pass in wrongly-sized values leading to potential stack
corruption.
* Ensure that memcpy is never called with invalid or null pointers as
this is undefined behavior, and when the engine is built with
optimizations turned on leads to memory corruption and hard to debug
crashes.
* Replace enum values only used as static values with constexpr static
const values instead. This has no runtime overhead. This makes it so
that the size of the enums is explicit.
* Make sure that nan and inf is handled consistently in String.
* Implement a _to_int template to ensure that all of the paths use the
same algorhithm, and correct the negative integer case.
* Changed the way the json serializer precision work, and added tests to
verify the new behavior. The behavior doesn't quite match master in
particulary for negative doubles as the original code tried to cast -inf
to an int. This then led to negative doubles losing all but one of
their decimal points when serializing. Behavior in GDScript remains
unchanged.
Basically, when we first encounter the document, we parse it as before, but we also note the offsets of other glyphs and store the remaining XML. The next time we see another glyph, we can simply parse that glyph node and insert it back into the stored XML.
- Fix crashing bug when invoking class constructor with parameters
- Add support for accessing class constants
- Add support for Godot Callable arguments. A Godot Callable can be wrapped by a Java Runnable to allow Java logic to run arbitrary Godot lambdas
- Automatically convert java.lang.CharSequence to Godot String as needed
- Code cleanup
My friends, gather around as I learned something about the C standard
that is horrifying and may keep you, dear reader, up at night.
My journey began trying to fix something entirely unrelated and not
wanting to wait for ubsan builds when changing a testcase. So me, in my
infinite naivete just built the engine with tests=yes, but
optimizations turned on.
This resulted in a segfault on "[Audio][AudioStreamWAV] Save empty file".
Well, then, I thought. Lets built with asan then and find out where this
happens! Would it surprise you, my fellow traveler, that the results
were that no such crash occurred?
Thus, to the debugger I go! Fearless, with great optimism. Where I find
that through many an indirection the crash came because, somehow,
CowData::_unref() was getting called with a _ptr set to 0x1.
This can of course only end in tears. Or segmentation faults as we try
to read an atomic variable at the somewhat inconveniently situated
address at 0xfffffffffffffff0.
So I went and looked at the likely culprit, blaming many an innocent
recent change along the way. I shall spare you the falsly accused. But
if for some reason you slept poorly last night, I can assure you that
the voodoo dolls have been put away and will not be harmed further.
So in AudioStreamWAV::get_data() we go, where we find a perfectly
reasonable function! It checks to see whether or not its data is empty,
and if it is not it will resize a temporary Vector to have data_bytes of
space, after which it will do a perfectly pedestrian memcpy() and all is
well in the world.
Or so it seems! After many an hour of despair and disassembly I, at
last, decided to look at where the data gets set! A breakthrough!
Because of the padding data is never empty! So the code always runs!
Eureka! One would think. But then, foolishly, I looked into the
get_data() function one more. My mortal enemy was staring me in the
face, laughing. Because it did not care about this. Sure, the check was
worthless but still... What are we left with.
At this point I could feel the method mocking me.
"I resize the vector to 0, I then memcpy zero bytes into it." It said,
DARING me to object to this state of affairs.
And yet, if I changed the function to check for "data_bytes" rather
than data.is_empty() no crashes.
Was this a compiler bug? Am I losing my mind? But then... I remembered
the mantra of the wise compiler druids... "It Is Not A Compiler Bug".
But what then! The bug does not happen when memory is being watched!
Valgrind agreed that while accessing the SafeRefCount at
0xfffffffffffffff0 was incredibly rude, it did not inform me of anything
else untoward happening. So I read the memcpy() manpage... nothing... I
read the the memcpy() posix spec... nothing.
Finally, in despair and because I had nothing left to lose... The ISO C
language specification. As I was reading, I could hear
AudioStreamWAV::get_data() cackling, knowing that its time was up, but
proud of the madness it caused in my soul. Knowing I would never be the
same.
The behavior is undefined if either dest or src is an invalid or null pointer.
So... Here I stand before you, a broken person. But one richer in
knowledge.
I write you this from the depths of madness in the hopes that you, dear
reader, can be spared this ordeal.
May god have mercy on our souls.
We trigger the following sequence of events:
* memcpy(null, null, 0) is UB, thus dest and src cannot be null
* we inline the calls to the ctor and dtor
* now we have a function that does something that "proves"
dest cannot be null
* we inline cowdata::_unref() which does a null check, on something
that the compiler just convinced itself cannot be null
* the compiler removes the dead code branch where _ptr == nullptr
* we start to do pointer arithmetic on a nullptr and get send to
uninitialized memory.
Co-Authored-By: Jason Beckmann <jasonabeckmann@gmail.com>
