Have PNaCl use debug pexe URL instead of stripped pexe w/ kEnableNaClDebug

Debug metadata is not part of the PNaCl stable bitcode format
so is stripped before deployment. In order to make debugging
easier (no separate translation), allow a separate debug URL
in the NMF file which can point to the unstripped
LLVM-formatted pexe which should still have debug metadata.

BUG= https://code.google.com/p/nativeclient/issues/detail?id=3765
TEST=browser_tests --gtest_filter=*PnaclDebugURL

Review URL: https://codereview.chromium.org/181153002

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

8 files changed, 67 insertions, 22 deletions

diff --git a/ppapi/api/private/ppb_nacl_private.idl b/ppapi/api/private/ppb_nacl_private.idl
index af5ba47..5873377 100644
--- a/ppapi/api/private/ppb_nacl_private.idl
+++ b/ppapi/api/private/ppb_nacl_private.idl
@@ -266,4 +266,9 @@ interface PPB_NaCl_Private {
 
   /* Performs internal cleanup when an instance is destroyed. */
   void InstanceDestroyed([in] PP_Instance instance);
+
+  /* Return true if the NaCl debug stub is enabled and the loaded app
+   * will be attached to a debugger.
+   */
+  PP_Bool NaClDebugStubEnabled();
 };
diff --git a/ppapi/c/private/ppb_nacl_private.h b/ppapi/c/private/ppb_nacl_private.h
index 5ea32a7d..1862803 100644
--- a/ppapi/c/private/ppb_nacl_private.h
+++ b/ppapi/c/private/ppb_nacl_private.h
@@ -3,7 +3,7 @@
  * found in the LICENSE file.
  */
 
-/* From private/ppb_nacl_private.idl modified Thu Feb 27 08:19:06 2014. */
+/* From private/ppb_nacl_private.idl modified Thu Feb 27 14:06:31 2014. */
 
 #ifndef PPAPI_C_PRIVATE_PPB_NACL_PRIVATE_H_
 #define PPAPI_C_PRIVATE_PPB_NACL_PRIVATE_H_
@@ -273,6 +273,10 @@ struct PPB_NaCl_Private_1_0 {
                           PP_Bool is_installed);
   /* Performs internal cleanup when an instance is destroyed. */
   void (*InstanceDestroyed)(PP_Instance instance);
+  /* Return true if the NaCl debug stub is enabled and the loaded app
+   * will be attached to a debugger.
+   */
+  PP_Bool (*NaClDebugStubEnabled)(void);
 };
 
 typedef struct PPB_NaCl_Private_1_0 PPB_NaCl_Private;
diff --git a/ppapi/native_client/src/trusted/plugin/json_manifest.cc b/ppapi/native_client/src/trusted/plugin/json_manifest.cc
index be9b01b..d1d5839 100644
--- a/ppapi/native_client/src/trusted/plugin/json_manifest.cc
+++ b/ppapi/native_client/src/trusted/plugin/json_manifest.cc
@@ -37,6 +37,7 @@ const char* const kArmKey =         "arm";
 const char* const kPortableKey =    "portable";
 
 // Url Resolution keys
+const char* const kPnaclDebugKey =     "pnacl-debug";
 const char* const kPnaclTranslateKey = "pnacl-translate";
 const char* const kUrlKey =            "url";
 
@@ -74,8 +75,11 @@ const char* const kOptLevelKey = "optlevel";
 //   "program": {
 //     "portable": {
 //       "pnacl-translate": {
-//         "url": "myprogram.pexe",
-//         "optlevel": 0
+//         "url": "myprogram.pexe"
+//       },
+//       "pnacl-debug": {
+//         "url": "myprogram.debug.pexe",
+//         "opt_level": 0
 //       }
 //     }
 //   },
@@ -213,21 +217,26 @@ bool IsValidUrlSpec(const Json::Value& url_spec,
   return true;
 }
 
