semantic_analyser: Run more passes to resolve types

src/semantic_analyser.cpp

@@ -83,7 +83,16 @@ void SemanticAnalyser::visit(Map &map)
     bpftrace_.map_args_.insert({map.ident, args});
   }
 
-  type_ = bpftrace_.map_val_.find(map.ident)->second;
+  auto search_val = bpftrace_.map_val_.find(map.ident);
+  if (search_val != bpftrace_.map_val_.end()) {
+    type_ = search_val->second;
+  }
+  else {
+    if (is_final_pass()) {
+      err_ << "Undefined map: " << map.ident << std::endl;
+    }
+    type_ = Type::none;
+  }
 }
 
 void SemanticAnalyser::visit(Binop &binop)
@@ -94,7 +103,7 @@ void SemanticAnalyser::visit(Binop &binop)
   binop.right->accept(*this);
   rhs = type_;
 
-  if (pass_ == 2 && lhs != rhs) {
+  if (is_final_pass() && lhs != rhs) {
     err_ << "Type mismatch for '" << opstr(binop) << "': ";
     err_ << "comparing '" << typestr(lhs) << "' ";
     err_ << "with '" << typestr(rhs) << "'" << std::endl;
@@ -122,9 +131,17 @@ void SemanticAnalyser::visit(AssignMapStatement &assignment)
   std::string map_ident = assignment.map->ident;
   auto search = bpftrace_.map_val_.find(map_ident);
   if (search != bpftrace_.map_val_.end()) {
-    if (search->second != type_) {
+    if (search->second == Type::none) {
+      if (is_final_pass()) {
+        err_ << "Undefined map: " << map_ident << std::endl;
+      }
+      else {
+        search->second = type_;
+      }
+    }
+    else if (search->second != type_) {
       err_ << "Type mismatch for " << map_ident << ": ";
-      err_ << "trying to assign variable of type '" << typestr(type_);
+      err_ << "trying to assign value of type '" << typestr(type_);
       err_ << "'\n\twhen map already contains a value of type '";
       err_ << typestr(search->second) << "'\n" << std::endl;
     }
@@ -143,7 +160,15 @@ void SemanticAnalyser::visit(AssignMapCallStatement &assignment)
   std::string map_ident = assignment.map->ident;
   auto search = bpftrace_.map_val_.find(map_ident);
   if (search != bpftrace_.map_val_.end()) {
-    if (search->second != type_) {
+    if (search->second == Type::none) {
+      if (is_final_pass()) {
+        err_ << "Undefined map: " << map_ident << std::endl;
+      }
+      else {
+        search->second = type_;
+      }
+    }
+    else if (search->second != type_) {
       err_ << "Type mismatch for " << map_ident << ": ";
       err_ << "trying to assign result of '" << assignment.call->func;
       err_ << "()'\n\twhen map already contains a value of type '";
@@ -170,7 +195,7 @@ void SemanticAnalyser::visit(Probe &probe)
     stmt->accept(*this);
   }
 
-  if (pass_ == 2 && bpftrace_.add_probe(probe)) {
+  if (is_final_pass() && bpftrace_.add_probe(probe)) {
     err_ << "Invalid probe type: '" << probe.type << "'" << std::endl;
   }
 }
@@ -184,16 +209,14 @@ void SemanticAnalyser::visit(Program &program)
 
 int SemanticAnalyser::analyse()
 {
-  // Two pass analysis, to handle variables being used before they are defined:
-  // - First pass checks assignments
-  // - Second pass checks expressions
+  // Multiple passes to handle variables being used before they are defined
   std::string errors;
 
-  for (pass_ = 1; pass_ <= 2; pass_++) {
+  for (pass_ = 1; pass_ <= num_passes_; pass_++) {
     root_->accept(*this);
     errors = err_.str();
     if (!errors.empty()) {
-      std::cerr << errors;
+      out_ << errors;
       return pass_;
     }
   }
@@ -201,6 +224,11 @@ int SemanticAnalyser::analyse()
   return 0;
 }
 
+bool SemanticAnalyser::is_final_pass() const
+{
+  return pass_ == num_passes_;
+}
+
 } // namespace ast
 } // namespace bpftrace
 } // namespace ebpf

src/semantic_analyser.h

@@ -12,9 +12,10 @@ namespace ast {
 
 class SemanticAnalyser : public Visitor {
 public:
-  explicit SemanticAnalyser(Node *root, BPFtrace &bpftrace)
+  explicit SemanticAnalyser(Node *root, BPFtrace &bpftrace, std::ostream &out = std::cerr)
     : root_(root),
-      bpftrace_(bpftrace) { }
+      bpftrace_(bpftrace),
+      out_(out) { }
 
   void visit(Integer &integer) override;
   void visit(Builtin &builtin) override;
@@ -34,11 +35,15 @@ public:
 private:
   Node *root_;
   BPFtrace &bpftrace_;
+  std::ostream &out_;
   std::ostringstream err_;
   int pass_;
+  const int num_passes_ = 10;
 
   using Type = ebpf::bpftrace::Type;
   Type type_;
+
+  bool is_final_pass() const;
 };
 
 } // namespace ast

tests/semantic_analyser.cpp

 #include "gmock/gmock.h"
 #include "gtest/gtest.h"
 #include "bpftrace.h"
+#include "parser.tab.hh"
 #include "semantic_analyser.h"
 
 namespace ebpf {
@@ -20,6 +21,8 @@ TEST(semantic_analyser, probe_count)
   MockBPFtrace bpftrace;
   EXPECT_CALL(bpftrace, add_probe(_)).Times(2);
 
+  // kprobe:kprobe { 123; }
+  // kprobe:kprobe { 123; }
   Integer expr(123);
   ExprStatement stmt(&expr);
   StatementList stmts = {&stmt};
@@ -28,10 +31,43 @@ TEST(semantic_analyser, probe_count)
   Probe p2(str, str, &stmts);
   ProbeList pl = {&p1, &p2};
   Program root(&pl);
+
   ebpf::bpftrace::ast::SemanticAnalyser semantics(&root, bpftrace);
   semantics.analyse();
 }
 
+TEST(semantic_analyser, undefined_map)
+{
+  BPFtrace bpftrace;
+
+  // kprobe:kprobe / @mymap1 == 123 / { 123; }
+  std::string str = "kprobe";
+  std::string mapstr1 = "mymap1";
+  Integer myint(123);
+  Map map1(mapstr1);
+  Binop binop(&map1, ebpf::bpftrace::Parser::token::EQ, &myint);
+  Predicate pred(&binop);
+  ExprStatement stmt(&myint);
+  StatementList stmts = {&stmt};
+  Probe p(str, str, &pred, &stmts);
+  ProbeList pl = {&p};
+  Program root(&pl);
+
+  std::ostringstream out1;
+  ebpf::bpftrace::ast::SemanticAnalyser semantics1(&root, bpftrace, out1);
+  EXPECT_EQ(semantics1.analyse(), 10);
+
+  // kprobe:kprobe / @mymap1 == 123 / { 123; @mymap1 = @mymap2; }
+  std::string mapstr2 = "mymap2";
+  Map map2(mapstr2);
+  AssignMapStatement assign(&map1, &map2);
+  stmts.push_back(&assign);
+
+  std::ostringstream out2;
+  ebpf::bpftrace::ast::SemanticAnalyser semantics2(&root, bpftrace, out2);
+  EXPECT_EQ(semantics2.analyse(), 10);
+}
+
 } // namespace ast
 } // namespace bpftrace
 } // namespace ebpf