Fix /emergeblocks crashing in debug builds (#11461)

Wuzzy2 · web-flow · commit e7cd4cfa2548 · 2021-07-31T19:54:40.000+02:00
The reason for the bug was an u16 overflow, thus failing the assert. This only happened in Debug build but not in Release builds.
diff --git 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
@@ -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<v3s16, BlockEmergeData>::iterator it;
-	std::unordered_map<u16, u16>::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
@@ -194,11 +194,11 @@ class EmergeManager {
 
 	std::mutex m_queue_mutex;
 	std::map<v3s16, BlockEmergeData> m_blocks_enqueued;
-	std::unordered_map<u16, u16> m_peer_queue_count;
+	std::unordered_map<u16, u32> 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
@@ -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
@@ -186,6 +186,7 @@ class Settings {
 	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;

Original file line number	Diff line number	Diff line change
`@@ -755,6 +755,15 @@ bool Settings::getS16NoEx(const std::string &name, s16 &val) const`
`755`	`755`	`}`
`756`	`756`	`}`
`757`	`757`
	`758`	`+bool Settings::getU32NoEx(const std::string &name, u32 &val) const`
	`759`	`+{`
	`760`	`+ try {`
	`761`	`+ val = getU32(name);`
	`762`	`+ return true;`
	`763`	`+ } catch (SettingNotFoundException &e) {`
	`764`	`+ return false;`
	`765`	`+ }`
	`766`	`+}`
`758`	`767`
`759`	`768`	`bool Settings::getS32NoEx(const std::string &name, s32 &val) const`
`760`	`769`	`{`