Set file tokens for NaCl main nexe if available to enable validation caching.

ppapi/native_client/src/trusted/plugin/plugin.cc

 // 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.
 
 #ifdef _MSC_VER
 // Do not warn about use of std::copy with raw pointers.
 #pragma warning(disable : 4996)
 #endif
 
 #include "ppapi/native_client/src/trusted/plugin/plugin.h"
 
 #include <sys/stat.h>
 #include <sys/types.h>
 
 #include <algorithm>
 #include <string>
 #include <vector>
 
 #include "native_client/src/include/nacl_base.h"
 #include "native_client/src/include/nacl_macros.h"
 #include "native_client/src/include/nacl_scoped_ptr.h"
 #include "native_client/src/include/nacl_string.h"
 #include "native_client/src/include/portability.h"
 #include "native_client/src/include/portability_io.h"
 #include "native_client/src/include/portability_string.h"
 #include "native_client/src/shared/platform/nacl_check.h"
 #include "native_client/src/trusted/desc/nacl_desc_wrapper.h"
+#include "native_client/src/trusted/desc_cacheability/desc_cacheability.h"
 #include "native_client/src/trusted/nonnacl_util/sel_ldr_launcher.h"
 #include "native_client/src/trusted/service_runtime/nacl_error_code.h"
 
 #include "ppapi/c/pp_errors.h"
 #include "ppapi/c/ppb_var.h"
 #include "ppapi/c/private/ppb_nacl_private.h"
 #include "ppapi/cpp/dev/url_util_dev.h"
 #include "ppapi/cpp/module.h"
 
 #include "ppapi/native_client/src/trusted/plugin/file_utils.h"
@@ skipping 443 matching lines @@
       pp_error,
       info.get_desc(),
       nexe_downloader_.status_code(),
       nexe_bytes_read,
       nexe_downloader_.url().c_str(),
       download_time / 1000);
 
   if (nexe_bytes_read == -1)
     return;
 
+  // Hand desc ownership over to the wrapper. Keep the tokens in tmp_info
+  // before losing the tokens.
+  tmp_info = info.Release();
   nacl::scoped_ptr<nacl::DescWrapper>
-      wrapper(wrapper_factory()->MakeFileDesc(info.Release().desc, O_RDONLY));
+      wrapper(wrapper_factory()->MakeFileDesc(tmp_info.desc, O_RDONLY));
+  tmp_info.desc = -1;
+  if (tmp_info.file_token.lo != 0 && tmp_info.file_token.hi != 0) {

[teravest, 2014/04/30 20:12:54]

Should this be ||, not &&? Is it possible that lo or hi could be 0 for valid
token data?

[jvoung (off chromium), 2014/04/30 21:49:02]

Yeah I think having only one of them be 0 is still accepted.  I guess the
concern is if some code accidentally left one of the hi/lo at 0 all the time
(instead of just picking 0 by chance):

https://chromium.googlesource.com/native_client/src/native_client/+/refs/head...

And see NaClBrowser::PutFilePath which marches forth once of the tokens is
randomly chosen to be non-zero.

https://chromium.googlesource.com/chromium/chromium/+/trunk/components/nacl/b...

[teravest, 2014/05/02 15:07:25]

I'm confused. If you want to use && here, you should probably change
NaClBrowser::PutFilePath to match.

[jvoung (off chromium), 2014/05/02 15:33:23]

Oops sorry, messed up the negation logic. Changed to || to match.

[Nick Bray (chromium), 2014/05/02 18:18:19]

Justin is correct, although I'd go further and suggest this for clarity:
!(file_token->lo == 0 && file_token->hi == 0) .

See:
https://code.google.com/p/chromium/codesearch#chromium/src/native_client/src/...

Maybe this should even be factored into its own function, given a name, and
shared?  Unclear if that would be a win.

[jvoung (off chromium), 2014/05/02 19:26:15]

Yeah, I could put something in nacl_file_info.h, so that what's considered valid
is not just a comment in that file but a function that could be reused.

Then I can clean this up after a NaCl -> Chrome roll.

This particular check could also be folded into the desc_wrapper factory method
proposed in the other CL if that makes sense.

+    if (!NaClDescSetFileToken(wrapper->desc(), &tmp_info.file_token)) {

[teravest, 2014/04/30 20:12:54]

It's a shame that there's not a call to MakeFileDesc() that can take a
NaClFileInfo instead.

[jvoung (off chromium), 2014/04/30 21:49:02]

Okay, maybe we can add that:
https://codereview.chromium.org/261683002/

Thoughts, Nick?

[jvoung (off chromium), 2014/05/20 16:12:42]

Changed to use the new ctor function that passes both the desc and file token.

+      NaClLog(4, "NexeFileDidOpen: NaClDescSetFileToken failed\n");
+    }
+  } else {
+    NaClLog(4, "NexeFileDidOpen: No file tokens for NaCl main nexe\n");
+  }
   NaClLog(4, "NexeFileDidOpen: invoking LoadNaClModule\n");
   LoadNaClModule(
       wrapper.release(),
       uses_nonsfi_mode_,
       true, /* enable_dyncode_syscalls */
       true, /* enable_exception_handling */
       false, /* enable_crash_throttling */
       callback_factory_.NewCallback(&Plugin::NexeFileDidOpenContinuation),
       callback_factory_.NewCallback(&Plugin::NexeDidCrash));
 }
@@ skipping 476 matching lines @@
 
 void Plugin::SetExitStatusOnMainThread(int32_t pp_error,
                                        int exit_status) {
   DCHECK(pp::Module::Get()->core()->IsMainThread());
   DCHECK(nacl_interface_);
   nacl_interface_->SetExitStatus(pp_instance(), exit_status);
 }
 
 
 }  // namespace plugin