Treat external calls as having unavailable output if their input coul…

…dn't be captured, fixes mozilla#68
bvaughn · Feb 12, 2020 · 7e65986 · 7e65986
1 parent eb7ea89
commit 7e65986
Show file tree

Hide file tree

Showing 4 changed files with 35 additions and 34 deletions.
diff --git a/toolkit/recordreplay/ExternalCall.cpp b/toolkit/recordreplay/ExternalCall.cpp
@@ -109,13 +109,23 @@ bool OnExternalCall(size_t aCallId, CallArguments* aArguments, bool aDiverged) {
     ExternalCallContext cx(call, aArguments,
                            ExternalCallPhase::SaveInput);
     redirection.mExternalCall(cx);
+
     if (cx.mFailed) {
-      delete call;
-      if (!child::UnhandledDivergenceAllowed() && aDiverged) {
-        child::ReportFatalError("External call input failed: %s\n",
-                                redirection.mName);
+      // We weren't able to capture the call's inputs, so we can't compute its ID.
+      if (aDiverged) {
+        // Use dummy outputs for the call if possible.
+        Print("ExternalCall CantSaveInput: %s %s\n", redirection.mName, messageName);
+        if (cx.mCantSaveOutput) {
+          delete call;
+          return false;
+        }
+        call->mOutputUnavailable = true;
+        ExternalCallContext ncx(call, aArguments,
+                                ExternalCallPhase::RestoreOutput);
+        redirection.mExternalCall(ncx);
+        delete call;
       }
-      return false;
+      return true;
     }
   }
 

diff --git a/toolkit/recordreplay/ExternalCall.h b/toolkit/recordreplay/ExternalCall.h
@@ -146,11 +146,16 @@ struct ExternalCallContext {
   ExternalCallPhase mPhase;
 
   // During the SaveInput phase, whether capturing input data has failed.
-  // In such cases the call cannot be placed in the external call graph and,
-  // if the thread has diverged from the recording, an unhandled divergence
-  // will occur.
+  // In such cases the call cannot be placed in the external call graph.
   bool mFailed = false;
 
+  // If mFailed is set during the SaveInput phase and the thread has diverged
+  // from the recording, this describes how to handle the call's outputs.
+  // If this is false then dummy outputs will be used, as if the output was
+  // unavailable. If this is true then an unhandled recording divergence will
+  // occur.
+  bool mCantSaveOutput = false;
+
   // This can be set in the RestoreInput phase to avoid executing the call
   // in the external process.
   bool mSkipExecuting = false;
@@ -194,9 +199,12 @@ struct ExternalCallContext {
     }
   }
 
-  void MarkAsFailed() {
+  void MarkAsFailed(bool aCantSaveOutput = false) {
     MOZ_RELEASE_ASSERT(mPhase == ExternalCallPhase::SaveInput);
     mFailed = true;
+    if (aCantSaveOutput) {
+      mCantSaveOutput = true;
+    }
   }
 
   void WriteInputBytes(const void* aBuffer, size_t aSize) {

diff --git a/toolkit/recordreplay/ProcessRedirect.cpp b/toolkit/recordreplay/ProcessRedirect.cpp
@@ -88,22 +88,15 @@ __attribute__((used)) int RecordReplayInterceptCall(int aCallId,
     }
 
     // If the redirection has an external call hook, try to get its result
-    // from another process.
+    // from another process or from call information stored in the recording.
     if (redirection.mExternalCall) {
       if (OnExternalCall(aCallId, aArguments, /* aPopulateOutput = */ true)) {
         return 0;
       }
     }
 
-    if (!child::UnhandledDivergenceAllowed()) {
-      // EnsureNotDivergedFromRecording is going to force us to crash, so fail
-      // earlier with a more helpful error message.
-      child::ReportFatalError("Could not perform external call: %s\n",
-                              redirection.mName);
-    }
-
-    // Calling any redirection which performs the standard steps will cause
-    // debugger operations that have diverged from the recording to fail.
+    // Without an external call hook we can't handle this call, and have an
+    // unhandled recording divergence.
     EnsureNotDivergedFromRecording(Some(aCallId));
     Unreachable();
   }

diff --git a/toolkit/recordreplay/ProcessRedirectDarwin.cpp b/toolkit/recordreplay/ProcessRedirectDarwin.cpp
@@ -1559,25 +1559,12 @@ static void EX_objc_msgSend(ExternalCallContext& aCx) {
       if (info.mExternalCall && !aCx.mFailed) {
         info.mExternalCall(aCx);
       }
-      if (aCx.mFailed && HasDivergedFromRecording()) {
-        PrintSpew("Middleman message failure: %s\n", message);
-        if (!child::UnhandledDivergenceAllowed()) {
-          child::ReportFatalError("Middleman message failure: %s\n", message);
-        }
-      }
       return;
     }
   }
 
   if (aCx.mPhase == ExternalCallPhase::SaveInput) {
-    aCx.MarkAsFailed();
-    if (HasDivergedFromRecording()) {
-      PrintSpew("Middleman message failure: %s\n", message);
-      if (!child::UnhandledDivergenceAllowed()) {
-        child::ReportFatalError("Could not perform middleman message: %s\n",
-                                message);
-      }
-    }
+    aCx.MarkAsFailed(/* aCantSaveOutput */ true);
   }
 }
 
@@ -1851,7 +1838,10 @@ static void EX_CGBitmapContextCreateWithData(ExternalCallContext& aCx) {
     releaseInfo = nullptr;
   }
 
-  if (aCx.AccessOutput()) {
+  if (aCx.AccessOutput() ||
+      (aCx.mPhase == ExternalCallPhase::SaveInput &&
+       aCx.mFailed &&
+       !HasDivergedFromRecording())) {
     gContextData.emplaceBack(rval, data, height * bytesPerRow);
   }
 }