NaCl: Make hardware exception handling work on 64-bit Windows

In order to support untrusted hardware exception handling on 64-bit
Windows, we need to have a 64-bit process attach as a debugger to the
64-bit NaCl loader process, so we extend the NaCl broker process to do
this.

The new code in nacl_broker_listener.cc for attaching a 64-bit
debugger is similar to the 32-bit code in nacl_process_host.cc.  I
intend to unify the two in a later change.

BUG=http://code.google.com/p/nativeclient/issues/detail?id=2651
TEST=run_inbrowser_exception_test in nacl_integration

Review URL: https://chromiumcodereview.appspot.com/9835003

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

10 files changed, 188 insertions, 42 deletions

diff --git a/chrome/browser/nacl_host/nacl_broker_host_win.cc b/chrome/browser/nacl_host/nacl_broker_host_win.cc
index 92e1453..3df9731 100644
--- a/chrome/browser/nacl_host/nacl_broker_host_win.cc
+++ b/chrome/browser/nacl_host/nacl_broker_host_win.cc
@@ -54,6 +54,8 @@ bool NaClBrokerHost::OnMessageReceived(const IPC::Message& msg) {
   bool handled = true;
   IPC_BEGIN_MESSAGE_MAP(NaClBrokerHost, msg)
     IPC_MESSAGE_HANDLER(NaClProcessMsg_LoaderLaunched, OnLoaderLaunched)
+    IPC_MESSAGE_HANDLER(NaClProcessMsg_DebugExceptionHandlerLaunched,
+                        OnDebugExceptionHandlerLaunched)
     IPC_MESSAGE_UNHANDLED(handled = false)
   IPC_END_MESSAGE_MAP()
   return handled;
@@ -70,6 +72,14 @@ void NaClBrokerHost::OnLoaderLaunched(const std::wstring& loader_channel_id,
   NaClBrokerService::GetInstance()->OnLoaderLaunched(loader_channel_id, handle);
 }
 
+bool NaClBrokerHost::LaunchDebugExceptionHandler(int32 pid) {
+  return process_->Send(new NaClProcessMsg_LaunchDebugExceptionHandler(pid));
+}
+
+void NaClBrokerHost::OnDebugExceptionHandlerLaunched(int32 pid) {
+  NaClBrokerService::GetInstance()->OnDebugExceptionHandlerLaunched(pid);
+}
+
 void NaClBrokerHost::StopBroker() {
   process_->Send(new NaClProcessMsg_StopBroker());
 }
diff --git a/chrome/browser/nacl_host/nacl_broker_host_win.h b/chrome/browser/nacl_host/nacl_broker_host_win.h
index 3ea6a33..353badd 100644
--- a/chrome/browser/nacl_host/nacl_broker_host_win.h
+++ b/chrome/browser/nacl_host/nacl_broker_host_win.h
@@ -28,6 +28,8 @@ class NaClBrokerHost : public content::BrowserChildProcessHostDelegate {
   // a Native Client loader process.
   bool LaunchLoader(const std::wstring& loader_channel_id);
 
+  bool LaunchDebugExceptionHandler(int32 pid);
+
   // Stop the broker process.
   void StopBroker();
 
@@ -35,6 +37,8 @@ class NaClBrokerHost : public content::BrowserChildProcessHostDelegate {
   // Handler for NaClProcessMsg_LoaderLaunched message
   void OnLoaderLaunched(const std::wstring& loader_channel_id,
                         base::ProcessHandle handle);
+  // Handler for NaClProcessMsg_DebugExceptionHandlerLaunched message
+  void OnDebugExceptionHandlerLaunched(int32 pid);
 
   // BrowserChildProcessHostDelegate implementation:
   virtual bool OnMessageReceived(const IPC::Message& msg);
diff --git a/chrome/browser/nacl_host/nacl_broker_service_win.cc b/chrome/browser/nacl_host/nacl_broker_service_win.cc
index 16bceb5..a338f09 100644
--- a/chrome/browser/nacl_host/nacl_broker_service_win.cc
+++ b/chrome/browser/nacl_host/nacl_broker_service_win.cc
@@ -65,6 +65,25 @@ void NaClBrokerService::OnLoaderDied() {
   }
 }
 
+bool NaClBrokerService::LaunchDebugExceptionHandler(
+    NaClProcessHost* nacl_process_host, int32 pid) {
+  pending_debuggers_[pid] = nacl_process_host;
+  NaClBrokerHost* broker_host = GetBrokerHost();
+  if (!broker_host)
+    return false;
+  return broker_host->LaunchDebugExceptionHandler(pid);
+}
+
+void NaClBrokerService::OnDebugExceptionHandlerLaunched(int32 pid) {
+  PendingDebugExceptionHandlersMap::iterator it = pending_debuggers_.find(pid);
+  if (pending_debuggers_.end() == it)
+    NOTREACHED();
+
+  NaClProcessHost* client = it->second;
+  client->OnDebugExceptionHandlerLaunchedByBroker();
+  pending_debuggers_.erase(it);
+}
+
 NaClBrokerHost* NaClBrokerService::GetBrokerHost() {
   BrowserChildProcessHostIterator iter(content::PROCESS_TYPE_NACL_BROKER);
   if (iter.Done())
diff --git a/chrome/browser/nacl_host/nacl_broker_service_win.h b/chrome/browser/nacl_host/nacl_broker_service_win.h
index c69da7b..2b04938 100644
--- a/chrome/browser/nacl_host/nacl_broker_service_win.h
+++ b/chrome/browser/nacl_host/nacl_broker_service_win.h
@@ -1,4 +1,4 @@
-// Copyright (c) 2011 The Chromium Authors. All rights reserved.
+// Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
@@ -34,9 +34,15 @@ class NaClBrokerService {
   // Called by NaClProcessHost when a loader process is terminated
   void OnLoaderDied();
 
+  bool LaunchDebugExceptionHandler(NaClProcessHost* client, int32 pid);
+
+  // Called by NaClBrokerHost to notify the service that a debug
+  // exception handler was started.
+  void OnDebugExceptionHandlerLaunched(int32 pid);
+
  private:
-  typedef std::map<std::wstring, NaClProcessHost*>
-      PendingLaunchesMap;
+  typedef std::map<std::wstring, NaClProcessHost*> PendingLaunchesMap;
+  typedef std::map<int, NaClProcessHost*> PendingDebugExceptionHandlersMap;
 
   friend struct DefaultSingletonTraits<NaClBrokerService>;
 
@@ -47,6 +53,7 @@ class NaClBrokerService {
 
   int loaders_running_;
   PendingLaunchesMap pending_launches_;
+  PendingDebugExceptionHandlersMap pending_debuggers_;
 
   DISALLOW_COPY_AND_ASSIGN(NaClBrokerService);
 };
diff --git a/chrome/browser/nacl_host/nacl_process_host.cc b/chrome/browser/nacl_host/nacl_process_host.cc
index 1cbab6f..65ce107 100644
--- a/chrome/browser/nacl_host/nacl_process_host.cc
+++ b/chrome/browser/nacl_host/nacl_process_host.cc
@@ -258,9 +258,7 @@ NaClProcessHost::NaClProcessHost(const std::wstring& url)
       getenv("NACL_UNTRUSTED_EXCEPTION_HANDLING") != NULL) {
     enable_exception_handling_ = true;
 #if defined(OS_WIN)
-    if (!RunningOnWOW64()) {
-      debug_context_ = new DebugContext();
-    }
+    debug_context_ = new DebugContext();
 #endif
   }
 }
@@ -505,14 +503,18 @@ bool NaClProcessHost::LaunchSelLdr() {
   return true;
 }
 
-void NaClProcessHost::OnProcessLaunchedByBroker(base::ProcessHandle handle) {
 #if defined(OS_WIN)
+void NaClProcessHost::OnProcessLaunchedByBroker(base::ProcessHandle handle) {
   process_launched_by_broker_ = true;
-#endif
   process_->SetHandle(handle);
   OnProcessLaunched();
 }
 
+void NaClProcessHost::OnDebugExceptionHandlerLaunchedByBroker() {
+  debug_context_->AllowAndSendStartMessage();
+}
+#endif
+
 void NaClProcessHost::OnProcessCrashed(int exit_code) {
   std::string message = base::StringPrintf(
       "NaCl process exited with status %i (0x%x)", exit_code, exit_code);
@@ -646,38 +648,45 @@ void NaClProcessHost::OnChannelConnected(int32 peer_pid) {
   if (debug_context_ == NULL) {
     return;
   }
-  // Start new thread for debug loop
   debug_context_->SetChildProcessHost(process_->GetHost());
-  // We can't use process_->GetData().handle because it doesn't have necessary
-  // access rights.
-  base::ProcessHandle process;
-  if (!base::OpenProcessHandleWithAccess(
-           peer_pid,
-           base::kProcessAccessQueryInformation |
-           base::kProcessAccessSuspendResume |
-           base::kProcessAccessTerminate |
-           base::kProcessAccessVMOperation |
-           base::kProcessAccessVMRead |
-           base::kProcessAccessVMWrite |
-           base::kProcessAccessWaitForTermination,
-           &process)) {
-    LOG(ERROR) << "Failed to open the process";
-    debug_context_->AllowAndSendStartMessage();
-    return;
-  }
-  base::Thread* dbg_thread = new base::Thread("Debug thread");
-  if (!dbg_thread->Start()) {
-    LOG(ERROR) << "Debug thread not started";
-    debug_context_->AllowAndSendStartMessage();
-    base::CloseProcessHandle(process);
-    return;
+  if (RunningOnWOW64()) {
+    if (!NaClBrokerService::GetInstance()->LaunchDebugExceptionHandler(
+             this, peer_pid)) {
+      debug_context_->AllowAndSendStartMessage();
+    }
+  } else {
+    // Start new thread for debug loop
+    // We can't use process_->GetData().handle because it doesn't have necessary
+    // access rights.
+    base::ProcessHandle process;
+    if (!base::OpenProcessHandleWithAccess(
+             peer_pid,
+             base::kProcessAccessQueryInformation |
+             base::kProcessAccessSuspendResume |
+             base::kProcessAccessTerminate |
+             base::kProcessAccessVMOperation |
+             base::kProcessAccessVMRead |
+             base::kProcessAccessVMWrite |
+             base::kProcessAccessWaitForTermination,
+             &process)) {
+      LOG(ERROR) << "Failed to open the process";
+      debug_context_->AllowAndSendStartMessage();
+      return;
+    }
+    base::Thread* dbg_thread = new base::Thread("Debug thread");
+    if (!dbg_thread->Start()) {
+      LOG(ERROR) << "Debug thread not started";
+      debug_context_->AllowAndSendStartMessage();
+      base::CloseProcessHandle(process);
+      return;
+    }
+    debug_context_->SetDebugThread(dbg_thread);
+    // System can not reallocate pid until we close process handle. So using
+    // pid in different thread is fine.
+    dbg_thread->message_loop()->PostTask(FROM_HERE,
+        base::Bind(&NaClProcessHost::DebugContext::AttachDebugger,
+                   debug_context_, peer_pid, process));
   }
-  debug_context_->SetDebugThread(dbg_thread);
-  // System can not reallocate pid until we close process handle. So using
-  // pid in different thread is fine.
-  dbg_thread->message_loop()->PostTask(FROM_HERE,
-      base::Bind(&NaClProcessHost::DebugContext::AttachDebugger,
-                 debug_context_, peer_pid, process));
 }
 #else
 void NaClProcessHost::OnChannelConnected(int32 peer_pid) {
diff --git a/chrome/browser/nacl_host/nacl_process_host.h b/chrome/browser/nacl_host/nacl_process_host.h
index d050179..f5e51ec 100644
--- a/chrome/browser/nacl_host/nacl_process_host.h
+++ b/chrome/browser/nacl_host/nacl_process_host.h
@@ -47,7 +47,10 @@ class NaClProcessHost : public content::BrowserChildProcessHostDelegate {
 
   virtual void OnChannelConnected(int32 peer_pid) OVERRIDE;
 
+#if defined(OS_WIN)
   void OnProcessLaunchedByBroker(base::ProcessHandle handle);
+  void OnDebugExceptionHandlerLaunchedByBroker();
+#endif
 
  private:
   // Internal class that holds the nacl::Handle objecs so that
diff --git a/chrome/common/nacl_messages.h b/chrome/common/nacl_messages.h
index 4a348ce..9ab79c6 100644
--- a/chrome/common/nacl_messages.h
+++ b/chrome/common/nacl_messages.h
@@ -33,6 +33,17 @@ IPC_MESSAGE_CONTROL2(NaClProcessMsg_LoaderLaunched,
                      std::wstring,  /* channel ID for the loader */
                      base::ProcessHandle /* loader process handle */)
 
+// Tells the NaCl broker to attach a debug exception handler to the
+// given NaCl loader process.
+IPC_MESSAGE_CONTROL1(NaClProcessMsg_LaunchDebugExceptionHandler,
+                     int32 /* pid */)
+
+// Notify the browser process that the broker process finished
+// attaching a debug exception handler to the given NaCl loader
+// process.
+IPC_MESSAGE_CONTROL1(NaClProcessMsg_DebugExceptionHandlerLaunched,
+                     int32 /* pid */)
+
 // Notify the broker that all loader processes have been terminated and it
 // should shutdown.
 IPC_MESSAGE_CONTROL0(NaClProcessMsg_StopBroker)
diff --git a/chrome/nacl/nacl_broker_listener.cc b/chrome/nacl/nacl_broker_listener.cc
index 32fc9ef..97346e9 100644
--- a/chrome/nacl/nacl_broker_listener.cc
+++ b/chrome/nacl/nacl_broker_listener.cc
@@ -1,19 +1,87 @@
-// Copyright (c) 2010 The Chromium Authors. All rights reserved.
+// Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "chrome/nacl/nacl_broker_listener.h"
 
 #include "base/base_switches.h"
+#include "base/bind.h"
 #include "base/command_line.h"
 #include "base/message_loop.h"
+#include "base/message_loop_proxy.h"
 #include "base/path_service.h"
 #include "base/process_util.h"
+#include "base/threading/platform_thread.h"
 #include "chrome/common/nacl_cmd_line.h"
 #include "chrome/common/nacl_messages.h"
 #include "content/common/sandbox_policy.h"
 #include "content/public/common/content_switches.h"
 #include "ipc/ipc_switches.h"
+#include "native_client/src/trusted/service_runtime/win/debug_exception_handler.h"
+
+namespace {
+
+void SendReply(IPC::Channel* channel, int32 pid) {
+  channel->Send(new NaClProcessMsg_DebugExceptionHandlerLaunched(pid));
+}
+
+class DebugExceptionHandler : public base::PlatformThread::Delegate {
+ public:
+  DebugExceptionHandler(base::MessageLoopProxy* message_loop,
+                        IPC::Channel* channel, int32 pid)
+      : message_loop_(message_loop), channel_(channel), pid_(pid) {
+  }
+
+  virtual void ThreadMain() OVERRIDE {
+    // In the Windows API, the set of processes being debugged is
+    // thread-local, so we have to attach to the process (using
+    // DebugActiveProcess()) on the same thread on which
+    // NaClDebugLoop() receives debug events for the process.
+    BOOL attached = false;
+    base::ProcessHandle process_handle = base::kNullProcessHandle;
+    if (!base::OpenProcessHandleWithAccess(
+             pid_,
+             base::kProcessAccessQueryInformation |
+             base::kProcessAccessSuspendResume |
+             base::kProcessAccessTerminate |
+             base::kProcessAccessVMOperation |
+             base::kProcessAccessVMRead |
+             base::kProcessAccessVMWrite |
+             base::kProcessAccessWaitForTermination,
+             &process_handle)) {
+      LOG(ERROR) << "Failed to get process handle";
+    } else {
+      attached = DebugActiveProcess(pid_);
+      if (!attached) {
+        LOG(ERROR) << "Failed to connect to the process";
+      }
+    }
+    // At the moment we do not say in the reply whether attaching as a
+    // debugger succeeded.  In the future, when we attach on demand
+    // when an exception handler is first registered, we can make the
+    // NaCl syscall indicate whether attaching succeeded.
+    message_loop_->PostDelayedTask(FROM_HERE,
+        base::Bind(SendReply, channel_, pid_), base::TimeDelta());
+
+    if (attached) {
+      DWORD exit_code;
+      NaClDebugLoop(process_handle, &exit_code);
+    }
+    if (process_handle != base::kNullProcessHandle) {
+      base::CloseProcessHandle(process_handle);
+    }
+    delete this;
+  }
+
+ private:
+  base::MessageLoopProxy* message_loop_;
+  IPC::Channel* channel_;
+  int32 pid_;
+
+  DISALLOW_COPY_AND_ASSIGN(DebugExceptionHandler);
+};
+
+}  // namespace
 
 NaClBrokerListener::NaClBrokerListener()
     : browser_handle_(base::kNullProcessHandle) {
@@ -43,6 +111,8 @@ bool NaClBrokerListener::OnMessageReceived(const IPC::Message& msg) {
   IPC_BEGIN_MESSAGE_MAP(NaClBrokerListener, msg)
     IPC_MESSAGE_HANDLER(NaClProcessMsg_LaunchLoaderThroughBroker,
                         OnLaunchLoaderThroughBroker)
+    IPC_MESSAGE_HANDLER(NaClProcessMsg_LaunchDebugExceptionHandler,
+                        OnLaunchDebugExceptionHandler)
     IPC_MESSAGE_HANDLER(NaClProcessMsg_StopBroker, OnStopBroker)
     IPC_MESSAGE_UNHANDLED(handled = false)
   IPC_END_MESSAGE_MAP()
@@ -85,6 +155,17 @@ void NaClBrokerListener::OnLaunchLoaderThroughBroker(
                                                    loader_handle_in_browser));
 }
 
+void NaClBrokerListener::OnLaunchDebugExceptionHandler(int32 pid) {
+  // The new PlatformThread will take ownership of the
+  // DebugExceptionHandler object, which will delete itself on exit.
+  DebugExceptionHandler* handler = new DebugExceptionHandler(
+      base::MessageLoopProxy::current(), channel_.get(), pid);
+  if (!base::PlatformThread::CreateNonJoinable(0, handler)) {
+    SendReply(channel_.get(), pid);
+    delete handler;
+  }
+}
+
 void NaClBrokerListener::OnStopBroker() {
   MessageLoop::current()->Quit();
 }
diff --git a/chrome/nacl/nacl_broker_listener.h b/chrome/nacl/nacl_broker_listener.h
index e1c3dd7..dccf57c 100644
--- a/chrome/nacl/nacl_broker_listener.h
+++ b/chrome/nacl/nacl_broker_listener.h
@@ -27,6 +27,7 @@ class NaClBrokerListener : public IPC::Channel::Listener {
 
  private:
   void OnLaunchLoaderThroughBroker(const std::wstring& loader_channel_id);
+  void OnLaunchDebugExceptionHandler(int32 pid);
   void OnStopBroker();
 
   base::ProcessHandle browser_handle_;
diff --git a/chrome/test/nacl_test_injection/buildbot_nacl_integration.py b/chrome/test/nacl_test_injection/buildbot_nacl_integration.py
index e571f02..f44a4e6 100755
--- a/chrome/test/nacl_test_injection/buildbot_nacl_integration.py
+++ b/chrome/test/nacl_test_injection/buildbot_nacl_integration.py
@@ -80,10 +80,11 @@ def Main(args):
   if sys.platform in ('win32', 'cygwin'):
     tests_to_disable.append('run_ppapi_ppp_input_event_browser_test')
 
-    # TODO(mseaborn): Remove this when exception handling works inside
-    # Chromium on Windows for both x86-32 and x86-64.
+    # TODO(mseaborn): Enable this test for 32-bit Windows.
     # See http://code.google.com/p/nativeclient/issues/detail?id=2602
-    tests_to_disable.append('run_inbrowser_exception_test')
+    if not ('64' in os.environ.get('PROCESSOR_ARCHITECTURE', '') or
+            '64' in os.environ.get('PROCESSOR_ARCHITEW6432', '')):
+      tests_to_disable.append('run_inbrowser_exception_test')
 
   script_dir = os.path.dirname(os.path.abspath(__file__))
   test_dir = os.path.dirname(script_dir)