Bug 1510481 - Fix handling of messages sent to NSAutoreleasePools after diverging from the recording, r=lsmyth.
authorBrian Hackett <bhackett1024@gmail.com>
Tue, 27 Nov 2018 14:17:07 -1000
changeset 505108 e17e9b14a4349edc70178cf8d9fe49ea537ef6d3
parent 505107 9a1c57557089eec2d1256912c983ae8d63440fbb
child 505109 1a669c064a396860699bf8a194b70198a095858f
push id10290
push userffxbld-merge
push dateMon, 03 Dec 2018 16:23:23 +0000
treeherdermozilla-beta@700bed2445e6 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerslsmyth
bugs1510481
milestone65.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1510481 - Fix handling of messages sent to NSAutoreleasePools after diverging from the recording, r=lsmyth.
toolkit/recordreplay/MiddlemanCall.cpp
toolkit/recordreplay/MiddlemanCall.h
toolkit/recordreplay/ProcessRedirectDarwin.cpp
--- a/toolkit/recordreplay/MiddlemanCall.cpp
+++ b/toolkit/recordreplay/MiddlemanCall.cpp
@@ -167,22 +167,26 @@ ProcessMiddlemanCall(const char* aInputD
     call->DecodeInput(inputStream);
 
     const Redirection& redirection = GetRedirection(call->mCallId);
     MOZ_RELEASE_ASSERT(redirection.mMiddlemanCall);
 
     CallArguments arguments;
     call->mArguments.CopyTo(&arguments);
 
+    bool skipCall;
     {
       MiddlemanCallContext cx(call, &arguments, MiddlemanCallPhase::MiddlemanInput);
       redirection.mMiddlemanCall(cx);
+      skipCall = cx.mSkipCallInMiddleman;
     }
 
-    RecordReplayInvokeCall(redirection.mBaseFunction, &arguments);
+    if (!skipCall) {
+      RecordReplayInvokeCall(redirection.mBaseFunction, &arguments);
+    }
 
     {
       MiddlemanCallContext cx(call, &arguments, MiddlemanCallPhase::MiddlemanOutput);
       redirection.mMiddlemanCall(cx);
     }
 
     call->mArguments.CopyFrom(&arguments);
     call->EncodeOutput(outputStream);
--- a/toolkit/recordreplay/MiddlemanCall.h
+++ b/toolkit/recordreplay/MiddlemanCall.h
@@ -167,16 +167,20 @@ struct MiddlemanCallContext
   MiddlemanCallPhase mPhase;
 
   // During the ReplayPreface or ReplayInput phases, whether capturing input
   // data has failed. In such cases the call cannot be sent to the middleman
   // and, if the thread has diverged from the recording, an unhandled
   // divergence and associated rewind will occur.
   bool mFailed;
 
+  // This can be set in the MiddlemanInput phase to avoid performing the call
+  // in the middleman process.
+  bool mSkipCallInMiddleman;
+
   // During the ReplayInput phase, this can be used to fill in any middleman
   // calls whose output the current one depends on.
   InfallibleVector<MiddlemanCall*>* mDependentCalls;
 
   // Streams of data that can be accessed during the various phases. Streams
   // need to be read or written from at the same points in the phases which use
   // them, so that callbacks operating on these streams can be composed without
   // issues.
@@ -193,17 +197,18 @@ struct MiddlemanCallContext
 
   // During the ReplayOutput phase, this is set if the call was made sometime
   // in the past and pointers referred to in the arguments may no longer be
   // valid.
   bool mReplayOutputIsOld;
 
   MiddlemanCallContext(MiddlemanCall* aCall, CallArguments* aArguments, MiddlemanCallPhase aPhase)
     : mCall(aCall), mArguments(aArguments), mPhase(aPhase),
-      mFailed(false), mDependentCalls(nullptr), mReplayOutputIsOld(false)
+      mFailed(false), mSkipCallInMiddleman(false),
+      mDependentCalls(nullptr), mReplayOutputIsOld(false)
   {
     switch (mPhase) {
     case MiddlemanCallPhase::ReplayPreface:
       mPrefaceStream.emplace(&mCall->mPreface);
       break;
     case MiddlemanCallPhase::ReplayInput:
       mPrefaceStream.emplace(mCall->mPreface.begin(), mCall->mPreface.length());
       mInputStream.emplace(&mCall->mInput);
@@ -424,16 +429,25 @@ static inline void
 MM_StackArgumentData(MiddlemanCallContext& aCx)
 {
   if (aCx.AccessPreface()) {
     auto stack = aCx.mArguments->StackAddress<0>();
     aCx.ReadOrWritePrefaceBytes(stack, ByteSize);
   }
 }
 
+// Avoid calling a function in the middleman process.
+static inline void
+MM_SkipInMiddleman(MiddlemanCallContext& aCx)
+{
+  if (aCx.mPhase == MiddlemanCallPhase::MiddlemanInput) {
+    aCx.mSkipCallInMiddleman = true;
+  }
+}
+
 static inline void
 MM_NoOp(MiddlemanCallContext& aCx)
 {
 }
 
 template <MiddlemanCallFn Fn0,
           MiddlemanCallFn Fn1,
           MiddlemanCallFn Fn2 = MM_NoOp,
--- a/toolkit/recordreplay/ProcessRedirectDarwin.cpp
+++ b/toolkit/recordreplay/ProcessRedirectDarwin.cpp
@@ -194,16 +194,17 @@ MM_ObjCInput(MiddlemanCallContext& aCx, 
   MOZ_RELEASE_ASSERT(aCx.AccessInput());
 
   if (aCx.mPhase == MiddlemanCallPhase::ReplayInput) {
     // Try to determine where this object came from.
 
     // List of the Objective C classes which messages might be sent to directly.
     static const char* gStaticClasses[] = {
       // Standard classes.
+      "NSAutoreleasePool",
       "NSBezierPath",
       "NSButtonCell",
       "NSColor",
       "NSComboBoxCell",
       "NSDictionary",
       "NSGraphicsContext",
       "NSFont",
       "NSFontManager",
@@ -1126,41 +1127,33 @@ RR_objc_msgSend(Stream& aEvents, CallArg
     aEvents.RecordOrReplayValue(&len);
     if (IsReplaying()) {
       rval = NewLeakyArray<char>(len + 1);
     }
     aEvents.RecordOrReplayBytes(rval, len + 1);
   }
 }
 
-static PreambleResult
-MiddlemanPreamble_objc_msgSend(CallArguments* aArguments)
+static void
+MM_Alloc(MiddlemanCallContext& aCx)
 {
-  auto obj = aArguments->Arg<0, id>();
-  auto message = aArguments->Arg<1, const char*>();
-
-  // Fake object value which allows null checks in the caller to pass.
-  static const size_t FakeId = 1;
-
-  // Ignore uses of NSAutoreleasePool after diverging from the recording.
-  // These are not performed in the middleman because the middleman has its
-  // own autorelease pool, and because the middleman can process calls from
-  // multiple threads which will cause these messages to behave differently.
-  // release messages are also ignored, as for CFRelease.
-  if ((!strcmp(message, "alloc") && obj == (id) objc_lookUpClass("NSAutoreleasePool")) ||
-      (!strcmp(message, "init") && obj == (id) FakeId) ||
-      !strcmp(message, "drain") ||
-      !strcmp(message, "release")) {
-    // Fake a return value in case the caller null checks it.
-    aArguments->Rval<size_t>() = 1;
-    return PreambleResult::Veto;
+  if (aCx.mPhase == MiddlemanCallPhase::MiddlemanInput) {
+    // Refuse to allocate NSAutoreleasePools in the middleman: the order in
+    // which middleman calls happen does not guarantee the pools will be
+    // created and released in LIFO order, as the pools require. Instead,
+    // allocate an NSString instead so we have something to release later.
+    // Messages sent to NSAutoreleasePools will all be skipped in the
+    // middleman, so no one should notice this subterfuge.
+    auto& obj = aCx.mArguments->Arg<0, id>();
+    if (obj == (id) objc_lookUpClass("NSAutoreleasePool")) {
+      obj = (id) objc_lookUpClass("NSString");
+    }
   }
 
-  // Other messages will be handled by MM_objc_msgSend.
-  return PreambleResult::Redirect;
+  MM_CreateCFTypeRval(aCx);
 }
 
 static void
 MM_PerformSelector(MiddlemanCallContext& aCx)
 {
   MM_CString<2>(aCx);
   MM_CFTypeArg<3>(aCx);
 
@@ -1250,25 +1243,29 @@ struct ObjCMessageInfo
   bool mUpdatesObject;
 };
 
 // All Objective C messages that can be called in the middleman, and hooks for
 // capturing any inputs and outputs other than the object, message, and scalar
 // arguments / return values.
 static ObjCMessageInfo gObjCMiddlemanCallMessages[] = {
   // Generic
-  { "alloc", MM_CreateCFTypeRval },
+  { "alloc", MM_Alloc },
   { "init", MM_AutoreleaseCFTypeRval },
   { "performSelector:withObject:", MM_PerformSelector },
+  { "release", MM_SkipInMiddleman },
   { "respondsToSelector:", MM_CString<2> },
 
   // NSAppearance
   { "_drawInRect:context:options:",
     MM_Compose<MM_StackArgumentData<sizeof(CGRect)>, MM_CFTypeArg<2>, MM_CFTypeArg<3>> },
 
+  // NSAutoreleasePool
+  { "drain", MM_SkipInMiddleman },
+
   // NSArray
   { "count" },
   { "objectAtIndex:", MM_AutoreleaseCFTypeRval },
 
   // NSBezierPath
   { "addClip", MM_NoOp, true },
   { "bezierPathWithRoundedRect:xRadius:yRadius:", MM_AutoreleaseCFTypeRval },
 
@@ -2083,17 +2080,17 @@ static SystemRedirection gSystemRedirect
   /////////////////////////////////////////////////////////////////////////////
 
   { "class_getClassMethod", RR_ScalarRval },
   { "class_getInstanceMethod", RR_ScalarRval },
   { "method_exchangeImplementations" },
   { "objc_autoreleasePoolPop" },
   { "objc_autoreleasePoolPush", RR_ScalarRval },
   { "objc_msgSend",
-    RR_objc_msgSend, Preamble_objc_msgSend, MM_objc_msgSend, MiddlemanPreamble_objc_msgSend },
+    RR_objc_msgSend, Preamble_objc_msgSend, MM_objc_msgSend },
 
   /////////////////////////////////////////////////////////////////////////////
   // Cocoa and CoreFoundation library functions
   /////////////////////////////////////////////////////////////////////////////
 
   { "AcquireFirstMatchingEventInQueue", RR_ScalarRval },
   { "CFArrayAppendValue" },
   { "CFArrayCreate", RR_ScalarRval, nullptr, MM_CFArrayCreate },