diff --git a/src/script_opt/UsageAnalyzer.cc b/src/script_opt/UsageAnalyzer.cc index c1b5f2d8cb..ac76861c11 100644 --- a/src/script_opt/UsageAnalyzer.cc +++ b/src/script_opt/UsageAnalyzer.cc @@ -98,8 +98,60 @@ UsageAnalyzer::UsageAnalyzer(std::vector& 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 ids; // List of IDs found in attributes. + std::set 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 diff --git a/testing/btest/Baseline/language.usage-analyzer-record-attributes/.stderr b/testing/btest/Baseline/language.usage-analyzer-record-attributes/.stderr new file mode 100644 index 0000000000..49d861c74c --- /dev/null +++ b/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. diff --git a/testing/btest/Baseline/language.usage-analyzer-table-attributes/.stderr b/testing/btest/Baseline/language.usage-analyzer-table-attributes/.stderr new file mode 100644 index 0000000000..49d861c74c --- /dev/null +++ b/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. diff --git a/testing/btest/Baseline/language.usage-analyzer/.stderr b/testing/btest/Baseline/language.usage-analyzer/.stderr new file mode 100644 index 0000000000..a6b8910065 --- /dev/null +++ b/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) diff --git a/testing/btest/language/usage-analyzer-record-attributes.zeek b/testing/btest/language/usage-analyzer-record-attributes.zeek new file mode 100644 index 0000000000..3d90b4f684 --- /dev/null +++ b/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)); })(); +}; diff --git a/testing/btest/language/usage-analyzer-table-attributes.zeek b/testing/btest/language/usage-analyzer-table-attributes.zeek new file mode 100644 index 0000000000..5625f39980 --- /dev/null +++ b/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); +}; diff --git a/testing/btest/language/usage-analyzer.zeek b/testing/btest/language/usage-analyzer.zeek new file mode 100644 index 0000000000..81a8bb9d40 --- /dev/null +++ b/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(); }