From a93ede7c2e4ad11a2d80ab2d6f17429afe81f644 Mon Sep 17 00:00:00 2001 From: Mark Stemm Date: Wed, 24 Aug 2022 17:11:27 -0700 Subject: [PATCH 1/2] Fix(engine): include parse positions in compile errors Now that ASTs have parse positions and the compiler will return the position of the last error, use that in falco rules to return errors within condition strings instead of reporting the position as the beginning of the condition. This led to a change in the filter_ruleset interface--now, an ast is compiled to a filter before being passed to the filter_ruleset object. That avoids polluting the interface with a lot of details about rule_loader contexts, errors, etc. The ast is still provided in case the filter_ruleset wants to do indexing/analysis of the filter. Signed-off-by: Mark Stemm --- tests/engine/test_rulesets.cpp | 45 ++++++++++++++++------ userspace/engine/evttype_index_ruleset.cpp | 3 +- userspace/engine/evttype_index_ruleset.h | 1 + userspace/engine/filter_ruleset.h | 14 ++++--- userspace/engine/rule_loader_compiler.cpp | 38 ++++++++++++------ 5 files changed, 71 insertions(+), 30 deletions(-) diff --git a/tests/engine/test_rulesets.cpp b/tests/engine/test_rulesets.cpp index d782a1764ac..0f45f7e682c 100644 --- a/tests/engine/test_rulesets.cpp +++ b/tests/engine/test_rulesets.cpp @@ -27,30 +27,51 @@ static uint16_t other_non_default_ruleset = 2; static std::set tags = {"some_tag", "some_other_tag"}; static std::set evttypes = { ppm_event_type::PPME_GENERIC_E }; -static std::shared_ptr create_filter() +static std::shared_ptr create_factory() +{ + std::shared_ptr ret(new sinsp_filter_factory(NULL)); + + return ret; +} + +static std::shared_ptr create_ast( + std::shared_ptr f) { libsinsp::filter::parser parser("evt.type=open"); std::shared_ptr ret(parser.parse()); + return ret; } -static std::shared_ptr create_ruleset() +static std::shared_ptr create_filter( + std::shared_ptr f, + std::shared_ptr ast) +{ + sinsp_filter_compiler compiler(f, ast.get()); + std::shared_ptr filter(compiler.compile()); + + return filter; +} + +static std::shared_ptr create_ruleset( + std::shared_ptr f) { - std::shared_ptr f(new sinsp_filter_factory(NULL)); std::shared_ptr ret(new evttype_index_ruleset(f)); return ret; } TEST_CASE("Should enable/disable on ruleset", "[rulesets]") { - auto r = create_ruleset(); - auto filter = create_filter(); + auto f = create_factory(); + auto r = create_ruleset(f); + auto ast = create_ast(f); + auto filter = create_filter(f, ast); falco_rule rule; rule.name = "one_rule"; rule.source = falco_common::syscall_source; rule.tags = tags; - r->add(rule, filter); + r->add(rule, filter, ast); SECTION("Should enable/disable for exact match w/ default ruleset") { @@ -184,21 +205,23 @@ TEST_CASE("Should enable/disable on ruleset", "[rulesets]") TEST_CASE("Should enable/disable on ruleset for incremental adding tags", "[rulesets]") { - auto r = create_ruleset(); + auto f = create_factory(); + auto r = create_ruleset(f); + auto ast = create_ast(f); - auto rule1_filter = create_filter(); + auto rule1_filter = create_filter(f, ast); falco_rule rule1; rule1.name = "one_rule"; rule1.source = falco_common::syscall_source; rule1.tags = {"rule1_tag"}; - r->add(rule1, rule1_filter); + r->add(rule1, rule1_filter, ast); - auto rule2_filter = create_filter(); + auto rule2_filter = create_filter(f, ast); falco_rule rule2; rule2.name = "two_rule"; rule2.source = falco_common::syscall_source; rule2.tags = {"rule2_tag"}; - r->add(rule2, rule2_filter); + r->add(rule2, rule2_filter, ast); std::set want_tags; diff --git a/userspace/engine/evttype_index_ruleset.cpp b/userspace/engine/evttype_index_ruleset.cpp index e52655eb3f6..58c9e055f7a 100644 --- a/userspace/engine/evttype_index_ruleset.cpp +++ b/userspace/engine/evttype_index_ruleset.cpp @@ -153,12 +153,11 @@ void evttype_index_ruleset::ruleset_filters::evttypes_for_ruleset(std::set filter, std::shared_ptr condition) { try { - sinsp_filter_compiler compiler(m_filter_factory, condition.get()); - shared_ptr filter(compiler.compile()); std::shared_ptr wrap(new filter_wrapper()); wrap->rule = rule; wrap->filter = filter; diff --git a/userspace/engine/evttype_index_ruleset.h b/userspace/engine/evttype_index_ruleset.h index 176bab09600..e7644abc52a 100644 --- a/userspace/engine/evttype_index_ruleset.h +++ b/userspace/engine/evttype_index_ruleset.h @@ -41,6 +41,7 @@ class evttype_index_ruleset: public filter_ruleset void add( const falco_rule& rule, + std::shared_ptr filter, std::shared_ptr condition) override; void clear() override; diff --git a/userspace/engine/filter_ruleset.h b/userspace/engine/filter_ruleset.h index 77625139b16..c50632e7ea3 100644 --- a/userspace/engine/filter_ruleset.h +++ b/userspace/engine/filter_ruleset.h @@ -32,16 +32,20 @@ class filter_ruleset virtual ~filter_ruleset() = default; /*! - \brief Adds a rule and its filtering condition inside the manager. - An exception is thrown is case of error. This method only adds the rule - inside the internal collection, but does not enable it for any ruleset. - The rule must be enabled for one or more rulesets with the enable() or - enable_tags() methods. + \brief Adds a rule and its filtering filter + condition inside the manager. + This method only adds the rule inside the internal collection, + but does not enable it for any ruleset. The rule must be enabled + for one or more rulesets with the enable() or enable_tags() methods. + The ast representation of the rule's condition is provided to allow + the filter_ruleset object to parse the ast to obtain event types + or do other analysis/indexing of the condition. \param rule The rule to be added + \param the filter representing the rule's filtering condition. \param condition The AST representing the rule's filtering condition */ virtual void add( const falco_rule& rule, + std::shared_ptr filter, std::shared_ptr condition) = 0; /*! diff --git a/userspace/engine/rule_loader_compiler.cpp b/userspace/engine/rule_loader_compiler.cpp index ddd85429f66..b54d84aed3f 100644 --- a/userspace/engine/rule_loader_compiler.cpp +++ b/userspace/engine/rule_loader_compiler.cpp @@ -234,6 +234,7 @@ static bool resolve_list(std::string& cnd, const rule_loader::list_info& list) static void resolve_macros( indexed_vector& macros, std::shared_ptr& ast, + const std::string& condition, uint32_t visibility, const rule_loader::context &ctx) { @@ -248,15 +249,22 @@ static void resolve_macros( macro_resolver.run(ast); // Note: only complaining about the first unknown macro - THROW(!macro_resolver.get_unknown_macros().empty(), - std::string("Undefined macro '") - + *macro_resolver.get_unknown_macros().begin() - + "' used in filter.", - ctx); + const filter_macro_resolver::macro_info_map& unresolved_macros = macro_resolver.get_unknown_macros(); + if(!unresolved_macros.empty()) + { + auto it = unresolved_macros.begin(); + const rule_loader::context cond_ctx(it->second, condition, ctx); + + THROW(true, + std::string("Undefined macro '") + + it->first + + "' used in filter.", + cond_ctx); + } - for (auto &m : macro_resolver.get_resolved_macros()) + for (auto &it : macro_resolver.get_resolved_macros()) { - macros.at(m)->used = true; + macros.at(it.first)->used = true; } } @@ -363,7 +371,7 @@ void rule_loader::compiler::compile_macros_infos( for (auto &m : out) { - resolve_macros(out, m.cond_ast, m.visibility, m.ctx); + resolve_macros(out, m.cond_ast, m.cond, m.visibility, m.ctx); } } @@ -404,7 +412,7 @@ void rule_loader::compiler::compile_rule_infos( r.exceptions, rule.exception_fields, condition); } auto ast = parse_condition(condition, lists, r.cond_ctx); - resolve_macros(macros, ast, MAX_VISIBILITY, r.ctx); + resolve_macros(macros, ast, condition, MAX_VISIBILITY, r.ctx); // check for warnings in the filtering condition warn_codes.clear(); @@ -444,10 +452,12 @@ void rule_loader::compiler::compile_rule_infos( // This also compiles the filter, and might throw a // falco_exception with details on the compilation // failure. + sinsp_filter_compiler compiler(cfg.sources.at(r.source)->filter_factory, ast.get()); try { - source->ruleset->add(*out.at(rule_id), ast); + shared_ptr filter(compiler.compile()); + source->ruleset->add(*out.at(rule_id), filter, ast); } - catch (const falco_exception& e) + catch (const sinsp_exception& e) { // Allow errors containing "nonexistent field" if // skip_if_unknown_filter is true @@ -463,10 +473,14 @@ void rule_loader::compiler::compile_rule_infos( } else { + rule_loader::context ctx(compiler.get_pos(), + condition, + r.cond_ctx); + throw rule_loader::rule_load_exception( falco::load_result::load_result::LOAD_ERR_COMPILE_CONDITION, e.what(), - r.cond_ctx); + ctx); } } From 5ec79535b8b87d2a324cd73b3f5b88ecb3a12bd9 Mon Sep 17 00:00:00 2001 From: Mark Stemm Date: Wed, 24 Aug 2022 17:16:01 -0700 Subject: [PATCH 2/2] Fix(engine) Save parse positions when finding unresolved macros Now that ASTs contain parse positions, use them when reporting errors about unknown macros. When doing the first pass to find all macro references, save macros as a map instead of a set. While making that change, change the visitor struct to use references instead of pointers. In the second pass, when reporting any unresolved macro references, also report the parse position. The unit tests also check that the positions of macros are properly returned in the resolved/unresolved maps. Signed-off-by: Mark Stemm --- tests/engine/test_filter_macro_resolver.cpp | 110 ++++++++++++++------ userspace/engine/filter_macro_resolver.cpp | 24 ++--- userspace/engine/filter_macro_resolver.h | 27 +++-- 3 files changed, 108 insertions(+), 53 deletions(-) diff --git a/tests/engine/test_filter_macro_resolver.cpp b/tests/engine/test_filter_macro_resolver.cpp index d98fe7ce576..b2b3ce06f02 100644 --- a/tests/engine/test_filter_macro_resolver.cpp +++ b/tests/engine/test_filter_macro_resolver.cpp @@ -20,9 +20,27 @@ limitations under the License. using namespace std; using namespace libsinsp::filter::ast; +static pos_info create_pos(uint32_t idx, uint32_t line, uint32_t col) +{ + pos_info ret; + ret.idx = idx; + ret.line = line; + ret.col = col; + + return ret; +} + +static bool operator==(const pos_info& p1, const pos_info& p2) +{ + return (p1.idx == p2.idx) && + (p1.line == p2.line) && + (p1.col == p2.col); +} + TEST_CASE("Should resolve macros on a filter AST", "[rule_loader]") { string macro_name = "test_macro"; + pos_info macro_pos = create_pos(12, 85, 27); SECTION("in the general case") { @@ -31,7 +49,7 @@ TEST_CASE("Should resolve macros on a filter AST", "[rule_loader]") std::vector> filter_and; filter_and.push_back(unary_check_expr::create("evt.name", "", "exists")); - filter_and.push_back(not_expr::create(value_expr::create(macro_name))); + filter_and.push_back(not_expr::create(value_expr::create(macro_name, macro_pos))); std::shared_ptr filter = std::move(and_expr::create(filter_and)); std::vector> expected_and; @@ -45,7 +63,8 @@ TEST_CASE("Should resolve macros on a filter AST", "[rule_loader]") // first run REQUIRE(resolver.run(filter) == true); REQUIRE(resolver.get_resolved_macros().size() == 1); - REQUIRE(*resolver.get_resolved_macros().begin() == macro_name); + REQUIRE(resolver.get_resolved_macros().begin()->first == macro_name); + REQUIRE(resolver.get_resolved_macros().begin()->second == macro_pos); REQUIRE(resolver.get_unknown_macros().empty()); REQUIRE(filter->is_equal(expected.get())); @@ -61,7 +80,7 @@ TEST_CASE("Should resolve macros on a filter AST", "[rule_loader]") std::shared_ptr macro = std::move( unary_check_expr::create("test.field", "", "exists")); - std::shared_ptr filter = std::move(value_expr::create(macro_name)); + std::shared_ptr filter = std::move(value_expr::create(macro_name, macro_pos)); filter_macro_resolver resolver; resolver.set_macro(macro_name, macro); @@ -71,7 +90,8 @@ TEST_CASE("Should resolve macros on a filter AST", "[rule_loader]") REQUIRE(resolver.run(filter) == true); REQUIRE(filter.get() != old_filter_ptr); REQUIRE(resolver.get_resolved_macros().size() == 1); - REQUIRE(*resolver.get_resolved_macros().begin() == macro_name); + REQUIRE(resolver.get_resolved_macros().begin()->first == macro_name); + REQUIRE(resolver.get_resolved_macros().begin()->second == macro_pos); REQUIRE(resolver.get_unknown_macros().empty()); REQUIRE(filter->is_equal(macro.get())); @@ -89,14 +109,17 @@ TEST_CASE("Should resolve macros on a filter AST", "[rule_loader]") string a_macro_name = macro_name + "_1"; string b_macro_name = macro_name + "_2"; + pos_info a_macro_pos = create_pos(11, 75, 43); + pos_info b_macro_pos = create_pos(91, 21, 9); + std::shared_ptr a_macro = std::move( unary_check_expr::create("one.field", "", "exists")); std::shared_ptr b_macro = std::move( unary_check_expr::create("another.field", "", "exists")); std::vector> filter_or; - filter_or.push_back(value_expr::create(a_macro_name)); - filter_or.push_back(value_expr::create(b_macro_name)); + filter_or.push_back(value_expr::create(a_macro_name, a_macro_pos)); + filter_or.push_back(value_expr::create(b_macro_name, b_macro_pos)); std::shared_ptr filter = std::move(or_expr::create(filter_or)); std::vector> expected_or; @@ -111,11 +134,16 @@ TEST_CASE("Should resolve macros on a filter AST", "[rule_loader]") // first run REQUIRE(resolver.run(filter) == true); REQUIRE(resolver.get_resolved_macros().size() == 2); - REQUIRE(resolver.get_resolved_macros().find(a_macro_name) - != resolver.get_resolved_macros().end()); - REQUIRE(resolver.get_resolved_macros().find(b_macro_name) - != resolver.get_resolved_macros().end()); + auto a_resolved_itr = resolver.get_resolved_macros().find(a_macro_name); + REQUIRE(a_resolved_itr != resolver.get_resolved_macros().end()); + REQUIRE(a_resolved_itr->first == a_macro_name); + REQUIRE(a_resolved_itr->second == a_macro_pos); + + auto b_resolved_itr = resolver.get_resolved_macros().find(b_macro_name); + REQUIRE(b_resolved_itr != resolver.get_resolved_macros().end()); REQUIRE(resolver.get_unknown_macros().empty()); + REQUIRE(b_resolved_itr->first == b_macro_name); + REQUIRE(b_resolved_itr->second == b_macro_pos); REQUIRE(filter->is_equal(expected_filter.get())); // second run @@ -130,15 +158,18 @@ TEST_CASE("Should resolve macros on a filter AST", "[rule_loader]") string a_macro_name = macro_name + "_1"; string b_macro_name = macro_name + "_2"; + pos_info a_macro_pos = create_pos(47, 1, 76); + pos_info b_macro_pos = create_pos(111, 65, 2); + std::vector> a_macro_and; a_macro_and.push_back(unary_check_expr::create("one.field", "", "exists")); - a_macro_and.push_back(value_expr::create(b_macro_name)); + a_macro_and.push_back(value_expr::create(b_macro_name, b_macro_pos)); std::shared_ptr a_macro = std::move(and_expr::create(a_macro_and)); std::shared_ptr b_macro = std::move( unary_check_expr::create("another.field", "", "exists")); - std::shared_ptr filter = std::move(value_expr::create(a_macro_name)); + std::shared_ptr filter = std::move(value_expr::create(a_macro_name, a_macro_pos)); std::vector> expected_and; expected_and.push_back(unary_check_expr::create("one.field", "", "exists")); @@ -152,10 +183,17 @@ TEST_CASE("Should resolve macros on a filter AST", "[rule_loader]") // first run REQUIRE(resolver.run(filter) == true); REQUIRE(resolver.get_resolved_macros().size() == 2); - REQUIRE(resolver.get_resolved_macros().find(a_macro_name) - != resolver.get_resolved_macros().end()); - REQUIRE(resolver.get_resolved_macros().find(b_macro_name) - != resolver.get_resolved_macros().end()); + auto a_resolved_itr = resolver.get_resolved_macros().find(a_macro_name); + REQUIRE(a_resolved_itr != resolver.get_resolved_macros().end()); + REQUIRE(a_resolved_itr->first == a_macro_name); + REQUIRE(a_resolved_itr->second == a_macro_pos); + + auto b_resolved_itr = resolver.get_resolved_macros().find(b_macro_name); + REQUIRE(b_resolved_itr != resolver.get_resolved_macros().end()); + REQUIRE(resolver.get_unknown_macros().empty()); + REQUIRE(b_resolved_itr->first == b_macro_name); + REQUIRE(b_resolved_itr->second == b_macro_pos); + REQUIRE(resolver.get_unknown_macros().empty()); REQUIRE(filter->is_equal(expected_filter.get())); @@ -170,18 +208,20 @@ TEST_CASE("Should resolve macros on a filter AST", "[rule_loader]") TEST_CASE("Should find unknown macros", "[rule_loader]") { string macro_name = "test_macro"; + pos_info macro_pos = create_pos(9, 4, 2); SECTION("in the general case") { std::vector> filter_and; filter_and.push_back(unary_check_expr::create("evt.name", "", "exists")); - filter_and.push_back(not_expr::create(value_expr::create(macro_name))); + filter_and.push_back(not_expr::create(value_expr::create(macro_name, macro_pos))); std::shared_ptr filter = std::move(and_expr::create(filter_and)); filter_macro_resolver resolver; REQUIRE(resolver.run(filter) == false); REQUIRE(resolver.get_unknown_macros().size() == 1); - REQUIRE(*resolver.get_unknown_macros().begin() == macro_name); + REQUIRE(resolver.get_unknown_macros().begin()->first == macro_name); + REQUIRE(resolver.get_unknown_macros().begin()->second == macro_pos); REQUIRE(resolver.get_resolved_macros().empty()); } @@ -190,12 +230,15 @@ TEST_CASE("Should find unknown macros", "[rule_loader]") string a_macro_name = macro_name + "_1"; string b_macro_name = macro_name + "_2"; + pos_info a_macro_pos = create_pos(32, 84, 9); + pos_info b_macro_pos = create_pos(1, 0, 5); + std::vector> a_macro_and; a_macro_and.push_back(unary_check_expr::create("one.field", "", "exists")); - a_macro_and.push_back(value_expr::create(b_macro_name)); + a_macro_and.push_back(value_expr::create(b_macro_name, b_macro_pos)); std::shared_ptr a_macro = std::move(and_expr::create(a_macro_and)); - std::shared_ptr filter = std::move(value_expr::create(a_macro_name)); + std::shared_ptr filter = std::move(value_expr::create(a_macro_name, a_macro_pos)); auto expected_filter = clone(a_macro.get()); filter_macro_resolver resolver; @@ -204,9 +247,11 @@ TEST_CASE("Should find unknown macros", "[rule_loader]") // first run REQUIRE(resolver.run(filter) == true); REQUIRE(resolver.get_resolved_macros().size() == 1); - REQUIRE(*resolver.get_resolved_macros().begin() == a_macro_name); + REQUIRE(resolver.get_resolved_macros().begin()->first == a_macro_name); + REQUIRE(resolver.get_resolved_macros().begin()->second == a_macro_pos); REQUIRE(resolver.get_unknown_macros().size() == 1); - REQUIRE(*resolver.get_unknown_macros().begin() == b_macro_name); + REQUIRE(resolver.get_unknown_macros().begin()->first == b_macro_name); + REQUIRE(resolver.get_unknown_macros().begin()->second == b_macro_pos); REQUIRE(filter->is_equal(expected_filter.get())); } } @@ -214,15 +259,19 @@ TEST_CASE("Should find unknown macros", "[rule_loader]") TEST_CASE("Should undefine macro", "[rule_loader]") { string macro_name = "test_macro"; + pos_info macro_pos_1 = create_pos(12, 9, 3); + pos_info macro_pos_2 = create_pos(9, 6, 3); + std::shared_ptr macro = std::move(unary_check_expr::create("test.field", "", "exists")); - std::shared_ptr a_filter = std::move(value_expr::create(macro_name)); - std::shared_ptr b_filter = std::move(value_expr::create(macro_name)); + std::shared_ptr a_filter = std::move(value_expr::create(macro_name, macro_pos_1)); + std::shared_ptr b_filter = std::move(value_expr::create(macro_name, macro_pos_2)); filter_macro_resolver resolver; resolver.set_macro(macro_name, macro); REQUIRE(resolver.run(a_filter) == true); REQUIRE(resolver.get_resolved_macros().size() == 1); - REQUIRE(*resolver.get_resolved_macros().begin() == macro_name); + REQUIRE(resolver.get_resolved_macros().begin()->first == macro_name); + REQUIRE(resolver.get_resolved_macros().begin()->second == macro_pos_1); REQUIRE(resolver.get_unknown_macros().empty()); REQUIRE(a_filter->is_equal(macro.get())); @@ -230,21 +279,24 @@ TEST_CASE("Should undefine macro", "[rule_loader]") REQUIRE(resolver.run(b_filter) == false); REQUIRE(resolver.get_resolved_macros().empty()); REQUIRE(resolver.get_unknown_macros().size() == 1); - REQUIRE(*resolver.get_unknown_macros().begin() == macro_name); + REQUIRE(resolver.get_unknown_macros().begin()->first == macro_name); + REQUIRE(resolver.get_unknown_macros().begin()->second == macro_pos_2); } // checks that the macro AST is cloned and not shared across resolved filters TEST_CASE("Should clone macro AST", "[rule_loader]") { string macro_name = "test_macro"; + pos_info macro_pos = create_pos(5, 2, 8888); std::shared_ptr macro = std::move(unary_check_expr::create("test.field", "", "exists")); - std::shared_ptr filter = std::move(value_expr::create(macro_name)); + std::shared_ptr filter = std::move(value_expr::create(macro_name, macro_pos)); filter_macro_resolver resolver; - + resolver.set_macro(macro_name, macro); REQUIRE(resolver.run(filter) == true); REQUIRE(resolver.get_resolved_macros().size() == 1); - REQUIRE(*resolver.get_resolved_macros().begin() == macro_name); + REQUIRE(resolver.get_resolved_macros().begin()->first == macro_name); + REQUIRE(resolver.get_resolved_macros().begin()->second == macro_pos); REQUIRE(resolver.get_unknown_macros().empty()); REQUIRE(filter->is_equal(macro.get())); diff --git a/userspace/engine/filter_macro_resolver.cpp b/userspace/engine/filter_macro_resolver.cpp index 0e86136f309..81cf598d2b7 100644 --- a/userspace/engine/filter_macro_resolver.cpp +++ b/userspace/engine/filter_macro_resolver.cpp @@ -21,12 +21,10 @@ using namespace libsinsp::filter; bool filter_macro_resolver::run(libsinsp::filter::ast::expr*& filter) { - visitor v; m_unknown_macros.clear(); m_resolved_macros.clear(); - v.m_unknown_macros = &m_unknown_macros; - v.m_resolved_macros = &m_resolved_macros; - v.m_macros = &m_macros; + + visitor v(m_unknown_macros, m_resolved_macros, m_macros); v.m_node_substitute = nullptr; filter->accept(&v); if (v.m_node_substitute) @@ -39,12 +37,10 @@ bool filter_macro_resolver::run(libsinsp::filter::ast::expr*& filter) bool filter_macro_resolver::run(std::shared_ptr& filter) { - visitor v; m_unknown_macros.clear(); m_resolved_macros.clear(); - v.m_unknown_macros = &m_unknown_macros; - v.m_resolved_macros = &m_resolved_macros; - v.m_macros = &m_macros; + + visitor v(m_unknown_macros, m_resolved_macros, m_macros); v.m_node_substitute = nullptr; filter->accept(&v); if (v.m_node_substitute) @@ -61,12 +57,12 @@ void filter_macro_resolver::set_macro( m_macros[name] = macro; } -const unordered_set& filter_macro_resolver::get_unknown_macros() const +const filter_macro_resolver::macro_info_map& filter_macro_resolver::get_unknown_macros() const { return m_unknown_macros; } -const unordered_set& filter_macro_resolver::get_resolved_macros() const +const filter_macro_resolver::macro_info_map& filter_macro_resolver::get_resolved_macros() const { return m_resolved_macros; } @@ -129,8 +125,8 @@ void filter_macro_resolver::visitor::visit(ast::value_expr* e) // we are supposed to get here only in case // of identier-only children from either a 'not', // an 'and' or an 'or'. - auto macro = m_macros->find(e->value); - if (macro != m_macros->end() && macro->second) // skip null-ptr macros + auto macro = m_macros.find(e->value); + if (macro != m_macros.end() && macro->second) // skip null-ptr macros { m_node_substitute = nullptr; auto new_node = ast::clone(macro->second.get()); @@ -141,11 +137,11 @@ void filter_macro_resolver::visitor::visit(ast::value_expr* e) { m_node_substitute = std::move(new_node); } - m_resolved_macros->insert(e->value); + m_resolved_macros[e->value] = e->get_pos(); } else { m_node_substitute = nullptr; - m_unknown_macros->insert(e->value); + m_unknown_macros[e->value] = e->get_pos(); } } diff --git a/userspace/engine/filter_macro_resolver.h b/userspace/engine/filter_macro_resolver.h index 8f71c43d202..8c798e81fc5 100644 --- a/userspace/engine/filter_macro_resolver.h +++ b/userspace/engine/filter_macro_resolver.h @@ -40,7 +40,7 @@ class filter_macro_resolver \return true if at least one of the defined macros is resolved */ bool run(libsinsp::filter::ast::expr*& filter); - + /*! \brief Version of run() that works with shared pointers */ @@ -58,12 +58,17 @@ class filter_macro_resolver std::string name, std::shared_ptr macro); + /*! + \brief used in get_{resolved,unknown}_macros + */ + typedef std::unordered_map macro_info_map; + /*! \brief Returns a set containing the names of all the macros substituted during the last invocation of run(). Should be non-empty if the last invocation of run() returned true. */ - const std::unordered_set& get_resolved_macros() const; + const macro_info_map& get_resolved_macros() const; /*! \brief Returns a set containing the names of all the macros @@ -71,8 +76,8 @@ class filter_macro_resolver A macro remains unresolved if it is found inside the processed filter but it was not defined with set_macro(); */ - const std::unordered_set& get_unknown_macros() const; - + const macro_info_map& get_unknown_macros() const; + private: typedef std::unordered_map< std::string, @@ -81,16 +86,18 @@ class filter_macro_resolver struct visitor : public libsinsp::filter::ast::expr_visitor { - visitor() = default; + visitor(macro_info_map& unknown_macros, macro_info_map& resolved_macros, macro_defs& macros) + : m_unknown_macros(unknown_macros), m_resolved_macros(resolved_macros), m_macros(macros) {} visitor(visitor&&) = default; visitor& operator = (visitor&&) = default; visitor(const visitor&) = delete; visitor& operator = (const visitor&) = delete; std::unique_ptr m_node_substitute; - std::unordered_set* m_unknown_macros; - std::unordered_set* m_resolved_macros; - macro_defs* m_macros; + macro_info_map& m_unknown_macros; + macro_info_map& m_resolved_macros; + + macro_defs& m_macros; void visit(libsinsp::filter::ast::and_expr* e) override; void visit(libsinsp::filter::ast::or_expr* e) override; @@ -101,7 +108,7 @@ class filter_macro_resolver void visit(libsinsp::filter::ast::binary_check_expr* e) override; }; - std::unordered_set m_unknown_macros; - std::unordered_set m_resolved_macros; + macro_info_map m_unknown_macros; + macro_info_map m_resolved_macros; macro_defs m_macros; };