Use late-binding to finalize snprintf/sscanf

Not all users require the snprintf/sscanf helpers, and in some types the
finalization can take a long time. Defer the finalization until the
first use. Wrap the invocation with a bound function such that the
BPFTable and other users don't need to keep an explicit BPFModule
reference. Use-after-free will be avoided by resetting the function to
an error-returning stub when/if the BPFModule is freed.

Signed-off-by: Brenden Blanco <bblanco@gmail.com>

diff --git a/src/cc/BPFTable.h b/src/cc/BPFTable.h
index 509cd9ef8c6a8612bdf8a8f8ababf7e2736877ab..f2d2e3e8526289925cac208ef58ad0c94c0fb435 100644 (file)
--- a/src/cc/BPFTable.h
+++ b/src/cc/BPFTable.h
@@ -39,41 +39,27 @@ class BPFTableBase {
   size_t capacity() { return desc.max_entries; }
 
   StatusTuple string_to_key(const std::string& key_str, KeyType* key) {
-    if (!desc.key_sscanf)
-      return StatusTuple(-1, "Key sscanf not available");
-    if (desc.key_sscanf(key_str.c_str(), key) != 0)
-      return StatusTuple(-1, "Error on key_sscanff: %s", std::strerror(errno));
-    return StatusTuple(0);
+    return desc.key_sscanf(key_str.c_str(), key);
   }
 
   StatusTuple string_to_leaf(const std::string& value_str, ValueType* value) {
-    if (!desc.leaf_sscanf)
-      return StatusTuple(-1, "Leaf sscanf not available");
-    if (desc.leaf_sscanf(value_str.c_str(), value) != 0)
-      return StatusTuple(-1, "Error on leaf_sscanff: %s", std::strerror(errno));
-    return StatusTuple(0);
+    return desc.leaf_sscanf(value_str.c_str(), value);
   }
 
   StatusTuple key_to_string(const KeyType* key, std::string& key_str) {
     char buf[8 * desc.key_size];
-    if (!desc.key_snprintf)
-      return StatusTuple(-1, "Key snprintf not available");
-    if (desc.key_snprintf(buf, sizeof(buf), key) < 0)
-      return StatusTuple(-1, "Error on key_sprintf: %s", std::strerror(errno));
-
-    key_str.assign(buf);
-    return StatusTuple(0);
+    StatusTuple rc = desc.key_snprintf(buf, sizeof(buf), key);
+    if (!rc.code())
+      key_str.assign(buf);
+    return rc;
   }
 
   StatusTuple leaf_to_string(const ValueType* value, std::string& value_str) {
     char buf[8 * desc.leaf_size];
-    if (!desc.leaf_snprintf)
-      return StatusTuple(-1, "Leaf snprintf not available");
-    if (desc.leaf_snprintf(buf, sizeof(buf), value) < 0)
-      return StatusTuple(-1, "Error on leaf_sprintf: %s", std::strerror(errno));
-
-    value_str.assign(buf);
-    return StatusTuple(0);
+    StatusTuple rc = desc.leaf_snprintf(buf, sizeof(buf), value);
+    if (!rc.code())
+      value_str.assign(buf);
+    return rc;
   }
 
  protected:

diff --git a/src/cc/bpf_module.cc b/src/cc/bpf_module.cc
index f31ff1f2a5a56ef518304e57fd20b894f0b8fb05..a9a8535322318147a565a8450cb672c8a8397482 100644 (file)
--- a/src/cc/bpf_module.cc
+++ b/src/cc/bpf_module.cc
@@ -116,16 +116,23 @@ BPFModule::BPFModule(unsigned flags, TableStorage *ts)
   }
 }
 
