From f648758c2a3da2dd03c8f57e98598c085b2030a6 Mon Sep 17 00:00:00 2001 From: Oz Linden Date: Wed, 16 Jan 2019 11:05:55 -0500 Subject: SL-10297: Modify LL_ERRS and other deliberate crashes to avoid a common stack frame --- indra/test/test.cpp | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) (limited to 'indra/test') diff --git a/indra/test/test.cpp b/indra/test/test.cpp index b14c2eb255..125de72b79 100644 --- a/indra/test/test.cpp +++ b/indra/test/test.cpp @@ -146,7 +146,7 @@ public: mOldSettings(LLError::saveAndResetSettings()), mRecorder(new RecordToTempFile(pool)) { - LLError::setFatalFunction(wouldHaveCrashed); + LLError::overrideCrashOnError(wouldHaveCrashed); LLError::setDefaultLevel(level); LLError::addRecorder(mRecorder); } @@ -508,7 +508,7 @@ void stream_groups(std::ostream& s, const char* app) void wouldHaveCrashed(const std::string& message) { - tut::fail("llerrs message: " + message); + tut::fail("fatal error message: " + message); } static LLTrace::ThreadRecorder* sMasterThreadRecorder = NULL; @@ -532,7 +532,7 @@ int main(int argc, char **argv) LLError::initForApplication(".", ".", false /* do not log to stderr */); LLError::setDefaultLevel(LLError::LEVEL_DEBUG); } - LLError::setFatalFunction(wouldHaveCrashed); + LLError::overrideCrashOnError(wouldHaveCrashed); std::string test_app_name(argv[0]); std::string test_log = test_app_name + ".log"; LLFile::remove(test_log); -- cgit v1.3 From e409c0492f1b1ce63606c0b693c92cdb36dcc28b Mon Sep 17 00:00:00 2001 From: Oz Linden Date: Sat, 2 Mar 2019 11:58:11 -0500 Subject: convert to an explicit USE_BUGSPLAT switch in cmake, revise LL_ERRS approach --- build.sh | 5 ---- indra/CMakeLists.txt | 10 +++++++ indra/cmake/Copy3rdPartyLibs.cmake | 4 +-- indra/cmake/LLAddBuildTest.cmake | 13 ++++++++- indra/cmake/Variables.cmake | 1 - indra/cmake/bugsplat.cmake | 53 +++++++++++++++++++--------------- indra/llcommon/CMakeLists.txt | 9 +++--- indra/llcommon/llerror.cpp | 21 +++++++++----- indra/llcommon/llerror.h | 20 +++++++------ indra/llcommon/llerrorcontrol.h | 14 +++++++-- indra/llcommon/llleap.cpp | 30 +------------------ indra/llcommon/tests/wrapllerrs.h | 5 ++-- indra/llcorehttp/CMakeLists.txt | 1 + indra/llimage/CMakeLists.txt | 1 + indra/llmath/CMakeLists.txt | 1 + indra/newview/CMakeLists.txt | 36 +++++++++++------------ indra/newview/llappviewer.cpp | 59 +++++++++++++++++++++++++++++++++----- indra/newview/llwatchdog.cpp | 8 +++--- indra/test/CMakeLists.txt | 6 +++- indra/test/test.cpp | 10 +++---- 20 files changed, 185 insertions(+), 122 deletions(-) (limited to 'indra/test') diff --git a/build.sh b/build.sh index 9f070f2d10..aa6ba475a3 100755 --- a/build.sh +++ b/build.sh @@ -129,11 +129,6 @@ pre_build() then # show that we're doing this, just not the contents echo source "$bugsplat_sh" source "$bugsplat_sh" - # important: we test this and use its value in [grand-]child processes - if [ -n "${BUGSPLAT_DB:-}" ] - then echo export BUGSPLAT_DB - export BUGSPLAT_DB - fi fi set -x diff --git a/indra/CMakeLists.txt b/indra/CMakeLists.txt index 62a8f3f003..cf6029c4ae 100644 --- a/indra/CMakeLists.txt +++ b/indra/CMakeLists.txt @@ -13,6 +13,7 @@ project(${ROOT_PROJECT_NAME}) set(CMAKE_MODULE_PATH "${CMAKE_SOURCE_DIR}/cmake") include(Variables) +include(bugsplat) include(BuildVersion) if (NOT CMAKE_BUILD_TYPE) @@ -89,6 +90,15 @@ set_property( PROPERTY VS_STARTUP_PROJECT secondlife-bin ) +if (USE_BUGSPLAT) + if (BUGSPLAT_DB) + message(STATUS "Building with BugSplat; database '${BUGSPLAT_DB}'") + else (BUGSPLAT_DB) + message(WARNING "Building with BugSplat, but no database name set (BUGSPLAT_DB)") + endif (BUGSPLAT_DB) +else (USE_BUGSPLAT) + message(STATUS "Not building with BugSplat") +endif (USE_BUGSPLAT) if (LL_TESTS) # Define after the custom targets are created so # individual apps can add themselves as dependencies diff --git a/indra/cmake/Copy3rdPartyLibs.cmake b/indra/cmake/Copy3rdPartyLibs.cmake index c73a1fdb47..ebb6379aed 100644 --- a/indra/cmake/Copy3rdPartyLibs.cmake +++ b/indra/cmake/Copy3rdPartyLibs.cmake @@ -50,7 +50,7 @@ if(WINDOWS) # Filenames are different for 32/64 bit BugSplat file and we don't # have any control over them so need to branch. - if (BUGSPLAT_DB) + if (USE_BUGSPLAT) if(ADDRESS_SIZE EQUAL 32) set(release_files ${release_files} BugSplat.dll) set(release_files ${release_files} BugSplatRc.dll) @@ -60,7 +60,7 @@ if(WINDOWS) set(release_files ${release_files} BugSplatRc64.dll) set(release_files ${release_files} BsSndRpt64.exe) endif(ADDRESS_SIZE EQUAL 32) - endif (BUGSPLAT_DB) + endif (USE_BUGSPLAT) if (FMODEX) diff --git a/indra/cmake/LLAddBuildTest.cmake b/indra/cmake/LLAddBuildTest.cmake index b3f42c1a5e..fa10a9d443 100644 --- a/indra/cmake/LLAddBuildTest.cmake +++ b/indra/cmake/LLAddBuildTest.cmake @@ -2,6 +2,7 @@ include(00-Common) include(LLTestCommand) include(GoogleMock) +include(bugsplat) include(Tut) #***************************************************************************** @@ -22,7 +23,6 @@ MACRO(LL_ADD_PROJECT_UNIT_TESTS project sources) # there is another branch that will conflict heavily with any changes here. INCLUDE(GoogleMock) - IF(LL_TEST_VERBOSE) MESSAGE("LL_ADD_PROJECT_UNIT_TESTS UNITTEST_PROJECT_${project} sources: ${sources}") ENDIF(LL_TEST_VERBOSE) @@ -87,6 +87,12 @@ INCLUDE(GoogleMock) IF(LL_TEST_VERBOSE) MESSAGE("LL_ADD_PROJECT_UNIT_TESTS ${name}_test_SOURCE_FILES ${${name}_test_SOURCE_FILES}") ENDIF(LL_TEST_VERBOSE) + + if (USE_BUGSPLAT) + SET_PROPERTY(SOURCE ${${name}_test_SOURCE_FILES} + APPEND PROPERTY COMPILE_DEFINITIONS "${BUGSPLAT_DEFINE}") + endif (USE_BUGSPLAT) + # Headers GET_OPT_SOURCE_FILE_PROPERTY(${name}_test_additional_HEADER_FILES ${source} LL_TEST_ADDITIONAL_HEADER_FILES) SET(${name}_test_HEADER_FILES ${name}.h ${${name}_test_additional_HEADER_FILES}) @@ -223,6 +229,11 @@ FUNCTION(LL_ADD_INTEGRATION_TEST SET_TARGET_PROPERTIES(INTEGRATION_TEST_${testname} PROPERTIES COMPILE_FLAGS -I"${TUT_INCLUDE_DIR}") endif(USESYSTEMLIBS) + if (USE_BUGSPLAT) + SET_PROPERTY(SOURCE ${source_files} + APPEND PROPERTY COMPILE_DEFINITIONS "${BUGSPLAT_DEFINE}") + endif (USE_BUGSPLAT) + # The following was copied to llcorehttp/CMakeLists.txt's texture_load target. # Any changes made here should be replicated there. if (WINDOWS) diff --git a/indra/cmake/Variables.cmake b/indra/cmake/Variables.cmake index a5770c5528..c81b22e572 100644 --- a/indra/cmake/Variables.cmake +++ b/indra/cmake/Variables.cmake @@ -34,7 +34,6 @@ set(LL_TESTS ON CACHE BOOL "Build and run unit and integration tests (disable fo set(INCREMENTAL_LINK OFF CACHE BOOL "Use incremental linking on win32 builds (enable for faster links on some machines)") set(ENABLE_MEDIA_PLUGINS ON CACHE BOOL "Turn off building media plugins if they are imported by third-party library mechanism") set(VIEWER_SYMBOL_FILE "" CACHE STRING "Name of tarball into which to place symbol files") -set(BUGSPLAT_DB "" CACHE STRING "BugSplat database name, if BugSplat crash reporting is desired") if(LIBS_CLOSED_DIR) file(TO_CMAKE_PATH "${LIBS_CLOSED_DIR}" LIBS_CLOSED_DIR) diff --git a/indra/cmake/bugsplat.cmake b/indra/cmake/bugsplat.cmake index 59644b73ce..749ea05403 100644 --- a/indra/cmake/bugsplat.cmake +++ b/indra/cmake/bugsplat.cmake @@ -1,25 +1,32 @@ -# BugSplat is engaged by setting BUGSPLAT_DB to the target BugSplat database -# name. -if (BUGSPLAT_DB) - if (USESYSTEMLIBS) - message(STATUS "Looking for system BugSplat") - set(BUGSPLAT_FIND_QUIETLY ON) - set(BUGSPLAT_FIND_REQUIRED ON) - include(FindBUGSPLAT) - else (USESYSTEMLIBS) - message(STATUS "Engaging autobuild BugSplat") - include(Prebuilt) - use_prebuilt_binary(bugsplat) - if (WINDOWS) - set(BUGSPLAT_LIBRARIES - ${ARCH_PREBUILT_DIRS_RELEASE}/bugsplat.lib - ) - elseif (DARWIN) - find_library(BUGSPLAT_LIBRARIES BugsplatMac - PATHS "${ARCH_PREBUILT_DIRS_RELEASE}") - else (WINDOWS) +if (INSTALL_PROPRIETARY) + set(USE_BUGSPLAT ON CACHE BOOL "Use the BugSplat crash reporting system") +else (INSTALL_PROPRIETARY) + set(USE_BUGSPLAT OFF CACHE BOOL "Use the BugSplat crash reporting system") +endif (INSTALL_PROPRIETARY) + +if (USE_BUGSPLAT) + if (NOT USESYSTEMLIBS) + include(Prebuilt) + use_prebuilt_binary(bugsplat) + if (WINDOWS) + set(BUGSPLAT_LIBRARIES + ${ARCH_PREBUILT_DIRS_RELEASE}/bugsplat.lib + ) + elseif (DARWIN) + find_library(BUGSPLAT_LIBRARIES BugsplatMac + PATHS "${ARCH_PREBUILT_DIRS_RELEASE}") + else (WINDOWS) + message(FATAL_ERROR "BugSplat is not supported; add -DUSE_BUGSPLAT=OFF") + endif (WINDOWS) + else (NOT USESYSTEMLIBS) + set(BUGSPLAT_FIND_QUIETLY ON) + set(BUGSPLAT_FIND_REQUIRED ON) + include(FindBUGSPLAT) + endif (NOT USESYSTEMLIBS) + + set(BUGSPLAT_DB "" CACHE STRING "BugSplat crash database name") - endif (WINDOWS) set(BUGSPLAT_INCLUDE_DIR ${LIBS_PREBUILT_DIR}/include/bugsplat) - endif (USESYSTEMLIBS) -endif (BUGSPLAT_DB) + set(BUGSPLAT_DEFINE "LL_BUGSPLAT") +endif (USE_BUGSPLAT) + diff --git a/indra/llcommon/CMakeLists.txt b/indra/llcommon/CMakeLists.txt index af41b9e460..e3afa5eca4 100644 --- a/indra/llcommon/CMakeLists.txt +++ b/indra/llcommon/CMakeLists.txt @@ -5,6 +5,7 @@ project(llcommon) include(00-Common) include(LLCommon) +include(bugsplat) include(Linking) include(Boost) include(LLSharedLibs) @@ -257,10 +258,10 @@ set(llcommon_HEADER_FILES set_source_files_properties(${llcommon_HEADER_FILES} PROPERTIES HEADER_FILE_ONLY TRUE) -if (BUGSPLAT_DB) - set_source_files_properties(llapp.cpp - PROPERTIES COMPILE_DEFINITIONS "LL_BUGSPLAT") -endif (BUGSPLAT_DB) +if (USE_BUGSPLAT) + set_source_files_properties(${llcommon_SOURCE_FILES} + PROPERTIES COMPILE_DEFINITIONS "${BUGSPLAT_DEFINE}") +endif (USE_BUGSPLAT) list(APPEND llcommon_SOURCE_FILES ${llcommon_HEADER_FILES}) diff --git a/indra/llcommon/llerror.cpp b/indra/llcommon/llerror.cpp index 0ddce353f1..77d7fe1b24 100644 --- a/indra/llcommon/llerror.cpp +++ b/indra/llcommon/llerror.cpp @@ -25,6 +25,7 @@ * $/LicenseInfo$ */ +#define _LLERROR_CPP_ #include "linden_common.h" #include "llerror.h" @@ -724,7 +725,7 @@ namespace LLError s->mCrashFunction = fatal_function; } - void restoreCrashOnError() + void restoreCrashOnError() { SettingsConfigPtr s = Settings::getInstance()->getSettingsConfig(); s->mCrashFunction = NULL; @@ -1310,7 +1311,7 @@ namespace LLError LLMutexTrylock lock(&gLogMutex,5); if (!lock.isLocked()) { - return true; + return false; // because this wasn't logged, it cannot be fatal } // If we hit a logging request very late during shutdown processing, @@ -1318,7 +1319,7 @@ namespace LLError // DO NOT resurrect them. if (Settings::wasDeleted() || Globals::wasDeleted()) { - return true; + return false; // because this wasn't logged, it cannot be fatal } Globals* g = Globals::getInstance(); @@ -1352,7 +1353,7 @@ namespace LLError } else { - return true; + return false; // because this wasn't logged, it cannot be fatal } } else @@ -1372,11 +1373,17 @@ namespace LLError if (s->mCrashFunction) { s->mCrashFunction(message); - return false; + return false; // because an override is in effect + } + else + { + return true; // calling macro should crash } } - - return true; + else + { + return false; // not ERROR, so do not crash + } } } diff --git a/indra/llcommon/llerror.h b/indra/llcommon/llerror.h index cbade88f61..07aa5c6f8b 100644 --- a/indra/llcommon/llerror.h +++ b/indra/llcommon/llerror.h @@ -202,7 +202,7 @@ namespace LLError static void flush(std::ostringstream* out, char* message); - // returns false iff there is a fatal crash override in effect + // returns false iff the calling macro should crash static bool flush(std::ostringstream*, const CallSite&); static std::string demangle(const char* mangled); @@ -371,18 +371,22 @@ typedef LLError::NoClassInfo _LL_CLASS_TO_LOG; #define LL_NEWLINE '\n' +#ifdef _LLERROR_CPP_ +volatile int* gCauseCrash = NULL; +#else +volatile extern int* gCauseCrash; +#endif // _LLERROR_CPP_ + // 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 LLERROR_CRASH \ +{ \ + *gCauseCrash = 0; \ + exit(*gCauseCrash); \ } #define LL_ENDL \ LLError::End(); \ - if (LLError::Log::flush(_out, _site) \ - && _site.mLevel == LLError::LEVEL_ERROR) \ + if (LLError::Log::flush(_out, _site)) \ LLERROR_CRASH \ } \ } while(0) diff --git a/indra/llcommon/llerrorcontrol.h b/indra/llcommon/llerrorcontrol.h index 7ca6ddb737..af46430f74 100644 --- a/indra/llcommon/llerrorcontrol.h +++ b/indra/llcommon/llerrorcontrol.h @@ -92,11 +92,19 @@ namespace LLError /* Control functions. */ + typedef boost::function FatalFunction; - LL_COMMON_API void overrideCrashOnError(const FatalFunction&); - LL_COMMON_API void restoreCrashOnError(); - + /// Override the default behavior of crashing on LL_ERRS; this should NEVER be used except in test code + LL_COMMON_API void overrideCrashOnError(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. + + /// Undo the effect of the overrideCrashOnError above + LL_COMMON_API void restoreCrashOnError(); + LL_COMMON_API std::string getFatalMessage(); // Retrieve the message last passed to LL_ERRS, if any diff --git a/indra/llcommon/llleap.cpp b/indra/llcommon/llleap.cpp index f7bfa36bb5..bf20c87c89 100644 --- a/indra/llcommon/llleap.cpp +++ b/indra/llcommon/llleap.cpp @@ -59,6 +59,7 @@ public: // pump name -- so it should NOT need tweaking for uniqueness. mReplyPump(LLUUID::generateNewID().asString()), mExpect(0), + // 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 @@ -144,9 +145,6 @@ public: mStderrConnection = childerr.getPump() .listen("LLLeap", boost::bind(&LLLeapImpl::rstderr, this, _1)); - // For our lifespan, intercept any LL_ERRS so we can notify plugin - 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! wstdin(mReplyPump.getName(), @@ -161,8 +159,6 @@ public: virtual ~LLLeapImpl() { LL_DEBUGS("LLLeap") << "destroying LLLeap(\"" << mDesc << "\")" << LL_ENDL; - // Restore original fatal crash behavior for LL_ERRS - LLError::restoreCrashOnError(); } // Listener for failed launch attempt @@ -376,30 +372,6 @@ public: return false; } - void fatalFunction(const std::string& error) - { - // Notify plugin - LLSD event; - event["type"] = "error"; - event["error"] = error; - mReplyPump.post(event); - - // All the above really accomplished was to buffer the serialized - // event in our WritePipe. Have to pump mainloop a couple times to - // really write it out there... but time out in case we can't write. - LLProcess::WritePipe& childin(mChild->getWritePipe(LLProcess::STDIN)); - LLEventPump& mainloop(LLEventPumps::instance().obtain("mainloop")); - LLSD nop; - F64 until = (LLTimer::getElapsedSeconds() + 2).value(); - while (childin.size() && LLTimer::getElapsedSeconds() < until) - { - mainloop.post(nop); - } - - // go ahead and do the crash that LLError would have done - LLERROR_CRASH - } - private: /// We always want to listen on mReplyPump with wstdin(); under some /// circumstances we'll also echo other LLEventPumps to the plugin. diff --git a/indra/llcommon/tests/wrapllerrs.h b/indra/llcommon/tests/wrapllerrs.h index b280271476..fedc17dbe9 100644 --- a/indra/llcommon/tests/wrapllerrs.h +++ b/indra/llcommon/tests/wrapllerrs.h @@ -50,11 +50,11 @@ extern void wouldHaveCrashed(const std::string& message); struct WrapLLErrs { - WrapLLErrs(): + 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()) + :mPriorErrorSettings(LLError::saveAndResetSettings()) { // Make LL_ERRS call our own operator() method LLError::overrideCrashOnError(boost::bind(&WrapLLErrs::operator(), this, _1)); @@ -210,6 +210,7 @@ public: { LLError::removeRecorder(mRecorder); LLError::restoreSettings(mOldSettings); + LLError::restoreCrashOnError(); } /// Don't assume the message we want is necessarily the LAST log message diff --git a/indra/llcorehttp/CMakeLists.txt b/indra/llcorehttp/CMakeLists.txt index 9dbc6f447e..286c275b90 100644 --- a/indra/llcorehttp/CMakeLists.txt +++ b/indra/llcorehttp/CMakeLists.txt @@ -13,6 +13,7 @@ include(LLAddBuildTest) include(LLMessage) include(LLCommon) include(Tut) +include(bugsplat) include_directories (${CMAKE_CURRENT_SOURCE_DIR}) diff --git a/indra/llimage/CMakeLists.txt b/indra/llimage/CMakeLists.txt index 293ada7548..28b8e8c06d 100644 --- a/indra/llimage/CMakeLists.txt +++ b/indra/llimage/CMakeLists.txt @@ -11,6 +11,7 @@ include(LLKDU) include(LLImageJ2COJ) include(ZLIB) include(LLAddBuildTest) +include(bugsplat) include(Tut) include_directories( diff --git a/indra/llmath/CMakeLists.txt b/indra/llmath/CMakeLists.txt index 379c3ee9ea..625459823c 100644 --- a/indra/llmath/CMakeLists.txt +++ b/indra/llmath/CMakeLists.txt @@ -4,6 +4,7 @@ project(llmath) include(00-Common) include(LLCommon) +include(bugsplat) include(Boost) include_directories( diff --git a/indra/newview/CMakeLists.txt b/indra/newview/CMakeLists.txt index 04f6c9b7f0..bbfa838827 100644 --- a/indra/newview/CMakeLists.txt +++ b/indra/newview/CMakeLists.txt @@ -8,9 +8,7 @@ include(00-Common) include(Linking) include(Boost) -if (BUGSPLAT_DB) - include(bugsplat) -endif (BUGSPLAT_DB) +include(bugsplat) include(BuildPackagesInfo) include(BuildVersion) include(CMakeCopyIfDifferent) @@ -97,18 +95,18 @@ include_directories( ${CMAKE_CURRENT_SOURCE_DIR} ) -if (BUGSPLAT_DB) - include_directories( - ${BUGSPLAT_INCLUDE_DIR} - ) -endif (BUGSPLAT_DB) - include_directories(SYSTEM ${LLCOMMON_SYSTEM_INCLUDE_DIRS} ${LLXML_SYSTEM_INCLUDE_DIRS} ${LLPHYSICSEXTENSIONS_INCLUDE_DIRS} ) +if (USE_BUGSPLAT) + include_directories(AFTER + ${BUGSPLAT_INCLUDE_DIR} + ) +endif (USE_BUGSPLAT) + set(viewer_SOURCE_FILES groupchatlistener.cpp llaccountingcostmanager.cpp @@ -1390,11 +1388,11 @@ if (DARWIN) ${COREAUDIO_LIBRARY} ) - if (BUGSPLAT_DB) + if (USE_BUGSPLAT) list(APPEND viewer_LIBRARIES ${BUGSPLAT_LIBRARIES} ) - endif (BUGSPLAT_DB) + endif (USE_BUGSPLAT) # Add resource files to the project. set(viewer_RESOURCE_FILES @@ -1729,10 +1727,10 @@ if (SDL_FOUND) ) endif (SDL_FOUND) -if (BUGSPLAT_DB) +if (USE_BUGSPLAT) set_property(TARGET ${VIEWER_BINARY_NAME} PROPERTY COMPILE_DEFINITIONS "LL_BUGSPLAT") -endif (BUGSPLAT_DB) +endif (USE_BUGSPLAT) # add package files file(GLOB EVENT_HOST_SCRIPT_GLOB_LIST @@ -2018,11 +2016,11 @@ target_link_libraries(${VIEWER_BINARY_NAME} ${LLAPPEARANCE_LIBRARIES} ) -if (BUGSPLAT_DB) +if (USE_BUGSPLAT) target_link_libraries(${VIEWER_BINARY_NAME} ${BUGSPLAT_LIBRARIES} ) -endif (BUGSPLAT_DB) +endif (USE_BUGSPLAT) set(ARTWORK_DIR ${CMAKE_CURRENT_SOURCE_DIR} CACHE PATH "Path to artwork files.") @@ -2206,7 +2204,7 @@ endif (INSTALL) # Note that the conventional VIEWER_SYMBOL_FILE is set by ../../build.sh if (PACKAGE AND (RELEASE_CRASH_REPORTING OR NON_RELEASE_CRASH_REPORTING) AND VIEWER_SYMBOL_FILE) - if (NOT BUGSPLAT_DB) + if (NOT USE_BUGSPLAT) # Breakpad symbol-file generation set(SYMBOL_SEARCH_DIRS "") if (WINDOWS) @@ -2261,7 +2259,7 @@ if (PACKAGE AND (RELEASE_CRASH_REPORTING OR NON_RELEASE_CRASH_REPORTING) AND VIE add_dependencies(generate_symbols "${VIEWER_COPY_MANIFEST}") endif (WINDOWS OR LINUX) - else (NOT BUGSPLAT_DB) + else (NOT USE_BUGSPLAT) # BugSplat symbol-file generation if (WINDOWS) # Just pack up a tarball containing only the .pdb file for the @@ -2345,9 +2343,9 @@ if (PACKAGE AND (RELEASE_CRASH_REPORTING OR NON_RELEASE_CRASH_REPORTING) AND VIE if (LINUX) # TBD endif (LINUX) - endif (NOT BUGSPLAT_DB) + endif (NOT USE_BUGSPLAT) - # for both BUGSPLAT_DB and Breakpad + # for both Bugsplat and Breakpad add_dependencies(llpackage generate_symbols) endif () diff --git a/indra/newview/llappviewer.cpp b/indra/newview/llappviewer.cpp index 1fda07edd6..f9ee22d20a 100644 --- a/indra/newview/llappviewer.cpp +++ b/indra/newview/llappviewer.cpp @@ -719,14 +719,14 @@ LLAppViewer::LLAppViewer() // from the previous viewer run between this constructor call and the // init() call, which will overwrite the static_debug_info.log file for // THIS run. So setDebugFileNames() early. -#if LL_BUGSPLAT +# ifdef LL_BUGSPLAT // MAINT-8917: don't create a dump directory just for the // static_debug_info.log file std::string logdir = gDirUtilp->getExpandedFilename(LL_PATH_LOGS, ""); -#else // ! LL_BUGSPLAT +# else // ! LL_BUGSPLAT // write Google Breakpad minidump files to a per-run dump directory to avoid multiple viewer issues. std::string logdir = gDirUtilp->getExpandedFilename(LL_PATH_DUMP, ""); -#endif // ! LL_BUGSPLAT +# endif // ! LL_BUGSPLAT mDumpPath = logdir; setMiniDumpDir(logdir); setDebugFileNames(logdir); @@ -751,6 +751,16 @@ public: } }; +namespace { +// With Xcode 6, _exit() is too magical to use with boost::bind(), so provide +// this little helper function. +void fast_exit(int rc) +{ + _exit(rc); +} + + +} bool LLAppViewer::init() { @@ -790,6 +800,18 @@ bool LLAppViewer::init() initMaxHeapSize() ; LLCoros::instance().setStackSize(gSavedSettings.getS32("CoroutineStackSize")); + // Although initLoggingAndGetLastDuration() is the right place to mess with + // overrideCrashOnError(), we can't query gSavedSettings until after + // initConfiguration(). + S32 rc(gSavedSettings.getS32("QAModeTermCode")); + if (rc >= 0) + { + // QAModeTermCode set, terminate with that rc on LL_ERRS. Use + // fast_exit() rather than exit() because normal cleanup depends too + // much on successful startup! + LLError::overrideCrashOnError(boost::bind(fast_exit, rc)); + } + mAlloc.setProfilingEnabled(gSavedSettings.getBOOL("MemProfiling")); // Initialize the non-LLCurl libcurl library. Should be called @@ -2096,9 +2118,6 @@ bool LLAppViewer::cleanup() return true; } - gDebugInfo["FatalMessage"] = error_string; - LLAppViewer::instance()->writeDebugInfo(); - bool LLAppViewer::initThreads() { static const bool enable_threads = true; @@ -2133,6 +2152,24 @@ bool LLAppViewer::initThreads() return true; } +#ifndef LL_BUGSPLAT +void errorCallback(const std::string &error_string) +{ +#ifndef LL_RELEASE_FOR_DOWNLOAD + OSMessageBox(error_string, LLTrans::getString("MBFatalError"), OSMB_OK); +#endif + + //Set the ErrorActivated global so we know to create a marker file + gLLErrorActivated = true; + + gDebugInfo["FatalMessage"] = error_string; + // We're not already crashing -- we simply *intend* to crash. Since we + // haven't actually trashed anything yet, we can afford to write the whole + // static info file. + LLAppViewer::instance()->writeDebugInfo(); +} +#endif // ! LL_BUGSPLAT + void LLAppViewer::initLoggingAndGetLastDuration() { // @@ -2889,7 +2926,6 @@ bool LLAppViewer::initWindow() // Need to load feature table before cheking to start watchdog. bool use_watchdog = false; int watchdog_enabled_setting = gSavedSettings.getS32("WatchdogEnabled"); - if (watchdog_enabled_setting == -1) { use_watchdog = !LLFeatureManager::getInstance()->isFeatureAvailable("WatchdogDisabled"); @@ -3352,10 +3388,19 @@ void LLAppViewer::writeSystemInfo() gDebugInfo["MainloopThreadID"] = (S32)thread_id; #endif +#ifndef LL_BUGSPLAT + // "CrashNotHandled" is set here, while things are running well, + // in case of a freeze. If there is a freeze, the crash logger will be launched + // and can read this value from the debug_info.log. + // If the crash is handled by LLAppViewer::handleViewerCrash, ie not a freeze, + // then the value of "CrashNotHandled" will be set to true. + gDebugInfo["CrashNotHandled"] = (LLSD::Boolean)true; +#else // LL_BUGSPLAT // "CrashNotHandled" is obsolete; it used (not very successsfully) // to try to distinguish crashes from freezes gDebugInfo["CrashNotHandled"] = (LLSD::Boolean)false; gDebugInfo["FatalMessage"] = LLError::getFatalMessage(); +#endif // ! LL_BUGSPLAT // Insert crash host url (url to post crash log to) if configured. This insures // that the crash report will go to the proper location in the case of a diff --git a/indra/newview/llwatchdog.cpp b/indra/newview/llwatchdog.cpp index 2f3e5db84f..0a01113224 100644 --- a/indra/newview/llwatchdog.cpp +++ b/indra/newview/llwatchdog.cpp @@ -145,10 +145,10 @@ void LLWatchdogTimeout::ping(const std::string& state) } // LLWatchdog -LLWatchdog::LLWatchdog() : - mSuspectsAccessMutex(), - mTimer(NULL), - mLastClockCount(0) +LLWatchdog::LLWatchdog() + :mSuspectsAccessMutex() + ,mTimer(NULL) + ,mLastClockCount(0) { } diff --git a/indra/test/CMakeLists.txt b/indra/test/CMakeLists.txt index 8344cead57..7ac1f1db23 100644 --- a/indra/test/CMakeLists.txt +++ b/indra/test/CMakeLists.txt @@ -13,7 +13,7 @@ include(LLXML) include(Linking) include(Tut) include(LLAddBuildTest) - +include(bugsplat) include(GoogleMock) include_directories( @@ -82,6 +82,10 @@ list(APPEND test_SOURCE_FILES ${test_HEADER_FILES}) add_executable(lltest ${test_SOURCE_FILES}) +if (USE_BUGSPLAT) + set_target_properties(lltest PROPERTIES COMPILE_DEFINITIONS "${BUGSPLAT_DEFINE}") +endif (USE_BUGSPLAT) + target_link_libraries(lltest ${LLDATABASE_LIBRARIES} ${LLINVENTORY_LIBRARIES} diff --git a/indra/test/test.cpp b/indra/test/test.cpp index 125de72b79..5dabcbbc58 100644 --- a/indra/test/test.cpp +++ b/indra/test/test.cpp @@ -75,7 +75,10 @@ #include -void wouldHaveCrashed(const std::string& message); +void wouldHaveCrashed(const std::string& message) +{ + tut::fail("fatal error message: " + message); +} namespace tut { @@ -506,11 +509,6 @@ void stream_groups(std::ostream& s, const char* app) } } -void wouldHaveCrashed(const std::string& message) -{ - tut::fail("fatal error message: " + message); -} - static LLTrace::ThreadRecorder* sMasterThreadRecorder = NULL; int main(int argc, char **argv) -- cgit v1.3 From e86c0b3d0fbc5f91090241be959ef19bfffd8636 Mon Sep 17 00:00:00 2001 From: Oz Linden Date: Tue, 5 Mar 2019 04:44:29 -0500 Subject: fix error message reporting? --- indra/llcommon/llerror.cpp | 18 +++++++-------- indra/llcommon/llerror.h | 6 +++-- indra/llcommon/llerrorcontrol.h | 11 ++++++--- indra/llcommon/tests/llerror_test.cpp | 8 +++++-- indra/llcommon/tests/wrapllerrs.h | 8 +++---- indra/newview/llappviewer.cpp | 43 ++++++++++++----------------------- indra/newview/llappviewermacosx.cpp | 6 ++--- indra/test/test.cpp | 7 +++--- 8 files changed, 51 insertions(+), 56 deletions(-) (limited to 'indra/test') diff --git a/indra/llcommon/llerror.cpp b/indra/llcommon/llerror.cpp index 77d7fe1b24..6fa9e590cb 100644 --- a/indra/llcommon/llerror.cpp +++ b/indra/llcommon/llerror.cpp @@ -719,7 +719,7 @@ namespace LLError commonInit(user_dir, app_dir, log_to_stderr); } - void overrideCrashOnError(const FatalFunction& fatal_function) + void setFatalHandler(const FatalFunction& fatal_function) { SettingsConfigPtr s = Settings::getInstance()->getSettingsConfig(); s->mCrashFunction = fatal_function; @@ -1306,12 +1306,12 @@ namespace LLError return ; } - bool Log::flush(std::ostringstream* out, const CallSite& site) + ErrCrashHandlerResult Log::flush(std::ostringstream* out, const CallSite& site) { LLMutexTrylock lock(&gLogMutex,5); if (!lock.isLocked()) { - return false; // because this wasn't logged, it cannot be fatal + return ERR_DO_NOT_CRASH; // because this wasn't logged, it cannot be fatal } // If we hit a logging request very late during shutdown processing, @@ -1319,7 +1319,7 @@ namespace LLError // DO NOT resurrect them. if (Settings::wasDeleted() || Globals::wasDeleted()) { - return false; // because this wasn't logged, it cannot be fatal + return ERR_DO_NOT_CRASH; // because this wasn't logged, it cannot be fatal } Globals* g = Globals::getInstance(); @@ -1353,7 +1353,7 @@ namespace LLError } else { - return false; // because this wasn't logged, it cannot be fatal + return ERR_DO_NOT_CRASH; // because this wasn't logged, it cannot be fatal } } else @@ -1369,20 +1369,18 @@ namespace LLError if (site.mLevel == LEVEL_ERROR) { - g->mFatalMessage = message; if (s->mCrashFunction) { - s->mCrashFunction(message); - return false; // because an override is in effect + return s->mCrashFunction(message); } else { - return true; // calling macro should crash + return ERR_CRASH; // calling macro should crash } } else { - return false; // not ERROR, so do not crash + return ERR_DO_NOT_CRASH; // not ERROR, so do not crash } } } diff --git a/indra/llcommon/llerror.h b/indra/llcommon/llerror.h index 07aa5c6f8b..2a73ccb36a 100644 --- a/indra/llcommon/llerror.h +++ b/indra/llcommon/llerror.h @@ -194,6 +194,8 @@ namespace LLError struct CallSite; + enum ErrCrashHandlerResult { ERR_DO_NOT_CRASH, ERR_CRASH }; + class LL_COMMON_API Log { public: @@ -203,7 +205,7 @@ namespace LLError static void flush(std::ostringstream* out, char* message); // returns false iff the calling macro should crash - static bool flush(std::ostringstream*, const CallSite&); + static ErrCrashHandlerResult flush(std::ostringstream*, const CallSite&); static std::string demangle(const char* mangled); }; @@ -386,7 +388,7 @@ volatile extern int* gCauseCrash; #define LL_ENDL \ LLError::End(); \ - if (LLError::Log::flush(_out, _site)) \ + if (LLError::ERR_CRASH == LLError::Log::flush(_out, _site)) \ LLERROR_CRASH \ } \ } while(0) diff --git a/indra/llcommon/llerrorcontrol.h b/indra/llcommon/llerrorcontrol.h index af46430f74..2fa220ba5a 100644 --- a/indra/llcommon/llerrorcontrol.h +++ b/indra/llcommon/llerrorcontrol.h @@ -93,16 +93,21 @@ namespace LLError Control functions. */ - typedef boost::function FatalFunction; + // A FatalFunction is called if set using setFatalHandler; its return controls + // whether or not the calling error logging code should crash. + // ERR_DO_NOT_CRASH should be used only in test code. + typedef boost::function FatalFunction; + /// Override the default behavior of crashing on LL_ERRS; this should NEVER be used except in test code - LL_COMMON_API void overrideCrashOnError(const FatalFunction&); + LL_COMMON_API void setFatalHandler(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. + // The - /// Undo the effect of the overrideCrashOnError above + /// Undo the effect of the setFatalHandler above LL_COMMON_API void restoreCrashOnError(); LL_COMMON_API std::string getFatalMessage(); diff --git a/indra/llcommon/tests/llerror_test.cpp b/indra/llcommon/tests/llerror_test.cpp index 2f8923d2de..9dcb5c145f 100644 --- a/indra/llcommon/tests/llerror_test.cpp +++ b/indra/llcommon/tests/llerror_test.cpp @@ -70,7 +70,11 @@ namespace namespace { static bool fatalWasCalled; - void fatalCall(const std::string&) { fatalWasCalled = true; } + LLError::ErrCrashHandlerResult fatalCall(const std::string&) + { + fatalWasCalled = true; + return LLError::ERR_DO_NOT_CRASH; + } } namespace tut @@ -120,7 +124,7 @@ namespace tut mPriorErrorSettings = LLError::saveAndResetSettings(); LLError::setDefaultLevel(LLError::LEVEL_DEBUG); - LLError::overrideCrashOnError(fatalCall); + LLError::setFatalHandler(fatalCall); LLError::addRecorder(mRecorder); } diff --git a/indra/llcommon/tests/wrapllerrs.h b/indra/llcommon/tests/wrapllerrs.h index fedc17dbe9..a412e12a04 100644 --- a/indra/llcommon/tests/wrapllerrs.h +++ b/indra/llcommon/tests/wrapllerrs.h @@ -46,7 +46,7 @@ // statically reference the function in test.cpp... it's short, we could // replicate, but better to reuse -extern void wouldHaveCrashed(const std::string& message); +extern LLError::ErrCrashHandlerResult wouldHaveCrashed(const std::string& message); struct WrapLLErrs { @@ -57,7 +57,7 @@ struct WrapLLErrs :mPriorErrorSettings(LLError::saveAndResetSettings()) { // Make LL_ERRS call our own operator() method - LLError::overrideCrashOnError(boost::bind(&WrapLLErrs::operator(), this, _1)); + LLError::setFatalHandler(boost::bind(&WrapLLErrs::operator(), this, _1)); } ~WrapLLErrs() @@ -71,7 +71,7 @@ struct WrapLLErrs FatalException(const std::string& what): LLException(what) {} }; - void operator()(const std::string& message) + LLError::ErrCrashHandlerResult operator()(const std::string& message) { // Save message for later in case consumer wants to sense the result directly error = message; @@ -201,7 +201,7 @@ public: mOldSettings(LLError::saveAndResetSettings()), mRecorder(new CaptureLogRecorder()) { - LLError::overrideCrashOnError(wouldHaveCrashed); + LLError::setFatalHandler(wouldHaveCrashed); LLError::setDefaultLevel(level); LLError::addRecorder(mRecorder); } diff --git a/indra/newview/llappviewer.cpp b/indra/newview/llappviewer.cpp index f9ee22d20a..8b9f9085b1 100644 --- a/indra/newview/llappviewer.cpp +++ b/indra/newview/llappviewer.cpp @@ -751,16 +751,7 @@ public: } }; -namespace { -// With Xcode 6, _exit() is too magical to use with boost::bind(), so provide -// this little helper function. -void fast_exit(int rc) -{ - _exit(rc); -} - - -} +static S32 sQAModeTermCode = 0; // set from QAModeTermCode to specify exit code for LL_ERRS bool LLAppViewer::init() { @@ -800,17 +791,8 @@ bool LLAppViewer::init() initMaxHeapSize() ; LLCoros::instance().setStackSize(gSavedSettings.getS32("CoroutineStackSize")); - // Although initLoggingAndGetLastDuration() is the right place to mess with - // overrideCrashOnError(), we can't query gSavedSettings until after - // initConfiguration(). - S32 rc(gSavedSettings.getS32("QAModeTermCode")); - if (rc >= 0) - { - // QAModeTermCode set, terminate with that rc on LL_ERRS. Use - // fast_exit() rather than exit() because normal cleanup depends too - // much on successful startup! - LLError::overrideCrashOnError(boost::bind(fast_exit, rc)); - } + // if a return code is set for error exit, save it here for use in fatalErrorHandler + sQAModeTermCode = gSavedSettings.getS32("QAModeTermCode"); mAlloc.setProfilingEnabled(gSavedSettings.getBOOL("MemProfiling")); @@ -2152,8 +2134,7 @@ bool LLAppViewer::initThreads() return true; } -#ifndef LL_BUGSPLAT -void errorCallback(const std::string &error_string) +LLError::ErrCrashHandlerResult fatalErrorHandler(const std::string &error_string) { #ifndef LL_RELEASE_FOR_DOWNLOAD OSMessageBox(error_string, LLTrans::getString("MBFatalError"), OSMB_OK); @@ -2167,8 +2148,15 @@ void errorCallback(const std::string &error_string) // haven't actually trashed anything yet, we can afford to write the whole // static info file. LLAppViewer::instance()->writeDebugInfo(); + + if (sQAModeTermCode) + { + _exit(sQAModeTermCode); + return LLError::ERR_DO_NOT_CRASH; // notreached + } + + return LLError::ERR_CRASH; } -#endif // ! LL_BUGSPLAT void LLAppViewer::initLoggingAndGetLastDuration() { @@ -2178,6 +2166,7 @@ void LLAppViewer::initLoggingAndGetLastDuration() LLError::initForApplication( gDirUtilp->getExpandedFilename(LL_PATH_USER_SETTINGS, "") ,gDirUtilp->getExpandedFilename(LL_PATH_APP_SETTINGS, "") ); + LLError::setFatalHandler(fatalErrorHandler); // Remove the last ".old" log file. std::string old_log_file = gDirUtilp->getExpandedFilename(LL_PATH_LOGS, @@ -3728,12 +3717,8 @@ void LLAppViewer::processMarkerFiles() else if (marker_is_same_version) { // the file existed, is ours, and matched our version, so we can report on what it says - LL_INFOS("MarkerFile") << "Exec marker '"<< mMarkerFileName << "' found; last exec FROZE" << LL_ENDL; -# if LL_BUGSPLAT + LL_INFOS("MarkerFile") << "Exec marker '"<< mMarkerFileName << "' found; last exec crashed" << LL_ENDL; gLastExecEvent = LAST_EXEC_OTHER_CRASH; -# else - gLastExecEvent = LAST_EXEC_FROZE; -# endif } else { diff --git a/indra/newview/llappviewermacosx.cpp b/indra/newview/llappviewermacosx.cpp index 3111540a13..9c2e6eadca 100644 --- a/indra/newview/llappviewermacosx.cpp +++ b/indra/newview/llappviewermacosx.cpp @@ -188,17 +188,17 @@ CrashMetadataSingleton::CrashMetadataSingleton() LLSD info; if (! static_file.is_open()) { - LL_INFOS() << "Can't open '" << staticDebugPathname + LL_WARNS() << "Can't open '" << staticDebugPathname << "'; no metadata about previous run" << LL_ENDL; } else if (! LLSDSerialize::deserialize(info, static_file, LLSDSerialize::SIZE_UNLIMITED)) { - LL_INFOS() << "Can't parse '" << staticDebugPathname + LL_WARNS() << "Can't parse '" << staticDebugPathname << "'; no metadata about previous run" << LL_ENDL; } else { - LL_INFOS() << "Metadata from '" << staticDebugPathname << "':" << LL_ENDL; + LL_INFOS() << "Previous run metadata from '" << staticDebugPathname << "':" << LL_ENDL; logFilePathname = get_metadata(info, "SLLog"); userSettingsPathname = get_metadata(info, "SettingsFilename"); OSInfo = get_metadata(info, "OSInfo"); diff --git a/indra/test/test.cpp b/indra/test/test.cpp index 5dabcbbc58..4f966aede8 100644 --- a/indra/test/test.cpp +++ b/indra/test/test.cpp @@ -75,9 +75,10 @@ #include -void wouldHaveCrashed(const std::string& message) +LLError::ErrCrashHandlerResult wouldHaveCrashed(const std::string& message) { tut::fail("fatal error message: " + message); + return LLError::ERR_DO_NOT_CRASH; } namespace tut @@ -149,7 +150,7 @@ public: mOldSettings(LLError::saveAndResetSettings()), mRecorder(new RecordToTempFile(pool)) { - LLError::overrideCrashOnError(wouldHaveCrashed); + LLError::setFatalHandler(wouldHaveCrashed); LLError::setDefaultLevel(level); LLError::addRecorder(mRecorder); } @@ -530,7 +531,7 @@ int main(int argc, char **argv) LLError::initForApplication(".", ".", false /* do not log to stderr */); LLError::setDefaultLevel(LLError::LEVEL_DEBUG); } - LLError::overrideCrashOnError(wouldHaveCrashed); + LLError::setFatalHandler(wouldHaveCrashed); std::string test_app_name(argv[0]); std::string test_log = test_app_name + ".log"; LLFile::remove(test_log); -- cgit v1.3 From 07870a9ebb17e7a31df276f4ac2d8f52fec2a3b0 Mon Sep 17 00:00:00 2001 From: Oz Linden Date: Tue, 5 Mar 2019 13:57:02 -0500 Subject: rename crash handler to hook, and make them chainable by putting back the get and documenting it --- indra/llcommon/llerror.cpp | 18 +++++++++--------- indra/llcommon/llerror.h | 6 +++--- indra/llcommon/llerrorcontrol.h | 26 ++++++++++++++------------ indra/llcommon/llleap.cpp | 32 +++++++++++++++++++++++++++++++- indra/llcommon/tests/llerror_test.cpp | 4 ++-- indra/llcommon/tests/wrapllerrs.h | 23 +++++++++++++---------- indra/newview/llappviewer.cpp | 4 ++-- indra/test/test.cpp | 6 +++--- 8 files changed, 77 insertions(+), 42 deletions(-) (limited to 'indra/test') diff --git a/indra/llcommon/llerror.cpp b/indra/llcommon/llerror.cpp index 6fa9e590cb..5f3edf7f84 100644 --- a/indra/llcommon/llerror.cpp +++ b/indra/llcommon/llerror.cpp @@ -483,7 +483,7 @@ namespace LLError LevelMap mTagLevelMap; std::map mUniqueLogMessages; - LLError::FatalFunction mCrashFunction; + LLError::FatalHook mFatalHook; LLError::TimeFunction mTimeFunction; Recorders mRecorders; @@ -523,7 +523,7 @@ namespace LLError mFileLevelMap(), mTagLevelMap(), mUniqueLogMessages(), - mCrashFunction(NULL), + mFatalHook(NULL), mTimeFunction(NULL), mRecorders(), mFileRecorder(), @@ -719,16 +719,16 @@ namespace LLError commonInit(user_dir, app_dir, log_to_stderr); } - void setFatalHandler(const FatalFunction& fatal_function) + void setFatalHook(const FatalHook& fatal_hook) { SettingsConfigPtr s = Settings::getInstance()->getSettingsConfig(); - s->mCrashFunction = fatal_function; + s->mFatalHook = fatal_hook; } - void restoreCrashOnError() + FatalHook getFatalHook() { SettingsConfigPtr s = Settings::getInstance()->getSettingsConfig(); - s->mCrashFunction = NULL; + return s->mFatalHook; } std::string getFatalMessage() @@ -1306,7 +1306,7 @@ namespace LLError return ; } - ErrCrashHandlerResult Log::flush(std::ostringstream* out, const CallSite& site) + ErrFatalHookResult Log::flush(std::ostringstream* out, const CallSite& site) { LLMutexTrylock lock(&gLogMutex,5); if (!lock.isLocked()) @@ -1369,9 +1369,9 @@ namespace LLError if (site.mLevel == LEVEL_ERROR) { - if (s->mCrashFunction) + if (s->mFatalHook) { - return s->mCrashFunction(message); + return s->mFatalHook(message); } else { diff --git a/indra/llcommon/llerror.h b/indra/llcommon/llerror.h index 2a73ccb36a..6bdb2e852f 100644 --- a/indra/llcommon/llerror.h +++ b/indra/llcommon/llerror.h @@ -194,7 +194,7 @@ namespace LLError struct CallSite; - enum ErrCrashHandlerResult { ERR_DO_NOT_CRASH, ERR_CRASH }; + enum ErrFatalHookResult { ERR_DO_NOT_CRASH, ERR_CRASH }; class LL_COMMON_API Log { @@ -205,7 +205,7 @@ namespace LLError static void flush(std::ostringstream* out, char* message); // returns false iff the calling macro should crash - static ErrCrashHandlerResult flush(std::ostringstream*, const CallSite&); + static ErrFatalHookResult flush(std::ostringstream*, const CallSite&); static std::string demangle(const char* mangled); }; @@ -272,7 +272,7 @@ namespace LLError //when LLAppViewer::handleViewerCrash() is triggered. // //Note: to be simple, efficient and necessary to keep track of correct call stacks, - //LLCallStacks is designed not to be thread-safe. + //LLCallStacks is designed not to be thread-safe. //so try not to use it in multiple parallel threads at same time. //Used in a single thread at a time is fine. class LL_COMMON_API LLCallStacks diff --git a/indra/llcommon/llerrorcontrol.h b/indra/llcommon/llerrorcontrol.h index 2fa220ba5a..e7c5241cc9 100644 --- a/indra/llcommon/llerrorcontrol.h +++ b/indra/llcommon/llerrorcontrol.h @@ -93,22 +93,24 @@ namespace LLError Control functions. */ - // A FatalFunction is called if set using setFatalHandler; its return controls + // A FatalHook is called if set using setFatalHook; its return controls // whether or not the calling error logging code should crash. // ERR_DO_NOT_CRASH should be used only in test code. - typedef boost::function FatalFunction; - - - /// Override the default behavior of crashing on LL_ERRS; this should NEVER be used except in test code - LL_COMMON_API void setFatalHandler(const FatalFunction&); - // The fatal function will be called when an message of LEVEL_ERROR + typedef boost::function FatalHook; + + /// Supplement and control the default behavior of crashing on LL_ERRS + /// This may be used to suppress crashes only in test code; + /// otherwise, the FatalHook should always either return + /// the result from the previous hook (see getFatalHook below), + /// or return LLError::ERR_CRASH + LL_COMMON_API void setFatalHook(const FatalHook& fatal_hook); + // The fatal_hook 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. - // The + // (by, for example, setting a class level to LEVEL_NONE), will also + // prevent the fatal_hook being called and the resulting deliberate crash - /// Undo the effect of the setFatalHandler above - LL_COMMON_API void restoreCrashOnError(); + /// Retrieve the previously-set FatalHook + LL_COMMON_API FatalHook getFatalHook(); LL_COMMON_API std::string getFatalMessage(); // Retrieve the message last passed to LL_ERRS, if any diff --git a/indra/llcommon/llleap.cpp b/indra/llcommon/llleap.cpp index bf20c87c89..161d25bc34 100644 --- a/indra/llcommon/llleap.cpp +++ b/indra/llcommon/llleap.cpp @@ -59,7 +59,7 @@ public: // pump name -- so it should NOT need tweaking for uniqueness. mReplyPump(LLUUID::generateNewID().asString()), mExpect(0), - + mPrevFatalHook(LLError::getFatalHook()), // 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 @@ -145,6 +145,9 @@ public: mStderrConnection = childerr.getPump() .listen("LLLeap", boost::bind(&LLLeapImpl::rstderr, this, _1)); + // For our lifespan, intercept any LL_ERRS so we can notify plugin + LLError::setFatalHook(boost::bind(&LLLeapImpl::fatalHook, this, _1)); + // Send child a preliminary event reporting our own reply-pump name -- // which would otherwise be pretty tricky to guess! wstdin(mReplyPump.getName(), @@ -159,6 +162,8 @@ public: virtual ~LLLeapImpl() { LL_DEBUGS("LLLeap") << "destroying LLLeap(\"" << mDesc << "\")" << LL_ENDL; + // Restore original FatalHook + LLError::setFatalHook(mPrevFatalHook); } // Listener for failed launch attempt @@ -372,6 +377,30 @@ public: return false; } + LLError::ErrFatalHookResult fatalHook(const std::string& error) + { + // Notify plugin + LLSD event; + event["type"] = "error"; + event["error"] = error; + mReplyPump.post(event); + + // All the above really accomplished was to buffer the serialized + // event in our WritePipe. Have to pump mainloop a couple times to + // really write it out there... but time out in case we can't write. + LLProcess::WritePipe& childin(mChild->getWritePipe(LLProcess::STDIN)); + LLEventPump& mainloop(LLEventPumps::instance().obtain("mainloop")); + LLSD nop; + F64 until = (LLTimer::getElapsedSeconds() + 2).value(); + while (childin.size() && LLTimer::getElapsedSeconds() < until) + { + mainloop.post(nop); + } + + // forward the call to the previous FatalHook, default to crashing if there isn't one + return mPrevFatalHook ? mPrevFatalHook(error) : LLError::ERR_CRASH; + } + private: /// We always want to listen on mReplyPump with wstdin(); under some /// circumstances we'll also echo other LLEventPumps to the plugin. @@ -392,6 +421,7 @@ private: mStdinConnection, mStdoutConnection, mStdoutDataConnection, mStderrConnection; boost::scoped_ptr mBlocker; LLProcess::ReadPipe::size_type mExpect; + LLError::FatalHook mPrevFatalHook; boost::scoped_ptr mListener; }; diff --git a/indra/llcommon/tests/llerror_test.cpp b/indra/llcommon/tests/llerror_test.cpp index 9dcb5c145f..9895abc1f3 100644 --- a/indra/llcommon/tests/llerror_test.cpp +++ b/indra/llcommon/tests/llerror_test.cpp @@ -70,7 +70,7 @@ namespace namespace { static bool fatalWasCalled; - LLError::ErrCrashHandlerResult fatalCall(const std::string&) + LLError::ErrFatalHookResult fatalHook(const std::string&) { fatalWasCalled = true; return LLError::ERR_DO_NOT_CRASH; @@ -124,7 +124,7 @@ namespace tut mPriorErrorSettings = LLError::saveAndResetSettings(); LLError::setDefaultLevel(LLError::LEVEL_DEBUG); - LLError::setFatalHandler(fatalCall); + LLError::setFatalHook(fatalHook); LLError::addRecorder(mRecorder); } diff --git a/indra/llcommon/tests/wrapllerrs.h b/indra/llcommon/tests/wrapllerrs.h index a412e12a04..a6c44d5fdd 100644 --- a/indra/llcommon/tests/wrapllerrs.h +++ b/indra/llcommon/tests/wrapllerrs.h @@ -46,7 +46,7 @@ // statically reference the function in test.cpp... it's short, we could // replicate, but better to reuse -extern LLError::ErrCrashHandlerResult wouldHaveCrashed(const std::string& message); +extern LLError::ErrFatalHookResult wouldHaveCrashed(const std::string& message); struct WrapLLErrs { @@ -55,14 +55,15 @@ struct WrapLLErrs // stderr. Otherwise, expected llerrs (LL_ERRS) messages clutter the // console output of successful tests, potentially confusing things. :mPriorErrorSettings(LLError::saveAndResetSettings()) + ,mPriorFatalHook(LLError::getFatalHook()) { // Make LL_ERRS call our own operator() method - LLError::setFatalHandler(boost::bind(&WrapLLErrs::operator(), this, _1)); + LLError::setFatalHook(boost::bind(&WrapLLErrs::operator(), this, _1)); } ~WrapLLErrs() { - LLError::restoreCrashOnError(); + LLError::setFatalHook(mPriorFatalHook); LLError::restoreSettings(mPriorErrorSettings); } @@ -71,7 +72,7 @@ struct WrapLLErrs FatalException(const std::string& what): LLException(what) {} }; - LLError::ErrCrashHandlerResult operator()(const std::string& message) + LLError::ErrFatalHookResult operator()(const std::string& message) { // Save message for later in case consumer wants to sense the result directly error = message; @@ -107,7 +108,7 @@ struct WrapLLErrs std::string error; LLError::SettingsStoragePtr mPriorErrorSettings; - LLError::FatalFunction mPriorFatal; + LLError::FatalHook mPriorFatalHook; }; /** @@ -197,11 +198,12 @@ public: // with that output. If it turns out that saveAndResetSettings() has // some bad effect, give up and just let the DEBUG level log messages // display. - : boost::noncopyable(), - mOldSettings(LLError::saveAndResetSettings()), - mRecorder(new CaptureLogRecorder()) + : boost::noncopyable() + , mOldSettings(LLError::saveAndResetSettings()) + , mPriorFatalHook(LLError::getFatalHook()) + , mRecorder(new CaptureLogRecorder()) { - LLError::setFatalHandler(wouldHaveCrashed); + LLError::setFatalHook(wouldHaveCrashed); LLError::setDefaultLevel(level); LLError::addRecorder(mRecorder); } @@ -210,7 +212,7 @@ public: { LLError::removeRecorder(mRecorder); LLError::restoreSettings(mOldSettings); - LLError::restoreCrashOnError(); + LLError::setFatalHook(mPriorFatalHook); } /// Don't assume the message we want is necessarily the LAST log message @@ -228,6 +230,7 @@ public: private: LLError::SettingsStoragePtr mOldSettings; + LLError::FatalHook mPriorFatalHook; LLError::RecorderPtr mRecorder; }; diff --git a/indra/newview/llappviewer.cpp b/indra/newview/llappviewer.cpp index 8b9f9085b1..8616fe7c76 100644 --- a/indra/newview/llappviewer.cpp +++ b/indra/newview/llappviewer.cpp @@ -2134,7 +2134,7 @@ bool LLAppViewer::initThreads() return true; } -LLError::ErrCrashHandlerResult fatalErrorHandler(const std::string &error_string) +LLError::ErrFatalHookResult fatalErrorHook(const std::string &error_string) { #ifndef LL_RELEASE_FOR_DOWNLOAD OSMessageBox(error_string, LLTrans::getString("MBFatalError"), OSMB_OK); @@ -2166,7 +2166,7 @@ void LLAppViewer::initLoggingAndGetLastDuration() LLError::initForApplication( gDirUtilp->getExpandedFilename(LL_PATH_USER_SETTINGS, "") ,gDirUtilp->getExpandedFilename(LL_PATH_APP_SETTINGS, "") ); - LLError::setFatalHandler(fatalErrorHandler); + LLError::setFatalHook(fatalErrorHook); // Remove the last ".old" log file. std::string old_log_file = gDirUtilp->getExpandedFilename(LL_PATH_LOGS, diff --git a/indra/test/test.cpp b/indra/test/test.cpp index 4f966aede8..52ac855d9f 100644 --- a/indra/test/test.cpp +++ b/indra/test/test.cpp @@ -75,7 +75,7 @@ #include -LLError::ErrCrashHandlerResult wouldHaveCrashed(const std::string& message) +LLError::ErrFatalHookResult wouldHaveCrashed(const std::string& message) { tut::fail("fatal error message: " + message); return LLError::ERR_DO_NOT_CRASH; @@ -150,7 +150,7 @@ public: mOldSettings(LLError::saveAndResetSettings()), mRecorder(new RecordToTempFile(pool)) { - LLError::setFatalHandler(wouldHaveCrashed); + LLError::setFatalHook(wouldHaveCrashed); LLError::setDefaultLevel(level); LLError::addRecorder(mRecorder); } @@ -531,7 +531,7 @@ int main(int argc, char **argv) LLError::initForApplication(".", ".", false /* do not log to stderr */); LLError::setDefaultLevel(LLError::LEVEL_DEBUG); } - LLError::setFatalHandler(wouldHaveCrashed); + LLError::setFatalHook(wouldHaveCrashed); std::string test_app_name(argv[0]); std::string test_log = test_app_name + ".log"; LLFile::remove(test_log); -- cgit v1.3 From 46fb225134ef871a686d0571cc55256923eee110 Mon Sep 17 00:00:00 2001 From: Oz Linden Date: Tue, 21 May 2019 12:09:26 -0400 Subject: suppress unreachable code warning because tut::fail triggers it --- indra/test/test.cpp | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) (limited to 'indra/test') diff --git a/indra/test/test.cpp b/indra/test/test.cpp index 52ac855d9f..9d327ff3be 100644 --- a/indra/test/test.cpp +++ b/indra/test/test.cpp @@ -57,16 +57,11 @@ #include #endif -#if LL_MSVC -#pragma warning (push) +#if LL_WINDOWS #pragma warning (disable : 4702) // warning C4702: unreachable code #endif #include #include -#if LL_MSVC -#pragma warning (pop) -#endif - #include #include #include -- cgit v1.3 From 4e4564dcf31766469ccf41e0eef82bc234fda72c Mon Sep 17 00:00:00 2001 From: Oz Linden Date: Wed, 26 Aug 2020 09:53:38 -0400 Subject: SL-10297 fix MSVC warning controls (merge error) --- indra/test/test.cpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) (limited to 'indra/test') diff --git a/indra/test/test.cpp b/indra/test/test.cpp index b16fcb84d6..748d042631 100644 --- a/indra/test/test.cpp +++ b/indra/test/test.cpp @@ -58,7 +58,8 @@ #include #endif -#if LL_WINDOWS +#if LL_MSVC +#pragma warning (push) #pragma warning (disable : 4702) // warning C4702: unreachable code #endif #include -- cgit v1.3 From 1aa140d3affa28d85a41d55f2566a19fb9d2cff5 Mon Sep 17 00:00:00 2001 From: Oz Linden Date: Wed, 26 Aug 2020 15:33:20 -0400 Subject: SL-10297 fix MSVC unreachable code warning --- indra/test/test.cpp | 6 ++++++ 1 file changed, 6 insertions(+) (limited to 'indra/test') diff --git a/indra/test/test.cpp b/indra/test/test.cpp index 748d042631..55dad94bc1 100644 --- a/indra/test/test.cpp +++ b/indra/test/test.cpp @@ -77,8 +77,14 @@ LLError::ErrFatalHookResult wouldHaveCrashed(const std::string& message) { +#if LL_MSVC +#pragma warning (push) +#pragma warning (disable : 4702) // warning C4702: unreachable code tut::fail("fatal error message: " + message); return LLError::ERR_DO_NOT_CRASH; +#if LL_MSVC +#pragma warning (pop) +#endif } namespace tut -- cgit v1.3 From 6007475c87d3edbb023cc64bb097d33e6c04dfd4 Mon Sep 17 00:00:00 2001 From: Oz Linden Date: Wed, 26 Aug 2020 16:37:23 -0400 Subject: SL-10297 fix MSVC unreachable code warning (for real) --- indra/test/test.cpp | 1 + 1 file changed, 1 insertion(+) (limited to 'indra/test') diff --git a/indra/test/test.cpp b/indra/test/test.cpp index 55dad94bc1..571f502861 100644 --- a/indra/test/test.cpp +++ b/indra/test/test.cpp @@ -80,6 +80,7 @@ LLError::ErrFatalHookResult wouldHaveCrashed(const std::string& message) #if LL_MSVC #pragma warning (push) #pragma warning (disable : 4702) // warning C4702: unreachable code +#endif tut::fail("fatal error message: " + message); return LLError::ERR_DO_NOT_CRASH; #if LL_MSVC -- cgit v1.3 From 28862ab53b9e532bf6ff789743d80b33af10f395 Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Wed, 12 May 2021 14:21:27 -0400 Subject: SL-10297: Clean up a few merge glitches. --- indra/newview/llappviewer.cpp | 29 ++++++++++++----------------- indra/test/test.cpp | 22 ++++++++-------------- 2 files changed, 20 insertions(+), 31 deletions(-) (limited to 'indra/test') diff --git a/indra/newview/llappviewer.cpp b/indra/newview/llappviewer.cpp index c44690ac25..7d7d41fe22 100644 --- a/indra/newview/llappviewer.cpp +++ b/indra/newview/llappviewer.cpp @@ -2298,10 +2298,6 @@ void LLAppViewer::initLoggingAndGetLastDuration() { LL_WARNS("MarkerFile") << duration_log_msg << LL_ENDL; } - - // TBD - temporary location for fatal hook (should be above, but for now it logs...) - LL_DEBUGS("FatalHook") << "initial setting of default fatalhook" << LL_ENDL; - LLError::setFatalHook(fatalErrorHook); } bool LLAppViewer::loadSettingsFromDirectory(const std::string& location_key, @@ -2998,16 +2994,15 @@ bool LLAppViewer::initWindow() use_watchdog = bool(watchdog_enabled_setting); } - LL_INFOS("AppInit") << "watchdog" - << (use_watchdog ? " " : " NOT ") - << "enabled" - << " (setting = " << watchdog_enabled_setting << ")" - << LL_ENDL; + LL_INFOS("AppInit") << "watchdog" + << (use_watchdog ? " " : " NOT ") + << "enabled" + << " (setting = " << watchdog_enabled_setting << ")" + << LL_ENDL; if (use_watchdog) { - LLWatchdog::getInstance()->init( - [](){ LL_ERRS() << "Watchdog killer event" << LL_ENDL; }); + LLWatchdog::getInstance()->init(); } LLNotificationsUI::LLNotificationManager::getInstance(); @@ -3441,8 +3436,8 @@ void LLAppViewer::writeSystemInfo() gDebugInfo["CPUInfo"]["CPUSSE"] = gSysCPU.hasSSE(); gDebugInfo["CPUInfo"]["CPUSSE2"] = gSysCPU.hasSSE2(); - gDebugInfo["RAMInfo"]["Physical"] = (LLSD::Integer)(gSysMemory.getPhysicalMemoryKB().value()); - gDebugInfo["RAMInfo"]["Allocated"] = (LLSD::Integer)(gMemoryAllocated.valueInUnits()); + gDebugInfo["RAMInfo"]["Physical"] = LLSD::Integer(gSysMemory.getPhysicalMemoryKB().value()); + gDebugInfo["RAMInfo"]["Allocated"] = LLSD::Integer(gMemoryAllocated.valueInUnits()); gDebugInfo["OSInfo"] = LLOSInfo::instance().getOSStringSimple(); // The user is not logged on yet, but record the current grid choice login url @@ -3461,11 +3456,11 @@ void LLAppViewer::writeSystemInfo() // and can read this value from the debug_info.log. // If the crash is handled by LLAppViewer::handleViewerCrash, ie not a freeze, // then the value of "CrashNotHandled" will be set to true. - gDebugInfo["CrashNotHandled"] = (LLSD::Boolean)true; + gDebugInfo["CrashNotHandled"] = LLSD::Boolean(true); #else // LL_BUGSPLAT // "CrashNotHandled" is obsolete; it used (not very successsfully) // to try to distinguish crashes from freezes - the intent here to to avoid calling it a freeze - gDebugInfo["CrashNotHandled"] = (LLSD::Boolean)false; + gDebugInfo["CrashNotHandled"] = LLSD::Boolean(false); #endif // ! LL_BUGSPLAT // Insert crash host url (url to post crash log to) if configured. This insures @@ -3497,7 +3492,7 @@ void LLAppViewer::writeSystemInfo() gDebugInfo["SettingsFilename"] = gSavedSettings.getString("ClientSettingsFile"); gDebugInfo["ViewerExePath"] = gDirUtilp->getExecutablePathAndName(); gDebugInfo["CurrentPath"] = gDirUtilp->getCurPath(); - gDebugInfo["FirstLogin"] = (LLSD::Boolean) gAgent.isFirstLogin(); + gDebugInfo["FirstLogin"] = LLSD::Boolean(gAgent.isFirstLogin()); gDebugInfo["FirstRunThisInstall"] = gSavedSettings.getBOOL("FirstRunThisInstall"); gDebugInfo["StartupState"] = LLStartUp::getStartupStateString(); @@ -3627,7 +3622,7 @@ void LLAppViewer::handleViewerCrash() // The crash is being handled here so set this value to false. // Otherwise the crash logger will think this crash was a freeze. - gDebugInfo["Dynamic"]["CrashNotHandled"] = (LLSD::Boolean)false; + gDebugInfo["Dynamic"]["CrashNotHandled"] = LLSD::Boolean(false); //Write out the crash status file //Use marker file style setup, as that's the simplest, especially since diff --git a/indra/test/test.cpp b/indra/test/test.cpp index 571f502861..87c4a8d8a3 100644 --- a/indra/test/test.cpp +++ b/indra/test/test.cpp @@ -75,18 +75,7 @@ #include -LLError::ErrFatalHookResult wouldHaveCrashed(const std::string& message) -{ -#if LL_MSVC -#pragma warning (push) -#pragma warning (disable : 4702) // warning C4702: unreachable code -#endif - tut::fail("fatal error message: " + message); - return LLError::ERR_DO_NOT_CRASH; -#if LL_MSVC -#pragma warning (pop) -#endif -} +void wouldHaveCrashed(const std::string& message); namespace tut { @@ -157,7 +146,7 @@ public: mOldSettings(LLError::saveAndResetSettings()), mRecorder(new RecordToTempFile(pool)) { - LLError::setFatalHook(wouldHaveCrashed); + LLError::setFatalFunction(wouldHaveCrashed); LLError::setDefaultLevel(level); LLError::addRecorder(mRecorder); } @@ -523,6 +512,11 @@ void stream_groups(std::ostream& s, const char* app) } } +void wouldHaveCrashed(const std::string& message) +{ + tut::fail("llerrs message: " + message); +} + static LLTrace::ThreadRecorder* sMasterThreadRecorder = NULL; int main(int argc, char **argv) @@ -632,7 +626,7 @@ int main(int argc, char **argv) replayer.reset(new LLReplayLogReal(level, gAPRPoolp)); } } - LLError::setFatalHook(wouldHaveCrashed); + LLError::setFatalFunction(wouldHaveCrashed); std::string test_app_name(argv[0]); std::string test_log = test_app_name + ".log"; LLFile::remove(test_log); -- cgit v1.3