Add crash fix and tests for minetest.find_nodes_in_area()

For some specific out of bounds values, the volume calculation in
minetest.find_nodes_in_area() is off by about four million nodes.
Unfortunately that behaviour made it trivial to crash Mineclonia,
as Minetest immediately crashes upon encountering large numbers.

This commit introduces a wrapper around minetest.find_nodes_in_area()
which should avoid a crash. Additionally, three self tests are executed
when a server starts; they crash Mineclonia in case the workaround fails.

mods/MISC/mcl_engine_workarounds/init.lua

@@ -0,0 +1,98 @@
+local clamp = function(value, min, max)
+	assert( min < max )
+	if value < min then
+		return min
+	end
+	if value > max then
+		return max
+	end
+	return value
+end
+
+assert( clamp(000, -100, 100) == 000 )
+assert( clamp(999, -100, 100) == 100 )
+assert( clamp(999, -999, 999) == 999 )
+assert( clamp(998, 999, 1999) == 999 )
+assert( clamp(999, 999, 1999) == 999 )
+
+local clamp_s16 = function(value)
+	-- seems minetest hangs on -32768 and 32767
+	return clamp(value, -32767, 32766)
+end
+
+assert( clamp_s16(000000) == 000000 )
+assert( clamp_s16(000001) == 000001 )
+assert( clamp_s16(000010) == 000010 )
+assert( clamp_s16(000100) == 000100 )
+assert( clamp_s16(001000) == 001000 )
+assert( clamp_s16(010000) == 010000 )
+assert( clamp_s16(100000) == 032766 )
+
+assert( clamp_s16(-00000) == -00000 )
+assert( clamp_s16(-00009) == -00009 )
+assert( clamp_s16(-00099) == -00099 )
+assert( clamp_s16(-00999) == -00999 )
+assert( clamp_s16(-09999) == -09999 )
+assert( clamp_s16(-99999) == -32767 )
+
+local minetest_find_nodes_in_area = minetest.find_nodes_in_area
+minetest.find_nodes_in_area = function(minp, maxp, ...)
+	if
+		minp.x >= 32767 or minp.x <= -32768 or
+		minp.y >= 32767 or minp.y <= -32768 or
+		minp.z >= 32767 or minp.z <= -32768 or
+		maxp.x >= 32767 or maxp.x <= -32768 or
+		maxp.y >= 32767 or maxp.y <= -32768 or
+		maxp.z >= 32767 or maxp.z <= -32768
+	then
+		minetest.log(
+			"warning",
+			"find_nodes_in_area() called with coords outside interval (-32768, 32767), clamping arguments: " ..
+			"minp { x=" .. minp.x .. ", y=" .. minp.y .. " z=" .. minp.z .. " } " ..
+			"maxp { x=" .. maxp.x .. ", y=" .. maxp.y .. " z=" .. maxp.z .. " } "
+		)
+		return minetest_find_nodes_in_area(
+			{ x=clamp_s16(minp.x), y=clamp_s16(minp.y), z=clamp_s16(minp.z) },
+			{ x=clamp_s16(maxp.x), y=clamp_s16(maxp.y), z=clamp_s16(maxp.z) },
+			...
+		)
+	else
+		return minetest_find_nodes_in_area(
+			minp,
+			maxp,
+			...
+		)
+	end
+end
+
+local test_minetest_find_nodes_in_area_implementation_equivalence = function()
+	-- If any assertion in this test function fails, the wrapper
+	-- for minetest.find_nodes_in_area() does not behave like the
+	-- original function. If you are reading the code because your
+	-- server crashed, please inform the Mineclonia developers.
+	for x = -31000, 31000, 15500 do
+		for y = -31000, 31000, 15500 do
+			for z = -31000, 31000, 15500 do
+				for d = 1, 9, 3 do
+					local minp = { x=x, y=y, z=z }
+					local maxp = { x=x+d, y=y+d, z=z+d }
+					local npos_1, nnum_1 = minetest_find_nodes_in_area(
+						minp,
+						maxp,
+						{ "air", "ignore" }
+					)
+					local npos_2, nnum_2 = minetest.find_nodes_in_area(
+						minp,
+						maxp,
+						{ "air", "ignore" }
+					)
+					assert(#npos_1 == #npos_2)
+					assert(nnum_1["air"] == nnum_2["air"])
+					assert(nnum_1["ignore"] == nnum_2["ignore"])
+				end
+			end
+		end
+	end
+end
+
+minetest.after( 0, test_minetest_find_nodes_in_area_implementation_equivalence )

mods/MISC/mcl_selftests/init.lua

@@ -0,0 +1,113 @@
+local test_minetest_find_nodes_in_area_can_count = function(dtime)
+	-- This function tests that minetest.find_nodes_in_area() can
+	-- count nodes correctly. If it fails, the engine can not be
+	-- trusted to actually count how many nodes of a given type
+	-- are in a given volume. Yes, *it* is bad if that happens.
+	--
+	-- If you are looking at this function because it executes at
+	-- startup and crashes the game, by far the most stupid thing
+	-- you could do is disabling it. Only an absolute moron would
+	-- disable tests that ensure basic functionality still works.
+	--
+	-- Experience has taught me that such warnings are mostly not
+	-- taken seriously by both Minetest mod & core developers. As
+	-- there are very few situations in which someone would read
+	-- the code of the function without a crash, you are probably
+	-- asking yourself how bad it can be. Surely you will want an
+	-- example of what will break if this test breaks. The answer
+	-- to this simple question is equally simple and consists of a
+	-- heartfelt “What the fuck did you say, you stupid fuckwad?”.
+	--
+	-- Alrighty then, let's get it on …
+
+	local pos = { x=30999, y=30999, z=30999 }
+	-- You think there is nothing there? Well, here is the thing:
+	-- With standard settings you can only see map until x=30927,
+	-- although the renderer can actually render up to x=31007 if
+	-- you configure it to. Any statements given by minetest core
+	-- devs that contradict the above assertion are probably lies.
+	--
+	-- In any way, this area should be so far out that no one has
+	-- built here … yet. Now that you know it is possible, I know
+	-- you want to. How though? I suggest to figure the technique
+	-- out yourself, then go on and build invisible lag machines.
+
+	local radius = 3
+	local minp = vector.subtract(pos, radius)
+	local maxp = vector.add(pos, radius)
+	local nodename = "air"
+	local c_nodename = minetest.get_content_id(nodename)
+
+	-- Why not use minetest.set_node() here? Well, some mods do
+	-- trigger on every placement of a node in the entire map …
+	-- and we do not want to crash those mods in this test case.
+	-- (Voxelmanip does not trigger callbacks – so all is well.)
+	--
+	-- And now for a funny story: I initially copied the following
+	-- code from the Minetest developer wiki. Can you spot a typo?
+	-- <https://dev.minetest.net/index.php?title=minetest.get_content_id&action=edit>
+	local vm = minetest.get_voxel_manip()
+	local emin, emax = vm:read_from_map(
+		minp,
+		maxp
+	)
+	local area = VoxelArea:new({
+		MinEdge=emin,
+		MaxEdge=emax
+	})
+	local data = vm:get_data()
+	for z = minp.z, maxp.z do
+		for y = minp.y, maxp.y do
+			local vi = area:index(minp.x, y, z)
+			for x = minp.x, maxp.y do
+				data[vi] = c_nodename
+				vi = vi + 1
+			end
+		end
+	end
+	vm:set_data(data)
+	vm:write_to_map()
+	local npos, nnum = minetest.find_nodes_in_area(
+		minp,
+		maxp,
+		{ nodename }
+	)
+	local nodes_expected = math.pow( 1 + (2 * radius), 3 )
+	local nodes_counted = nnum[nodename]
+	local nodes_difference = nodes_expected - nodes_counted
+	-- Yes, the following line is supposed to crash the game in
+	-- the case that Minetest forgot how to count the number of
+	-- nodes in a three dimensional volume it just filled. This
+	-- function contains an excellent explanation on what not to
+	-- do further up. I strongly suggest to pester Minetest core
+	-- devs about this issue and not the author of the function,
+	-- should this test ever fail.
+	assert ( 0 == nodes_difference )
+end
+
+minetest.after( 0, test_minetest_find_nodes_in_area_can_count )
+
+local test_minetest_find_nodes_in_area_crash = function(dtime)
+	-- And now for our feature presentation, where we call the
+	-- function “minetest.find_nodes_in_area()” with a position
+	-- out of bounds! Will it crash? Who knows‽ If it does, the
+	-- workaround is not working though, so it should be patched.
+
+	local pos = { x=32767, y=32767, z=32767 }
+	-- Note that not all out of bounds values actually crash the
+	-- function minetest.find_nodes_in_area()“. In fact, the vast
+	-- majority of out of bounds values do not crash the function.
+
+	local radius = 3
+	local minp = vector.subtract(pos, radius)
+	local maxp = vector.add(pos, radius)
+	local nodename = "air"
+	local npos, nnum = minetest.find_nodes_in_area(
+		minp,
+		maxp,
+		{ nodename }
+	)
+	-- That's all, folks!
+end
+
+minetest.after( 0, test_minetest_find_nodes_in_area_crash )