SL-10297: Modify LL_ERRS and other deliberate crashes to avoid a common stack frame

7 files changed, 49 insertions, 96 deletions

diff --git a/indra/llcommon/llerror.cpp b/indra/llcommon/llerror.cpp
index 335a0995fe..0ddce353f1 100644
--- a/indra/llcommon/llerror.cpp
+++ b/indra/llcommon/llerror.cpp
@@ -683,7 +683,6 @@ namespace
 		LLError::setDefaultLevel(LLError::LEVEL_INFO);
         LLError::setAlwaysFlush(true);
         LLError::setEnabledLogTypesMask(0xFFFFFFFF);
-		LLError::setFatalFunction(LLError::crashAndLoop);
 		LLError::setTimeFunction(LLError::utcTime);
 
 		// log_to_stderr is only false in the unit and integration tests to keep builds quieter
@@ -719,16 +718,16 @@ namespace LLError
 		commonInit(user_dir, app_dir, log_to_stderr);
 	}
 
-	void setFatalFunction(const FatalFunction& f)
+	void overrideCrashOnError(const FatalFunction& fatal_function)
 	{
 		SettingsConfigPtr s = Settings::getInstance()->getSettingsConfig();
-		s->mCrashFunction = f;
+		s->mCrashFunction = fatal_function;
 	}
 
-	FatalFunction getFatalFunction()
+	void restoreCrashOnError()
 	{
 		SettingsConfigPtr s = Settings::getInstance()->getSettingsConfig();
-		return s->mCrashFunction;
+		s->mCrashFunction = NULL;
 	}
 
 	std::string getFatalMessage()
@@ -1306,12 +1305,12 @@ namespace LLError
 		return ;
 	}
 
-	void Log::flush(std::ostringstream* out, const CallSite& site)
+	bool Log::flush(std::ostringstream* out, const CallSite& site)
 	{
 		LLMutexTrylock lock(&gLogMutex,5);
 		if (!lock.isLocked())
 		{
-			return;
+			return true;
 		}
 
 		// If we hit a logging request very late during shutdown processing,
@@ -1319,7 +1318,7 @@ namespace LLError
 		// DO NOT resurrect them.
 		if (Settings::wasDeleted() || Globals::wasDeleted())
 		{
-			return;
+			return true;
 		}
 
 		Globals* g = Globals::getInstance();
@@ -1353,7 +1352,7 @@ namespace LLError
 				} 
 				else
 				{
-					return;
+					return true;
 				}
 			}
 			else 
@@ -1370,11 +1369,14 @@ namespace LLError
 		if (site.mLevel == LEVEL_ERROR)
 		{
 			g->mFatalMessage = message;
-			if (s->mCrashFunction)
-			{
-				s->mCrashFunction(message);
-			}
+            if (s->mCrashFunction)
+            {
+                s->mCrashFunction(message);
+                return false;
+            }
 		}
+
+        return true;
 	}
 }
 
@@ -1437,29 +1439,6 @@ namespace LLError
 		return s->mShouldLogCallCounter;
 	}
 
-#if LL_WINDOWS
-		// VC80 was optimizing the error away.
-		#pragma optimize("", off)
-#endif
-	void crashAndLoop(const std::string& message)
-	{
-		// Now, we go kaboom!
-		int* make_me_crash = NULL;
-
-		*make_me_crash = 0;
-
-		while(true)
-		{
-			// Loop forever, in case the crash didn't work?
-		}
-		
-		// this is an attempt to let Coverity and other semantic scanners know that this function won't be returning ever.
-		exit(EXIT_FAILURE);
-	}
-#if LL_WINDOWS
-		#pragma optimize("", on)
-#endif
-
 	std::string utcTime()
 	{
 		time_t now = time(NULL);
diff --git a/indra/llcommon/llerror.h b/indra/llcommon/llerror.h
index 0a78229555..cbade88f61 100644
--- a/indra/llcommon/llerror.h
+++ b/indra/llcommon/llerror.h
@@ -199,8 +199,12 @@ namespace LLError
 	public:
 		static bool shouldLog(CallSite&);
 		static std::ostringstream* out();
+
 		static void flush(std::ostringstream* out, char* message);
-		static void flush(std::ostringstream*, const CallSite&);
+
+        // returns false iff there is a fatal crash override in effect
+		static bool flush(std::ostringstream*, const CallSite&);
+
 		static std::string demangle(const char* mangled);
 	};
 	
