From 240f510fa786ee4af6a34ca9712a5d5d9745d637 Mon Sep 17 00:00:00 2001 From: HP van Braam Date: Sat, 14 Dec 2024 02:17:09 +0100 Subject: [PATCH] Core ubsan fixes 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. --- core/config/project_settings.cpp | 8 +- core/config/project_settings.h | 6 +- core/crypto/aes_context.h | 2 +- core/crypto/hashing_context.h | 2 +- core/input/input_enums.h | 4 + core/io/compression.h | 2 +- core/io/dir_access.h | 2 +- core/io/file_access.cpp | 10 +- core/io/file_access.h | 8 +- core/io/file_access_compressed.cpp | 8 +- core/io/file_access_encrypted.cpp | 16 +++- core/io/file_access_encrypted.h | 2 +- core/io/file_access_memory.cpp | 12 ++- core/io/image.h | 2 +- core/io/json.cpp | 18 ++-- core/io/marshalls.cpp | 8 +- core/io/net_socket.h | 4 +- core/io/pck_packer.cpp | 6 +- core/io/remote_filesystem_client.cpp | 2 +- core/io/resource_format_binary.cpp | 90 +++++++++--------- core/io/resource_importer.h | 2 +- core/io/resource_uid.cpp | 4 +- core/io/resource_uid.h | 4 +- core/io/stream_peer.cpp | 35 +++++-- core/math/bvh_pair.inc | 2 +- core/math/bvh_tree.h | 4 +- core/math/color.h | 4 +- core/math/random_pcg.cpp | 2 +- core/object/class_db.cpp | 2 +- core/object/object_id.h | 2 +- core/os/memory.cpp | 4 +- core/string/string_buffer.h | 2 +- core/string/ustring.cpp | 135 ++++++++++++--------------- core/templates/command_queue_mt.h | 4 +- core/templates/hashfuncs.h | 44 ++++----- core/templates/local_vector.h | 8 +- core/templates/vector.h | 7 +- core/variant/variant.cpp | 58 ++++++------ core/variant/variant_parser.cpp | 1 + tests/core/io/test_json.h | 85 +++++++++++++++++ 40 files changed, 373 insertions(+), 248 deletions(-) diff --git a/core/config/project_settings.cpp b/core/config/project_settings.cpp index 30cdd789a44..4a85b96f97b 100644 --- a/core/config/project_settings.cpp +++ b/core/config/project_settings.cpp @@ -898,7 +898,7 @@ Error ProjectSettings::_save_settings_binary(const String &p_file, const RBMapstore_32(count + 1); + file->store_32(uint32_t(count + 1)); String key = CoreStringName(_custom_features); file->store_pascal_string(key); @@ -911,12 +911,12 @@ Error ProjectSettings::_save_settings_binary(const String &p_file, const RBMapstore_32(len); + file->store_32(uint32_t(len)); file->store_buffer(buff.ptr(), buff.size()); } else { // Store how many properties are saved. - file->store_32(count); + file->store_32(uint32_t(count)); } for (const KeyValue> &E : p_props) { @@ -943,7 +943,7 @@ Error ProjectSettings::_save_settings_binary(const String &p_file, const RBMapstore_32(len); + file->store_32(uint32_t(len)); file->store_buffer(buff.ptr(), buff.size()); } } diff --git a/core/config/project_settings.h b/core/config/project_settings.h index 922c88c1513..72877526812 100644 --- a/core/config/project_settings.h +++ b/core/config/project_settings.h @@ -47,10 +47,8 @@ public: typedef HashMap CustomMap; static const String PROJECT_DATA_DIR_NAME_SUFFIX; - enum { - // Properties that are not for built in values begin from this value, so builtin ones are displayed first. - NO_BUILTIN_ORDER_BASE = 1 << 16 - }; + // Properties that are not for built in values begin from this value, so builtin ones are displayed first. + constexpr static const int32_t NO_BUILTIN_ORDER_BASE = 1 << 16; #ifdef TOOLS_ENABLED const static PackedStringArray get_required_features(); diff --git a/core/crypto/aes_context.h b/core/crypto/aes_context.h index f6aeab221f2..ab0792ac8be 100644 --- a/core/crypto/aes_context.h +++ b/core/crypto/aes_context.h @@ -38,7 +38,7 @@ class AESContext : public RefCounted { GDCLASS(AESContext, RefCounted); public: - enum Mode { + enum Mode : int32_t { MODE_ECB_ENCRYPT, MODE_ECB_DECRYPT, MODE_CBC_ENCRYPT, diff --git a/core/crypto/hashing_context.h b/core/crypto/hashing_context.h index ab7affabaa3..436afd837ee 100644 --- a/core/crypto/hashing_context.h +++ b/core/crypto/hashing_context.h @@ -37,7 +37,7 @@ class HashingContext : public RefCounted { GDCLASS(HashingContext, RefCounted); public: - enum HashType { + enum HashType : int32_t { HASH_MD5, HASH_SHA1, HASH_SHA256 diff --git a/core/input/input_enums.h b/core/input/input_enums.h index 7974ee639d2..182a5aabe54 100644 --- a/core/input/input_enums.h +++ b/core/input/input_enums.h @@ -31,6 +31,8 @@ #ifndef INPUT_ENUMS_H #define INPUT_ENUMS_H +#include "core/error/error_macros.h" + enum class HatDir { UP = 0, RIGHT = 1, @@ -131,6 +133,8 @@ enum class MouseButtonMask { }; inline MouseButtonMask mouse_button_to_mask(MouseButton button) { + ERR_FAIL_COND_V(button == MouseButton::NONE, MouseButtonMask::NONE); + return MouseButtonMask(1 << ((int)button - 1)); } diff --git a/core/io/compression.h b/core/io/compression.h index a5a2d657dad..ea56ab8eb2a 100644 --- a/core/io/compression.h +++ b/core/io/compression.h @@ -43,7 +43,7 @@ public: static int zstd_window_log_size; static int gzip_chunk; - enum Mode { + enum Mode : int32_t { MODE_FASTLZ, MODE_DEFLATE, MODE_ZSTD, diff --git a/core/io/dir_access.h b/core/io/dir_access.h index bdeb4324831..588e03774b4 100644 --- a/core/io/dir_access.h +++ b/core/io/dir_access.h @@ -40,7 +40,7 @@ class DirAccess : public RefCounted { GDCLASS(DirAccess, RefCounted); public: - enum AccessType { + enum AccessType : int32_t { ACCESS_RESOURCES, ACCESS_USERDATA, ACCESS_FILESYSTEM, diff --git a/core/io/file_access.cpp b/core/io/file_access.cpp index c04ad1cf495..230184ad04b 100644 --- a/core/io/file_access.cpp +++ b/core/io/file_access.cpp @@ -383,7 +383,7 @@ double FileAccess::get_double() const { String FileAccess::get_token() const { CharString token; - char32_t c = get_8(); + uint8_t c = get_8(); while (!eof_reached()) { if (c <= ' ') { @@ -391,7 +391,7 @@ String FileAccess::get_token() const { break; } } else { - token += c; + token += char(c); } c = get_8(); } @@ -448,14 +448,14 @@ public: String FileAccess::get_line() const { CharBuffer line; - char32_t c = get_8(); + uint8_t c = get_8(); while (!eof_reached()) { if (c == '\n' || c == '\0') { line.push_back(0); return String::utf8(line.get_data()); } else if (c != '\r') { - line.push_back(c); + line.push_back(char(c)); } c = get_8(); @@ -786,7 +786,7 @@ bool FileAccess::store_var(const Variant &p_var, bool p_full_objects) { err = encode_variant(p_var, &w[0], len, p_full_objects); ERR_FAIL_COND_V_MSG(err != OK, false, "Error when trying to encode Variant."); - return store_32(len) && store_buffer(buff); + return store_32(uint32_t(len)) && store_buffer(buff); } Vector FileAccess::get_file_as_bytes(const String &p_path, Error *r_error) { diff --git a/core/io/file_access.h b/core/io/file_access.h index da3cf0e28a8..f53f1f183a5 100644 --- a/core/io/file_access.h +++ b/core/io/file_access.h @@ -46,7 +46,7 @@ class FileAccess : public RefCounted { GDCLASS(FileAccess, RefCounted); public: - enum AccessType { + enum AccessType : int32_t { ACCESS_RESOURCES, ACCESS_USERDATA, ACCESS_FILESYSTEM, @@ -54,14 +54,14 @@ public: ACCESS_MAX }; - enum ModeFlags { + enum ModeFlags : int32_t { READ = 1, WRITE = 2, READ_WRITE = 3, WRITE_READ = 7, }; - enum UnixPermissionFlags { + enum UnixPermissionFlags : int32_t { UNIX_EXECUTE_OTHER = 0x001, UNIX_WRITE_OTHER = 0x002, UNIX_READ_OTHER = 0x004, @@ -76,7 +76,7 @@ public: UNIX_SET_USER_ID = 0x800, }; - enum CompressionMode { + enum CompressionMode : int32_t { COMPRESSION_FASTLZ = Compression::MODE_FASTLZ, COMPRESSION_DEFLATE = Compression::MODE_DEFLATE, COMPRESSION_ZSTD = Compression::MODE_ZSTD, diff --git a/core/io/file_access_compressed.cpp b/core/io/file_access_compressed.cpp index 53ef93d09b9..736896994fd 100644 --- a/core/io/file_access_compressed.cpp +++ b/core/io/file_access_compressed.cpp @@ -125,7 +125,7 @@ void FileAccessCompressed::_close() { f->store_buffer((const uint8_t *)mgc.get_data(), mgc.length()); //write header 4 f->store_32(cmode); //write compression mode 4 f->store_32(block_size); //write block size 4 - f->store_32(write_max); //max amount of data written 4 + f->store_32(uint32_t(write_max)); //max amount of data written 4 uint32_t bc = (write_max / block_size) + 1; for (uint32_t i = 0; i < bc; i++) { @@ -147,7 +147,7 @@ void FileAccessCompressed::_close() { f->seek(16); //ok write block sizes for (uint32_t i = 0; i < bc; i++) { - f->store_32(block_sizes[i]); + f->store_32(uint32_t(block_sizes[i])); } f->seek_end(); f->store_buffer((const uint8_t *)mgc.get_data(), mgc.length()); //magic at the end too @@ -310,7 +310,9 @@ bool FileAccessCompressed::store_buffer(const uint8_t *p_src, uint64_t p_length) write_ptr = buffer.ptrw(); } - memcpy(write_ptr + write_pos, p_src, p_length); + if (p_length) { + memcpy(write_ptr + write_pos, p_src, p_length); + } write_pos += p_length; return true; diff --git a/core/io/file_access_encrypted.cpp b/core/io/file_access_encrypted.cpp index c899c860c6d..de71e9f140c 100644 --- a/core/io/file_access_encrypted.cpp +++ b/core/io/file_access_encrypted.cpp @@ -210,10 +210,16 @@ bool FileAccessEncrypted::eof_reached() const { } uint64_t FileAccessEncrypted::get_buffer(uint8_t *p_dst, uint64_t p_length) const { - ERR_FAIL_COND_V(!p_dst && p_length > 0, -1); ERR_FAIL_COND_V_MSG(writing, -1, "File has not been opened in read mode."); + if (!p_length) { + return 0; + } + + ERR_FAIL_NULL_V(p_dst, -1); + uint64_t to_copy = MIN(p_length, get_length() - pos); + memcpy(p_dst, data.ptr() + pos, to_copy); pos += to_copy; @@ -230,7 +236,12 @@ Error FileAccessEncrypted::get_error() const { bool FileAccessEncrypted::store_buffer(const uint8_t *p_src, uint64_t p_length) { ERR_FAIL_COND_V_MSG(!writing, false, "File has not been opened in write mode."); - ERR_FAIL_COND_V(!p_src && p_length > 0, false); + + if (!p_length) { + return true; + } + + ERR_FAIL_NULL_V(p_src, false); if (pos + p_length >= get_length()) { ERR_FAIL_COND_V(data.resize(pos + p_length) != OK, false); @@ -238,6 +249,7 @@ bool FileAccessEncrypted::store_buffer(const uint8_t *p_src, uint64_t p_length) memcpy(data.ptrw() + pos, p_src, p_length); pos += p_length; + return true; } diff --git a/core/io/file_access_encrypted.h b/core/io/file_access_encrypted.h index a373fc0cb5d..570955b7835 100644 --- a/core/io/file_access_encrypted.h +++ b/core/io/file_access_encrypted.h @@ -37,7 +37,7 @@ class FileAccessEncrypted : public FileAccess { public: - enum Mode { + enum Mode : int32_t { MODE_READ, MODE_WRITE_AES256, MODE_MAX diff --git a/core/io/file_access_memory.cpp b/core/io/file_access_memory.cpp index b5ab18407b3..1977c87258b 100644 --- a/core/io/file_access_memory.cpp +++ b/core/io/file_access_memory.cpp @@ -123,7 +123,11 @@ bool FileAccessMemory::eof_reached() const { } uint64_t FileAccessMemory::get_buffer(uint8_t *p_dst, uint64_t p_length) const { - ERR_FAIL_COND_V(!p_dst && p_length > 0, -1); + if (!p_length) { + return 0; + } + + ERR_FAIL_NULL_V(p_dst, -1); ERR_FAIL_NULL_V(data, -1); uint64_t left = length - pos; @@ -148,7 +152,11 @@ void FileAccessMemory::flush() { } bool FileAccessMemory::store_buffer(const uint8_t *p_src, uint64_t p_length) { - ERR_FAIL_COND_V(!p_src && p_length > 0, false); + if (!p_length) { + return true; + } + + ERR_FAIL_NULL_V(p_src, false); uint64_t left = length - pos; uint64_t write = MIN(p_length, left); diff --git a/core/io/image.h b/core/io/image.h index f6504c7be88..c31597ac401 100644 --- a/core/io/image.h +++ b/core/io/image.h @@ -70,7 +70,7 @@ public: MAX_PIXELS = 268435456 // 16384 ^ 2 }; - enum Format { + enum Format : int32_t { FORMAT_L8, // Luminance FORMAT_LA8, // Luminance-Alpha FORMAT_R8, diff --git a/core/io/json.cpp b/core/io/json.cpp index b6b1a88479b..bb93e2fd5d2 100644 --- a/core/io/json.cpp +++ b/core/io/json.cpp @@ -71,14 +71,18 @@ String JSON::_stringify(const Variant &p_var, const String &p_indent, int p_cur_ return itos(p_var); case Variant::FLOAT: { double num = p_var; - if (p_full_precision) { - // Store unreliable digits (17) instead of just reliable - // digits (14) so that the value can be decoded exactly. - return String::num(num, 17 - (int)floor(log10(num))); - } else { - // Store only reliable digits (14) by default. - return String::num(num, 14 - (int)floor(log10(num))); + + // Only for exactly 0. If we have approximately 0 let the user decide how much + // precision they want. + if (num == double(0)) { + return String("0.0"); } + + double magnitude = log10(Math::abs(num)); + int total_digits = p_full_precision ? 17 : 14; + int precision = MAX(1, total_digits - (int)Math::floor(magnitude)); + + return String::num(num, precision); } case Variant::PACKED_INT32_ARRAY: case Variant::PACKED_INT64_ARRAY: diff --git a/core/io/marshalls.cpp b/core/io/marshalls.cpp index d9149a6e736..19efe28931e 100644 --- a/core/io/marshalls.cpp +++ b/core/io/marshalls.cpp @@ -217,7 +217,7 @@ Error decode_variant(Variant &r_variant, const uint8_t *p_buffer, int p_len, int case Variant::INT: { if (header & HEADER_DATA_FLAG_64) { ERR_FAIL_COND_V(len < 8, ERR_INVALID_DATA); - int64_t val = decode_uint64(buf); + int64_t val = int64_t(decode_uint64(buf)); r_variant = val; if (r_len) { (*r_len) += 8; @@ -225,7 +225,7 @@ Error decode_variant(Variant &r_variant, const uint8_t *p_buffer, int p_len, int } else { ERR_FAIL_COND_V(len < 4, ERR_INVALID_DATA); - int32_t val = decode_uint32(buf); + int32_t val = int32_t(decode_uint32(buf)); r_variant = val; if (r_len) { (*r_len) += 4; @@ -1450,13 +1450,13 @@ Error encode_variant(const Variant &p_variant, uint8_t *r_buffer, int &r_len, bo if (header & HEADER_DATA_FLAG_64) { // 64 bits. if (buf) { - encode_uint64(p_variant.operator int64_t(), buf); + encode_uint64(p_variant.operator uint64_t(), buf); } r_len += 8; } else { if (buf) { - encode_uint32(p_variant.operator int32_t(), buf); + encode_uint32(p_variant.operator uint32_t(), buf); } r_len += 4; diff --git a/core/io/net_socket.h b/core/io/net_socket.h index c12bab622af..49b23575a79 100644 --- a/core/io/net_socket.h +++ b/core/io/net_socket.h @@ -41,13 +41,13 @@ protected: public: static NetSocket *create(); - enum PollType { + enum PollType : int32_t { POLL_TYPE_IN, POLL_TYPE_OUT, POLL_TYPE_IN_OUT }; - enum Type { + enum Type : int32_t { TYPE_NONE, TYPE_TCP, TYPE_UDP, diff --git a/core/io/pck_packer.cpp b/core/io/pck_packer.cpp index c832ef5700a..b9fe121ea6d 100644 --- a/core/io/pck_packer.cpp +++ b/core/io/pck_packer.cpp @@ -182,7 +182,7 @@ Error PCKPacker::flush(bool p_verbose) { } // write the index - file->store_32(files.size()); + file->store_32(uint32_t(files.size())); Ref fae; Ref fhead = file; @@ -201,7 +201,7 @@ Error PCKPacker::flush(bool p_verbose) { int string_len = files[i].path.utf8().length(); int pad = _get_pad(4, string_len); - fhead->store_32(string_len + pad); + fhead->store_32(uint32_t(string_len + pad)); fhead->store_buffer((const uint8_t *)files[i].path.utf8().get_data(), string_len); for (int j = 0; j < pad; j++) { fhead->store_8(0); @@ -231,7 +231,7 @@ Error PCKPacker::flush(bool p_verbose) { file->store_8(0); } - int64_t file_base = file->get_position(); + uint64_t file_base = file->get_position(); file->seek(file_base_ofs); file->store_64(file_base); // update files base file->seek(file_base); diff --git a/core/io/remote_filesystem_client.cpp b/core/io/remote_filesystem_client.cpp index c3f9a0016cd..9ee5ab76379 100644 --- a/core/io/remote_filesystem_client.cpp +++ b/core/io/remote_filesystem_client.cpp @@ -237,7 +237,7 @@ Error RemoteFilesystemClient::_synchronize_with_server(const String &p_host, int tcp_client->poll(); ERR_FAIL_COND_V_MSG(tcp_client->get_status() != StreamPeerTCP::STATUS_CONNECTED, ERR_CONNECTION_ERROR, "Remote filesystem server disconnected after sending header."); - uint32_t file_count = tcp_client->get_32(); + uint32_t file_count = tcp_client->get_u32(); ERR_FAIL_COND_V_MSG(tcp_client->get_status() != StreamPeerTCP::STATUS_CONNECTED, ERR_CONNECTION_ERROR, "Remote filesystem server disconnected while waiting for file list"); diff --git a/core/io/resource_format_binary.cpp b/core/io/resource_format_binary.cpp index 542b6335501..2f8e3800d5a 100644 --- a/core/io/resource_format_binary.cpp +++ b/core/io/resource_format_binary.cpp @@ -911,7 +911,7 @@ void ResourceLoaderBinary::set_translation_remapped(bool p_remapped) { static void save_ustring(Ref f, const String &p_string) { CharString utf8 = p_string.utf8(); - f->store_32(utf8.length() + 1); + f->store_32(uint32_t(utf8.length() + 1)); f->store_buffer((const uint8_t *)utf8.get_data(), utf8.length() + 1); } @@ -1051,7 +1051,7 @@ void ResourceLoaderBinary::open(Ref p_f, bool p_no_resources, bool p f->real_is_double = (flags & ResourceFormatSaverBinaryInstance::FORMAT_FLAG_REAL_T_IS_DOUBLE) != 0; if (using_uids) { - uid = f->get_64(); + uid = ResourceUID::ID(f->get_64()); } else { f->get_64(); // skip over uid field uid = ResourceUID::INVALID_ID; @@ -1084,7 +1084,7 @@ void ResourceLoaderBinary::open(Ref p_f, bool p_no_resources, bool p er.type = get_unicode_string(); er.path = get_unicode_string(); if (using_uids) { - er.uid = f->get_64(); + er.uid = ResourceUID::ID(f->get_64()); if (!p_keep_uuid_paths && er.uid != ResourceUID::INVALID_ID) { if (ResourceUID::get_singleton()->has_id(er.uid)) { // If a UID is found and the path is valid, it will be used, otherwise, it falls back to the path. @@ -1477,7 +1477,7 @@ Error ResourceFormatLoaderBinary::rename_dependencies(const String &p_path, cons if (using_uids) { ResourceUID::ID uid = ResourceSaver::get_resource_id_for_path(full_path); - fw->store_64(uid); + fw->store_64(uint64_t(uid)); } } @@ -1609,11 +1609,11 @@ void ResourceFormatSaverBinaryInstance::write_variant(Ref f, const V int64_t val = p_property; if (val > 0x7FFFFFFF || val < -(int64_t)0x80000000) { f->store_32(VARIANT_INT64); - f->store_64(val); + f->store_64(uint64_t(val)); } else { f->store_32(VARIANT_INT); - f->store_32(int32_t(p_property)); + f->store_32(uint32_t(p_property)); } } break; @@ -1645,8 +1645,8 @@ void ResourceFormatSaverBinaryInstance::write_variant(Ref f, const V case Variant::VECTOR2I: { f->store_32(VARIANT_VECTOR2I); Vector2i val = p_property; - f->store_32(val.x); - f->store_32(val.y); + f->store_32(uint32_t(val.x)); + f->store_32(uint32_t(val.y)); } break; case Variant::RECT2: { @@ -1661,10 +1661,10 @@ void ResourceFormatSaverBinaryInstance::write_variant(Ref f, const V case Variant::RECT2I: { f->store_32(VARIANT_RECT2I); Rect2i val = p_property; - f->store_32(val.position.x); - f->store_32(val.position.y); - f->store_32(val.size.x); - f->store_32(val.size.y); + f->store_32(uint32_t(val.position.x)); + f->store_32(uint32_t(val.position.y)); + f->store_32(uint32_t(val.size.x)); + f->store_32(uint32_t(val.size.y)); } break; case Variant::VECTOR3: { @@ -1678,9 +1678,9 @@ void ResourceFormatSaverBinaryInstance::write_variant(Ref f, const V case Variant::VECTOR3I: { f->store_32(VARIANT_VECTOR3I); Vector3i val = p_property; - f->store_32(val.x); - f->store_32(val.y); - f->store_32(val.z); + f->store_32(uint32_t(val.x)); + f->store_32(uint32_t(val.y)); + f->store_32(uint32_t(val.z)); } break; case Variant::VECTOR4: { @@ -1695,10 +1695,10 @@ void ResourceFormatSaverBinaryInstance::write_variant(Ref f, const V case Variant::VECTOR4I: { f->store_32(VARIANT_VECTOR4I); Vector4i val = p_property; - f->store_32(val.x); - f->store_32(val.y); - f->store_32(val.z); - f->store_32(val.w); + f->store_32(uint32_t(val.x)); + f->store_32(uint32_t(val.y)); + f->store_32(uint32_t(val.z)); + f->store_32(uint32_t(val.w)); } break; case Variant::PLANE: { @@ -1821,14 +1821,14 @@ void ResourceFormatSaverBinaryInstance::write_variant(Ref f, const V f->store_16(snc); for (int i = 0; i < np.get_name_count(); i++) { if (string_map.has(np.get_name(i))) { - f->store_32(string_map[np.get_name(i)]); + f->store_32(uint32_t(string_map[np.get_name(i)])); } else { save_unicode_string(f, np.get_name(i), true); } } for (int i = 0; i < np.get_subname_count(); i++) { if (string_map.has(np.get_subname(i))) { - f->store_32(string_map[np.get_subname(i)]); + f->store_32(uint32_t(string_map[np.get_subname(i)])); } else { save_unicode_string(f, np.get_subname(i), true); } @@ -1839,7 +1839,7 @@ void ResourceFormatSaverBinaryInstance::write_variant(Ref f, const V f->store_32(VARIANT_RID); WARN_PRINT("Can't save RIDs."); RID val = p_property; - f->store_32(val.get_id()); + f->store_32(uint32_t(val.get_id())); } break; case Variant::OBJECT: { f->store_32(VARIANT_OBJECT); @@ -1851,7 +1851,7 @@ void ResourceFormatSaverBinaryInstance::write_variant(Ref f, const V if (!res->is_built_in()) { f->store_32(OBJECT_EXTERNAL_RESOURCE_INDEX); - f->store_32(external_resources[res]); + f->store_32(uint32_t(external_resources[res])); } else { if (!resource_map.has(res)) { f->store_32(OBJECT_EMPTY); @@ -1859,7 +1859,7 @@ void ResourceFormatSaverBinaryInstance::write_variant(Ref f, const V } f->store_32(OBJECT_INTERNAL_RESOURCE); - f->store_32(resource_map[res]); + f->store_32(uint32_t(resource_map[res])); //internal resource } @@ -1900,7 +1900,7 @@ void ResourceFormatSaverBinaryInstance::write_variant(Ref f, const V f->store_32(VARIANT_PACKED_BYTE_ARRAY); Vector arr = p_property; int len = arr.size(); - f->store_32(len); + f->store_32(uint32_t(len)); const uint8_t *r = arr.ptr(); f->store_buffer(r, len); _pad_buffer(f, len); @@ -1910,10 +1910,10 @@ void ResourceFormatSaverBinaryInstance::write_variant(Ref f, const V f->store_32(VARIANT_PACKED_INT32_ARRAY); Vector arr = p_property; int len = arr.size(); - f->store_32(len); + f->store_32(uint32_t(len)); const int32_t *r = arr.ptr(); for (int i = 0; i < len; i++) { - f->store_32(r[i]); + f->store_32(uint32_t(r[i])); } } break; @@ -1921,10 +1921,10 @@ void ResourceFormatSaverBinaryInstance::write_variant(Ref f, const V f->store_32(VARIANT_PACKED_INT64_ARRAY); Vector arr = p_property; int len = arr.size(); - f->store_32(len); + f->store_32(uint32_t(len)); const int64_t *r = arr.ptr(); for (int i = 0; i < len; i++) { - f->store_64(r[i]); + f->store_64(uint64_t(r[i])); } } break; @@ -1932,7 +1932,7 @@ void ResourceFormatSaverBinaryInstance::write_variant(Ref f, const V f->store_32(VARIANT_PACKED_FLOAT32_ARRAY); Vector arr = p_property; int len = arr.size(); - f->store_32(len); + f->store_32(uint32_t(len)); const float *r = arr.ptr(); for (int i = 0; i < len; i++) { f->store_float(r[i]); @@ -1943,7 +1943,7 @@ void ResourceFormatSaverBinaryInstance::write_variant(Ref f, const V f->store_32(VARIANT_PACKED_FLOAT64_ARRAY); Vector arr = p_property; int len = arr.size(); - f->store_32(len); + f->store_32(uint32_t(len)); const double *r = arr.ptr(); for (int i = 0; i < len; i++) { f->store_double(r[i]); @@ -1954,7 +1954,7 @@ void ResourceFormatSaverBinaryInstance::write_variant(Ref f, const V f->store_32(VARIANT_PACKED_STRING_ARRAY); Vector arr = p_property; int len = arr.size(); - f->store_32(len); + f->store_32(uint32_t(len)); const String *r = arr.ptr(); for (int i = 0; i < len; i++) { save_unicode_string(f, r[i]); @@ -1965,7 +1965,7 @@ void ResourceFormatSaverBinaryInstance::write_variant(Ref f, const V f->store_32(VARIANT_PACKED_VECTOR2_ARRAY); Vector arr = p_property; int len = arr.size(); - f->store_32(len); + f->store_32(uint32_t(len)); const Vector2 *r = arr.ptr(); for (int i = 0; i < len; i++) { f->store_real(r[i].x); @@ -1977,7 +1977,7 @@ void ResourceFormatSaverBinaryInstance::write_variant(Ref f, const V f->store_32(VARIANT_PACKED_VECTOR3_ARRAY); Vector arr = p_property; int len = arr.size(); - f->store_32(len); + f->store_32(uint32_t(len)); const Vector3 *r = arr.ptr(); for (int i = 0; i < len; i++) { f->store_real(r[i].x); @@ -1990,7 +1990,7 @@ void ResourceFormatSaverBinaryInstance::write_variant(Ref f, const V f->store_32(VARIANT_PACKED_COLOR_ARRAY); Vector arr = p_property; int len = arr.size(); - f->store_32(len); + f->store_32(uint32_t(len)); const Color *r = arr.ptr(); for (int i = 0; i < len; i++) { f->store_float(r[i].r); @@ -2004,7 +2004,7 @@ void ResourceFormatSaverBinaryInstance::write_variant(Ref f, const V f->store_32(VARIANT_PACKED_VECTOR4_ARRAY); Vector arr = p_property; int len = arr.size(); - f->store_32(len); + f->store_32(uint32_t(len)); const Vector4 *r = arr.ptr(); for (int i = 0; i < len; i++) { f->store_real(r[i].x); @@ -2115,9 +2115,9 @@ void ResourceFormatSaverBinaryInstance::_find_resources(const Variant &p_variant void ResourceFormatSaverBinaryInstance::save_unicode_string(Ref p_f, const String &p_string, bool p_bit_on_len) { CharString utf8 = p_string.utf8(); if (p_bit_on_len) { - p_f->store_32((utf8.length() + 1) | 0x80000000); + p_f->store_32(uint32_t((utf8.length() + 1) | 0x80000000)); } else { - p_f->store_32(utf8.length() + 1); + p_f->store_32(uint32_t(utf8.length() + 1)); } p_f->store_buffer((const uint8_t *)utf8.get_data(), utf8.length() + 1); } @@ -2218,7 +2218,7 @@ Error ResourceFormatSaverBinaryInstance::save(const String &p_path, const Refstore_32(format_flags); } ResourceUID::ID uid = ResourceSaver::get_resource_id_for_path(p_path, true); - f->store_64(uid); + f->store_64(uint64_t(uid)); if (!script_class.is_empty()) { save_unicode_string(f, script_class); } @@ -2284,7 +2284,7 @@ Error ResourceFormatSaverBinaryInstance::save(const String &p_path, const Refstore_32(strings.size()); //string table size + f->store_32(uint32_t(strings.size())); //string table size for (int i = 0; i < strings.size(); i++) { save_unicode_string(f, strings[i]); } @@ -2304,10 +2304,10 @@ Error ResourceFormatSaverBinaryInstance::save(const String &p_path, const Refget_path(), false); - f->store_64(ruid); + f->store_64(uint64_t(ruid)); } // save internal resource table - f->store_32(saved_resources.size()); //amount of internal resources + f->store_32(uint32_t(saved_resources.size())); //amount of internal resources Vector ofs_pos; HashSet used_unique_ids; @@ -2362,10 +2362,10 @@ Error ResourceFormatSaverBinaryInstance::save(const String &p_path, const Refget_position()); save_unicode_string(f, rd.type); - f->store_32(rd.properties.size()); + f->store_32(uint32_t(rd.properties.size())); for (const Property &p : rd.properties) { - f->store_32(p.name_idx); + f->store_32(uint32_t(p.name_idx)); write_variant(f, p.value, resource_map, external_resources, string_map, p.pi); } } @@ -2473,7 +2473,7 @@ Error ResourceFormatSaverBinaryInstance::set_uid(const String &p_path, ResourceU f->get_64(); // Skip previous UID fw->store_32(flags); - fw->store_64(p_uid); + fw->store_64(uint64_t(p_uid)); if (flags & ResourceFormatSaverBinaryInstance::FORMAT_FLAG_HAS_SCRIPT_CLASS) { save_ustring(fw, get_ustring(f)); diff --git a/core/io/resource_importer.h b/core/io/resource_importer.h index c3d3c4b67e9..1d00ef3b824 100644 --- a/core/io/resource_importer.h +++ b/core/io/resource_importer.h @@ -46,7 +46,7 @@ class ResourceFormatImporter : public ResourceFormatLoader { String importer; String group_file; Variant metadata; - uint64_t uid = ResourceUID::INVALID_ID; + ResourceUID::ID uid = ResourceUID::INVALID_ID; }; Error _get_path_and_type(const String &p_path, PathAndType &r_path_and_type, bool *r_valid = nullptr) const; diff --git a/core/io/resource_uid.cpp b/core/io/resource_uid.cpp index 2c221b36760..e5e847983c8 100644 --- a/core/io/resource_uid.cpp +++ b/core/io/resource_uid.cpp @@ -178,7 +178,7 @@ Error ResourceUID::save_to_cache() { cache_entries = 0; for (KeyValue &E : unique_ids) { - f->store_64(E.key); + f->store_64(uint64_t(E.key)); uint32_t s = E.value.cs.length(); f->store_32(s); f->store_buffer((const uint8_t *)E.value.cs.ptr(), s); @@ -241,7 +241,7 @@ Error ResourceUID::update_cache() { } f->seek_end(); } - f->store_64(E.key); + f->store_64(uint64_t(E.key)); uint32_t s = E.value.cs.length(); f->store_32(s); f->store_buffer((const uint8_t *)E.value.cs.ptr(), s); diff --git a/core/io/resource_uid.h b/core/io/resource_uid.h index 7b735d296ab..bdbd87dcad5 100644 --- a/core/io/resource_uid.h +++ b/core/io/resource_uid.h @@ -39,9 +39,7 @@ class ResourceUID : public Object { GDCLASS(ResourceUID, Object) public: typedef int64_t ID; - enum { - INVALID_ID = -1 - }; + constexpr const static ID INVALID_ID = -1; static String get_cache_file(); diff --git a/core/io/stream_peer.cpp b/core/io/stream_peer.cpp index b5abf15303c..bc7ea8a0001 100644 --- a/core/io/stream_peer.cpp +++ b/core/io/stream_peer.cpp @@ -204,11 +204,13 @@ void StreamPeer::put_float(float p_val) { void StreamPeer::put_double(double p_val) { uint8_t buf[8]; + encode_double(p_val, buf); if (big_endian) { uint64_t *p64 = (uint64_t *)buf; *p64 = BSWAP64(*p64); } + put_data(buf, 8); } @@ -243,75 +245,87 @@ uint8_t StreamPeer::get_u8() { int8_t StreamPeer::get_8() { uint8_t buf[1] = {}; get_data(buf, 1); - return buf[0]; + return int8_t(buf[0]); } uint16_t StreamPeer::get_u16() { uint8_t buf[2]; get_data(buf, 2); + uint16_t r = decode_uint16(buf); if (big_endian) { r = BSWAP16(r); } + return r; } int16_t StreamPeer::get_16() { uint8_t buf[2]; get_data(buf, 2); + uint16_t r = decode_uint16(buf); if (big_endian) { r = BSWAP16(r); } - return r; + + return int16_t(r); } uint32_t StreamPeer::get_u32() { uint8_t buf[4]; get_data(buf, 4); + uint32_t r = decode_uint32(buf); if (big_endian) { r = BSWAP32(r); } + return r; } int32_t StreamPeer::get_32() { uint8_t buf[4]; get_data(buf, 4); + uint32_t r = decode_uint32(buf); if (big_endian) { r = BSWAP32(r); } - return r; + + return int32_t(r); } uint64_t StreamPeer::get_u64() { uint8_t buf[8]; get_data(buf, 8); + uint64_t r = decode_uint64(buf); if (big_endian) { r = BSWAP64(r); } + return r; } int64_t StreamPeer::get_64() { uint8_t buf[8]; get_data(buf, 8); + uint64_t r = decode_uint64(buf); if (big_endian) { r = BSWAP64(r); } - return r; + + return int64_t(r); } float StreamPeer::get_half() { uint8_t buf[2]; get_data(buf, 2); - uint16_t *p16 = (uint16_t *)buf; if (big_endian) { + uint16_t *p16 = (uint16_t *)buf; *p16 = BSWAP16(*p16); } @@ -344,7 +358,7 @@ double StreamPeer::get_double() { String StreamPeer::get_string(int p_bytes) { if (p_bytes < 0) { - p_bytes = get_u32(); + p_bytes = get_32(); } ERR_FAIL_COND_V(p_bytes < 0, String()); @@ -359,7 +373,7 @@ String StreamPeer::get_string(int p_bytes) { String StreamPeer::get_utf8_string(int p_bytes) { if (p_bytes < 0) { - p_bytes = get_u32(); + p_bytes = get_32(); } ERR_FAIL_COND_V(p_bytes < 0, String()); @@ -498,7 +512,7 @@ void StreamPeerBuffer::_bind_methods() { } Error StreamPeerBuffer::put_data(const uint8_t *p_data, int p_bytes) { - if (p_bytes <= 0) { + if (p_bytes <= 0 || !p_data) { return OK; } @@ -529,6 +543,11 @@ Error StreamPeerBuffer::get_data(uint8_t *p_buffer, int p_bytes) { } Error StreamPeerBuffer::get_partial_data(uint8_t *p_buffer, int p_bytes, int &r_received) { + if (!p_bytes) { + r_received = 0; + return OK; + } + if (pointer + p_bytes > data.size()) { r_received = data.size() - pointer; if (r_received <= 0) { diff --git a/core/math/bvh_pair.inc b/core/math/bvh_pair.inc index 7b9c7ce6ae4..0e48b23a318 100644 --- a/core/math/bvh_pair.inc +++ b/core/math/bvh_pair.inc @@ -37,7 +37,7 @@ struct ItemPairs { return n; } } - return -1; + return uint32_t(-1); } bool contains_pair_to(BVHHandle h) const { diff --git a/core/math/bvh_tree.h b/core/math/bvh_tree.h index 0faa50555fb..2a2a6d6715a 100644 --- a/core/math/bvh_tree.h +++ b/core/math/bvh_tree.h @@ -109,9 +109,7 @@ struct BVHHandle { template class BVH_IterativeInfo { public: - enum { - ALLOCA_STACK_SIZE = 128 - }; + constexpr static const size_t ALLOCA_STACK_SIZE = 128; int32_t depth = 1; int32_t threshold = ALLOCA_STACK_SIZE - 2; diff --git a/core/math/color.h b/core/math/color.h index 70fad78acbd..850252d138e 100644 --- a/core/math/color.h +++ b/core/math/color.h @@ -147,7 +147,7 @@ struct [[nodiscard]] Color { // of the mantissa, rounding the truncated bits. union { float f; - int32_t i; + uint32_t i; } R, G, B, E; E.f = MaxChannel; @@ -168,7 +168,7 @@ struct [[nodiscard]] Color { // Combine the fields. RGB floats have unwanted data in the upper 9 // bits. Only red needs to mask them off because green and blue shift // it out to the left. - return E.i | (B.i << 18) | (G.i << 9) | (R.i & 511); + return E.i | (B.i << 18U) | (G.i << 9U) | (R.i & 511U); } _FORCE_INLINE_ Color blend(const Color &p_over) const { diff --git a/core/math/random_pcg.cpp b/core/math/random_pcg.cpp index c286a604218..2b0f8f12ee4 100644 --- a/core/math/random_pcg.cpp +++ b/core/math/random_pcg.cpp @@ -80,5 +80,5 @@ int RandomPCG::random(int p_from, int p_to) { if (p_from == p_to) { return p_from; } - return rand(abs(p_from - p_to) + 1) + MIN(p_from, p_to); + return int(rand(uint32_t(Math::abs(p_from - p_to)) + 1U)) + MIN(p_from, p_to); } diff --git a/core/object/class_db.cpp b/core/object/class_db.cpp index 865b6acc423..48e6d61d8aa 100644 --- a/core/object/class_db.cpp +++ b/core/object/class_db.cpp @@ -454,7 +454,7 @@ uint32_t ClassDB::get_api_hash(APIType p_api) { for (const StringName &F : snames) { hash = hash_murmur3_one_64(F.hash(), hash); - hash = hash_murmur3_one_64(t->constant_map[F], hash); + hash = hash_murmur3_one_64(uint64_t(t->constant_map[F]), hash); } } diff --git a/core/object/object_id.h b/core/object/object_id.h index 7b677cb05b8..b04e2df128c 100644 --- a/core/object/object_id.h +++ b/core/object/object_id.h @@ -46,7 +46,7 @@ public: _ALWAYS_INLINE_ bool is_valid() const { return id != 0; } _ALWAYS_INLINE_ bool is_null() const { return id == 0; } _ALWAYS_INLINE_ operator uint64_t() const { return id; } - _ALWAYS_INLINE_ operator int64_t() const { return id; } + _ALWAYS_INLINE_ operator int64_t() const { return (int64_t)id; } _ALWAYS_INLINE_ bool operator==(const ObjectID &p_id) const { return id == p_id.id; } _ALWAYS_INLINE_ bool operator!=(const ObjectID &p_id) const { return id != p_id.id; } diff --git a/core/os/memory.cpp b/core/os/memory.cpp index dae0a31fe02..ce4ccae5312 100644 --- a/core/os/memory.cpp +++ b/core/os/memory.cpp @@ -87,7 +87,9 @@ void *Memory::realloc_aligned_static(void *p_memory, size_t p_bytes, size_t p_pr } void *ret = alloc_aligned_static(p_bytes, p_alignment); - memcpy(ret, p_memory, p_prev_bytes); + if (ret) { + memcpy(ret, p_memory, p_prev_bytes); + } free_aligned_static(p_memory); return ret; } diff --git a/core/string/string_buffer.h b/core/string/string_buffer.h index e094c7627ea..c1cdd38ec8e 100644 --- a/core/string/string_buffer.h +++ b/core/string/string_buffer.h @@ -118,7 +118,7 @@ StringBuffer &StringBuffer::append(const c template StringBuffer &StringBuffer::reserve(int p_size) { - if (p_size < SHORT_BUFFER_SIZE || p_size < buffer.size()) { + if (p_size < SHORT_BUFFER_SIZE || p_size < buffer.size() || !p_size) { return *this; } diff --git a/core/string/ustring.cpp b/core/string/ustring.cpp index e1588d3752f..61ff176b6a1 100644 --- a/core/string/ustring.cpp +++ b/core/string/ustring.cpp @@ -42,10 +42,6 @@ #include "core/variant/variant.h" #include "core/version_generated.gen.h" -#include -#include -#include - #ifdef _MSC_VER #define _CRT_SECURE_NO_WARNINGS // to disable build-time warning which suggested to use strcpy_s instead strcpy #endif @@ -1804,6 +1800,10 @@ String String::num_uint64(uint64_t p_num, int base, bool capitalize_hex) { } String String::num_real(double p_num, bool p_trailing) { + if (Math::is_nan(p_num) || Math::is_inf(p_num)) { + return num(p_num, 0); + } + if (p_num == (double)(int64_t)p_num) { if (p_trailing) { return num_int64((int64_t)p_num) + ".0"; @@ -1811,6 +1811,7 @@ String String::num_real(double p_num, bool p_trailing) { return num_int64((int64_t)p_num); } } + int decimals = 14; // We want to align the digits to the above sane default, so we only need // to subtract log10 for numbers with a positive power of ten magnitude. @@ -1818,10 +1819,15 @@ String String::num_real(double p_num, bool p_trailing) { if (abs_num > 10) { decimals -= (int)floor(log10(abs_num)); } + return num(p_num, decimals); } String String::num_real(float p_num, bool p_trailing) { + if (Math::is_nan(p_num) || Math::is_inf(p_num)) { + return num(p_num, 0); + } + if (p_num == (float)(int64_t)p_num) { if (p_trailing) { return num_int64((int64_t)p_num) + ".0"; @@ -1840,16 +1846,8 @@ String String::num_real(float p_num, bool p_trailing) { } String String::num_scientific(double p_num) { - if (Math::is_nan(p_num)) { - return "nan"; - } - - if (Math::is_inf(p_num)) { - if (signbit(p_num)) { - return "-inf"; - } else { - return "inf"; - } + if (Math::is_nan(p_num) || Math::is_inf(p_num)) { + return num(p_num, 0); } char buf[256]; @@ -1947,7 +1945,7 @@ CharString String::ascii(bool p_allow_extended) const { for (int i = 0; i < size(); i++) { char32_t c = this_ptr[i]; if ((c <= 0x7f) || (c <= 0xff && p_allow_extended)) { - cs_ptrw[i] = c; + cs_ptrw[i] = char(c); } else { print_unicode_error(vformat("Invalid unicode codepoint (%x), cannot represent as ASCII/Latin-1", (uint32_t)c)); cs_ptrw[i] = 0x20; // ASCII doesn't have a replacement character like unicode, 0x1a is sometimes used but is kinda arcane. @@ -2487,6 +2485,42 @@ int64_t String::bin_to_int() const { return binary * sign; } +template +_ALWAYS_INLINE_ int64_t _to_int(const T &p_in, int to) { + // Accumulate the total number in an unsigned integer as the range is: + // +9223372036854775807 to -9223372036854775808 and the smallest negative + // number does not fit inside an int64_t. So we accumulate the positive + // number in an unsigned, and then at the very end convert to its signed + // form. + uint64_t integer = 0; + uint8_t digits = 0; + bool positive = true; + + for (int i = 0; i < to; i++) { + C c = p_in[i]; + if (is_digit(c)) { + // No need to do expensive checks unless we're approaching INT64_MAX / INT64_MIN. + if (unlikely(digits > 18)) { + bool overflow = (integer > INT64_MAX / 10) || (integer == INT64_MAX / 10 && ((positive && c > '7') || (!positive && c > '8'))); + ERR_FAIL_COND_V_MSG(overflow, positive ? INT64_MAX : INT64_MIN, "Cannot represent " + String(p_in) + " as a 64-bit signed integer, since the value is " + (positive ? "too large." : "too small.")); + } + + integer *= 10; + integer += c - '0'; + ++digits; + + } else if (integer == 0 && c == '-') { + positive = !positive; + } + } + + if (positive) { + return int64_t(integer); + } else { + return int64_t(integer * uint64_t(-1)); + } +} + int64_t String::to_int() const { if (length() == 0) { return 0; @@ -2494,23 +2528,7 @@ int64_t String::to_int() const { int to = (find_char('.') >= 0) ? find_char('.') : length(); - int64_t integer = 0; - int64_t sign = 1; - - for (int i = 0; i < to; i++) { - char32_t c = operator[](i); - if (is_digit(c)) { - bool overflow = (integer > INT64_MAX / 10) || (integer == INT64_MAX / 10 && ((sign == 1 && c > '7') || (sign == -1 && c > '8'))); - ERR_FAIL_COND_V_MSG(overflow, sign == 1 ? INT64_MAX : INT64_MIN, "Cannot represent " + *this + " as a 64-bit signed integer, since the value is " + (sign == 1 ? "too large." : "too small.")); - integer *= 10; - integer += c - '0'; - - } else if (integer == 0 && c == '-') { - sign = -sign; - } - } - - return integer * sign; + return _to_int(*this, to); } int64_t String::to_int(const char *p_str, int p_len) { @@ -2523,25 +2541,7 @@ int64_t String::to_int(const char *p_str, int p_len) { } } - int64_t integer = 0; - int64_t sign = 1; - - for (int i = 0; i < to; i++) { - char c = p_str[i]; - if (is_digit(c)) { - bool overflow = (integer > INT64_MAX / 10) || (integer == INT64_MAX / 10 && ((sign == 1 && c > '7') || (sign == -1 && c > '8'))); - ERR_FAIL_COND_V_MSG(overflow, sign == 1 ? INT64_MAX : INT64_MIN, "Cannot represent " + String(p_str).substr(0, to) + " as a 64-bit signed integer, since the value is " + (sign == 1 ? "too large." : "too small.")); - integer *= 10; - integer += c - '0'; - - } else if (c == '-' && integer == 0) { - sign = -sign; - } else if (c != ' ') { - break; - } - } - - return integer * sign; + return _to_int(p_str, to); } int64_t String::to_int(const wchar_t *p_str, int p_len) { @@ -2554,25 +2554,7 @@ int64_t String::to_int(const wchar_t *p_str, int p_len) { } } - int64_t integer = 0; - int64_t sign = 1; - - for (int i = 0; i < to; i++) { - wchar_t c = p_str[i]; - if (is_digit(c)) { - bool overflow = (integer > INT64_MAX / 10) || (integer == INT64_MAX / 10 && ((sign == 1 && c > '7') || (sign == -1 && c > '8'))); - ERR_FAIL_COND_V_MSG(overflow, sign == 1 ? INT64_MAX : INT64_MIN, "Cannot represent " + String(p_str).substr(0, to) + " as a 64-bit signed integer, since the value is " + (sign == 1 ? "too large." : "too small.")); - integer *= 10; - integer += c - '0'; - - } else if (c == '-' && integer == 0) { - sign = -sign; - } else if (c != ' ') { - break; - } - } - - return integer * sign; + return _to_int(p_str, to); } bool String::is_numeric() const { @@ -3969,7 +3951,7 @@ static String _replace_common(const String &p_this, const String &p_key, const S return p_this; } - const int key_length = p_key.length(); + const size_t key_length = p_key.length(); int search_from = 0; int result = 0; @@ -3978,6 +3960,7 @@ static String _replace_common(const String &p_this, const String &p_key, const S while ((result = (p_case_insensitive ? p_this.findn(p_key, search_from) : p_this.find(p_key, search_from))) >= 0) { found.push_back(result); + ERR_FAIL_COND_V_MSG((result + key_length) > INT32_MAX, p_this, "Key length too long"); search_from = result + key_length; } @@ -3990,7 +3973,7 @@ static String _replace_common(const String &p_this, const String &p_key, const S const int with_length = p_with.length(); const int old_length = p_this.length(); - new_string.resize(old_length + found.size() * (with_length - key_length) + 1); + new_string.resize(old_length + int(found.size()) * (with_length - key_length) + 1); char32_t *new_ptrw = new_string.ptrw(); const char32_t *old_ptr = p_this.ptr(); @@ -4021,7 +4004,7 @@ static String _replace_common(const String &p_this, const String &p_key, const S } static String _replace_common(const String &p_this, char const *p_key, char const *p_with, bool p_case_insensitive) { - int key_length = strlen(p_key); + size_t key_length = strlen(p_key); if (key_length == 0 || p_this.is_empty()) { return p_this; @@ -4034,6 +4017,7 @@ static String _replace_common(const String &p_this, char const *p_key, char cons while ((result = (p_case_insensitive ? p_this.findn(p_key, search_from) : p_this.find(p_key, search_from))) >= 0) { found.push_back(result); + ERR_FAIL_COND_V_MSG((result + key_length) > INT32_MAX, p_this, "Key length too long"); search_from = result + key_length; } @@ -4048,7 +4032,7 @@ static String _replace_common(const String &p_this, char const *p_key, char cons const int with_length = with_string.length(); const int old_length = p_this.length(); - new_string.resize(old_length + found.size() * (with_length - key_length) + 1); + new_string.resize(old_length + int(found.size()) * (with_length - key_length) + 1); char32_t *new_ptrw = new_string.ptrw(); const char32_t *old_ptr = p_this.ptr(); @@ -4639,8 +4623,9 @@ bool String::is_valid_string() const { String String::uri_encode() const { const CharString temp = utf8(); String res; + for (int i = 0; i < temp.length(); ++i) { - uint8_t ord = temp[i]; + uint8_t ord = uint8_t(temp[i]); if (ord == '.' || ord == '-' || ord == '~' || is_ascii_identifier_char(ord)) { res += ord; } else { diff --git a/core/templates/command_queue_mt.h b/core/templates/command_queue_mt.h index 8ef5dd30642..e877a03a2fb 100644 --- a/core/templates/command_queue_mt.h +++ b/core/templates/command_queue_mt.h @@ -339,7 +339,9 @@ class CommandQueueMT { template T *allocate() { // alloc size is size+T+safeguard - uint32_t alloc_size = ((sizeof(T) + 8 - 1) & ~(8 - 1)); + static_assert(sizeof(T) < UINT32_MAX, "Type too large to fit in the command queue."); + + uint32_t alloc_size = ((sizeof(T) + 8U - 1U) & ~(8U - 1U)); uint64_t size = command_mem.size(); command_mem.resize(size + alloc_size + 8); *(uint64_t *)&command_mem[size] = alloc_size; diff --git a/core/templates/hashfuncs.h b/core/templates/hashfuncs.h index e681835c5a4..a2cf57a05ee 100644 --- a/core/templates/hashfuncs.h +++ b/core/templates/hashfuncs.h @@ -323,9 +323,9 @@ struct HashMapHasherDefault { static _FORCE_INLINE_ uint32_t hash(const String &p_string) { return p_string.hash(); } static _FORCE_INLINE_ uint32_t hash(const char *p_cstr) { return hash_djb2(p_cstr); } - static _FORCE_INLINE_ uint32_t hash(const wchar_t p_wchar) { return hash_fmix32(p_wchar); } - static _FORCE_INLINE_ uint32_t hash(const char16_t p_uchar) { return hash_fmix32(p_uchar); } - static _FORCE_INLINE_ uint32_t hash(const char32_t p_uchar) { return hash_fmix32(p_uchar); } + static _FORCE_INLINE_ uint32_t hash(const wchar_t p_wchar) { return hash_fmix32(uint32_t(p_wchar)); } + static _FORCE_INLINE_ uint32_t hash(const char16_t p_uchar) { return hash_fmix32(uint32_t(p_uchar)); } + static _FORCE_INLINE_ uint32_t hash(const char32_t p_uchar) { return hash_fmix32(uint32_t(p_uchar)); } static _FORCE_INLINE_ uint32_t hash(const RID &p_rid) { return hash_one_uint64(p_rid.get_id()); } static _FORCE_INLINE_ uint32_t hash(const CharString &p_char_string) { return hash_djb2(p_char_string.get_data()); } static _FORCE_INLINE_ uint32_t hash(const StringName &p_string_name) { return p_string_name.hash(); } @@ -333,31 +333,31 @@ struct HashMapHasherDefault { static _FORCE_INLINE_ uint32_t hash(const ObjectID &p_id) { return hash_one_uint64(p_id); } static _FORCE_INLINE_ uint32_t hash(const uint64_t p_int) { return hash_one_uint64(p_int); } - static _FORCE_INLINE_ uint32_t hash(const int64_t p_int) { return hash_one_uint64(p_int); } + static _FORCE_INLINE_ uint32_t hash(const int64_t p_int) { return hash_one_uint64(uint64_t(p_int)); } static _FORCE_INLINE_ uint32_t hash(const float p_float) { return hash_murmur3_one_float(p_float); } static _FORCE_INLINE_ uint32_t hash(const double p_double) { return hash_murmur3_one_double(p_double); } static _FORCE_INLINE_ uint32_t hash(const uint32_t p_int) { return hash_fmix32(p_int); } - static _FORCE_INLINE_ uint32_t hash(const int32_t p_int) { return hash_fmix32(p_int); } - static _FORCE_INLINE_ uint32_t hash(const uint16_t p_int) { return hash_fmix32(p_int); } - static _FORCE_INLINE_ uint32_t hash(const int16_t p_int) { return hash_fmix32(p_int); } - static _FORCE_INLINE_ uint32_t hash(const uint8_t p_int) { return hash_fmix32(p_int); } - static _FORCE_INLINE_ uint32_t hash(const int8_t p_int) { return hash_fmix32(p_int); } + static _FORCE_INLINE_ uint32_t hash(const int32_t p_int) { return hash_fmix32(uint32_t(p_int)); } + static _FORCE_INLINE_ uint32_t hash(const uint16_t p_int) { return hash_fmix32(uint32_t(p_int)); } + static _FORCE_INLINE_ uint32_t hash(const int16_t p_int) { return hash_fmix32(uint32_t(p_int)); } + static _FORCE_INLINE_ uint32_t hash(const uint8_t p_int) { return hash_fmix32(uint32_t(p_int)); } + static _FORCE_INLINE_ uint32_t hash(const int8_t p_int) { return hash_fmix32(uint32_t(p_int)); } static _FORCE_INLINE_ uint32_t hash(const Vector2i &p_vec) { - uint32_t h = hash_murmur3_one_32(p_vec.x); - h = hash_murmur3_one_32(p_vec.y, h); + uint32_t h = hash_murmur3_one_32(uint32_t(p_vec.x)); + h = hash_murmur3_one_32(uint32_t(p_vec.y), h); return hash_fmix32(h); } static _FORCE_INLINE_ uint32_t hash(const Vector3i &p_vec) { - uint32_t h = hash_murmur3_one_32(p_vec.x); - h = hash_murmur3_one_32(p_vec.y, h); - h = hash_murmur3_one_32(p_vec.z, h); + uint32_t h = hash_murmur3_one_32(uint32_t(p_vec.x)); + h = hash_murmur3_one_32(uint32_t(p_vec.y), h); + h = hash_murmur3_one_32(uint32_t(p_vec.z), h); return hash_fmix32(h); } static _FORCE_INLINE_ uint32_t hash(const Vector4i &p_vec) { - uint32_t h = hash_murmur3_one_32(p_vec.x); - h = hash_murmur3_one_32(p_vec.y, h); - h = hash_murmur3_one_32(p_vec.z, h); - h = hash_murmur3_one_32(p_vec.w, h); + uint32_t h = hash_murmur3_one_32(uint32_t(p_vec.x)); + h = hash_murmur3_one_32(uint32_t(p_vec.y), h); + h = hash_murmur3_one_32(uint32_t(p_vec.z), h); + h = hash_murmur3_one_32(uint32_t(p_vec.w), h); return hash_fmix32(h); } static _FORCE_INLINE_ uint32_t hash(const Vector2 &p_vec) { @@ -379,10 +379,10 @@ struct HashMapHasherDefault { return hash_fmix32(h); } static _FORCE_INLINE_ uint32_t hash(const Rect2i &p_rect) { - uint32_t h = hash_murmur3_one_32(p_rect.position.x); - h = hash_murmur3_one_32(p_rect.position.y, h); - h = hash_murmur3_one_32(p_rect.size.x, h); - h = hash_murmur3_one_32(p_rect.size.y, h); + uint32_t h = hash_murmur3_one_32(uint32_t(p_rect.position.x)); + h = hash_murmur3_one_32(uint32_t(p_rect.position.y), h); + h = hash_murmur3_one_32(uint32_t(p_rect.size.x), h); + h = hash_murmur3_one_32(uint32_t(p_rect.size.y), h); return hash_fmix32(h); } static _FORCE_INLINE_ uint32_t hash(const Rect2 &p_rect) { diff --git a/core/templates/local_vector.h b/core/templates/local_vector.h index c281d70d928..e859c585c68 100644 --- a/core/templates/local_vector.h +++ b/core/templates/local_vector.h @@ -297,7 +297,9 @@ public: Vector ret; ret.resize(size()); T *w = ret.ptrw(); - memcpy(w, data, sizeof(T) * count); + if (w) { + memcpy(w, data, sizeof(T) * count); + } return ret; } @@ -305,7 +307,9 @@ public: Vector ret; ret.resize(count * sizeof(T)); uint8_t *w = ret.ptrw(); - memcpy(w, data, sizeof(T) * count); + if (w) { + memcpy(w, data, sizeof(T) * count); + } return ret; } diff --git a/core/templates/vector.h b/core/templates/vector.h index 32e1339e959..3b60a35f0b5 100644 --- a/core/templates/vector.h +++ b/core/templates/vector.h @@ -156,8 +156,11 @@ public: if (is_empty()) { return ret; } - ret.resize(size() * sizeof(T)); - memcpy(ret.ptrw(), ptr(), sizeof(T) * size()); + size_t alloc_size = size() * sizeof(T); + ret.resize(alloc_size); + if (alloc_size) { + memcpy(ret.ptrw(), ptr(), alloc_size); + } return ret; } diff --git a/core/variant/variant.cpp b/core/variant/variant.cpp index c879efecb0d..cff617d9674 100644 --- a/core/variant/variant.cpp +++ b/core/variant/variant.cpp @@ -1494,11 +1494,11 @@ Variant::operator int64_t() const { case BOOL: return _data._bool ? 1 : 0; case INT: - return _data._int; + return int64_t(_data._int); case FLOAT: - return _data._float; + return int64_t(_data._float); case STRING: - return operator String().to_int(); + return int64_t(operator String().to_int()); default: { return 0; } @@ -1512,11 +1512,11 @@ Variant::operator int32_t() const { case BOOL: return _data._bool ? 1 : 0; case INT: - return _data._int; + return int32_t(_data._int); case FLOAT: - return _data._float; + return int32_t(_data._float); case STRING: - return operator String().to_int(); + return int32_t(operator String().to_int()); default: { return 0; } @@ -1530,11 +1530,11 @@ Variant::operator int16_t() const { case BOOL: return _data._bool ? 1 : 0; case INT: - return _data._int; + return int16_t(_data._int); case FLOAT: - return _data._float; + return int16_t(_data._float); case STRING: - return operator String().to_int(); + return int16_t(operator String().to_int()); default: { return 0; } @@ -1548,11 +1548,11 @@ Variant::operator int8_t() const { case BOOL: return _data._bool ? 1 : 0; case INT: - return _data._int; + return int8_t(_data._int); case FLOAT: - return _data._float; + return int8_t(_data._float); case STRING: - return operator String().to_int(); + return int8_t(operator String().to_int()); default: { return 0; } @@ -1566,11 +1566,11 @@ Variant::operator uint64_t() const { case BOOL: return _data._bool ? 1 : 0; case INT: - return _data._int; + return uint64_t(_data._int); case FLOAT: - return _data._float; + return uint64_t(_data._float); case STRING: - return operator String().to_int(); + return uint64_t(operator String().to_int()); default: { return 0; } @@ -1584,11 +1584,11 @@ Variant::operator uint32_t() const { case BOOL: return _data._bool ? 1 : 0; case INT: - return _data._int; + return uint32_t(_data._int); case FLOAT: - return _data._float; + return uint32_t(_data._float); case STRING: - return operator String().to_int(); + return uint32_t(operator String().to_int()); default: { return 0; } @@ -1602,11 +1602,11 @@ Variant::operator uint16_t() const { case BOOL: return _data._bool ? 1 : 0; case INT: - return _data._int; + return uint16_t(_data._int); case FLOAT: - return _data._float; + return uint16_t(_data._float); case STRING: - return operator String().to_int(); + return uint16_t(operator String().to_int()); default: { return 0; } @@ -1620,11 +1620,11 @@ Variant::operator uint8_t() const { case BOOL: return _data._bool ? 1 : 0; case INT: - return _data._int; + return uint8_t(_data._int); case FLOAT: - return _data._float; + return uint8_t(_data._float); case STRING: - return operator String().to_int(); + return uint8_t(operator String().to_int()); default: { return 0; } @@ -2484,22 +2484,22 @@ Variant::Variant(int8_t p_int8) : Variant::Variant(uint64_t p_uint64) : type(INT) { - _data._int = p_uint64; + _data._int = int64_t(p_uint64); } Variant::Variant(uint32_t p_uint32) : type(INT) { - _data._int = p_uint32; + _data._int = int64_t(p_uint32); } Variant::Variant(uint16_t p_uint16) : type(INT) { - _data._int = p_uint16; + _data._int = int64_t(p_uint16); } Variant::Variant(uint8_t p_uint8) : type(INT) { - _data._int = p_uint8; + _data._int = int64_t(p_uint8); } Variant::Variant(float p_float) : @@ -2514,7 +2514,7 @@ Variant::Variant(double p_double) : Variant::Variant(const ObjectID &p_id) : type(INT) { - _data._int = p_id; + _data._int = int64_t(p_id); } Variant::Variant(const StringName &p_string) : diff --git a/core/variant/variant_parser.cpp b/core/variant/variant_parser.cpp index bd0b0692b06..e9e1c964d30 100644 --- a/core/variant/variant_parser.cpp +++ b/core/variant/variant_parser.cpp @@ -99,6 +99,7 @@ bool VariantParser::StreamString::_is_eof() const { uint32_t VariantParser::StreamString::_read_buffer(char32_t *p_buffer, uint32_t p_num_chars) { // The buffer is assumed to include at least one character (for null terminator) ERR_FAIL_COND_V(!p_num_chars, 0); + ERR_FAIL_NULL_V(p_buffer, 0); int available = MAX(s.length() - pos, 0); if (available >= (int)p_num_chars) { diff --git a/tests/core/io/test_json.h b/tests/core/io/test_json.h index bf2ed427407..17a510dfc77 100644 --- a/tests/core/io/test_json.h +++ b/tests/core/io/test_json.h @@ -232,6 +232,91 @@ TEST_CASE("[JSON] Parsing escape sequences") { ERR_PRINT_ON } } + +TEST_CASE("[JSON] Serialization") { + JSON json; + + struct FpTestCase { + double number; + String json; + }; + + struct IntTestCase { + int64_t number; + String json; + }; + + struct UIntTestCase { + uint64_t number; + String json; + }; + + static FpTestCase fp_tests_default_precision[] = { + { 0.0, "0.0" }, + { 1000.1234567890123456789, "1000.12345678901" }, + { -1000.1234567890123456789, "-1000.12345678901" }, + { DBL_MAX, "179769313486231570814527423731704356798070567525844996598917476803157260780028538760589558632766878171540458953514382464234321326889464182768467546703537516986049910576551282076245490090389328944075868508455133942304583236903222948165808559332123348274797826204144723168738177180919299881250404026184124858368.0" }, + { DBL_MAX - 1, "179769313486231570814527423731704356798070567525844996598917476803157260780028538760589558632766878171540458953514382464234321326889464182768467546703537516986049910576551282076245490090389328944075868508455133942304583236903222948165808559332123348274797826204144723168738177180919299881250404026184124858368.0" }, + { pow(2, 53), "9007199254740992.0" }, + { -pow(2, 53), "-9007199254740992.0" }, + { 0.00000000000000011, "0.00000000000000011" }, + { -0.00000000000000011, "-0.00000000000000011" }, + { 1.0 / 3.0, "0.333333333333333" }, + { 0.9999999999999999, "1.0" }, + { 1.0000000000000001, "1.0" }, + }; + + static FpTestCase fp_tests_full_precision[] = { + { 0.0, "0.0" }, + { 1000.1234567890123456789, "1000.12345678901238" }, + { -1000.1234567890123456789, "-1000.12345678901238" }, + { DBL_MAX, "179769313486231570814527423731704356798070567525844996598917476803157260780028538760589558632766878171540458953514382464234321326889464182768467546703537516986049910576551282076245490090389328944075868508455133942304583236903222948165808559332123348274797826204144723168738177180919299881250404026184124858368.0" }, + { DBL_MAX - 1, "179769313486231570814527423731704356798070567525844996598917476803157260780028538760589558632766878171540458953514382464234321326889464182768467546703537516986049910576551282076245490090389328944075868508455133942304583236903222948165808559332123348274797826204144723168738177180919299881250404026184124858368.0" }, + { pow(2, 53), "9007199254740992.0" }, + { -pow(2, 53), "-9007199254740992.0" }, + { 0.00000000000000011, "0.00000000000000011" }, + { -0.00000000000000011, "-0.00000000000000011" }, + { 1.0 / 3.0, "0.333333333333333315" }, + { 0.9999999999999999, "0.999999999999999889" }, + { 1.0000000000000001, "1.0" }, + }; + + static IntTestCase int_tests[] = { + { 0, "0" }, + { INT64_MAX, "9223372036854775807" }, + { INT64_MIN, "-9223372036854775808" }, + }; + + SUBCASE("Floating point default precision") { + for (FpTestCase &test : fp_tests_default_precision) { + String json_value = json.stringify(test.number, "", true, false); + + CHECK_MESSAGE( + json_value == test.json, + vformat("Serializing `%.20d` to JSON should return the expected value.", test.number)); + } + } + + SUBCASE("Floating point full precision") { + for (FpTestCase &test : fp_tests_full_precision) { + String json_value = json.stringify(test.number, "", true, true); + + CHECK_MESSAGE( + json_value == test.json, + vformat("Serializing `%20f` to JSON should return the expected value.", test.number)); + } + } + + SUBCASE("Signed integer") { + for (IntTestCase &test : int_tests) { + String json_value = json.stringify(test.number, "", true, true); + + CHECK_MESSAGE( + json_value == test.json, + vformat("Serializing `%d` to JSON should return the expected value.", test.number)); + } + } +} } // namespace TestJSON #endif // TEST_JSON_H