Merge branch 'topic/awelzel/3122-attribute-expression-seeds'

* topic/awelzel/3122-attribute-expression-seeds:
  UsageAnalyzer: Collect identifiers found in attributes as seeds

CHANGES

@@ -1,3 +1,22 @@
+6.1.0-dev.254 | 2023-08-02 09:23:18 +0200
+
+  * GH-3122: UsageAnalyzer: Collect identifiers found in attributes as seeds (Arne Welzel, Corelight)
+
+    This marks every identifier used within an attribute as seeds. The scenario
+    this avoids is functions referenced through attributes on unused tables or
+    record types (&default, &expire_func, ...) being dinged as unused as
+    that's rather confusing.
+
+    Also adds test for the above and a light smoke test into language/ as it
+    doesn't appear we had coverage here.
+
+  * Force refresh of all CI docker images (Tim Wojtulewicz, Corelight)
+
+    There's something going on with the image cache on Cirrus where the images
+    are sometimes vanishing from the cache, thus causing builds to fail
+    because it can't load them. This forces a rebuild of all of the images,
+    thus refreshing the cached version of all of them.
+
 6.1.0-dev.250 | 2023-08-01 09:58:04 -0700
 
   * Fix memory leak in script_opt's Expr code (Tim Wojtulewicz, Corelight)

VERSION

@@ -1 +1 @@
-6.1.0-dev.250
+6.1.0-dev.254

src/script_opt/UsageAnalyzer.cc

@@ -98,8 +98,60 @@ UsageAnalyzer::UsageAnalyzer(std::vector<FuncInfo>& funcs)
 		}
 	}
 
+// Find all identifiers in attributes for seeding.
+//
+// General motivation: Dinging functions referenced from &default or &expire_func
+// on tables or &default on record types is confusing when really the containing
+// table/type is unused. Any IDs references from attributes are therefore
+// implicitly used as seeds.
+class AttrExprIdsCollector : public TraversalCallback
+	{
+public:
+	TraversalCode PreAttr(const Attr* attr) override
+		{
+		++attr_depth;
+		return TC_CONTINUE;
+		}
+
+	TraversalCode PostAttr(const Attr* attr) override
+		{
+		--attr_depth;
+		return TC_CONTINUE;
+		}
+
+	TraversalCode PreType(const Type* t) override
+		{
+		if ( analyzed_types.count(t) > 0 )
+			return TC_ABORTSTMT;
+
+		analyzed_types.insert(t);
+		return TC_CONTINUE;
+		}
+
+	TraversalCode PreID(const ID* id) override
+		{
+		if ( ids.count(id) > 0 )
+			return TC_ABORTSTMT;
+
+		if ( attr_depth > 0 )
+			ids.insert(id);
+
+		id->GetType()->Traverse(this);
+
+		if ( auto& attrs = id->GetAttrs() )
+			attrs->Traverse(this);
+
+		return TC_CONTINUE;
+		}
+
+	int attr_depth = 0; // Are we in an attribute?
+	std::set<const detail::ID*> ids; // List of IDs found in attributes.
+	std::set<const Type*> analyzed_types; // Endless recursion avoidance.
+	};
+
 void UsageAnalyzer::FindSeeds(IDSet& seeds) const
 	{
+	AttrExprIdsCollector attr_ids_collector;
 	for ( auto& gpair : global_scope()->Vars() )
 		{
 		auto& id = gpair.second;
@@ -125,7 +177,13 @@ void UsageAnalyzer::FindSeeds(IDSet& seeds) const
 		// use it.
 		if ( id->IsExport() || id->ModuleName() == "GLOBAL" )
 			seeds.insert(id.get());
+		else
+			// ...otherwise, find all IDs referenced from attribute expressions
+			// found through this identifier.
+			id->Traverse(&attr_ids_collector);
 		}
+
+	seeds.insert(attr_ids_collector.ids.begin(), attr_ids_collector.ids.end());
 	}
 
 const Func* UsageAnalyzer::GetFuncIfAny(const ID* id) const