-// Validate a "pnacl-translate" dictionary, assuming it was resolved from
-// container_key. E.g., "container_key" : { "pnacl_translate" : URLSpec }
+// Validate a "pnacl-translate" or "pnacl-debug" dictionary, assuming
+// it was resolved from container_key.
+// E.g., "container_key" : { "pnacl-translate" : URLSpec }
 bool IsValidPnaclTranslateSpec(const Json::Value& pnacl_spec,
                                const nacl::string& container_key,
                                const nacl::string& parent_key,
                                const nacl::string& sandbox_isa,
                                nacl::string* error_string) {
-  static const char* kManifestPnaclSpecProperties[] = {
+  static const char* kManifestPnaclSpecValid[] = {
+    kPnaclDebugKey,
+    kPnaclTranslateKey
+  };
+  static const char* kManifestPnaclSpecRequired[] = {
     kPnaclTranslateKey
   };
   if (!IsValidDictionary(pnacl_spec, container_key, parent_key,
-                         kManifestPnaclSpecProperties,
-                         NACL_ARRAY_SIZE(kManifestPnaclSpecProperties),
-                         kManifestPnaclSpecProperties,
-                         NACL_ARRAY_SIZE(kManifestPnaclSpecProperties),
+                         kManifestPnaclSpecValid,
+                         NACL_ARRAY_SIZE(kManifestPnaclSpecValid),
+                         kManifestPnaclSpecRequired,
+                         NACL_ARRAY_SIZE(kManifestPnaclSpecRequired),
                          error_string)) {
     return false;
   }
@@ -295,8 +304,9 @@ bool IsValidISADictionary(const Json::Value& dictionary,
                              isaPropertiesLength)) {
       // For NaCl, arch entries can only be
       //     "arch/portable" : URLSpec
-      // For PNaCl arch in "program" dictionary entries can only be
+      // For PNaCl arch in "program" dictionary entries can be
       //     "portable" : { "pnacl-translate": URLSpec }
+      //  or "portable" : { "pnacl-debug": URLSpec }
       // For PNaCl arch elsewhere, dictionary entries can only be
       //     "portable" : URLSpec
       if ((sandbox_isa != kPortableKey &&
@@ -361,6 +371,7 @@ void GrabUrlAndPnaclOptions(const Json::Value& url_spec,
                             nacl::string* url,
                             PnaclOptions* pnacl_options) {
   *url = url_spec[kUrlKey].asString();
+  pnacl_options->set_translate(true);
   if (url_spec.isMember(kOptLevelKey)) {
     int32_t opt_raw = url_spec[kOptLevelKey].asInt();
     // set_opt_level will normalize the values.
@@ -502,12 +513,14 @@ bool JsonManifest::GetURLFromISADictionary(const Json::Value& dictionary,
     chosen_isa = sandbox_isa_;
   }
   const Json::Value& isa_spec = dictionary[chosen_isa];
-  // Check if this requires a pnacl-translate, otherwise just grab the URL.
-  // We may have pnacl-translate for isa-specific bitcode for CPU tuning.
-  if (isa_spec.isMember(kPnaclTranslateKey)) {
-    // PNaCl
+  // If the PNaCl debug flag is turned on, look for pnacl-debug entries first.
+  // If found, mark that it is a debug URL. Otherwise, fall back to
+  // checking for pnacl-translate URLs, etc. and don't mark it as a debug URL.
+  if (pnacl_debug_ && isa_spec.isMember(kPnaclDebugKey)) {
+    GrabUrlAndPnaclOptions(isa_spec[kPnaclDebugKey], url, pnacl_options);
+    pnacl_options->set_debug(true);
+  } else if (isa_spec.isMember(kPnaclTranslateKey)) {
     GrabUrlAndPnaclOptions(isa_spec[kPnaclTranslateKey], url, pnacl_options);
-    pnacl_options->set_translate(true);
   } else {
     // NaCl
     *url = isa_spec[kUrlKey].asString();
diff --git a/ppapi/native_client/src/trusted/plugin/json_manifest.h b/ppapi/native_client/src/trusted/plugin/json_manifest.h
index 177ad2a..6760345 100644
--- a/ppapi/native_client/src/trusted/plugin/json_manifest.h
+++ b/ppapi/native_client/src/trusted/plugin/json_manifest.h
@@ -30,12 +30,14 @@ class PnaclOptions;
 class JsonManifest : public Manifest {
  public:
   JsonManifest(const pp::URLUtil_Dev* url_util,
-           const nacl::string& manifest_base_url,
-           const nacl::string& sandbox_isa)
+               const nacl::string& manifest_base_url,
+               const nacl::string& sandbox_isa,
+               bool pnacl_debug)
       : url_util_(url_util),
         manifest_base_url_(manifest_base_url),
         sandbox_isa_(sandbox_isa),
-        dictionary_(Json::nullValue) { }
+        dictionary_(Json::nullValue),
+        pnacl_debug_(pnacl_debug) { }
   virtual ~JsonManifest() { }
 
   // Initialize the manifest object for use by later lookups.  The return
@@ -93,6 +95,7 @@ class JsonManifest : public Manifest {
   nacl::string sandbox_isa_;
 
   Json::Value dictionary_;
+  bool pnacl_debug_;  // Search for a pnacl-debug entry.
 };
 
 
diff --git a/ppapi/native_client/src/trusted/plugin/plugin.cc b/ppapi/native_client/src/trusted/plugin/plugin.cc
index 894337b..9d091d8 100644
--- a/ppapi/native_client/src/trusted/plugin/plugin.cc
+++ b/ppapi/native_client/src/trusted/plugin/plugin.cc
@@ -1137,10 +1137,12 @@ bool Plugin::SetManifestObject(const nacl::string& manifest_json,
   // Determine whether lookups should use portable (i.e., pnacl versions)
   // rather than platform-specific files.
   bool is_pnacl = (mime_type() == kPnaclMIMEType);
+  bool pnacl_debug = GetNaClInterface()->NaClDebugStubEnabled();
   nacl::scoped_ptr<JsonManifest> json_manifest(
       new JsonManifest(url_util_,
                        manifest_base_url(),
-                       (is_pnacl ? kPortableISA : GetSandboxISA())));
+                       (is_pnacl ? kPortableISA : GetSandboxISA()),
+                       pnacl_debug));
   if (!json_manifest->Init(manifest_json, error_info)) {
     return false;
   }
diff --git a/ppapi/native_client/src/trusted/plugin/pnacl_options.cc b/ppapi/native_client/src/trusted/plugin/pnacl_options.cc
index 69858831..b02e1f0 100644
--- a/ppapi/native_client/src/trusted/plugin/pnacl_options.cc
+++ b/ppapi/native_client/src/trusted/plugin/pnacl_options.cc
@@ -11,7 +11,11 @@
 
 namespace plugin {
 
-PnaclOptions::PnaclOptions() : translate_(false), opt_level_(2) { }
+PnaclOptions::PnaclOptions()
+    : translate_(false),
+      is_debug_(false),
+      opt_level_(2) {
+}
 
 PnaclOptions::~PnaclOptions() {
 }
@@ -31,6 +35,10 @@ std::vector<char> PnaclOptions::GetOptCommandline() const {
 
   nacl::stringstream ss;
   ss << "-O" << opt_level_;
+  // Debug info is only available in LLVM format pexes,
+  // not in PNaCl format pexes.
+  if (is_debug_)
+    ss << "\x00-bitcode-format=llvm";
   str = ss.str();
 
   std::copy(str.begin(), str.end(), std::back_inserter(result));
diff --git a/ppapi/native_client/src/trusted/plugin/pnacl_options.h b/ppapi/native_client/src/trusted/plugin/pnacl_options.h
index df2a9aa..2d30c3b 100644
--- a/ppapi/native_client/src/trusted/plugin/pnacl_options.h
+++ b/ppapi/native_client/src/trusted/plugin/pnacl_options.h
@@ -25,6 +25,9 @@ class PnaclOptions {
   bool translate() const { return translate_; }
   void set_translate(bool t) { translate_ = t; }
 
+  bool is_debug() const { return is_debug_; }
+  void set_debug(bool t) { is_debug_ = t; }
+
   int32_t opt_level() const { return opt_level_; }
   void set_opt_level(int32_t l);
 
@@ -33,6 +36,7 @@ class PnaclOptions {
   // Currently the default copy constructor is good enough, but
   // double-check that it is the case when more fields are added.
   bool translate_;
+  bool is_debug_;
   int32_t opt_level_;
 };
 
diff --git a/ppapi/native_client/src/untrusted/pnacl_irt_shim/pnacl_shim.c b/ppapi/native_client/src/untrusted/pnacl_irt_shim/pnacl_shim.c
index 13aea0a..3ebcfc0 100644
--- a/ppapi/native_client/src/untrusted/pnacl_irt_shim/pnacl_shim.c
+++ b/ppapi/native_client/src/untrusted/pnacl_irt_shim/pnacl_shim.c
@@ -3166,6 +3166,11 @@ static void Pnacl_M25_PPB_NaCl_Private_InstanceDestroyed(PP_Instance instance) {
   iface->InstanceDestroyed(instance);
 }
 
+static PP_Bool Pnacl_M25_PPB_NaCl_Private_NaClDebugStubEnabled(void) {
+  const struct PPB_NaCl_Private_1_0 *iface = Pnacl_WrapperInfo_PPB_NaCl_Private_1_0.real_iface;
+  return iface->NaClDebugStubEnabled();
+}
+
 /* End wrapper methods for PPB_NaCl_Private_1_0 */
 
 /* Begin wrapper methods for PPB_NetAddress_Private_0_1 */
@@ -5051,7 +5056,8 @@ static const struct PPB_NaCl_Private_1_0 Pnacl_Wrappers_PPB_NaCl_Private_1_0 = {
     .DispatchEvent = (void (*)(PP_Instance instance, PP_NaClEventType event_type, const char* resource_url, PP_Bool length_is_computable, uint64_t loaded_bytes, uint64_t total_bytes))&Pnacl_M25_PPB_NaCl_Private_DispatchEvent,
     .SetReadOnlyProperty = (void (*)(PP_Instance instance, struct PP_Var key, struct PP_Var value))&Pnacl_M25_PPB_NaCl_Private_SetReadOnlyProperty,
     .ReportLoadError = (void (*)(PP_Instance instance, PP_NaClError error, PP_Bool is_installed))&Pnacl_M25_PPB_NaCl_Private_ReportLoadError,
-    .InstanceDestroyed = (void (*)(PP_Instance instance))&Pnacl_M25_PPB_NaCl_Private_InstanceDestroyed
+    .InstanceDestroyed = (void (*)(PP_Instance instance))&Pnacl_M25_PPB_NaCl_Private_InstanceDestroyed,
+    .NaClDebugStubEnabled = (PP_Bool (*)(void))&Pnacl_M25_PPB_NaCl_Private_NaClDebugStubEnabled
 };
 
 static const struct PPB_NetAddress_Private_0_1 Pnacl_Wrappers_PPB_NetAddress_Private_0_1 = {