Allocate unit bytecode and metadata in a read-only memory region

Only in RepoAuthoritative mode, where units can't be
deallocated.  I have a semi-reproducable bug where a unit's m_bc
region is getting corrupted (only occasionally in perflab).
Presumably moving it out of the malloc'd heap will make the bug
corrupt something else, so this probably doesn't really help much, but
it seemed like having a separate region for some cold, read-only,
process-lifetime metadata might make sense.

hphp/runtime/base/runtime_option.h

@@ -392,6 +392,7 @@ public:
   F(bool, PerfPidMap,                  true)                            \
   F(bool, KeepPerfPidMap,              false)                           \
   F(uint32_t, JitTargetCacheSize,      64 << 20)                        \
+  F(uint32_t, HHBCArenaChunkSize,      64 << 20)                        \
   F(bool, ProfileBC,                   false)                           \
   F(bool, ProfileHWEnable,             true)                            \
   F(string, ProfileHWEvents,           string(""))                      \

hphp/runtime/base/server/admin_request_handler.cpp

@@ -555,6 +555,10 @@ static bool send_status(Transport *transport, ServerStats::Format format,
   return true;
 }
 
+namespace VM {
+  extern size_t hhbc_arena_capacity();
+}
+
 bool AdminRequestHandler::handleCheckRequest(const std::string &cmd,
                                              Transport *transport) {
   if (cmd == "check-load") {
@@ -585,6 +589,7 @@ bool AdminRequestHandler::handleCheckRequest(const std::string &cmd,
     appendStat("load", server->getActiveWorker());
     appendStat("queued", server->getQueuedJobs());
     VM::Transl::Translator* tx = VM::Transl::Translator::Get();
+    appendStat("hhbc-roarena-capac", VM::hhbc_arena_capacity());
     appendStat("tc-size", tx->getCodeSize());
     appendStat("tc-stubsize", tx->getStubSize());
     appendStat("targetcache", tx->getTargetCacheSize());

hphp/runtime/vm/core_types.h

@@ -40,6 +40,15 @@ typedef TypedValue Var;
  */
 typedef uint8_t Opcode;
 
+/*
+ * Program counters in the bytecode interpreter.
+ *
+ * Normally points to an Opcode, but has type const uchar* because
+ * during a given instruction it is incremented while decoding
+ * immediates and may point to arbitrary bytes.
+ */
+typedef const uchar* PC;
+
 /*
  * Id type for various components of a unit that have to have unique
  * identifiers.  For example, function ids, class ids, string literal

hphp/runtime/vm/unit.cpp

@@ -16,30 +16,32 @@
 
 #include <sys/mman.h>
 
-#include "folly/ScopeGuard.h"
-
 #include <iostream>
 #include <iomanip>
 #include <tbb/concurrent_unordered_map.h>
 #include <boost/algorithm/string.hpp>
 
-#include <util/lock.h>
-#include <util/util.h>
-#include <util/atomic.h>
-#include <runtime/ext/ext_variable.h>
-#include <runtime/vm/bytecode.h>
-#include <runtime/vm/repo.h>
-#include <runtime/vm/blob_helper.h>
-#include <runtime/vm/translator/targetcache.h>
-#include <runtime/vm/translator/translator-deps.h>
-#include <runtime/vm/translator/translator-inline.h>
-#include <runtime/vm/translator/translator-x64.h>
-#include <runtime/vm/verifier/check.h>
-#include <runtime/base/strings.h>
-#include <runtime/vm/func_inline.h>
-#include <runtime/eval/runtime/file_repository.h>
-#include <runtime/vm/stats.h>
-#include <runtime/vm/treadmill.h>
+#include "folly/ScopeGuard.h"
+
+#include "util/lock.h"
+#include "util/util.h"
+#include "util/atomic.h"
+#include "util/read_only_arena.h"
+
+#include "runtime/ext/ext_variable.h"
+#include "runtime/vm/bytecode.h"
+#include "runtime/vm/repo.h"
+#include "runtime/vm/blob_helper.h"
+#include "runtime/vm/translator/targetcache.h"
+#include "runtime/vm/translator/translator-deps.h"
+#include "runtime/vm/translator/translator-inline.h"
+#include "runtime/vm/translator/translator-x64.h"
+#include "runtime/vm/verifier/check.h"
+#include "runtime/base/strings.h"
+#include "runtime/vm/func_inline.h"
+#include "runtime/eval/runtime/file_repository.h"
+#include "runtime/vm/stats.h"
+#include "runtime/vm/treadmill.h"
 
 namespace HPHP {
 namespace VM {
@@ -49,6 +51,32 @@ using Util::getDataRef;
 
 static const Trace::Module TRACEMOD = Trace::hhbc;
 
+ReadOnlyArena& get_readonly_arena() {
+  static ReadOnlyArena arena(RuntimeOption::EvalHHBCArenaChunkSize);
+  return arena;
+}
+
+// Exports for the admin server.
+size_t hhbc_arena_capacity() {
+  if (!RuntimeOption::RepoAuthoritative) return 0;
+  return get_readonly_arena().capacity();
+}
+
+static const unsigned char*
+allocateBCRegion(const unsigned char* bc, size_t bclen) {
+  if (RuntimeOption::RepoAuthoritative) {
+    // In RepoAuthoritative, we assume we won't ever deallocate units
+    // and that this is read-only, mostly cold data.  So we throw it
+    // in a bump-allocator that's mprotect'd to prevent writes.
+    return static_cast<const unsigned char*>(
+      get_readonly_arena().allocate(bc, bclen)
+    );
+  }
+  auto mem = static_cast<unsigned char*>(std::malloc(bclen));
+  std::copy(bc, bc + bclen, mem);
+  return mem;
+}
+
 Mutex Unit::s_classesMutex;
 /*
  * We hold onto references to elements of this map. If we use a different
@@ -333,12 +361,14 @@ Unit::Unit()
 }
 
 Unit::~Unit() {
-  if (debug) {
-    // poison released bytecode
-    memset(m_bc, 0xff, m_bclen);
+  if (!RuntimeOption::RepoAuthoritative) {
+    if (debug) {
+      // poison released bytecode
+      memset(const_cast<unsigned char*>(m_bc), 0xff, m_bclen);
+    }
+    free(const_cast<unsigned char*>(m_bc));
+    free(const_cast<unsigned char*>(m_bc_meta));
   }
-  free(m_bc);
-  free(m_bc_meta);
 
   if (m_mergeInfo) {
     // Delete all Func's.
@@ -2418,12 +2448,10 @@ Unit* UnitEmitter::create() {
   Unit* u = new Unit();
   u->m_repoId = m_repoId;
   u->m_sn = m_sn;
-  u->m_bc = (uchar*)malloc(m_bclen);
-  memcpy(u->m_bc, m_bc, m_bclen);
+  u->m_bc = allocateBCRegion(m_bc, m_bclen);
   u->m_bclen = m_bclen;
   if (m_bc_meta_len) {
-    u->m_bc_meta = (uchar*)malloc(m_bc_meta_len);
-    memcpy(u->m_bc_meta, m_bc_meta, m_bc_meta_len);
+    u->m_bc_meta = allocateBCRegion(m_bc_meta, m_bc_meta_len);
     u->m_bc_meta_len = m_bc_meta_len;
   }
   u->m_filepath = m_filepath;

hphp/runtime/vm/unit.h

@@ -109,8 +109,6 @@ struct UnitMergeInfo {
   void* mergeableData(int ix) { return (char*)m_mergeables + ix*sizeof(void*); }
 };
 
-typedef const uchar* PC;
-
 // Exception handler table entry.
 class EHEnt {
  public:
@@ -659,9 +657,9 @@ private:
   // pseudoMain's return value, or KindOfUninit if its not known.
   TypedValue m_mainReturn;
   int64_t m_sn;
-  uchar* m_bc;
+  uchar const* m_bc;
   size_t m_bclen;
-  uchar* m_bc_meta;
+  uchar const* m_bc_meta;
   size_t m_bc_meta_len;
   const StringData* m_filepath;
   const StringData* m_dirpath;

hphp/util/read_only_arena.cpp

@@ -0,0 +1,98 @@
+/*
+   +----------------------------------------------------------------------+
+   | HipHop for PHP                                                       |
+   +----------------------------------------------------------------------+
+   | Copyright (c) 2010- Facebook, Inc. (http://www.facebook.com)         |
+   +----------------------------------------------------------------------+
+   | This source file is subject to version 3.01 of the PHP license,      |
+   | that is bundled with this package in the file LICENSE, and is        |
+   | available through the world-wide-web at the following url:           |
+   | http://www.php.net/license/3_01.txt                                  |
+   | If you did not receive a copy of the PHP license and are unable to   |
+   | obtain it through the world-wide-web, please send a note to          |
+   | license@php.net so we can mail you a copy immediately.               |
+   +----------------------------------------------------------------------+
+*/
+#include "util/read_only_arena.h"
+
+#include <sys/mman.h>
+
+#include "folly/Exception.h"
+
+#include "util/assertions.h"
+
+namespace HPHP {
+
+//////////////////////////////////////////////////////////////////////
+
+typedef std::lock_guard<std::mutex> guard;
+
+//////////////////////////////////////////////////////////////////////
+
+ReadOnlyArena::ReadOnlyArena(size_t chunkSize)
+  : m_chunkSize(chunkSize)
+  , m_frontier(nullptr)
+  , m_end(nullptr)
+{
+  always_assert(m_chunkSize % Util::s_pageSize == 0);
+  grow();
+}
+
+ReadOnlyArena::~ReadOnlyArena() {
+  for (auto& chunk : m_chunks) {
+    munmap(chunk, m_chunkSize);
+  }
+}
+
+const void* ReadOnlyArena::allocate(const void* data, size_t dataLen) {
+  always_assert(dataLen <= m_chunkSize);
+  guard g(m_mutex);
+
+  // Round up to the minimal alignment.
+  dataLen = (dataLen + (kMinimalAlignment - 1)) & ~(kMinimalAlignment - 1);
+
+  if (m_frontier + dataLen > m_end) {
+    grow();
+  }
+  always_assert(m_frontier + dataLen <= m_end);
+
+  auto const ret = m_frontier;
+  assert((uintptr_t(ret) & (kMinimalAlignment - 1)) == 0);
+
+  m_frontier += dataLen;
+
+  auto pageAddr = reinterpret_cast<unsigned char*>(
+    uintptr_t(ret) & ~(Util::s_pageSize - 1)
+  );
+  mprotect(pageAddr, m_frontier - pageAddr, PROT_WRITE|PROT_READ);
+  auto const ucData = static_cast<const unsigned char*>(data);
+  std::copy(ucData, ucData + dataLen, ret);
+  mprotect(pageAddr, m_frontier - pageAddr, PROT_READ);
+
+  return ret;
+}
+
+// Pre: mutex already held, or no other threads may be able to access
+// this (i.e. it's the ctor).
+void ReadOnlyArena::grow() {
+  void* vp = mmap(nullptr, m_chunkSize, PROT_READ,
+    MAP_PRIVATE | MAP_ANONYMOUS, -1, 0);
+  if (vp == MAP_FAILED) {
+    folly::throwSystemError("failed to mmap in ReadOnlyArena");
+  }
+
+  auto uc = static_cast<unsigned char*>(vp);
+  m_chunks.push_back(uc);
+  m_frontier = uc;
+  m_end = uc + m_chunkSize;
+}
+
+size_t ReadOnlyArena::capacity() const {
+  guard g(m_mutex);
+  return m_chunks.size() * m_chunkSize;
+}
+
+//////////////////////////////////////////////////////////////////////
+
+}
+

hphp/util/read_only_arena.h

@@ -0,0 +1,101 @@
+/*
+   +----------------------------------------------------------------------+
+   | HipHop for PHP                                                       |
+   +----------------------------------------------------------------------+
+   | Copyright (c) 2010- Facebook, Inc. (http://www.facebook.com)         |
+   +----------------------------------------------------------------------+
+   | This source file is subject to version 3.01 of the PHP license,      |
+   | that is bundled with this package in the file LICENSE, and is        |
+   | available through the world-wide-web at the following url:           |
+   | http://www.php.net/license/3_01.txt                                  |
+   | If you did not receive a copy of the PHP license and are unable to   |
+   | obtain it through the world-wide-web, please send a note to          |
+   | license@php.net so we can mail you a copy immediately.               |
+   +----------------------------------------------------------------------+
+*/
+#ifndef incl_READ_ONLY_ARENA_H_
+#define incl_READ_ONLY_ARENA_H_
+
+#include <cstdlib>
+#include <thread>
+#include <mutex>
+#include <boost/noncopyable.hpp>
+
+#include "util/tiny_vector.h"
+#include "util/alloc.h"
+
+namespace HPHP {
+
+//////////////////////////////////////////////////////////////////////
+
+/*
+ * ReadOnlyArena is an arena allocator that can be used for
+ * arena-lifetime read only data.  In practice this is used in HPHP
+ * for process-lifetime cold runtime data.
+ *
+ * When allocating from this arena, you have to provide the data
+ * that's supposed to go in the block.  The allocator will temporarily
+ * make the page writeable and put the data in there, then mprotect it
+ * back to read only.
+ *
+ * One read only arena may safely be concurrently accessed by multiple
+ * threads.
+ */
+struct ReadOnlyArena : private boost::noncopyable {
+  /*
+   * All pointers returned from ReadOnlyArena will have at least this
+   * alignment.
+   */
+  static constexpr size_t kMinimalAlignment = 8;
+
+  /*
+   * Create a ReadOnlyArena that uses chunkSize bytes for each call to
+   * mmap.  `chunkSize' must be a multiple of the page size
+   * (s_pageSize).
+   *
+   * Note: s_pageSize is a dynamically initialized static, so do not
+   * create global ReadOnlyArenas.
+   */
+  explicit ReadOnlyArena(size_t chunkSize);
+
+  /*
+   * Destroying a ReadOnlyArena will munmap all the chunks it
+   * allocated, but generally ReadOnlyArenas should be used for
+   * extremely long-lived data.
+   */
+  ~ReadOnlyArena();
+
+  /*
+   * Returns: the number of bytes we've mmaped in this arena.
+   */
+  size_t capacity() const;
+
+  /*
+   * Pre: dataLen is less than or equal to the chunkSize.
+   *
+   * Returns: a pointer to a read only memory region that contains a
+   * copy of [data, data + dataLen).
+   *
+   * Throws: if we fail to mmap.
+   */
+  const void* allocate(const void* data, size_t dataLen);
+
+private:
+  void grow();
+
+private:
+  size_t const m_chunkSize;
+
+  mutable std::mutex m_mutex;
+  unsigned char* m_frontier;
+  unsigned char* m_end;
+  TinyVector<unsigned char*,4> m_chunks;
+};
+
+//////////////////////////////////////////////////////////////////////
+
+}
+
+
+#endif
+

hphp/util/test/read_only_arena.cpp

@@ -0,0 +1,46 @@
+/*
+   +----------------------------------------------------------------------+
+   | HipHop for PHP                                                       |
+   +----------------------------------------------------------------------+
+   | Copyright (c) 2010- Facebook, Inc. (http://www.facebook.com)         |
+   +----------------------------------------------------------------------+
+   | This source file is subject to version 3.01 of the PHP license,      |
+   | that is bundled with this package in the file LICENSE, and is        |
+   | available through the world-wide-web at the following url:           |
+   | http://www.php.net/license/3_01.txt                                  |
+   | If you did not receive a copy of the PHP license and are unable to   |
+   | obtain it through the world-wide-web, please send a note to          |
+   | license@php.net so we can mail you a copy immediately.               |
+   +----------------------------------------------------------------------+
+*/
+#include "util/read_only_arena.h"
+#include <gtest/gtest.h>
+
+#include <vector>
+
+namespace HPHP {
+
+//////////////////////////////////////////////////////////////////////
+
+TEST(ReadOnlyArena, simpleTest) {
+  ReadOnlyArena arena(4096 * 10);
+
+  const char foo[] = "abc";
+  auto strP = static_cast<const char*>(
+    arena.allocate(foo, sizeof foo)
+  );
+
+  EXPECT_EQ(strcmp(strP, foo), 0);
+
+  const char someJunk[] = "whatevs";
+
+  auto strP2 = static_cast<const char*>(
+    arena.allocate(someJunk, sizeof someJunk)
+  );
+
+  EXPECT_EQ(strcmp(strP2, someJunk), 0);
+}
+
+//////////////////////////////////////////////////////////////////////
+
+}