Instrument the allocator code so that we can track down the cause of recent crashes.

BUG=74777
TEST=This should give us a clearer picture of what's going wrong in the recent courgette crashes.

Review URL: http://codereview.chromium.org/6611027

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@76834 0039d316-1c4b-4281-b951-d872f2087c98

3 files changed, 66 insertions, 44 deletions

diff --git a/courgette/assembly_program.cc b/courgette/assembly_program.cc
index 345ca89..1dc9ecf 100644
--- a/courgette/assembly_program.cc
+++ b/courgette/assembly_program.cc
@@ -289,6 +289,9 @@ void AssemblyProgram::AssignRemainingIndexes(RVAToLabel* labels) {
 
 typedef void (EncodedProgram::*DefineLabelMethod)(int index, RVA value);
 
+#if defined(OS_WIN)
+__declspec(noinline)
+#endif
 static void DefineLabels(const RVAToLabel& labels,
                          EncodedProgram* encoded_format,
                          DefineLabelMethod define_label) {
diff --git a/courgette/memory_allocator.cc b/courgette/memory_allocator.cc
index 06a0653..d45d053 100644
--- a/courgette/memory_allocator.cc
+++ b/courgette/memory_allocator.cc
@@ -7,9 +7,21 @@
 #include <map>
 
 #include "base/file_util.h"
+#include "base/stringprintf.h"
 
 #ifdef OS_WIN
 
+// A helper function to help diagnose failures we've seen in the field.
+// The function constructs a string containing the error and throws a runtime
+// exception.  The calling convention is set to stdcall to force argument
+// passing via the stack.
+__declspec(noinline)
+void __stdcall RuntimeError(DWORD err) {
+  char buffer[20] = {0};
+  wsprintfA(buffer, "err: %u", err);
+  throw buffer;
+}
+
 namespace courgette {
 
 // TempFile
@@ -21,6 +33,13 @@ TempFile::~TempFile() {
   Close();
 }
 
+FilePath TempFile::PrepareTempFile() {
+  FilePath path;
+  if (!file_util::CreateTemporaryFile(&path))
+    RuntimeError(::GetLastError());
+  return path;
+}
+
 void TempFile::Close() {
   if (valid()) {
     base::ClosePlatformFile(file_);
@@ -29,21 +48,18 @@ void TempFile::Close() {
   }
 }
 
-bool TempFile::Create() {
+void TempFile::Create() {
   DCHECK(file_ == base::kInvalidPlatformFileValue);
-  FilePath path;
-  if (file_util::CreateTemporaryFile(&path)) {
-    bool created = false;
-    base::PlatformFileError error_code = base::PLATFORM_FILE_OK;
-    int flags = base::PLATFORM_FILE_OPEN_ALWAYS | base::PLATFORM_FILE_READ |
-                base::PLATFORM_FILE_WRITE |
-                base::PLATFORM_FILE_DELETE_ON_CLOSE |
-                base::PLATFORM_FILE_TEMPORARY;
-    file_ = base::CreatePlatformFile(path, flags, &created, &error_code);
-    PLOG_IF(ERROR, file_ == base::kInvalidPlatformFileValue)
-        << "CreatePlatformFile";
-  }
-  return valid();
+  FilePath path(PrepareTempFile());
+  bool created = false;
+  base::PlatformFileError error_code = base::PLATFORM_FILE_OK;
+  int flags = base::PLATFORM_FILE_OPEN_ALWAYS | base::PLATFORM_FILE_READ |
+              base::PLATFORM_FILE_WRITE |
+              base::PLATFORM_FILE_DELETE_ON_CLOSE |
+              base::PLATFORM_FILE_TEMPORARY;
+  file_ = base::CreatePlatformFile(path, flags, &created, &error_code);
+  if (file_ == base::kInvalidPlatformFileValue)
+    RuntimeError(error_code);
 }
 
 bool TempFile::valid() const {
@@ -58,11 +74,11 @@ size_t TempFile::size() const {
   return size_;
 }
 
-bool TempFile::SetSize(size_t size) {
-  bool ret = base::TruncatePlatformFile(file_, size);
-  if (ret)
-    size_ = size;
-  return ret;
+void TempFile::SetSize(size_t size) {
+  bool set = base::TruncatePlatformFile(file_, size);
+  if (!set)
+    RuntimeError(::GetLastError());
+  size_ = size;
 }
 
 // FileMapping
@@ -74,19 +90,22 @@ FileMapping::~FileMapping() {
   Close();
 }
 
-bool FileMapping::Create(HANDLE file, size_t size) {
+void FileMapping::InitializeView(size_t size) {
+  DCHECK(view_ == NULL);
+  DCHECK(mapping_ != NULL);
+  view_ = ::MapViewOfFile(mapping_, FILE_MAP_WRITE, 0, 0, size);
+  if (!view_)
+    RuntimeError(::GetLastError());
+}
+
+void FileMapping::Create(HANDLE file, size_t size) {
   DCHECK(file != INVALID_HANDLE_VALUE);
   DCHECK(!valid());
   mapping_ = ::CreateFileMapping(file, NULL, PAGE_READWRITE, 0, 0, NULL);
-  if (mapping_)
-    view_ = ::MapViewOfFile(mapping_, FILE_MAP_WRITE, 0, 0, size);
+  if (!mapping_)
+    RuntimeError(::GetLastError());
 
-  if (!valid()) {
-    PLOG(ERROR) << "Failed to map file";
-    Close();
-  }
-
-  return valid();
+  InitializeView(size);
 }
 
 void FileMapping::Close() {
@@ -114,19 +133,17 @@ TempMapping::TempMapping() {
 TempMapping::~TempMapping() {
 }
 
-bool TempMapping::Initialize(size_t size) {
+void TempMapping::Initialize(size_t size) {
   // TODO(tommi): The assumption here is that the alignment of pointers (this)
   // is as strict or stricter than the alignment of the element type.  This is
   // not always true, e.g. __m128 has 16-byte alignment.
   size += sizeof(this);
-  bool ret = file_.Create() && file_.SetSize(size) &&
-             mapping_.Create(file_.handle(), size);
-  if (ret) {
-    TempMapping** write = reinterpret_cast<TempMapping**>(mapping_.view());
-    write[0] = this;
-  }
+  file_.Create();
+  file_.SetSize(size);
+  mapping_.Create(file_.handle(), size);
 
-  return ret;
+  TempMapping** write = reinterpret_cast<TempMapping**>(mapping_.view());
+  write[0] = this;
 }
 
 void* TempMapping::memory() const {
diff --git a/courgette/memory_allocator.h b/courgette/memory_allocator.h
index 8e06e65..fce1e25 100644
--- a/courgette/memory_allocator.h
+++ b/courgette/memory_allocator.h
@@ -8,6 +8,7 @@
 #include <memory>
 
 #include "base/basictypes.h"
+#include "base/file_path.h"
 #include "base/logging.h"
 #include "base/platform_file.h"
 
@@ -24,9 +25,9 @@ class TempFile {
   TempFile();
   ~TempFile();
 
-  bool Create();
+  __declspec(noinline) void Create();
   void Close();
-  bool SetSize(size_t size);
+  __declspec(noinline) void SetSize(size_t size);
 
   // Returns true iff the temp file is currently open.
   bool valid() const;
@@ -40,6 +41,8 @@ class TempFile {
   size_t size() const;
 
  protected:
+  __declspec(noinline) FilePath PrepareTempFile();
+
   base::PlatformFile file_;
   size_t size_;
 };
@@ -51,7 +54,7 @@ class FileMapping {
   ~FileMapping();
 
   // Map a file from beginning to |size|.
-  bool Create(HANDLE file, size_t size);
+  __declspec(noinline) void Create(HANDLE file, size_t size);
   void Close();
 
   // Returns true iff a mapping has been created.
@@ -62,6 +65,8 @@ class FileMapping {
   void* view() const;
 
  protected:
+  __declspec(noinline) void InitializeView(size_t size);
+
   HANDLE mapping_;
   void* view_;
 };
@@ -77,7 +82,7 @@ class TempMapping {
 
   // Creates a temporary file of size |size| and maps it into the current
   // process' address space.
-  bool Initialize(size_t size);
+  __declspec(noinline) void Initialize(size_t size);
 
   // Returns a writable pointer to the reserved memory.
   void* memory() const;
@@ -179,10 +184,7 @@ class MemoryAllocator {
       // If either the heap allocation failed or the request exceeds the
       // max heap allocation threshold, we back the allocation with a temp file.
       TempMapping* mapping = new TempMapping();
-      if (!mapping->Initialize(bytes)) {
-        delete mapping;
-        throw std::bad_alloc("TempMapping::Initialize");
-      }
+      mapping->Initialize(bytes);
       mem = reinterpret_cast<uint8*>(mapping->memory());
       mem[0] = static_cast<uint8>(FILE_ALLOCATION);
     }