From e7cd4cfa25485610c05a906859e8365158a13f69 Mon Sep 17 00:00:00 2001 From: Wuzzy Date: Sat, 31 Jul 2021 17:54:40 +0000 Subject: [PATCH] Fix /emergeblocks crashing in debug builds (#11461) The reason for the bug was an u16 overflow, thus failing the assert. This only happened in Debug build but not in Release builds. --- builtin/settingtypes.txt | 6 +++--- src/emerge.cpp | 29 ++++++++++++----------------- src/emerge.h | 8 ++++---- src/settings.cpp | 9 +++++++++ src/settings.h | 1 + 5 files changed, 29 insertions(+), 24 deletions(-) diff --git a/builtin/settingtypes.txt b/builtin/settingtypes.txt index 420e9d49c..25a51b888 100644 --- a/builtin/settingtypes.txt +++ b/builtin/settingtypes.txt @@ -2218,15 +2218,15 @@ chunksize (Chunk size) int 5 enable_mapgen_debug_info (Mapgen debug) bool false # Maximum number of blocks that can be queued for loading. -emergequeue_limit_total (Absolute limit of queued blocks to emerge) int 1024 +emergequeue_limit_total (Absolute limit of queued blocks to emerge) int 1024 1 1000000 # Maximum number of blocks to be queued that are to be loaded from file. # This limit is enforced per player. -emergequeue_limit_diskonly (Per-player limit of queued blocks load from disk) int 128 +emergequeue_limit_diskonly (Per-player limit of queued blocks load from disk) int 128 1 1000000 # Maximum number of blocks to be queued that are to be generated. # This limit is enforced per player. -emergequeue_limit_generate (Per-player limit of queued blocks to generate) int 128 +emergequeue_limit_generate (Per-player limit of queued blocks to generate) int 128 1 1000000 # Number of emerge threads to use. # Value 0: diff --git a/src/emerge.cpp b/src/emerge.cpp index 3a2244d7e..bd1c1726d 100644 --- a/src/emerge.cpp +++ b/src/emerge.cpp @@ -165,20 +165,17 @@ EmergeManager::EmergeManager(Server *server) if (nthreads < 1) nthreads = 1; - m_qlimit_total = g_settings->getU16("emergequeue_limit_total"); + m_qlimit_total = g_settings->getU32("emergequeue_limit_total"); // FIXME: these fallback values are probably not good - if (!g_settings->getU16NoEx("emergequeue_limit_diskonly", m_qlimit_diskonly)) + if (!g_settings->getU32NoEx("emergequeue_limit_diskonly", m_qlimit_diskonly)) m_qlimit_diskonly = nthreads * 5 + 1; - if (!g_settings->getU16NoEx("emergequeue_limit_generate", m_qlimit_generate)) + if (!g_settings->getU32NoEx("emergequeue_limit_generate", m_qlimit_generate)) m_qlimit_generate = nthreads + 1; // don't trust user input for something very important like this - if (m_qlimit_total < 1) - m_qlimit_total = 1; - if (m_qlimit_diskonly < 1) - m_qlimit_diskonly = 1; - if (m_qlimit_generate < 1) - m_qlimit_generate = 1; + m_qlimit_total = rangelim(m_qlimit_total, 1, 1000000); + m_qlimit_diskonly = rangelim(m_qlimit_diskonly, 1, 1000000); + m_qlimit_generate = rangelim(m_qlimit_generate, 1, 1000000); for (s16 i = 0; i < nthreads; i++) m_threads.push_back(new EmergeThread(server, i)); @@ -425,14 +422,14 @@ bool EmergeManager::pushBlockEmergeData( void *callback_param, bool *entry_already_exists) { - u16 &count_peer = m_peer_queue_count[peer_requested]; + u32 &count_peer = m_peer_queue_count[peer_requested]; if ((flags & BLOCK_EMERGE_FORCE_QUEUE) == 0) { if (m_blocks_enqueued.size() >= m_qlimit_total) return false; if (peer_requested != PEER_ID_INEXISTENT) { - u16 qlimit_peer = (flags & BLOCK_EMERGE_ALLOW_GEN) ? + u32 qlimit_peer = (flags & BLOCK_EMERGE_ALLOW_GEN) ? m_qlimit_generate : m_qlimit_diskonly; if (count_peer >= qlimit_peer) return false; @@ -467,20 +464,18 @@ bool EmergeManager::pushBlockEmergeData( bool EmergeManager::popBlockEmergeData(v3s16 pos, BlockEmergeData *bedata) { - std::map::iterator it; - std::unordered_map::iterator it2; - - it = m_blocks_enqueued.find(pos); + auto it = m_blocks_enqueued.find(pos); if (it == m_blocks_enqueued.end()) return false; *bedata = it->second; - it2 = m_peer_queue_count.find(bedata->peer_requested); + auto it2 = m_peer_queue_count.find(bedata->peer_requested); if (it2 == m_peer_queue_count.end()) return false; - u16 &count_peer = it2->second; + u32 &count_peer = it2->second; + assert(count_peer != 0); count_peer--; diff --git a/src/emerge.h b/src/emerge.h index b060226f8..e2d727973 100644 --- a/src/emerge.h +++ b/src/emerge.h @@ -194,11 +194,11 @@ private: std::mutex m_queue_mutex; std::map m_blocks_enqueued; - std::unordered_map m_peer_queue_count; + std::unordered_map m_peer_queue_count; - u16 m_qlimit_total; - u16 m_qlimit_diskonly; - u16 m_qlimit_generate; + u32 m_qlimit_total; + u32 m_qlimit_diskonly; + u32 m_qlimit_generate; // Managers of various map generation-related components // Note that each Mapgen gets a copy(!) of these to work with diff --git a/src/settings.cpp b/src/settings.cpp index 0a9424994..ba4629a6f 100644 --- a/src/settings.cpp +++ b/src/settings.cpp @@ -755,6 +755,15 @@ bool Settings::getS16NoEx(const std::string &name, s16 &val) const } } +bool Settings::getU32NoEx(const std::string &name, u32 &val) const +{ + try { + val = getU32(name); + return true; + } catch (SettingNotFoundException &e) { + return false; + } +} bool Settings::getS32NoEx(const std::string &name, s32 &val) const { diff --git a/src/settings.h b/src/settings.h index 7791413b9..4e32a3488 100644 --- a/src/settings.h +++ b/src/settings.h @@ -186,6 +186,7 @@ public: bool getFlag(const std::string &name) const; bool getU16NoEx(const std::string &name, u16 &val) const; bool getS16NoEx(const std::string &name, s16 &val) const; + bool getU32NoEx(const std::string &name, u32 &val) const; bool getS32NoEx(const std::string &name, s32 &val) const; bool getU64NoEx(const std::string &name, u64 &val) const; bool getFloatNoEx(const std::string &name, float &val) const;