Fix weapon special filters can lead to infinite recursion

This adds a recursion counter which runs on a per-weapon basis; if the
recursion limit is reached then the last level of recursion is assumed
to have returned false. A warning is printed (with error severity).

At the user-visible layer, that gives the following logic:

* Abilities that depend on another ability being active will be
  inactive.
* Pairs of abilities, where X depends on Y being inactive and Y
  depends on X being inactive, will end up with them both inactive.

The limit is defined by ATTACK_RECURSION_LIMIT in attack_type.cpp.
The minimum for passing all the unit tests is 2, but I've left some
headroom by setting it to 4.

With the recursion limit set to 1, the following tests fail, which
seems reasonable because they're checking that the special is on
a particular type of weapon.
* event_test_filter_attack_specials
* event_test_filter_attack_opponent_weapon_condition
* event_test_filter_attack_student_weapon_condition

Output from the four_cycle_recursion_branching test:
```
error unit: Recursion limit reached for weapon 'melee_attack' while checking filter 'special_type_active = parry'
error unit: Recursion limit reached for weapon 'melee_attack' while checking filter 'special_type_active = poison'
error unit: Recursion limit reached for weapon 'melee_attack' while checking filter 'special_type_active = damage'
```

There's deduplication to prevent the logfiles getting spammed during
AI turns. As implemented, the two tests mentioned below print 3
messages each; these tests are added in separate commits because the
tests are shared between PRs while we discuss the right mechanism for
guarding against recursion.

With no rate limit:
* four_cycle_recursion_branching prints 1280 logs
* event_test_filter_attack_student_weapon_condition prints 320

With a rate limit via a flag that's reset in recursion_guard's destructor:
* four_cycle_recursion_branching prints 80 logs
* event_test_filter_attack_student_weapon_condition prints 160

Resetting the flag in specials_context_t's destructor gets better numbers,
but splits the logic across .cpp files. I think the trade-off isn't worth it:
* four_cycle_recursion_branching prints 40 logs
* event_test_filter_attack_student_weapon_condition prints 132

Co-authored-by: newfrenchy83 <eric.belleux@gmail.com>

changelog_entries/fix_infinite_recursion.md

@@ -0,0 +1,2 @@
+ ### WML Engine
+   * Fix crash when weapon specials' filters lead to infinite recursion (issue #8940)

src/units/attack_type.cpp

@@ -46,6 +46,27 @@ static lg::log_domain log_unit("unit");
 static lg::log_domain log_wml("wml");
 #define ERR_WML LOG_STREAM(err, log_wml)
 
+namespace {
+/**
+ * Value of attack_type::num_recursion_ at which allocations of further recursion_guards fail. This
+ * value is used per weapon, so if two weapon specials are depending on each other being active then
+ * with ATTACK_RECURSION_LIMIT = 3 the recursion could go 6 levels deep (and then return false on
+ * the 7th call to matches_simple_filter).
+ *
+ * The counter is checked at the start of matches_simple_filter, and even the first level needs an
+ * allocation; setting the limit to zero would make matches_simple_filter always return false.
+ *
+ * With the recursion limit set to 1, the following tests fail; they just need a reasonable depth.
+ * event_test_filter_attack_specials
+ * event_test_filter_attack_opponent_weapon_condition
+ * event_test_filter_attack_student_weapon_condition
+ *
+ * With the limit set to 2, all tests pass, but as the limit only affects cases that would otherwise
+ * lead to a crash, it seems reasonable to leave a little headroom for more complex logic.
+ */
+constexpr unsigned int ATTACK_RECURSION_LIMIT = 4;
+};
+
 attack_type::attack_type(const config& cfg) :
 	self_loc_(),
 	other_loc_(),
@@ -96,12 +117,50 @@ std::string attack_type::accuracy_parry_description() const
 	return s.str();
 }
 