@@ -367,10 +371,20 @@ typedef LLError::NoClassInfo _LL_CLASS_TO_LOG;
 
 #define LL_NEWLINE '\n'
 
-#define LL_ENDL                               \
-			LLError::End();                   \
-			LLError::Log::flush(_out, _site); \
-		}                                     \
+// Use this only in LL_ERRS or in a place that LL_ERRS may not be used
+#define LLERROR_CRASH         \
+{                             \
+    int* make_me_crash = NULL;\
+    *make_me_crash = 0;       \
+    exit(*make_me_crash);     \
+}
+
+#define LL_ENDL                                          \
+			LLError::End();                              \
+            if (LLError::Log::flush(_out, _site)         \
+                && _site.mLevel == LLError::LEVEL_ERROR) \
+                LLERROR_CRASH                            \
+        }                                                \
 	} while(0)
 
 // NEW Macros for debugging, allow the passing of a string tag
diff --git a/indra/llcommon/llerrorcontrol.h b/indra/llcommon/llerrorcontrol.h
index 276d22fc36..7ca6ddb737 100644
--- a/indra/llcommon/llerrorcontrol.h
+++ b/indra/llcommon/llerrorcontrol.h
@@ -92,41 +92,13 @@ namespace LLError
 	/*
 		Control functions.
 	*/
-
 	typedef boost::function<void(const std::string&)> FatalFunction;
-	LL_COMMON_API void crashAndLoop(const std::string& message);
-		// Default fatal function: access null pointer and loops forever
-
-	LL_COMMON_API void setFatalFunction(const FatalFunction&);
-		// The fatal function will be called when an message of LEVEL_ERROR
-		// is logged.  Note: supressing a LEVEL_ERROR message from being logged
-		// (by, for example, setting a class level to LEVEL_NONE), will keep
-		// the that message from causing the fatal funciton to be invoked.
-
-	LL_COMMON_API FatalFunction getFatalFunction();
-		// Retrieve the previously-set FatalFunction
 
+    LL_COMMON_API void overrideCrashOnError(const FatalFunction&);
+    LL_COMMON_API void restoreCrashOnError();
+    
 	LL_COMMON_API std::string getFatalMessage();
-		// Retrieve the message last passed to FatalFunction, if any
-
-	/// temporarily override the FatalFunction for the duration of a
-	/// particular scope, e.g. for unit tests
-	class LL_COMMON_API OverrideFatalFunction
-	{
-	public:
-		OverrideFatalFunction(const FatalFunction& func):
-			mPrev(getFatalFunction())
-		{
-			setFatalFunction(func);
-		}
-		~OverrideFatalFunction()
-		{
-			setFatalFunction(mPrev);
-		}
-
-	private:
-		FatalFunction mPrev;
-	};
+		// Retrieve the message last passed to LL_ERRS, if any
 
 	typedef std::string (*TimeFunction)();
 	LL_COMMON_API std::string utcTime();
diff --git a/indra/llcommon/llleap.cpp b/indra/llcommon/llleap.cpp
index cf8f8cc6a5..f7bfa36bb5 100644
--- a/indra/llcommon/llleap.cpp
+++ b/indra/llcommon/llleap.cpp
@@ -59,7 +59,6 @@ public:
         // pump name -- so it should NOT need tweaking for uniqueness.
         mReplyPump(LLUUID::generateNewID().asString()),
         mExpect(0),
-        mPrevFatalFunction(LLError::getFatalFunction()),
         // Instantiate a distinct LLLeapListener for this plugin. (Every
         // plugin will want its own collection of managed listeners, etc.)
         // Pass it a callback to our connect() method, so it can send events
@@ -146,7 +145,7 @@ public:
             .listen("LLLeap", boost::bind(&LLLeapImpl::rstderr, this, _1));
 
         // For our lifespan, intercept any LL_ERRS so we can notify plugin
-        LLError::setFatalFunction(boost::bind(&LLLeapImpl::fatalFunction, this, _1));
+        LLError::overrideCrashOnError(boost::bind(&LLLeapImpl::fatalFunction, this, _1));
 
         // Send child a preliminary event reporting our own reply-pump name --
         // which would otherwise be pretty tricky to guess!
