From c9fc4349b7d4ab1f5a7bfc0125014a96a07e51a3 Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Tue, 11 May 2021 21:42:14 -0400 Subject: SL-10297: Move LL_ERRS crash location into the LL_ERRS macro itself. Introduce Oz's LLERROR_CRASH macro analogous to the old LLError::crashAndLoop() function. Change LL_ENDL macro so that, after calling flush(), if the CallSite is for LEVEL_ERROR, we invoke LLERROR_CRASH right there. Change the meaning of LLError::FatalFunction. It used to be responsible for the actual crash (hence crashAndLoop()). Now, instead, its role is to disrupt control flow in some other way if you DON'T want to crash: throw an exception, or call exit() or some such. Any FatalFunction that returns normally will fall into the new crash in LL_ENDL. Accordingly, the new default FatalFunction is a no-op lambda. This eliminates the need to test for empty (not set) FatalFunction in Log::flush(). Remove LLError::crashAndLoop() because the official LL_ERRS crash is now in LL_ENDL. One of the two common use cases for setFatalFunction() used to be to intercept control in the last moments before crashing -- not to crash or to avoid crashing, but to capture the LL_ERRS message in some way. Especially when that's temporary, though (e.g. LLLeap), saving and restoring the previous FatalFunction only works when the lifespans of the relevant objects are strictly LIFO. Either way, that's a misuse of FatalFunction. Fortunately the Recorder mechanism exactly addresses that case. Introduce a GenericRecorder template subclass, with LLError::addGenericRecorder(callable) that accepts a callable with suitable (level, message) signature, instantiates a GenericRecorder, adds it to the logging machinery and returns the RecorderPtr for possible later use with removeRecorder(). Change llappviewer.cpp's errorCallback() to an addGenericRecorder() callable. Its role was simply to update gDebugInfo["FatalMessage"] with the LL_ERRS message, then call writeDebugInfo(), before calling crashAndLoop() to finish crashing. Remove the crashAndLoop() call, retaining the gDebugInfo logic. Pass errorCallback() to LLError::addGenericRecorder() instead of setFatalFunction(). Oddly, errorCallback()'s crashAndLoop() call was conditional on a compile-time SHADER_CRASH_NONFATAL symbol. The new mechanism provides no way to support SHADER_CRASH_NONFATAL -- it is a Bad Idea to return normally from any LL_ERRS invocation! Rename LLLeapImpl::fatalFunction() to onError(). Instead of passing it to LLError::setFatalFunction(), pass it to addGenericRecorder(). Capture the returned RecorderPtr in mRecorder, replacing mPrevFatalFunction. Then ~LLLeapImpl() calls removeRecorder(mRecorder) instead of restoring mPrevFatalFunction (which, as noted above, was order-sensitive). Of course, every enabled Recorder is called with every log message. onError() and errorCallback() must specifically test for calls with LEVEL_ERROR. LLSingletonBase::logerrs() used to call LLError::getFatalFunction(), check the return and call it if non-empty, else call LLError::crashAndLoop(). Replace all that with LLERROR_CRASH. Remove from llappviewer.cpp the watchdog_llerrs_callback() and watchdog_killer_callback() functions. watchdog_killer_callback(), passed to Watchdog::init(), used to setFatalFunction(watchdog_llerrs_callback) and then invoke LL_ERRS() -- which seems a bit roundabout. watchdog_llerrs_callback(), in turn, replicated much of the logic in the primary errorCallback() function before replicating the crash from llwatchdog.cpp's default_killer_callback(). Instead, pass LLWatchdog::init() a lambda that invokes the LL_ERRS() message formerly found in watchdog_killer_callback(). It no longer needs to override FatalFunction with watchdog_llerrs_callback() because errorCallback() will still be called as a Recorder, obviating watchdog_llerrs_callback()'s first half; and LL_ENDL will handle the crash, obviating the second half. Remove from llappviewer.cpp the static fast_exit() function, which was simply an alias for _exit() acceptable to boost::bind(). Use a lambda directly calling _exit() instead of using boost::bind() at all. In the CaptureLog class in llcommon/tests/wrapllerrs.h, instead of statically referencing the wouldHaveCrashed() function from test.cpp, simply save and restore the current FatalFunction across the LLError::saveAndResetSettings() call. llerror_test.cpp calls setFatalFunction(fatalCall), where fatalCall() was a function that simply set a fatalWasCalled bool rather than actually crashing in any way. Of course, that implementation would now lead to crashing the test program. Make fatalCall() throw a new FatalWasCalled exception. Introduce a CATCH(LL_ERRS("tag"), "message") macro that expands to: LL_ERRS("tag") << "message" << LL_ENDL; within a try/catch block that catches FatalWasCalled and sets the same bool. Change all existing LL_ERRS() in llerror_test.cpp to corresponding CATCH() calls. In fact there's also an LL_DEBUGS(bad tag) invocation that exercises an LL_ERRS internal to llerror.cpp; wrap that too. --- indra/llcommon/llerror.h | 22 +++++++++++++++++----- 1 file changed, 17 insertions(+), 5 deletions(-) (limited to 'indra/llcommon/llerror.h') diff --git a/indra/llcommon/llerror.h b/indra/llcommon/llerror.h index ffaa464d77..f8c0d03aea 100644 --- a/indra/llcommon/llerror.h +++ b/indra/llcommon/llerror.h @@ -382,11 +382,23 @@ typedef LLError::NoClassInfo _LL_CLASS_TO_LOG; #define LL_NEWLINE '\n' -#define LL_ENDL \ - LLError::End(); \ - LLError::Log::flush(_out, _site); \ - } \ - } while(0) +// 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(); \ + LLError::Log::flush(_out, _site); \ + if (_site.mLevel == LLError::LEVEL_ERROR) \ + { \ + LLERROR_CRASH \ + } \ + } \ + } while(0) // NEW Macros for debugging, allow the passing of a string tag -- cgit v1.2.3 From 5b96ee0e10923a00ddb3836d4dc3c5f912ca4330 Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Wed, 12 May 2021 12:02:57 -0400 Subject: SL-10297: Eliminate llerror.cpp's Globals::messageStream and bool. Instead of a single std::ostringstream instance shared by all callers, even those on different threads, make each of the relevant lllog_test_() and llcallstacks macros instantiate independent (stack) std::ostringstream objects. lllog_test_() is called by LL_DEBUGS(), LLINFOS(), LL_WARNS(), LL_ERRS(), LL_VLOGS() et al. Eliminate LLError::Log::out(), whose sole function was to arbitrate use of that shared std::ostringstream. Amusingly, if the lock couldn't be locked or if messageStreamInUse was set, out() would allocate a new (heap!) std::ostringstream anyway, which would then have to be freed by flush(). Make both LLError::Log::flush() overloads accept const std::ostringstream&. Make LL_ENDL pass the local _out instance. This eliminates the need to check whether the passed std::ostringstream* references the shared instance and (if so) reset it or (if not) delete it. Make LLError::LLCallStacks::insert() accept the local _out instance as non- const std::ostream&, rather than acquiring and returning std::ostringstream*. Make end() accept the local instance as const std::ostringstream&. --- indra/llcommon/llerror.h | 30 +++++++++++++++--------------- 1 file changed, 15 insertions(+), 15 deletions(-) (limited to 'indra/llcommon/llerror.h') diff --git a/indra/llcommon/llerror.h b/indra/llcommon/llerror.h index f8c0d03aea..51423350e6 100644 --- a/indra/llcommon/llerror.h +++ b/indra/llcommon/llerror.h @@ -198,9 +198,8 @@ 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&); + static void flush(const std::ostringstream& out, char* message); + static void flush(const std::ostringstream&, const CallSite&); static std::string demangle(const char* mangled); /// classname() template @@ -289,10 +288,10 @@ namespace LLError public: static void push(const char* function, const int line) ; - static std::ostringstream* insert(const char* function, const int line) ; + static void insert(std::ostream& out, const char* function, const int line) ; static void print() ; static void clear() ; - static void end(std::ostringstream* _out) ; + static void end(const std::ostringstream& out) ; static void cleanup(); }; @@ -306,10 +305,11 @@ namespace LLError //this is cheaper than llcallstacks if no need to output other variables to call stacks. #define LL_PUSH_CALLSTACKS() LLError::LLCallStacks::push(__FUNCTION__, __LINE__) -#define llcallstacks \ - { \ - std::ostringstream* _out = LLError::LLCallStacks::insert(__FUNCTION__, __LINE__) ; \ - (*_out) +#define llcallstacks \ + { \ + std::ostringstream _out; \ + LLError::LLCallStacks::insert(_out, __FUNCTION__, __LINE__) ; \ + _out #define llcallstacksendl \ LLError::End(); \ @@ -355,11 +355,11 @@ typedef LLError::NoClassInfo _LL_CLASS_TO_LOG; static LLError::CallSite _site(lllog_site_args_(level, once, tags)); \ lllog_test_() -#define lllog_test_() \ - if (LL_UNLIKELY(_site.shouldLog())) \ - { \ - std::ostringstream* _out = LLError::Log::out(); \ - (*_out) +#define lllog_test_() \ + if (LL_UNLIKELY(_site.shouldLog())) \ + { \ + std::ostringstream _out; \ + _out #define lllog_site_args_(level, once, tags) \ level, __FILE__, __LINE__, typeid(_LL_CLASS_TO_LOG), \ @@ -378,7 +378,7 @@ typedef LLError::NoClassInfo _LL_CLASS_TO_LOG; // LL_CONT << " for " << t << " seconds" << LL_ENDL; // //Such computation is done iff the message will be logged. -#define LL_CONT (*_out) +#define LL_CONT _out #define LL_NEWLINE '\n' -- cgit v1.2.3 From 91c20363eee4e1e02435e0ee74867cdb3f6c7136 Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Wed, 12 May 2021 13:37:24 -0400 Subject: SL-10297: Get rid of LLError::LLCallStacks::allocateStackBuffer(). Also freeStackBuffer() and all the funky classic-C string management of a big flat buffer divided into exactly 512 128-byte strings. Define StringVector as a std::vector, and use that instead. Retain the behavior of clearing the vector if it exceeds 512 entries. This eliminates the LLError::Log::flush(const std::ostringstream&, char*) overload as well, with its baffling mix of std::string and classic-C (e.g. strlen(out.str().c_str()). If we absolutely MUST use a big memory pool for performance reasons, let's use StringVector with allocators. --- indra/llcommon/llerror.h | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) (limited to 'indra/llcommon/llerror.h') diff --git a/indra/llcommon/llerror.h b/indra/llcommon/llerror.h index 51423350e6..d439136ca8 100644 --- a/indra/llcommon/llerror.h +++ b/indra/llcommon/llerror.h @@ -29,7 +29,9 @@ #define LL_LLERROR_H #include +#include #include +#include #include "stdtypes.h" @@ -198,7 +200,6 @@ namespace LLError { public: static bool shouldLog(CallSite&); - static void flush(const std::ostringstream& out, char* message); static void flush(const std::ostringstream&, const CallSite&); static std::string demangle(const char* mangled); /// classname() @@ -280,11 +281,8 @@ namespace LLError class LL_COMMON_API LLCallStacks { private: - static char** sBuffer ; - static S32 sIndex ; - - static void allocateStackBuffer(); - static void freeStackBuffer(); + typedef std::vector StringVector; + static StringVector sBuffer ; public: static void push(const char* function, const int line) ; -- cgit v1.2.3