+namespace {
+/**
+ * Print "Recursion limit reached" log messages, including deduplication if the same problem has
+ * already been logged.
+ */
+void show_recursion_warning(const attack_type& attack, const config& filter) {
+	// This function is only called when the recursion limit has already been reached, meaning the
+	// filter has already been parsed multiple times, so I'm not trying to optimize the performance
+	// of this; it's merely to prevent the logs getting spammed. For example, each of
+	// four_cycle_recursion_branching and event_test_filter_attack_student_weapon_condition only log
+	// 3 unique messages, but without deduplication they'd log 1280 and 392 respectively.
+	static std::vector<std::tuple<std::string, std::string>> already_shown;
+
+	auto identifier = std::tuple<std::string, std::string>{attack.id(), filter.debug()};
+	if(utils::contains(already_shown, identifier)) {
+		return;
+	}
+
+	std::string_view filter_text_view = std::get<1>(identifier);
+	utils::trim(filter_text_view);
+	ERR_UT << "Recursion limit reached for weapon '" << attack.id()
+		<< "' while checking filter '" << filter_text_view << "'";
+
+	// Arbitrary limit, just ensuring that having a huge number of specials causing recursion
+	// warnings can't lead to unbounded memory consumption here.
+	if(already_shown.size() > 100) {
+		already_shown.clear();
+	}
+	already_shown.push_back(std::move(identifier));
+}
+
 /**
  * Returns whether or not *this matches the given @a filter, ignoring the
  * complexities introduced by [and], [or], and [not].
  */
-static bool matches_simple_filter(const attack_type & attack, const config & filter, const std::string& tag_name)
+bool matches_simple_filter(const attack_type& attack, const config& filter, const std::string& tag_name)
 {
+	//update and check variable_recursion for prevent check special_id/type_active in case of infinite recursion.
+	attack_type::recursion_guard filter_lock= attack.update_variables_recursion();
+	if(!filter_lock) {
+		show_recursion_warning(attack, filter);
+		return false;
+	}
+
 	const std::set<std::string> filter_range = utils::split_set(filter["range"].str());
 	const std::string& filter_damage = filter["damage"];
 	const std::string& filter_attacks = filter["number"];
@@ -144,7 +203,10 @@ static bool matches_simple_filter(const attack_type & attack, const config & fil
 		return false;
 
 	if (!filter_type.empty()){
-		//if special is type "damage_type" then check attack.type() only for don't have infinite recursion by calling damage_type() below.
+		// Although there's a general guard against infinite recursion, the "damage_type" special
+		// should always use the base type of the weapon. Otherwise it will flip-flop between the
+		// special being active or inactive based on whether ATTACK_RECURSION_LIMIT is even or odd;
+		// without this it will also behave differently when calculating resistance_against.
 		if(tag_name == "damage_type"){
 			if (filter_type.count(attack.type()) == 0){
 				return false;
@@ -252,6 +314,7 @@ static bool matches_simple_filter(const attack_type & attack, const config & fil
 	// Passed all tests.
 	return true;
 }
+} // anonymous namespace
 
 /**
  * Returns whether or not *this matches the given @a filter.
@@ -572,6 +635,50 @@ bool attack_type::describe_modification(const config& cfg,std::string* descripti
 	return true;
 }
 
+attack_type::recursion_guard attack_type::update_variables_recursion() const
+{
+	if(num_recursion_ < ATTACK_RECURSION_LIMIT) {
+		return recursion_guard(*this);
+	}
+	return recursion_guard();
+}
+
+attack_type::recursion_guard::recursion_guard() = default;
+
+attack_type::recursion_guard::recursion_guard(const attack_type& weapon)
+	: parent(weapon.shared_from_this())
+{
+	weapon.num_recursion_++;
+}
+
+attack_type::recursion_guard::recursion_guard(attack_type::recursion_guard&& other)
+{
+	std::swap(parent, other.parent);
+}
+
+attack_type::recursion_guard::operator bool() const {
+	return bool(parent);
+}
+
+attack_type::recursion_guard& attack_type::recursion_guard::operator=(attack_type::recursion_guard&& other)
+{
+	// This is only intended to move ownership to a longer-living variable. Assigning to an instance that
+	// already has a parent implies that the caller is going to recurse and needs a recursion allocation,
+	// but is accidentally dropping one of the allocations that it already has; hence the asserts.
+	assert(this != &other);
+	assert(!parent);
+	std::swap(parent, other.parent);
+	return *this;
+}
+
+attack_type::recursion_guard::~recursion_guard()
+{
+	if(parent) {
+		assert(parent->num_recursion_ > 0);
+		parent->num_recursion_--;
+	}
+}
+
 void attack_type::write(config& cfg) const
 {
 	cfg["description"] = description_;

src/units/attack_type.hpp

@@ -150,6 +150,56 @@ public:
 	inline config to_config() const { config c; write(c); return c; }
 
 	void add_formula_context(wfl::map_formula_callable&) const;
+
+	/**
+	 * Helper similar to std::unique_lock for detecting when calculations such as has_special
+	 * have entered infinite recursion.
+	 *
+	 * This assumes that there's only a single thread accessing the attack_type, it's a lightweight
+	 * increment/decrement counter rather than a mutex.
+	 */
+	class recursion_guard {
+		friend class attack_type;
+		/**
+		 * Only expected to be called in update_variables_recursion(), which handles some of the checks.
+		 */
+		explicit recursion_guard(const attack_type& weapon);
+	public:
+		/**
+		 * Construct an empty instance, only useful for extending the lifetime of a
+		 * recursion_guard returned from weapon.update_variables_recursion() by
+		 * std::moving it to an instance declared in a larger scope.
+		 */
+		explicit recursion_guard();
+
+		/**
+		 * Returns true if a level of recursion was available at the time when update_variables_recursion()
+		 * created this object.
+		 */
+		operator bool() const;
+
+		recursion_guard(recursion_guard&& other);
+		recursion_guard(const recursion_guard& other) = delete;
+		recursion_guard& operator=(recursion_guard&&);
+		recursion_guard& operator=(const recursion_guard&) = delete;
+		~recursion_guard();
+	private:
+		std::shared_ptr<const attack_type> parent;
+	};
+
+	/**
+	 * Tests which might otherwise cause infinite recursion should call this, check that the
+	 * returned object evaluates to true, and then keep the object returned as long as the
+	 * recursion might occur, similar to a reentrant mutex that's limited to a small number of
+	 * reentrances.
+	 *
+	 * This is a cheap function, so no reason to optimise by doing some filters before calling it.
+	 * However, it only expects to be called in a single thread, but the whole of attack_type makes
+	 * that assumption, for example its' mutable members are assumed to be set up by the current
+	 * caller (or caller's caller, probably several layers up).
+	 */
+	recursion_guard update_variables_recursion() const;
+
 private:
 	// In unit_abilities.cpp:
 
@@ -365,6 +415,8 @@ private:
 	int parry_;
 	config specials_;
 	bool changed_;
+	/** Number of instances of recursion_guard that are currently allocated permission to recurse */
+	mutable unsigned int num_recursion_ = 0;
 };
 
 using attack_list = std::vector<attack_ptr>;