Incorporate shimming into the irt

src/untrusted/irt/irt_ppapi.c

Index: src/untrusted/irt/irt_ppapi.c
===================================================================
--- src/untrusted/irt/irt_ppapi.c	(revision 9574)
+++ src/untrusted/irt/irt_ppapi.c	(working copy)
@@ -1,16 +1,20 @@
 /*
- * Copyright (c) 2012 The Native Client Authors. All rights reserved.
+ * Copyright 2012 The Native Client 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 "native_client/src/shared/ppapi_proxy/ppruntime.h"
 #include "native_client/src/untrusted/irt/irt.h"
+#include "native_client/src/untrusted/irt/irt_interfaces.h"
 #include "native_client/src/untrusted/irt/irt_ppapi.h"
 #include "native_client/src/untrusted/irt/irt_private.h"
 
+#include "ppapi/generators/pnacl_shim.h"
+
 struct PP_StartFunctions g_pp_functions;
 
+/* register entry points to untrusted pepper plugin  */
 static int irt_ppapi_start(const struct PP_StartFunctions *funcs) {
   g_pp_functions = *funcs;
   g_is_main_thread = 1;
@@ -42,3 +46,69 @@
   irt_ppapi_start,
   PpapiPluginRegisterThreadCreator,
 };
+
+
+/*
+ * PNaCl Shimming magic
+ * ====================
+ * Note: this could be simplified by changing the actual
+ * GetInterface functions but for now we rely on some autogenerated files.
+ *
+ * Some background:
+ * pnacl has calling conventions which are slightly different from nacl-gcc.

[Mark Seaborn, 2012/08/28 15:51:20]

'pnacl' -> 'PNaCl'

[Robert Muth (chromium), 2012/08/28 18:13:30]

Done.

+ * This affects structure passing on x86-64 which is only used by Ppapi

[Mark Seaborn, 2012/08/28 15:51:20]

'Ppapi' -> 'PPAPI'

[Robert Muth (chromium), 2012/08/28 18:13:30]

Done.

+ * functions.
+ * There are two functions which are relevant:
+ * PPP_GetInterface and PPB_GetInterface.
+ * We interecept those functions, store the old version and then redirect

[Mark Seaborn, 2012/08/28 15:51:20]

"intercept"

[Robert Muth (chromium), 2012/08/28 18:13:30]

Done.

+ * to __Pnacl_PPPGetInterface and __Pnacl_PPBGetInterface.
+ */
+
+/* Holds the orginial PP_StartFunctions data if shimming is enabled. */
+struct PP_StartFunctions g_pp_functions_non_shim;

[Mark Seaborn, 2012/08/28 15:51:20]

This should be static, because it's only used within one file.

[Robert Muth (chromium), 2012/08/28 18:13:30]

Done.

+
+static int32_t PPP_InitializeModule_cc_shim(PP_Module module_id,
+                                            PPB_GetInterface get_browser_intf) {

[Mark Seaborn, 2012/08/28 15:51:20]

Please use the full name: "get_browser_interface", no need to abbreviate.

[Robert Muth (chromium), 2012/08/28 18:13:30]

Done.

+  /* save old version of PPB_GetInterface */
+  __set_real_Pnacl_PPBGetInterface(get_browser_intf);
+  /* redirect to new version of PPB_GetInterface */
+  return g_pp_functions_non_shim.PPP_InitializeModule(
+    module_id, &__Pnacl_PPBGetInterface);

[Mark Seaborn, 2012/08/28 15:51:20]

Nit: the usual style is to indent arguments by 4 spaces rather than 2

[Robert Muth (chromium), 2012/08/28 18:13:30]

Done.

+}
+
+static void PPP_ShutdownModule_cc_shim() {
+  /* redirect to non-shimed version */

[Mark Seaborn, 2012/08/28 15:51:20]

"shimmed"

[Robert Muth (chromium), 2012/08/28 18:13:30]

Done.

+  g_pp_functions_non_shim.PPP_ShutdownModule();
+}
+
+static const void *PPP_GetInterface_cc_shim(const char *interface_name) {
+  /* redirect to new version of PPP_GetInterface */
+  return __Pnacl_PPPGetInterface(interface_name);
+}
+
+static struct PP_StartFunctions g_pp_functions_cc_shim = {
+  &PPP_InitializeModule_cc_shim,

[Mark Seaborn, 2012/08/28 15:51:20]

We don't use the redundant "&" for getting function pointers elsewhere in the
file, so please don't use it here.

[Robert Muth (chromium), 2012/08/28 18:13:30]

Done.

+  &PPP_ShutdownModule_cc_shim,
+  &PPP_GetInterface_cc_shim
+};
+
+/*
+ * This function is called by nexe module to advertise its API
+ * to chrome.

[Mark Seaborn, 2012/08/28 15:51:20]

'chrome' -> 'Chrome'

[Robert Muth (chromium), 2012/08/28 18:13:30]

Done.

+ * We need to shim all these functions and also do some extra bookeeping

[Mark Seaborn, 2012/08/28 15:51:20]

Isn't this repeating what you said in the earlier comment?  The earlier comment
should be enough.

[Robert Muth (chromium), 2012/08/28 18:13:30]

emphasized the word *also*

On 2012/08/28 15:51:20, Mark Seaborn wrote:

+ * work so we sneak in our own replacements g_pp_functions_cc_shim
+ * which in turn call the original functions.
+ */
+static int irt_ppapi_start_shimmed(const struct PP_StartFunctions *funcs) {
+  /* save old version of PPP_GetInterface */
+  __set_real_Pnacl_PPPGetInterface(funcs->PPP_GetInterface);
+  g_pp_functions_non_shim = *funcs;
+  g_pp_functions = g_pp_functions_cc_shim;
+  g_is_main_thread = 1;
+  return PpapiPluginMain();

[Mark Seaborn, 2012/08/28 15:51:20]

Rather than duplicating code from irt_ppapi_start() above, please just call
irt_ppapi_start() here.

[Robert Muth (chromium), 2012/08/28 18:13:30]

Done.

+}
+
+const struct nacl_irt_ppapihook nacl_irt_ppapihook_shimmed = {
+  irt_ppapi_start_shimmed,
+  PpapiPluginRegisterThreadCreator,
+};