Exit cleanly for the SRPC "log" requests instead of generating crash reports.

src/trusted/service_runtime/nacl_secure_service.c

 /*
  * Copyright (c) 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/trusted/service_runtime/nacl_secure_service.h"
 
 #include <string.h>
 
@@ skipping 239 matching lines @@
 static void NaClSecureServiceLogRpc(
     struct NaClSrpcRpc      *rpc,
     struct NaClSrpcArg      **in_args,
     struct NaClSrpcArg      **out_args,
     struct NaClSrpcClosure  *done_cls) {
   int   severity = in_args[0]->u.ival;
   char  *msg = in_args[1]->arrays.str;
   UNREFERENCED_PARAMETER(out_args);
 
   NaClLog(5, "NaClSecureChannelLogRpc\n");
+  if (LOG_FATAL == severity) {

[Mark Seaborn, 2015/03/11 01:26:14]

Nit: Use the NaCl style for multiline comments, with "/*" and "*/" on separate
lines, like this:
/*
 * Blah...
 * blah.
 */

[jvoung (off chromium), 2015/03/11 18:02:06]

Done.

+    /* SRPC is deprecated and the only remaining user of this LogRpc
+     * corresponds to fairly benign errors.  Also, at this point the
+     * backtraces will not be terribly interesting.  Avoid spamming the
+     * crash report counter for such errors and exit cleanly.
+     */
+    NaClLog(LOG_ERROR, "%s\n", msg);
+    NaClExit(1);

[Mark Seaborn, 2015/03/11 01:26:14]

You wrote:
Does this change mean we lose some useful error reporting in the Javascript
console about why NaCl process startup failed?

Did you test an error case manually?  Can you add a TEST= line, please?

If this would lose some error reporting, should we call NaClReportLogMessages()
here (without its NaClAbort() call)?

[jvoung (off chromium), 2015/03/11 18:02:06]

I'm not sure if they are useful. For bad elf header and validation failures, we
mostly repeat in the NaClLog what shows up in the lastError string.

Just in case, I went ahead and added a getter for the set hook
(NaClReportLogMessages()), and called that hook here. I changed the hook to not
abort by itself, since the usual caller of the hook will abort anyway (and we
don't want to abort here).

Added some detail to the TEST= line

+  }
   NaClLog(severity, "%s\n", msg);
   NaClLog(5, "NaClSecureChannelLogRpc\n");
   rpc->result = NACL_SRPC_RESULT_OK;
   (*done_cls->Run)(done_cls);
 }
 
 static void NaClSecureServiceShutdownRpc(
     struct NaClSrpcRpc      *rpc,
     struct NaClSrpcArg      **in_args,
     struct NaClSrpcArg      **out_args,
@@ skipping 18 matching lines @@
 
 struct NaClSimpleServiceVtbl const kNaClSecureServiceVtbl = {
   {
     NaClSecureServiceDtor,
   },
   NaClSecureServiceConnectionFactory,
   NaClSecureServiceAcceptConnection,
   NaClSimpleServiceAcceptAndSpawnHandler,
   NaClSecureServiceRpcHandler,
 };