+static StatusTuple unimplemented_sscanf(const char *, void *) {
+  return StatusTuple(-1, "sscanf unimplemented");
+}
+static StatusTuple unimplemented_snprintf(char *, size_t, const void *) {
+  return StatusTuple(-1, "snprintf unimplemented");
+}
+
 BPFModule::~BPFModule() {
   engine_.reset();
   rw_engine_.reset();
   ctx_.reset();
 
   for (auto &v : tables_) {
-    v->key_sscanf = nullptr;
-    v->leaf_sscanf = nullptr;
-    v->key_snprintf = nullptr;
-    v->leaf_snprintf = nullptr;
+    v->key_sscanf = unimplemented_sscanf;
+    v->leaf_sscanf = unimplemented_sscanf;
+    v->key_snprintf = unimplemented_snprintf;
+    v->leaf_snprintf = unimplemented_snprintf;
   }
 
   ts_->DeletePrefix(Path({id_}));
@@ -189,7 +196,7 @@ static void parse_type(IRBuilder<> &B, vector<Value *> *args, string *fmt,
   }
 }
 
-Function * BPFModule::make_reader(Module *mod, Type *type) {
+string BPFModule::make_reader(Module *mod, Type *type) {
   auto fn_it = readers_.find(type);
   if (fn_it != readers_.end())
     return fn_it->second;
@@ -202,10 +209,11 @@ Function * BPFModule::make_reader(Module *mod, Type *type) {
 
   IRBuilder<> B(*ctx_);
 
+  string name = "reader" + std::to_string(readers_.size());
   vector<Type *> fn_args({B.getInt8PtrTy(), PointerType::getUnqual(type)});
   FunctionType *fn_type = FunctionType::get(B.getInt32Ty(), fn_args, /*isVarArg=*/false);
-  Function *fn = Function::Create(fn_type, GlobalValue::ExternalLinkage,
-                                  "reader" + std::to_string(readers_.size()), mod);
+  Function *fn =
+      Function::Create(fn_type, GlobalValue::ExternalLinkage, name, mod);
   auto arg_it = fn->arg_begin();
   Argument *arg_in = &*arg_it;
   ++arg_it;
@@ -251,11 +259,11 @@ Function * BPFModule::make_reader(Module *mod, Type *type) {
   B.SetInsertPoint(label_exit);
   B.CreateRet(B.getInt32(0));
 
-  readers_[type] = fn;
-  return fn;
+  readers_[type] = name;
+  return name;
 }
 
-Function * BPFModule::make_writer(Module *mod, Type *type) {
+string BPFModule::make_writer(Module *mod, Type *type) {
   auto fn_it = writers_.find(type);
   if (fn_it != writers_.end())
     return fn_it->second;
@@ -266,10 +274,11 @@ Function * BPFModule::make_writer(Module *mod, Type *type) {
 
   IRBuilder<> B(*ctx_);
 
+  string name = "writer" + std::to_string(writers_.size());
   vector<Type *> fn_args({B.getInt8PtrTy(), B.getInt64Ty(), PointerType::getUnqual(type)});
   FunctionType *fn_type = FunctionType::get(B.getInt32Ty(), fn_args, /*isVarArg=*/false);
-  Function *fn = Function::Create(fn_type, GlobalValue::ExternalLinkage,
-                                  "writer" + std::to_string(writers_.size()), mod);
+  Function *fn =
+      Function::Create(fn_type, GlobalValue::ExternalLinkage, name, mod);
   auto arg_it = fn->arg_begin();
   Argument *arg_out = &*arg_it;
   ++arg_it;
@@ -308,8 +317,8 @@ Function * BPFModule::make_writer(Module *mod, Type *type) {
 
   B.CreateRet(call);
 
-  writers_[type] = fn;
-  return fn;
+  writers_[type] = name;
+  return name;
 }
 
 unique_ptr<ExecutionEngine> BPFModule::finalize_rw(unique_ptr<Module> m) {
@@ -355,15 +364,6 @@ int BPFModule::annotate() {
   // separate module to hold the reader functions
   auto m = ebpf::make_unique<Module>("sscanf", *ctx_);
 
-  struct llvmfnpointers {
-    llvm::Function *key_sscanf;
-    llvm::Function *leaf_sscanf;
-    llvm::Function *key_snprintf;
-    llvm::Function *leaf_snprintf;
-  };
-
-  std::map<TableDesc *, llvmfnpointers> ptrs_map;
-
   size_t id = 0;
   Path path({id_});
   for (auto it = ts_->lower_bound(path), up = ts_->upper_bound(path); it != up; ++it) {
@@ -378,48 +378,51 @@ int BPFModule::annotate() {
         Type *key_type = st->elements()[0];
         Type *leaf_type = st->elements()[1];
 
-        llvmfnpointers fns;
-
-        fns.key_sscanf = make_reader(&*m, key_type);
-        if (!fns.key_sscanf)
-          errs() << "Failed to compile sscanf for " << *key_type << "\n";
-
-        fns.leaf_sscanf = make_reader(&*m, leaf_type);
-        if (!fns.leaf_sscanf)
-          errs() << "Failed to compile sscanf for " << *leaf_type << "\n";
-
-        fns.key_snprintf = make_writer(&*m, key_type);
-        if (!fns.key_snprintf)
-          errs() << "Failed to compile snprintf for " << *key_type << "\n";
-
-        fns.leaf_snprintf = make_writer(&*m, leaf_type);
-        if (!fns.leaf_snprintf)
-          errs() << "Failed to compile snprintf for " << *leaf_type << "\n";
-
-        ptrs_map[&it->second] = fns;
+        using std::placeholders::_1;
+        using std::placeholders::_2;
+        using std::placeholders::_3;
+        table.key_sscanf = std::bind(&BPFModule::sscanf, this,
+                                     make_reader(&*m, key_type), _1, _2);
+        table.leaf_sscanf = std::bind(&BPFModule::sscanf, this,
+                                      make_reader(&*m, leaf_type), _1, _2);
+        table.key_snprintf = std::bind(&BPFModule::snprintf, this,
+                                       make_writer(&*m, key_type), _1, _2, _3);
+        table.leaf_snprintf =
+            std::bind(&BPFModule::snprintf, this, make_writer(&*m, leaf_type),
+                      _1, _2, _3);
       }
     }
   }
 
   rw_engine_ = finalize_rw(move(m));
-  if (rw_engine_)
-    rw_engine_->finalizeObject();
-
-  for (auto &it : ptrs_map) {
-    auto t = it.first;
-    auto ptr = it.second;
-    t->key_sscanf = (sscanf_fn)rw_engine_->getPointerToFunction(ptr.key_sscanf);
-    t->leaf_sscanf =
-        (sscanf_fn)rw_engine_->getPointerToFunction(ptr.leaf_sscanf);
-    t->key_snprintf =
-        (snprintf_fn)rw_engine_->getPointerToFunction(ptr.key_snprintf);
-    t->leaf_snprintf =
-        (snprintf_fn)rw_engine_->getPointerToFunction(ptr.leaf_snprintf);
-  }
-
+  if (!rw_engine_)
+    return -1;
   return 0;
 }
 
+StatusTuple BPFModule::sscanf(string fn_name, const char *str, void *val) {
+  auto fn =
+      (int (*)(const char *, void *))rw_engine_->getFunctionAddress(fn_name);
+  if (!fn)
+    return StatusTuple(-1, "sscanf not available");
+  int rc = fn(str, val);
+  if (rc < 0)
+    return StatusTuple(rc, "error in sscanf: %s", std::strerror(errno));
+  return StatusTuple(rc);
+}
+
+StatusTuple BPFModule::snprintf(string fn_name, char *str, size_t sz,
+                                const void *val) {
+  auto fn = (int (*)(char *, size_t,
+                     const void *))rw_engine_->getFunctionAddress(fn_name);
+  if (!fn)
+    return StatusTuple(-1, "snprintf not available");
+  int rc = fn(str, sz, val);
+  if (rc < 0)
+    return StatusTuple(rc, "error in snprintf: %s", std::strerror(errno));
+  return StatusTuple(rc);
+}
+
 void BPFModule::dump_ir(Module &mod) {
   legacy::PassManager PM;
   PM.add(createPrintModulePass(errs()));
@@ -649,16 +652,12 @@ int BPFModule::table_key_printf(size_t id, char *buf, size_t buflen, const void
   if (id >= tables_.size())
     return -1;
   const TableDesc &desc = *tables_[id];
-  if (!desc.key_snprintf) {
-    fprintf(stderr, "Key snprintf not available\n");
-    return -1;
-  }
-  int rc = desc.key_snprintf(buf, buflen, key);
-  if (rc < 0) {
-    perror("snprintf");
+  StatusTuple rc = desc.key_snprintf(buf, buflen, key);
+  if (rc.code() < 0) {
+    fprintf(stderr, "%s\n", rc.msg().c_str());
     return -1;
   }
-  if ((size_t)rc >= buflen) {
+  if ((size_t)rc.code() == buflen) {
     fprintf(stderr, "snprintf ran out of buffer space\n");
     return -1;
   }
@@ -669,16 +668,12 @@ int BPFModule::table_leaf_printf(size_t id, char *buf, size_t buflen, const void
   if (id >= tables_.size())
     return -1;
   const TableDesc &desc = *tables_[id];
-  if (!desc.leaf_snprintf) {
-    fprintf(stderr, "Key snprintf not available\n");
+  StatusTuple rc = desc.leaf_snprintf(buf, buflen, leaf);
+  if (rc.code() < 0) {
+    fprintf(stderr, "%s\n", rc.msg().c_str());
     return -1;
   }
-  int rc = desc.leaf_snprintf(buf, buflen, leaf);
-  if (rc < 0) {
-    perror("snprintf");
-    return -1;
-  }
-  if ((size_t)rc >= buflen) {
+  if ((size_t)rc.code() == buflen) {
     fprintf(stderr, "snprintf ran out of buffer space\n");
     return -1;
   }
@@ -689,13 +684,9 @@ int BPFModule::table_key_scanf(size_t id, const char *key_str, void *key) {
   if (id >= tables_.size())
     return -1;
   const TableDesc &desc = *tables_[id];
-  if (!desc.key_sscanf) {
-    fprintf(stderr, "Key sscanf not available\n");
-    return -1;
-  }
-  int rc = desc.key_sscanf(key_str, key);
-  if (rc != 0) {
-    perror("sscanf");
+  StatusTuple rc = desc.key_sscanf(key_str, key);
+  if (rc.code() < 0) {
+    fprintf(stderr, "%s\n", rc.msg().c_str());
     return -1;
   }
   return 0;
@@ -705,13 +696,9 @@ int BPFModule::table_leaf_scanf(size_t id, const char *leaf_str, void *leaf) {
   if (id >= tables_.size())
     return -1;
   const TableDesc &desc = *tables_[id];
-  if (!desc.leaf_sscanf) {
-    fprintf(stderr, "Key sscanf not available\n");
-    return -1;
-  }
-  int rc = desc.leaf_sscanf(leaf_str, leaf);
-  if (rc != 0) {
-    perror("sscanf");
+  StatusTuple rc = desc.leaf_sscanf(leaf_str, leaf);
+  if (rc.code() < 0) {
+    fprintf(stderr, "%s\n", rc.msg().c_str());
     return -1;
   }
   return 0;

diff --git a/src/cc/bpf_module.h b/src/cc/bpf_module.h
index 236a8b5d6e2bcbb2fa58553fdc07f293a96e9bad..a4b0369705ce1eb00fdf850a5769de5f00e01ac1 100644 (file)
--- a/src/cc/bpf_module.h
+++ b/src/cc/bpf_module.h
@@ -22,6 +22,8 @@
 #include <string>
 #include <vector>
 
+#include "bcc_exception.h"
+
 namespace llvm {
 class ExecutionEngine;
 class Function;
@@ -44,14 +46,18 @@ class BPFModule {
   int finalize();
   int annotate();
   std::unique_ptr<llvm::ExecutionEngine> finalize_rw(std::unique_ptr<llvm::Module> mod);
-  llvm::Function * make_reader(llvm::Module *mod, llvm::Type *type);
-  llvm::Function * make_writer(llvm::Module *mod, llvm::Type *type);
+  std::string make_reader(llvm::Module *mod, llvm::Type *type);
+  std::string make_writer(llvm::Module *mod, llvm::Type *type);
   void dump_ir(llvm::Module &mod);
   int load_file_module(std::unique_ptr<llvm::Module> *mod, const std::string &file, bool in_memory);
   int load_includes(const std::string &text);
   int load_cfile(const std::string &file, bool in_memory, const char *cflags[], int ncflags);
   int kbuild_flags(const char *uname_release, std::vector<std::string> *cflags);
   int run_pass_manager(llvm::Module &mod);
+  StatusTuple sscanf(std::string fn_name, const char *str, void *val);
+  StatusTuple snprintf(std::string fn_name, char *str, size_t sz,
+                       const void *val);
+
  public:
   BPFModule(unsigned flags, TableStorage *ts = nullptr);
   ~BPFModule();
@@ -106,8 +112,8 @@ class BPFModule {
   std::vector<TableDesc *> tables_;
   std::map<std::string, size_t> table_names_;
   std::vector<std::string> function_names_;
-  std::map<llvm::Type *, llvm::Function *> readers_;
-  std::map<llvm::Type *, llvm::Function *> writers_;
+  std::map<llvm::Type *, std::string> readers_;
+  std::map<llvm::Type *, std::string> writers_;
   std::string id_;
   TableStorage *ts_;
   std::unique_ptr<TableStorage> local_ts_;

diff --git a/src/cc/table_desc.h b/src/cc/table_desc.h
index 66c5362b51c04d353ac7af3d5970b89bb49b4914..d1338202032ac2db53d5fef42720315683527d0d 100644 (file)
--- a/src/cc/table_desc.h
+++ b/src/cc/table_desc.h
@@ -18,15 +18,13 @@
 
 #include <unistd.h>
 #include <cstdint>
+#include <functional>
 #include <memory>
 #include <string>
 
+#include "bcc_exception.h"
 #include "common.h"
 
-namespace llvm {
-class Function;
-}
-
 namespace clang {
 class ASTContext;
 class QualType;
@@ -34,8 +32,8 @@ class QualType;
 
 namespace ebpf {
 
-typedef int (*sscanf_fn)(const char *, void *);
-typedef int (*snprintf_fn)(char *, size_t, const void *);
+typedef std::function<StatusTuple(const char *, void *)> sscanf_fn;
+typedef std::function<StatusTuple(char *, size_t, const void *)> snprintf_fn;
 
 /// TableDesc uniquely stores all of the runtime state for an active bpf table.
 /// The copy constructor/assign operator are disabled since the file handles
@@ -68,10 +66,6 @@ class TableDesc {
         leaf_size(0),
         max_entries(0),
         flags(0),
-        key_sscanf(nullptr),
-        leaf_sscanf(nullptr),
-        key_snprintf(nullptr),
-        leaf_snprintf(nullptr),
         is_shared(false),
         is_extern(false) {}
   TableDesc(const std::string &name, FileDesc &&fd, int type, size_t key_size,
@@ -83,10 +77,6 @@ class TableDesc {
         leaf_size(leaf_size),
         max_entries(max_entries),
         flags(flags),
-        key_sscanf(nullptr),
-        leaf_sscanf(nullptr),
-        key_snprintf(nullptr),
-        leaf_snprintf(nullptr),
         is_shared(false),
         is_extern(false) {}
   TableDesc(TableDesc &&that) = default;