From 6b5207166546a7a54db8b96b132b7f861014d969 Mon Sep 17 00:00:00 2001 From: Mark Stemm Date: Wed, 24 Aug 2022 17:16:01 -0700 Subject: [PATCH] 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. In the second pass, when reporting any unresolved macro references, also report the parse position. Signed-off-by: Mark Stemm --- tests/engine/test_filter_macro_resolver.cpp | 17 +++++++++-------- userspace/engine/filter_macro_resolver.cpp | 8 ++++---- userspace/engine/filter_macro_resolver.h | 20 +++++++++++++------- 3 files changed, 26 insertions(+), 19 deletions(-) diff --git a/tests/engine/test_filter_macro_resolver.cpp b/tests/engine/test_filter_macro_resolver.cpp index d98fe7ce576..560f528906c 100644 --- a/tests/engine/test_filter_macro_resolver.cpp +++ b/tests/engine/test_filter_macro_resolver.cpp @@ -45,7 +45,7 @@ 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_unknown_macros().empty()); REQUIRE(filter->is_equal(expected.get())); @@ -71,7 +71,7 @@ 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_unknown_macros().empty()); REQUIRE(filter->is_equal(macro.get())); @@ -181,7 +181,7 @@ TEST_CASE("Should find unknown macros", "[rule_loader]") 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_resolved_macros().empty()); } @@ -204,9 +204,9 @@ 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_unknown_macros().size() == 1); - REQUIRE(*resolver.get_unknown_macros().begin() == b_macro_name); + REQUIRE(resolver.get_unknown_macros().begin()->first == b_macro_name); REQUIRE(filter->is_equal(expected_filter.get())); } } @@ -222,7 +222,7 @@ TEST_CASE("Should undefine macro", "[rule_loader]") 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_unknown_macros().empty()); REQUIRE(a_filter->is_equal(macro.get())); @@ -231,6 +231,7 @@ TEST_CASE("Should undefine macro", "[rule_loader]") 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); } // checks that the macro AST is cloned and not shared across resolved filters @@ -240,11 +241,11 @@ TEST_CASE("Should clone macro 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)); 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_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..f8099287c8d 100644 --- a/userspace/engine/filter_macro_resolver.cpp +++ b/userspace/engine/filter_macro_resolver.cpp @@ -61,12 +61,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; } @@ -141,11 +141,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 e73bd983aa4..e25fe3dcddb 100644 --- a/userspace/engine/filter_macro_resolver.h +++ b/userspace/engine/filter_macro_resolver.h @@ -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, @@ -82,8 +87,9 @@ class filter_macro_resolver struct visitor : public libsinsp::filter::ast::expr_visitor { std::unique_ptr m_node_substitute; - 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; void visit(libsinsp::filter::ast::and_expr* e) override; @@ -95,7 +101,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; };