Merge pull request #9 from plumgrid/fix_issue8

Fix for Issue #8, pkt.rewrite_field

src/cc/codegen_llvm.cc

@@ -315,30 +315,34 @@ StatusTuple CodegenLLVM::visit_ident_expr_node(IdentExprNode *n) {
 
 StatusTuple CodegenLLVM::visit_assign_expr_node(AssignExprNode *n) {
   if (n->bitop_) {
-    TRY2(n->id_->accept(this));
+    TRY2(n->lhs_->accept(this));
     emit(" = (");
-    TRY2(n->id_->accept(this));
-    emit(" & ~((((%s)1 << %d) - 1) << %d)) | (", bits_to_uint(n->id_->bit_width_),
+    TRY2(n->lhs_->accept(this));
+    emit(" & ~((((%s)1 << %d) - 1) << %d)) | (", bits_to_uint(n->lhs_->bit_width_),
          n->bitop_->bit_width_, n->bitop_->bit_offset_);
     TRY2(n->rhs_->accept(this));
     emit(" << %d)", n->bitop_->bit_offset_);
     return mkstatus_(n, "unsupported");
   } else {
-    if (n->id_->flags_[ExprNode::PROTO]) {
-      auto f = n->id_->struct_type_->field(n->id_->sub_name_);
-      emit("bpf_dins(%s%s + %zu, %zu, %zu, ", n->id_->decl_->scope_id(), n->id_->c_str(),
-           f->bit_offset_ >> 3, f->bit_offset_ & 0x7, f->bit_width_);
-      TRY2(n->rhs_->accept(this));
-      emit(")");
+    if (n->lhs_->flags_[ExprNode::PROTO]) {
+      // auto f = n->lhs_->struct_type_->field(n->id_->sub_name_);
+      // emit("bpf_dins(%s%s + %zu, %zu, %zu, ", n->id_->decl_->scope_id(), n->id_->c_str(),
+      //      f->bit_offset_ >> 3, f->bit_offset_ & 0x7, f->bit_width_);
+      // TRY2(n->rhs_->accept(this));
+      // emit(")");
       return mkstatus_(n, "unsupported");
     } else {
       TRY2(n->rhs_->accept(this));
-      Value *rhs = pop_expr();
-      TRY2(n->id_->accept(this));
-      Value *lhs = pop_expr();
-      if (!n->rhs_->is_ref())
-        rhs = B.CreateIntCast(rhs, cast<PointerType>(lhs->getType())->getElementType(), false);
-      B.CreateStore(rhs, lhs);
+      if (n->lhs_->is_pkt()) {
+        TRY2(n->lhs_->accept(this));
+      } else {
+        Value *rhs = pop_expr();
+        TRY2(n->lhs_->accept(this));
+        Value *lhs = pop_expr();
+        if (!n->rhs_->is_ref())
+          rhs = B.CreateIntCast(rhs, cast<PointerType>(lhs->getType())->getElementType(), false);
+        B.CreateStore(rhs, lhs);
+      }
     }
   }
   return mkstatus(0);
@@ -682,6 +686,12 @@ StatusTuple CodegenLLVM::emit_log(MethodCallExprNode *n) {
   return mkstatus(0);
 }
 
+StatusTuple CodegenLLVM::emit_packet_rewrite_field(MethodCallExprNode *n) {
+  TRY2(n->args_[1]->accept(this));
+  TRY2(n->args_[0]->accept(this));
+  return mkstatus(0);
+}
+
 StatusTuple CodegenLLVM::emit_atomic_add(MethodCallExprNode *n) {
   TRY2(n->args_[0]->accept(this));
   Value *lhs = B.CreateBitCast(pop_expr(), Type::getInt64PtrTy(ctx()));
@@ -741,6 +751,8 @@ StatusTuple CodegenLLVM::visit_method_call_expr_node(MethodCallExprNode *n) {
       TRY2(emit_table_update(n));
     } else if (n->id_->sub_name_ == "delete") {
       TRY2(emit_table_delete(n));
+    } else if (n->id_->sub_name_ == "rewrite_field" && n->id_->name_ == "pkt") {
+      TRY2(emit_packet_rewrite_field(n));
     }
   } else if (n->id_->name_ == "atomic_add") {
     TRY2(emit_atomic_add(n));
@@ -952,24 +964,25 @@ StatusTuple CodegenLLVM::visit_struct_variable_decl_stmt_node(StructVariableDecl
       ConstantPointerNull *const_null = ConstantPointerNull::get(cast<PointerType>(ptr_stype));
       B.CreateStore(const_null, ptr_a);
     } else {
-      string var = n->scope_id() + n->id_->name_;
-      /* zero initialize array to be filled in with packet header */
-      emit("uint64_t __%s[%zu] = {}; uint8_t *%s = (uint8_t*)__%s;",
-           var.c_str(), ((decl->bit_width_ >> 3) + 7) >> 3, var.c_str(), var.c_str());
-      for (auto it = n->init_.begin(); it != n->init_.end(); ++it) {
-        auto asn = static_cast<AssignExprNode*>(it->get());
-        if (auto f = decl->field(asn->id_->sub_name_)) {
-          size_t bit_offset = f->bit_offset_;
-          size_t bit_width = f->bit_width_;
-          if (asn->bitop_) {
-            bit_offset += f->bit_width_ - (asn->bitop_->bit_offset_ + asn->bitop_->bit_width_);
-            bit_width = std::min(bit_width - asn->bitop_->bit_offset_, asn->bitop_->bit_width_);
-          }
-          emit(" bpf_dins(%s + %zu, %zu, %zu, ", var.c_str(), bit_offset >> 3, bit_offset & 0x7, bit_width);
-          TRY2(asn->rhs_->accept(this));
-          emit(");");
-        }
-      }
+      return mkstatus_(n, "unsupported");
+      // string var = n->scope_id() + n->id_->name_;
+      // /* zero initialize array to be filled in with packet header */
+      // emit("uint64_t __%s[%zu] = {}; uint8_t *%s = (uint8_t*)__%s;",
+      //      var.c_str(), ((decl->bit_width_ >> 3) + 7) >> 3, var.c_str(), var.c_str());
+      // for (auto it = n->init_.begin(); it != n->init_.end(); ++it) {
+      //   auto asn = static_cast<AssignExprNode*>(it->get());
+      //   if (auto f = decl->field(asn->id_->sub_name_)) {
+      //     size_t bit_offset = f->bit_offset_;
+      //     size_t bit_width = f->bit_width_;
+      //     if (asn->bitop_) {
+      //       bit_offset += f->bit_width_ - (asn->bitop_->bit_offset_ + asn->bitop_->bit_width_);
+      //       bit_width = std::min(bit_width - asn->bitop_->bit_offset_, asn->bitop_->bit_width_);
+      //     }
+      //     emit(" bpf_dins(%s + %zu, %zu, %zu, ", var.c_str(), bit_offset >> 3, bit_offset & 0x7, bit_width);
+      //     TRY2(asn->rhs_->accept(this));
+      //     emit(");");
+      //   }
+      // }
     }
   } else {
     if (n->is_pointer()) {

src/cc/codegen_llvm.h

@@ -70,20 +70,7 @@ class CodegenLLVM : public Visitor {
   STATUS_RETURN emit_table_lookup(MethodCallExprNode* n);
   STATUS_RETURN emit_table_update(MethodCallExprNode* n);
   STATUS_RETURN emit_table_delete(MethodCallExprNode* n);
-  STATUS_RETURN emit_channel_push(MethodCallExprNode* n);
-  STATUS_RETURN emit_channel_push_generic(MethodCallExprNode* n);
   STATUS_RETURN emit_log(MethodCallExprNode* n);
-  STATUS_RETURN emit_packet_forward(MethodCallExprNode* n);
-  STATUS_RETURN emit_packet_replicate(MethodCallExprNode* n);
-  STATUS_RETURN emit_packet_clone_forward(MethodCallExprNode* n);
-  STATUS_RETURN emit_packet_forward_self(MethodCallExprNode* n);
-  STATUS_RETURN emit_packet_drop(MethodCallExprNode* n);
-  STATUS_RETURN emit_packet_broadcast(MethodCallExprNode* n);
-  STATUS_RETURN emit_packet_multicast(MethodCallExprNode* n);
-  STATUS_RETURN emit_packet_push_header(MethodCallExprNode* n);
-  STATUS_RETURN emit_packet_pop_header(MethodCallExprNode* n);
-  STATUS_RETURN emit_packet_push_vlan(MethodCallExprNode* n);
-  STATUS_RETURN emit_packet_pop_vlan(MethodCallExprNode* n);
   STATUS_RETURN emit_packet_rewrite_field(MethodCallExprNode* n);
   STATUS_RETURN emit_atomic_add(MethodCallExprNode* n);
   STATUS_RETURN emit_cksum(MethodCallExprNode* n);

src/cc/node.h

@@ -150,7 +150,7 @@ class ExprNode : public Node {
   typedef unique_ptr<ExprNode> Ptr;
   virtual StatusTuple accept(Visitor* v) = 0;
   enum expr_type { STRUCT, INTEGER, STRING, VOID, UNKNOWN };
-  enum prop_flag { READ = 0, WRITE, PROTO, IS_LHS, IS_REF, LAST };
+  enum prop_flag { READ = 0, WRITE, PROTO, IS_LHS, IS_REF, IS_PKT, LAST };
   expr_type typeof_;
   StructDeclStmtNode *struct_type_;
   size_t bit_width_;
@@ -165,6 +165,7 @@ class ExprNode : public Node {
   }
   bool is_lhs() const { return flags_[IS_LHS]; }
   bool is_ref() const { return flags_[IS_REF]; }
+  bool is_pkt() const { return flags_[IS_PKT]; }
 };
 
 typedef vector<ExprNode::Ptr> ExprNodeList;
@@ -244,11 +245,18 @@ class AssignExprNode : public ExprNode {
  public:
   DECLARE(AssignExprNode)
 
-  IdentExprNode::Ptr id_;
+  //IdentExprNode *id_;
+  ExprNode::Ptr lhs_;
   ExprNode::Ptr rhs_;
   AssignExprNode(IdentExprNode::Ptr id, ExprNode::Ptr rhs)
-      : id_(move(id)), rhs_(move(rhs)) {
-    id_->flags_[ExprNode::IS_LHS] = true;
+      : lhs_(move(id)), rhs_(move(rhs)) {
+    //id_ = (IdentExprNode *)lhs_.get();
+    lhs_->flags_[ExprNode::IS_LHS] = true;
+  }
+  AssignExprNode(ExprNode::Ptr lhs, ExprNode::Ptr rhs)
+      : lhs_(move(lhs)), rhs_(move(rhs)) {
+    //id_ = nullptr;
+    lhs_->flags_[ExprNode::IS_LHS] = true;
   }
 };
 

src/cc/parser.cc

@@ -67,7 +67,8 @@ StructVariableDeclStmtNode *Parser::variable_add(StructVariableDeclStmtNode *dec
     for (auto arg = args->begin(); arg != args->end(); ++arg) {
       // decorate with the name of this decl
       auto n = static_cast<AssignExprNode *>(arg->get());
-      n->id_->prepend_dot(decl->id_->name_);
+      auto id = static_cast<IdentExprNode *>(n->lhs_.get());
+      id->prepend_dot(decl->id_->name_);
     }
   } else {
     fprintf(stderr, "must use key = value syntax\n");

src/cc/parser.yy

@@ -446,6 +446,9 @@ assign_expr
   | dotted_ident bitop TEQUAL expr
     { $$ = new AssignExprNode(IdentExprNode::Ptr($1), ExprNode::Ptr($4)); $$->bitop_ = BitopExprNode::Ptr($2);
       parser.set_loc($$, @$); }
+  | expr TEQUAL expr
+    { $$ = new AssignExprNode(ExprNode::Ptr($1), ExprNode::Ptr($3));
+      parser.set_loc($$, @$); }
   ;
 
 return_expr
@@ -471,9 +474,11 @@ expr
       parser.set_loc($$, @$); }
   | TDOLLAR dotted_ident
     { $$ = new PacketExprNode(IdentExprNode::Ptr($2));
+      $$->flags_[ExprNode::IS_PKT] = true;
       parser.set_loc($$, @$); }
   | TDOLLAR dotted_ident bitop
     { $$ = new PacketExprNode(IdentExprNode::Ptr($2)); $$->bitop_ = BitopExprNode::Ptr($3);
+      $$->flags_[ExprNode::IS_PKT] = true;
       parser.set_loc($$, @$); }
   | TGOTO scoped_ident
     { $$ = new GotoExprNode(IdentExprNode::Ptr($2), false);

src/cc/printer.cc

@@ -98,7 +98,7 @@ StatusTuple Printer::visit_ident_expr_node(IdentExprNode* n) {
 }
 
 StatusTuple Printer::visit_assign_expr_node(AssignExprNode* n) {
-  TRY2(n->id_->accept(this));
+  TRY2(n->lhs_->accept(this));
   fprintf(out_, " = ");
   TRY2(n->rhs_->accept(this));
   return mkstatus(0);

src/cc/type_check.cc

@@ -138,15 +138,15 @@ StatusTuple TypeCheck::visit_ident_expr_node(IdentExprNode *n) {
 
 StatusTuple TypeCheck::visit_assign_expr_node(AssignExprNode *n) {
   /// @todo check lhs is assignable
-  TRY2(n->id_->accept(this));
-  if (n->id_->typeof_ == ExprNode::STRUCT) {
+  TRY2(n->lhs_->accept(this));
+  if (n->lhs_->typeof_ == ExprNode::STRUCT) {
     TRY2(n->rhs_->accept(this));
     if (n->rhs_->typeof_ != ExprNode::STRUCT)
       return mkstatus_(n, "Right-hand side of assignment must be a struct");
   } else {
-    if (n->id_->typeof_ != ExprNode::INTEGER)
+    if (n->lhs_->typeof_ != ExprNode::INTEGER)
       return mkstatus_(n, "Left-hand side of assignment must be a numeric type");
-    if (!n->id_->flags_[ExprNode::WRITE])
+    if (!n->lhs_->flags_[ExprNode::WRITE])
       return mkstatus_(n, "Left-hand side of assignment is read-only");
     TRY2(n->rhs_->accept(this));
     if (n->rhs_->typeof_ != ExprNode::INTEGER)
@@ -173,7 +173,7 @@ StatusTuple TypeCheck::visit_packet_expr_node(PacketExprNode *n) {
     else
       n->bit_width_ = sub_decl->bit_width_;
   }
-  n->flags_[ExprNode::WRITE] = false;
+  n->flags_[ExprNode::WRITE] = true;
   return mkstatus(0);
 }
 
@@ -305,6 +305,9 @@ StatusTuple TypeCheck::visit_method_call_expr_node(MethodCallExprNode *n) {
       TRY2(check_update_method(n));
     } else if (n->id_->sub_name_ == "delete") {
       TRY2(check_delete_method(n));
+    } else if (n->id_->sub_name_ == "rewrite_field" && n->id_->name_ == "pkt") {
+      TRY2(expect_method_arg(n, 2));
+      n->args_[0]->flags_[ExprNode::IS_LHS] = true;
     }
   } else if (n->id_->name_ == "log") {
     if (n->args_.size() < 1)
@@ -382,7 +385,8 @@ StatusTuple TypeCheck::visit_struct_variable_decl_stmt_node(StructVariableDeclSt
     set<string> used;
     for (auto i = n->init_.begin(); i != n->init_.end(); ++i) {
       auto asn = static_cast<AssignExprNode*>(i->get());
-      used.insert(asn->id_->sub_name_);
+      auto id = static_cast<IdentExprNode *>(asn->lhs_.get());
+      used.insert(id->sub_name_);
     }
     for (auto f = type->stmts_.begin(); f != type->stmts_.end(); ++f) {
       if (used.find((*f)->id_->name_) == used.end()) {

tests/jit/test2.b

@@ -36,8 +36,9 @@ u32 main (struct proto::skbuff *skb) {
     on_valid(xleaf) {
       incr_cksum(@ip.hchecksum, orig_dip, xleaf.xdip);
       incr_cksum(@ip.hchecksum, orig_sip, xleaf.xsip);
+      // the below are equivalent
       pkt.rewrite_field($ip.dst, xleaf.xdip);
-      pkt.rewrite_field($ip.src, xleaf.xsip);
+      $ip.src = xleaf.xsip;
       atomic_add(xleaf.xlated_pkts, 1);
     }
   }