testing/btest/Baseline/language.usage-analyzer-record-attributes/.stderr

@@ -0,0 +1 @@
+### BTest baseline data generated by btest-diff. Do not edit. Use "btest -U/-u" to update. Requires BTest >= 0.63.

testing/btest/Baseline/language.usage-analyzer-table-attributes/.stderr

@@ -0,0 +1 @@
+### BTest baseline data generated by btest-diff. Do not edit. Use "btest -U/-u" to update. Requires BTest >= 0.63.

testing/btest/Baseline/language.usage-analyzer/.stderr

@@ -0,0 +1,5 @@
+### BTest baseline data generated by btest-diff. Do not edit. Use "btest -U/-u" to update. Requires BTest >= 0.63.
+warning in <...>/usage-analyzer.zeek, line 11: handler for non-existing event cannot be invoked (MyModule::unused)
+warning in <...>/usage-analyzer.zeek, line 7: non-exported function does not have any callers (MyModule::gen_id)
+warning in <...>/usage-analyzer.zeek, line 8: non-exported function does not have any callers (MyModule::gen_id2)
+warning in <...>/usage-analyzer.zeek, line 10: non-exported function does not have any callers (MyModule::helper)

testing/btest/language/usage-analyzer-record-attributes.zeek

@@ -0,0 +1,19 @@
+# @TEST-DOC: Usage analyzer marked lambdas and functions in attribute expressions of unused tables or record types as unused. That is a bit confusing. Regression test for #3122.
+# @TEST-EXEC: zeek -b %INPUT
+# @TEST-EXEC: TEST_DIFF_CANONIFIER=$SCRIPTS/diff-remove-abspath btest-diff .stderr
+
+module MyModule;
+
+function gen_id(): string {
+	return cat(rand(10000));
+}
+
+type R1: record {
+	xxx_id: string &default=gen_id();
+};
+
+# Seems we can't actually put functions on &default on records, so the
+# following uses a directly invoked lambda instead.
+type R2: record {
+	xxx_id: string &default=(function(): string { return cat(rand(10000)); })();
+};

testing/btest/language/usage-analyzer-table-attributes.zeek

@@ -0,0 +1,44 @@
+# @TEST-DOC: Usage analyzer marked lambdas and functions in attribute expressions of unused tables or record types as unused. That is a bit confusing. Regression test for #3122.
+# @TEST-EXEC: zeek -b %INPUT
+# @TEST-EXEC: TEST_DIFF_CANONIFIER=$SCRIPTS/diff-remove-abspath btest-diff .stderr
+
+module MyModule;
+
+## Lambda on table.
+const ids1: table[count] of string = {
+        [1] = "One",
+} &default=function(c: count): string {
+	return fmt("unknown-%d", c);
+};
+
+
+## External default function
+function default_id(c: count): string {
+	return fmt("unknown-%d", c);
+}
+
+const ids2: table[count] of string = {
+        [1] = "One",
+} &default=default_id;
+
+
+## &default expression using function
+function default_id2(): string {
+	return "";
+}
+
+const ids3: table[count] of string = {
+        [1] = "One",
+} &default=default_id2() + "";
+
+
+## &expire_func lambda using another function
+function expire_f(t: table[count] of string, c: count): interval {
+	return 0.0sec;
+}
+
+const ids4: table[count] of string = {
+        [1] = "One",
+} &expire_func=function(t: table[count] of string, c: count): interval {
+	return expire_f(t, c);
+};

testing/btest/language/usage-analyzer.zeek

@@ -0,0 +1,11 @@
+# @TEST-DOC: Simple testing for unused function/event detection to ensure nothing breaks when modifying it.
+# @TEST-EXEC: zeek -b %INPUT
+# @TEST-EXEC: TEST_DIFF_CANONIFIER=$SCRIPTS/diff-remove-abspath btest-diff .stderr
+
+module MyModule;
+
+function gen_id(): string { return cat(rand(10000)); }
+function gen_id2(): string { return gen_id2(); }
+
+function helper() { }
+event MyModule::unused(c: count) { helper(); }