@@ -162,8 +161,8 @@ public:
     virtual ~LLLeapImpl()
     {
         LL_DEBUGS("LLLeap") << "destroying LLLeap(\"" << mDesc << "\")" << LL_ENDL;
-        // Restore original FatalFunction
-        LLError::setFatalFunction(mPrevFatalFunction);
+        // Restore original fatal crash behavior for LL_ERRS
+        LLError::restoreCrashOnError();
     }
 
     // Listener for failed launch attempt
@@ -397,8 +396,8 @@ public:
             mainloop.post(nop);
         }
 
-        // forward the call to the previous FatalFunction
-        mPrevFatalFunction(error);
+        // go ahead and do the crash that LLError would have done
+        LLERROR_CRASH
     }
 
 private:
@@ -421,7 +420,6 @@ private:
         mStdinConnection, mStdoutConnection, mStdoutDataConnection, mStderrConnection;
     boost::scoped_ptr<LLEventPump::Blocker> mBlocker;
     LLProcess::ReadPipe::size_type mExpect;
-    LLError::FatalFunction mPrevFatalFunction;
     boost::scoped_ptr<LLLeapListener> mListener;
 };
 
diff --git a/indra/llcommon/llsingleton.cpp b/indra/llcommon/llsingleton.cpp
index c45c144570..6f254ef670 100644
--- a/indra/llcommon/llsingleton.cpp
+++ b/indra/llcommon/llsingleton.cpp
@@ -461,15 +461,7 @@ void LLSingletonBase::logerrs(const char* p1, const char* p2, const char* p3, co
     // https://www.youtube.com/watch?v=OMG7paGJqhQ (emphasis on OMG)
     std::ostringstream out;
     out << p1 << p2 << p3 << p4;
-    auto crash = LLError::getFatalFunction();
-    if (crash)
-    {
-        crash(out.str());
-    }
-    else
-    {
-        LLError::crashAndLoop(out.str());
-    }
+    LLERROR_CRASH;
 }
 
 std::string LLSingletonBase::demangle(const char* mangled)
diff --git a/indra/llcommon/tests/llerror_test.cpp b/indra/llcommon/tests/llerror_test.cpp
index 8e1f4c14ac..2f8923d2de 100644
--- a/indra/llcommon/tests/llerror_test.cpp
+++ b/indra/llcommon/tests/llerror_test.cpp
@@ -120,7 +120,7 @@ namespace tut
 
 			mPriorErrorSettings = LLError::saveAndResetSettings();
 			LLError::setDefaultLevel(LLError::LEVEL_DEBUG);
-			LLError::setFatalFunction(fatalCall);
+			LLError::overrideCrashOnError(fatalCall);
 			LLError::addRecorder(mRecorder);
 		}
 
diff --git a/indra/llcommon/tests/wrapllerrs.h b/indra/llcommon/tests/wrapllerrs.h
index b07d5afbd8..b280271476 100644
--- a/indra/llcommon/tests/wrapllerrs.h
+++ b/indra/llcommon/tests/wrapllerrs.h
@@ -54,17 +54,15 @@ struct WrapLLErrs
         // Resetting Settings discards the default Recorder that writes to
         // stderr. Otherwise, expected llerrs (LL_ERRS) messages clutter the
         // console output of successful tests, potentially confusing things.
-        mPriorErrorSettings(LLError::saveAndResetSettings()),
-        // Save shutdown function called by LL_ERRS
-        mPriorFatal(LLError::getFatalFunction())
+        mPriorErrorSettings(LLError::saveAndResetSettings())
     {
         // Make LL_ERRS call our own operator() method
-        LLError::setFatalFunction(boost::bind(&WrapLLErrs::operator(), this, _1));
+        LLError::overrideCrashOnError(boost::bind(&WrapLLErrs::operator(), this, _1));
     }
 
     ~WrapLLErrs()
     {
-        LLError::setFatalFunction(mPriorFatal);
+        LLError::restoreCrashOnError();
         LLError::restoreSettings(mPriorErrorSettings);
     }
 
@@ -203,7 +201,7 @@ public:
         mOldSettings(LLError::saveAndResetSettings()),
 		mRecorder(new CaptureLogRecorder())
     {
-        LLError::setFatalFunction(wouldHaveCrashed);
+        LLError::overrideCrashOnError(wouldHaveCrashed);
         LLError::setDefaultLevel(level);
         LLError::addRecorder(mRecorder);
     }