From 1fc7c994d6232e373fc9f36e72ed9855d4d7cd76 Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Thu, 12 Dec 2019 07:39:23 -0500 Subject: DRTVWR-494: Fix VS LLError::Log::demangle() vulnerability. The Windows implementation of demangle() assumed that a "mangled" class name produced by typeid(class).name() always starts with the prefix "class ", checked for that and removed it. If the mangled name didn't start with that prefix, it would emit a debug message and return the full name. When the class in question is actually a struct, the prefix is "struct " instead. But when demangle() was being called before logging had been fully initialized, the debug message remarking that it didn't start with "class " crashed. Look for either "class " or "struct " prefix. Remove whichever is found and return the rest of the name. If neither is found, only log if logging is available. --- indra/llcommon/llerror.cpp | 37 ++++++++++++++++++++++--------------- 1 file changed, 22 insertions(+), 15 deletions(-) (limited to 'indra/llcommon/llerror.cpp') diff --git a/indra/llcommon/llerror.cpp b/indra/llcommon/llerror.cpp index 335a0995fe..83d380fafd 100644 --- a/indra/llcommon/llerror.cpp +++ b/indra/llcommon/llerror.cpp @@ -302,28 +302,35 @@ namespace LLError { #ifdef __GNUC__ // GCC: type_info::name() returns a mangled class name,st demangle - // passing nullptr, 0 forces allocation of a unique buffer we can free - // fixing MAINT-8724 on OSX 10.14 + // passing nullptr, 0 forces allocation of a unique buffer we can free + // fixing MAINT-8724 on OSX 10.14 int status = -1; char* name = abi::__cxa_demangle(mangled, nullptr, 0, &status); - std::string result(name ? name : mangled); - free(name); - return result; -#elif LL_WINDOWS - // DevStudio: type_info::name() includes the text "class " at the start + std::string result(name ? name : mangled); + free(name); + return result; - static const std::string class_prefix = "class "; +#elif LL_WINDOWS + // Visual Studio: type_info::name() includes the text "class " at the start std::string name = mangled; - if (0 != name.compare(0, class_prefix.length(), class_prefix)) + for (const auto& prefix : std::vector{ "class ", "struct " }) + { + if (0 == name.compare(0, prefix.length(), prefix)) + { + return name.substr(prefix.length()); + } + } + // huh, that's odd, we should see one or the other prefix -- but don't + // try to log unless logging is already initialized + if (is_available()) { - LL_DEBUGS() << "Did not see '" << class_prefix << "' prefix on '" - << name << "'" << LL_ENDL; - return name; + // in Python, " or ".join(vector) -- but in C++, a PITB + LL_DEBUGS() << "Did not see 'class' or 'struct' prefix on '" + << name << "'" << LL_ENDL; } + return name; - return name.substr(class_prefix.length()); - -#else +#else // neither GCC nor Visual Studio return mangled; #endif } -- cgit v1.3 From 4c9e90de43670b0c641bc51bd686150d661c4203 Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Wed, 18 Dec 2019 12:25:45 -0500 Subject: DRTVWR-494: Get initialized LLMutexes for very early log calls. Use function-static LLMutex instances instead of module-static instances, since some log calls are evidently issued before we get around to initializing llerror.cpp module-static variables. --- indra/llcommon/llerror.cpp | 39 ++++++++++++++++++++++++++++----------- 1 file changed, 28 insertions(+), 11 deletions(-) (limited to 'indra/llcommon/llerror.cpp') diff --git a/indra/llcommon/llerror.cpp b/indra/llcommon/llerror.cpp index 83d380fafd..4bf4827119 100644 --- a/indra/llcommon/llerror.cpp +++ b/indra/llcommon/llerror.cpp @@ -1155,8 +1155,25 @@ namespace } namespace { - LLMutex gLogMutex; - LLMutex gCallStacksLogMutex; + // We need a couple different mutexes, but we want to use the same mechanism + // for both. Make getMutex() a template function with different instances + // for different MutexDiscriminator values. + enum MutexDiscriminator + { + LOG_MUTEX, + STACKS_MUTEX + }; + // Some logging calls happen very early in processing -- so early that our + // module-static variables aren't yet initialized. getMutex() wraps a + // function-static LLMutex so that early calls can still have a valid + // LLMutex instance. + template + LLMutex* getMutex() + { + // guaranteed to be initialized the first time control reaches here + static LLMutex sMutex; + return &sMutex; + } bool checkLevelMap(const LevelMap& map, const std::string& key, LLError::ELevel& level) @@ -1204,7 +1221,7 @@ namespace LLError bool Log::shouldLog(CallSite& site) { - LLMutexTrylock lock(&gLogMutex, 5); + LLMutexTrylock lock(getMutex(), 5); if (!lock.isLocked()) { return false; @@ -1255,7 +1272,7 @@ namespace LLError std::ostringstream* Log::out() { - LLMutexTrylock lock(&gLogMutex,5); + LLMutexTrylock lock(getMutex(),5); // If we hit a logging request very late during shutdown processing, // when either of the relevant LLSingletons has already been deleted, // DO NOT resurrect them. @@ -1275,7 +1292,7 @@ namespace LLError void Log::flush(std::ostringstream* out, char* message) { - LLMutexTrylock lock(&gLogMutex,5); + LLMutexTrylock lock(getMutex(),5); if (!lock.isLocked()) { return; @@ -1315,7 +1332,7 @@ namespace LLError void Log::flush(std::ostringstream* out, const CallSite& site) { - LLMutexTrylock lock(&gLogMutex,5); + LLMutexTrylock lock(getMutex(),5); if (!lock.isLocked()) { return; @@ -1514,7 +1531,7 @@ namespace LLError //static void LLCallStacks::push(const char* function, const int line) { - LLMutexTrylock lock(&gCallStacksLogMutex, 5); + LLMutexTrylock lock(getMutex(), 5); if (!lock.isLocked()) { return; @@ -1549,7 +1566,7 @@ namespace LLError //static void LLCallStacks::end(std::ostringstream* _out) { - LLMutexTrylock lock(&gCallStacksLogMutex, 5); + LLMutexTrylock lock(getMutex(), 5); if (!lock.isLocked()) { return; @@ -1565,13 +1582,13 @@ namespace LLError clear() ; } - LLError::Log::flush(_out, sBuffer[sIndex++]) ; + LLError::Log::flush(_out, sBuffer[sIndex++]) ; } //static void LLCallStacks::print() { - LLMutexTrylock lock(&gCallStacksLogMutex, 5); + LLMutexTrylock lock(getMutex(), 5); if (!lock.isLocked()) { return; @@ -1609,7 +1626,7 @@ namespace LLError bool debugLoggingEnabled(const std::string& tag) { - LLMutexTrylock lock(&gLogMutex, 5); + LLMutexTrylock lock(getMutex(), 5); if (!lock.isLocked()) { return false; -- cgit v1.3 From a6f31e9167c75982bb5eaba96ec6ac1f70ee31fb Mon Sep 17 00:00:00 2001 From: Brad Kittenbrink Date: Fri, 8 Mar 2019 13:39:56 -0800 Subject: Fixed variadic macro usage in LL_ERRS_IF and LL_WARNS_IF and improved LLError::shouldLogToStderr() behavior under xcode. --- indra/llcommon/llerror.cpp | 8 ++++---- indra/llcommon/llerror.h | 9 +++++++-- 2 files changed, 11 insertions(+), 6 deletions(-) (limited to 'indra/llcommon/llerror.cpp') diff --git a/indra/llcommon/llerror.cpp b/indra/llcommon/llerror.cpp index 4bf4827119..2d4898f7be 100644 --- a/indra/llcommon/llerror.cpp +++ b/indra/llcommon/llerror.cpp @@ -664,10 +664,10 @@ namespace // console log. It's generally considered bad form to spam too much // there. - // If stdin is a tty, assume the user launched from the command line and - // therefore wants to see stderr. Otherwise, assume we've been launched - // from the finder and shouldn't spam stderr. - return isatty(0); + // If stderr is a tty, assume the user launched from the command line or + // debugger and therefore wants to see stderr. Otherwise, assume we've + // been launched from the finder and shouldn't spam stderr. + return isatty(STDERR_FILENO); #else return true; #endif diff --git a/indra/llcommon/llerror.h b/indra/llcommon/llerror.h index 0a78229555..9613911531 100644 --- a/indra/llcommon/llerror.h +++ b/indra/llcommon/llerror.h @@ -381,8 +381,13 @@ typedef LLError::NoClassInfo _LL_CLASS_TO_LOG; #define LL_WARNS(...) lllog(LLError::LEVEL_WARN, false, ##__VA_ARGS__) #define LL_ERRS(...) lllog(LLError::LEVEL_ERROR, false, ##__VA_ARGS__) // alternative to llassert_always that prints explanatory message -#define LL_WARNS_IF(exp, ...) if (exp) LL_WARNS(##__VA_ARGS__) << "(" #exp ")" -#define LL_ERRS_IF(exp, ...) if (exp) LL_ERRS(##__VA_ARGS__) << "(" #exp ")" +// note ## token paste operator hack used above will only work in gcc following +// a comma and is completely unnecessary in VS since the comma is automatically +// suppressed +// https://gcc.gnu.org/onlinedocs/cpp/Variadic-Macros.html +// https://docs.microsoft.com/en-us/cpp/preprocessor/variadic-macros?view=vs-2015 +#define LL_WARNS_IF(exp, ...) if (exp) LL_WARNS(__VA_ARGS__) << "(" #exp ")" +#define LL_ERRS_IF(exp, ...) if (exp) LL_ERRS(__VA_ARGS__) << "(" #exp ")" // Only print the log message once (good for warnings or infos that would otherwise // spam the log file over and over, such as tighter loops). -- cgit v1.3 From 6b70493ddb1b95a2d3527e2189f5b94f5a2b606f Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Mon, 14 Oct 2019 15:41:09 -0400 Subject: DRTVWR-476: Make test program --debug switch work like LOGTEST=DEBUG. The comments within indra/test/test.cpp promise that --debug is, in fact, like LOGTEST=DEBUG. Until now, that was a lie. LOGTEST=level displayed log output on stderr as well as in testprogram.log, while --debug did not. Add LLError::logToStderr() function, and make initForApplication() (i.e. commonInit()) call that instead of instantiating RecordToStderr inline. Also call it when test.cpp recognizes --debug switch. Remove the mFileRecorder, mFixedBufferRecorder and mFileRecorderFileName members from SettingsConfig. That tactic doesn't scale. Instead, add findRecorder() and removeRecorder() template functions to locate (or remove) a RecorderPtr to an object of the specified subclass. Both are based on an underlying findRecorderPos() template function. Since we never expect to manage more than a handful of RecorderPtrs, and since access to the deleted members is very much application setup rather than any kind of ongoing access, a search loop suffices. logToFile() uses removeRecorder() rather than removing mFileRecorder (the only use of mFileRecorder). logToFixedBuffer() uses removeRecorder() rather than removing mFixedBufferRecorder (the only use of mFixedBufferRecorder). Make RecordToFile store the filename with which it was instantiated. Add a getFilename() method to retrieve it. logFileName() is now based on findRecorder() instead of mFileRecorderFileName (the only use of mFileRecorderFileName). Make RecordToStderr::mUseANSI a simple bool rather than a three-state enum, and set it immediately on construction. Apparently the reason it was set lazily was because it consults its own checkANSI() method, and of course 'this' doesn't acquire the leaf class type until the constructor has completed successfully. But since nothing in checkANSI() depends on anything else in RecordToStderr, making it static solves that problem. --- indra/llcommon/llerror.cpp | 196 +++++++++++++++++++++++++--------------- indra/llcommon/llerrorcontrol.h | 1 + indra/test/test.cpp | 3 + 3 files changed, 127 insertions(+), 73 deletions(-) (limited to 'indra/llcommon/llerror.cpp') diff --git a/indra/llcommon/llerror.cpp b/indra/llcommon/llerror.cpp index 2d4898f7be..acd863a316 100644 --- a/indra/llcommon/llerror.cpp +++ b/indra/llcommon/llerror.cpp @@ -118,27 +118,28 @@ namespace { class RecordToFile : public LLError::Recorder { public: - RecordToFile(const std::string& filename) + RecordToFile(const std::string& filename): + mName(filename) { mFile.open(filename.c_str(), std::ios_base::out | std::ios_base::app); if (!mFile) { LL_INFOS() << "Error setting log file to " << filename << LL_ENDL; } - else - { - if (!LLError::getAlwaysFlush()) - { - mFile.sync_with_stdio(false); - } - } + else + { + if (!LLError::getAlwaysFlush()) + { + mFile.sync_with_stdio(false); + } + } } - + ~RecordToFile() { mFile.close(); } - + virtual bool enabled() override { #ifdef LL_RELEASE_FOR_DOWNLOAD @@ -148,11 +149,13 @@ namespace { #endif } - bool okay() { return mFile.good(); } - - virtual void recordMessage(LLError::ELevel level, - const std::string& message) override - { + bool okay() const { return mFile.good(); } + + std::string getFilename() const { return mName; } + + virtual void recordMessage(LLError::ELevel level, + const std::string& message) override + { if (LLError::getAlwaysFlush()) { mFile << message << std::endl; @@ -161,9 +164,10 @@ namespace { { mFile << message << "\n"; } - } - + } + private: + const std::string mName; llofstream mFile; }; @@ -171,7 +175,7 @@ namespace { class RecordToStderr : public LLError::Recorder { public: - RecordToStderr(bool timestamp) : mUseANSI(ANSI_PROBE) + RecordToStderr(bool timestamp) : mUseANSI(checkANSI()) { this->showMultiline(true); } @@ -184,10 +188,7 @@ namespace { virtual void recordMessage(LLError::ELevel level, const std::string& message) override { - if (ANSI_PROBE == mUseANSI) - mUseANSI = (checkANSI() ? ANSI_YES : ANSI_NO); - - if (ANSI_YES == mUseANSI) + if (mUseANSI) { // Default all message levels to bold so we can distinguish our own messages from those dumped by subprocesses and libraries. colorANSI("1"); // bold @@ -206,16 +207,11 @@ namespace { } } fprintf(stderr, "%s\n", message.c_str()); - if (ANSI_YES == mUseANSI) colorANSI("0"); // reset + if (mUseANSI) colorANSI("0"); // reset } private: - enum ANSIState - { - ANSI_PROBE, - ANSI_YES, - ANSI_NO - } mUseANSI; + bool mUseANSI; void colorANSI(const std::string color) { @@ -223,7 +219,7 @@ namespace { fprintf(stderr, "\033[%sm", color.c_str() ); }; - bool checkANSI(void) + static bool checkANSI(void) { #if LL_LINUX || LL_DARWIN // Check whether it's okay to use ANSI; if stderr is @@ -491,14 +487,11 @@ namespace LLError LLError::FatalFunction mCrashFunction; LLError::TimeFunction mTimeFunction; - + Recorders mRecorders; - RecorderPtr mFileRecorder; - RecorderPtr mFixedBufferRecorder; - std::string mFileRecorderFileName; - - int mShouldLogCallCounter; - + + int mShouldLogCallCounter; + private: SettingsConfig(); }; @@ -532,9 +525,6 @@ namespace LLError mCrashFunction(NULL), mTimeFunction(NULL), mRecorders(), - mFileRecorder(), - mFixedBufferRecorder(), - mFileRecorderFileName(), mShouldLogCallCounter(0) { } @@ -686,20 +676,19 @@ namespace void commonInit(const std::string& user_dir, const std::string& app_dir, bool log_to_stderr = true) { LLError::Settings::getInstance()->reset(); - + LLError::setDefaultLevel(LLError::LEVEL_INFO); - LLError::setAlwaysFlush(true); - LLError::setEnabledLogTypesMask(0xFFFFFFFF); + 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 if (log_to_stderr && shouldLogToStderr()) { - LLError::RecorderPtr recordToStdErr(new RecordToStderr(stderrLogWantsTime())); - LLError::addRecorder(recordToStdErr); + LLError::logToStderr(); } - + #if LL_WINDOWS LLError::RecorderPtr recordToWinDebug(new RecordToWinDebug()); LLError::addRecorder(recordToWinDebug); @@ -997,49 +986,110 @@ namespace LLError s->mRecorders.erase(std::remove(s->mRecorders.begin(), s->mRecorders.end(), recorder), s->mRecorders.end()); } + + // Find an entry in SettingsConfig::mRecorders whose RecorderPtr points to + // a Recorder subclass of type RECORDER. Return, not a RecorderPtr (which + // points to the Recorder base class), but a shared_ptr which + // specifically points to the concrete RECORDER subclass instance, along + // with a Recorders::iterator indicating the position of that entry in + // mRecorders. The shared_ptr might be empty (operator!() returns true) if + // there was no such RECORDER subclass instance in mRecorders. + template + std::pair, Recorders::iterator> + findRecorderPos() + { + SettingsConfigPtr s = Settings::instance().getSettingsConfig(); + // Since we promise to return an iterator, use a classic iterator + // loop. + auto end{s->mRecorders.end()}; + for (Recorders::iterator it{s->mRecorders.begin()}; it != end; ++it) + { + // *it is a RecorderPtr, a shared_ptr. Use a + // dynamic_pointer_cast to try to downcast to test if it's also a + // shared_ptr. + auto ptr = boost::dynamic_pointer_cast(*it); + if (ptr) + { + // found the entry we want + return { ptr, it }; + } + } + // dropped out of the loop without finding any such entry -- instead + // of default-constructing Recorders::iterator (which might or might + // not be valid), return a value that is valid but not dereferenceable. + return { {}, end }; + } + + // Find an entry in SettingsConfig::mRecorders whose RecorderPtr points to + // a Recorder subclass of type RECORDER. Return, not a RecorderPtr (which + // points to the Recorder base class), but a shared_ptr which + // specifically points to the concrete RECORDER subclass instance. The + // shared_ptr might be empty (operator!() returns true) if there was no + // such RECORDER subclass instance in mRecorders. + template + boost::shared_ptr findRecorder() + { + return findRecorderPos().first; + } + + // Remove an entry from SettingsConfig::mRecorders whose RecorderPtr + // points to a Recorder subclass of type RECORDER. Return true if there + // was one and we removed it, false if there wasn't one to start with. + template + bool removeRecorder() + { + auto found = findRecorderPos(); + if (found.first) + { + SettingsConfigPtr s = Settings::instance().getSettingsConfig(); + s->mRecorders.erase(found.second); + } + return bool(found.first); + } } namespace LLError { void logToFile(const std::string& file_name) { - SettingsConfigPtr s = Settings::getInstance()->getSettingsConfig(); + // remove any previous Recorder filling this role + removeRecorder(); - removeRecorder(s->mFileRecorder); - s->mFileRecorder.reset(); - s->mFileRecorderFileName.clear(); - if (!file_name.empty()) { - RecorderPtr recordToFile(new RecordToFile(file_name)); - if (boost::dynamic_pointer_cast(recordToFile)->okay()) - { - s->mFileRecorderFileName = file_name; - s->mFileRecorder = recordToFile; - addRecorder(recordToFile); - } + boost::shared_ptr recordToFile(new RecordToFile(file_name)); + if (recordToFile->okay()) + { + addRecorder(recordToFile); + } } } - - void logToFixedBuffer(LLLineBuffer* fixedBuffer) + + std::string logFileName() { - SettingsConfigPtr s = Settings::getInstance()->getSettingsConfig(); + auto found = findRecorder(); + return found? found->getFilename() : std::string(); + } - removeRecorder(s->mFixedBufferRecorder); - s->mFixedBufferRecorder.reset(); - - if (fixedBuffer) - { - RecorderPtr recordToFixedBuffer(new RecordToFixedBuffer(fixedBuffer)); - s->mFixedBufferRecorder = recordToFixedBuffer; - addRecorder(recordToFixedBuffer); + void logToStderr() + { + if (! findRecorder()) + { + RecorderPtr recordToStdErr(new RecordToStderr(stderrLogWantsTime())); + addRecorder(recordToStdErr); } - } + } - std::string logFileName() + void logToFixedBuffer(LLLineBuffer* fixedBuffer) { - SettingsConfigPtr s = Settings::getInstance()->getSettingsConfig(); - return s->mFileRecorderFileName; + // remove any previous Recorder filling this role + removeRecorder(); + + if (fixedBuffer) + { + RecorderPtr recordToFixedBuffer(new RecordToFixedBuffer(fixedBuffer)); + addRecorder(recordToFixedBuffer); + } } } diff --git a/indra/llcommon/llerrorcontrol.h b/indra/llcommon/llerrorcontrol.h index 276d22fc36..bfa2269025 100644 --- a/indra/llcommon/llerrorcontrol.h +++ b/indra/llcommon/llerrorcontrol.h @@ -183,6 +183,7 @@ namespace LLError // each error message is passed to each recorder via recordMessage() LL_COMMON_API void logToFile(const std::string& filename); + LL_COMMON_API void logToStderr(); LL_COMMON_API void logToFixedBuffer(LLLineBuffer*); // Utilities to add recorders for logging to a file or a fixed buffer // A second call to the same function will remove the logger added diff --git a/indra/test/test.cpp b/indra/test/test.cpp index b14c2eb255..51f0e80043 100644 --- a/indra/test/test.cpp +++ b/indra/test/test.cpp @@ -611,6 +611,9 @@ int main(int argc, char **argv) wait_at_exit = true; break; case 'd': + // this is what LLError::initForApplication() does internally + // when you pass log_to_stderr=true + LLError::logToStderr(); LLError::setDefaultLevel(LLError::LEVEL_DEBUG); break; case 'x': -- cgit v1.3 From 7845f73c7691d338ef92d653be12337215ff0f49 Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Mon, 28 Oct 2019 14:33:36 -0400 Subject: DRTVWR-476: Try to log stderr output from classic-C libraries. Some of the libraries we use produce log output to stderr. Such output can be informative, but is invisible unless you launch the viewer from a console. In particular, it's invisible to anyone trying to diagnose a problem by reading someone else's SecondLife.log file. Make RecordToFile -- the Recorder subclass engaged by LLError::logToFile() -- redirect STDERR_FILENO to the newly-opened log file so that any subsequent writes to stderr (or cerr, for that matter) will be captured in the log file. But first duplicate the original stderr file handle, and restore it when RecordToFile is destroyed. That way, output written to stderr during the final moments of application shutdown should still appear on (console) stderr. --- indra/llcommon/llerror.cpp | 68 ++++++++++++++++++++++++++-------------------- 1 file changed, 38 insertions(+), 30 deletions(-) (limited to 'indra/llcommon/llerror.cpp') diff --git a/indra/llcommon/llerror.cpp b/indra/llcommon/llerror.cpp index acd863a316..6ef0fde886 100644 --- a/indra/llcommon/llerror.cpp +++ b/indra/llcommon/llerror.cpp @@ -119,59 +119,67 @@ namespace { { public: RecordToFile(const std::string& filename): - mName(filename) + mName(filename), + mFile(LLFile::fopen(filename, "a")) { - mFile.open(filename.c_str(), std::ios_base::out | std::ios_base::app); if (!mFile) { - LL_INFOS() << "Error setting log file to " << filename << LL_ENDL; + LL_WARNS() << "Error setting log file to " << filename << LL_ENDL; } else { - if (!LLError::getAlwaysFlush()) - { - mFile.sync_with_stdio(false); - } +#if LL_DARWIN || LL_LINUX + // We use a number of classic-C libraries, some of which write + // log output to stderr. The trouble with that is that unless + // you launch the viewer from a console, stderr output is + // lost. Redirect STDERR_FILENO to write into this log file. + // But first, save the original stream in case we want it later. + mSavedStderr = ::dup(STDERR_FILENO); + ::dup2(::fileno(mFile), STDERR_FILENO); +#endif } } ~RecordToFile() { +#if LL_DARWIN || LL_LINUX + // restore stderr to its original fileno so any subsequent output + // to stderr goes to original stream + ::dup2(mSavedStderr, STDERR_FILENO); +#endif mFile.close(); } - virtual bool enabled() override - { + virtual bool enabled() override + { #ifdef LL_RELEASE_FOR_DOWNLOAD - return 1; + return 1; #else - return LLError::getEnabledLogTypesMask() & 0x02; + return LLError::getEnabledLogTypesMask() & 0x02; #endif - } - - bool okay() const { return mFile.good(); } + } - std::string getFilename() const { return mName; } + bool okay() const { return bool(mFile); } - virtual void recordMessage(LLError::ELevel level, - const std::string& message) override - { - if (LLError::getAlwaysFlush()) - { - mFile << message << std::endl; - } - else - { - mFile << message << "\n"; - } - } + std::string getFilename() const { return mName; } + + virtual void recordMessage(LLError::ELevel level, + const std::string& message) override + { + fwrite(message.c_str(), sizeof(char), message.length(), mFile); + if (LLError::getAlwaysFlush()) + { + ::fflush(mFile); + } + } private: const std::string mName; - llofstream mFile; + LLUniqueFile mFile; + int mSavedStderr{0}; }; - - + + class RecordToStderr : public LLError::Recorder { public: -- cgit v1.3 From 07134aaee7a39f2141ee8f1e702aa0df989851df Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Mon, 28 Oct 2019 17:15:40 -0400 Subject: DRTVWR-476: Try to extend stderr redirection to Windows as well. Make the LLError::Settings LLSingleton duplicate the file handle for stderr (usually 2) on construction. Make its destructor restore the original target for that file handle. Provide a getDupStderr() method to obtain the duplicate file handle. Move Settings declaration up to the top of the file so other code can reference it. Make RecordToFile (the Recorder subclass engaged by LLError::logToFile()), instead of duplicating stderr's file handle itself, capture the duplicate stderr file handle from Settings to revert stderr redirection on destruction. Make RecordToStderr (the Recorder subclass engaged by LLError::logToStderr()) use fdopen() to create an LLFILE* targeting the duplicate file handle from Settings. Write output to that instead of to stderr so logToStderr() continues to provide output for the user instead of duplicating each line into the log file. --- indra/llcommon/llerror.cpp | 118 ++++++++++++++++++++++++++++++--------------- 1 file changed, 79 insertions(+), 39 deletions(-) (limited to 'indra/llcommon/llerror.cpp') diff --git a/indra/llcommon/llerror.cpp b/indra/llcommon/llerror.cpp index 6ef0fde886..a8e1481774 100644 --- a/indra/llcommon/llerror.cpp +++ b/indra/llcommon/llerror.cpp @@ -53,6 +53,46 @@ #include "llstl.h" #include "lltimer.h" +#if LL_WINDOWS +#define fhclose _close +#define fhdup _dup +#define fhdup2 _dup2 +#define fhfdopen _fdopen +#define fhfileno _fileno +#else +#define fhclose ::close +#define fhdup ::dup +#define fhdup2 ::dup2 +#define fhfdopen ::fdopen +#define fhfileno ::fileno +#endif + +namespace LLError +{ + + class SettingsConfig; + typedef LLPointer SettingsConfigPtr; + + class Settings : public LLSingleton + { + LLSINGLETON(Settings); + public: + SettingsConfigPtr getSettingsConfig(); + ~Settings(); + + void reset(); + SettingsStoragePtr saveAndReset(); + void restore(SettingsStoragePtr pSettingsStorage); + + int getDupStderr() const; + + private: + SettingsConfigPtr mSettingsConfig; + int mDupStderr; + }; + +} // namespace LLError + namespace { #if LL_WINDOWS void debugger_print(const std::string& s) @@ -120,7 +160,8 @@ namespace { public: RecordToFile(const std::string& filename): mName(filename), - mFile(LLFile::fopen(filename, "a")) + mFile(LLFile::fopen(filename, "a")), + mSavedStderr(LLError::Settings::instance().getDupStderr()) { if (!mFile) { @@ -128,25 +169,19 @@ namespace { } else { -#if LL_DARWIN || LL_LINUX // We use a number of classic-C libraries, some of which write // log output to stderr. The trouble with that is that unless // you launch the viewer from a console, stderr output is // lost. Redirect STDERR_FILENO to write into this log file. - // But first, save the original stream in case we want it later. - mSavedStderr = ::dup(STDERR_FILENO); - ::dup2(::fileno(mFile), STDERR_FILENO); -#endif + fhdup2(fhfileno(mFile), fhfileno(stderr)); } } ~RecordToFile() { -#if LL_DARWIN || LL_LINUX // restore stderr to its original fileno so any subsequent output // to stderr goes to original stream - ::dup2(mSavedStderr, STDERR_FILENO); -#endif + fhdup2(mSavedStderr, fhfileno(stderr)); mFile.close(); } @@ -166,7 +201,8 @@ namespace { virtual void recordMessage(LLError::ELevel level, const std::string& message) override { - fwrite(message.c_str(), sizeof(char), message.length(), mFile); + ::fwrite(message.c_str(), sizeof(char), message.length(), mFile); + ::fputc('\n', mFile); if (LLError::getAlwaysFlush()) { ::fflush(mFile); @@ -176,22 +212,26 @@ namespace { private: const std::string mName; LLUniqueFile mFile; - int mSavedStderr{0}; + int mSavedStderr; }; class RecordToStderr : public LLError::Recorder { public: - RecordToStderr(bool timestamp) : mUseANSI(checkANSI()) + RecordToStderr(bool timestamp) : + mUseANSI(checkANSI()), + // use duplicate stderr file handle so THIS output isn't affected + // by our internal redirection of all (other) stderr output + mStderr(fhfdopen(LLError::Settings::instance().getDupStderr(), "a")) { - this->showMultiline(true); + this->showMultiline(true); + } + + virtual bool enabled() override + { + return LLError::getEnabledLogTypesMask() & 0x04; } - - virtual bool enabled() override - { - return LLError::getEnabledLogTypesMask() & 0x04; - } virtual void recordMessage(LLError::ELevel level, const std::string& message) override @@ -214,17 +254,18 @@ namespace { break; } } - fprintf(stderr, "%s\n", message.c_str()); + fprintf(mStderr, "%s\n", message.c_str()); if (mUseANSI) colorANSI("0"); // reset } - + private: bool mUseANSI; + LLFILE* mStderr; void colorANSI(const std::string color) { // ANSI color code escape sequence - fprintf(stderr, "\033[%sm", color.c_str() ); + fprintf(mStderr, "\033[%sm", color.c_str() ); }; static bool checkANSI(void) @@ -233,7 +274,7 @@ namespace { // Check whether it's okay to use ANSI; if stderr is // a tty then we assume yes. Can be turned off with // the LL_NO_ANSI_COLOR env var. - return (0 != isatty(2)) && + return (0 != isatty(fhfileno(stderr))) && (NULL == getenv("LL_NO_ANSI_COLOR")); #endif // LL_LINUX return false; @@ -504,22 +545,6 @@ namespace LLError SettingsConfig(); }; - typedef LLPointer SettingsConfigPtr; - - class Settings : public LLSingleton - { - LLSINGLETON(Settings); - public: - SettingsConfigPtr getSettingsConfig(); - - void reset(); - SettingsStoragePtr saveAndReset(); - void restore(SettingsStoragePtr pSettingsStorage); - - private: - SettingsConfigPtr mSettingsConfig; - }; - SettingsConfig::SettingsConfig() : LLRefCount(), mDefaultLevel(LLError::LEVEL_DEBUG), @@ -543,8 +568,18 @@ namespace LLError } Settings::Settings(): - mSettingsConfig(new SettingsConfig()) + mSettingsConfig(new SettingsConfig()), + // duplicate stderr file handle right away + mDupStderr(fhdup(fhfileno(stderr))) + { + } + + Settings::~Settings() { + // restore original stderr + fhdup2(mDupStderr, fhfileno(stderr)); + // and close the duplicate + fhclose(mDupStderr); } SettingsConfigPtr Settings::getSettingsConfig() @@ -572,6 +607,11 @@ namespace LLError mSettingsConfig = newSettingsConfig; } + int Settings::getDupStderr() const + { + return mDupStderr; + } + bool is_available() { return Settings::instanceExists() && Globals::instanceExists(); -- cgit v1.3 From 7f1a2002142dfcda3cbada729d4fbe961b3bc7bb Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Tue, 29 Oct 2019 07:32:10 -0400 Subject: DRTVWR-476: On Windows, dup2() et al. need --- indra/llcommon/llerror.cpp | 2 ++ 1 file changed, 2 insertions(+) (limited to 'indra/llcommon/llerror.cpp') diff --git a/indra/llcommon/llerror.cpp b/indra/llcommon/llerror.cpp index a8e1481774..457965b1fd 100644 --- a/indra/llcommon/llerror.cpp +++ b/indra/llcommon/llerror.cpp @@ -39,6 +39,8 @@ #if !LL_WINDOWS # include # include +#else +# include #endif // !LL_WINDOWS #include #include "string.h" -- cgit v1.3 From ec2bd40d3e318baf6f22ee7a7ccbc57cb071af40 Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Thu, 31 Oct 2019 12:39:31 -0400 Subject: DRTVWR-476: Encapsulate dup()/dup2() fd saving as LLTempRedirect. --- indra/llcommon/CMakeLists.txt | 2 + indra/llcommon/llerror.cpp | 48 +++---------- indra/llcommon/lltempredirect.cpp | 138 ++++++++++++++++++++++++++++++++++++++ indra/llcommon/lltempredirect.h | 91 +++++++++++++++++++++++++ 4 files changed, 241 insertions(+), 38 deletions(-) create mode 100644 indra/llcommon/lltempredirect.cpp create mode 100644 indra/llcommon/lltempredirect.h (limited to 'indra/llcommon/llerror.cpp') diff --git a/indra/llcommon/CMakeLists.txt b/indra/llcommon/CMakeLists.txt index 035b379246..d17ee4c70a 100644 --- a/indra/llcommon/CMakeLists.txt +++ b/indra/llcommon/CMakeLists.txt @@ -104,6 +104,7 @@ set(llcommon_SOURCE_FILES llstring.cpp llstringtable.cpp llsys.cpp + lltempredirect.cpp llthread.cpp llthreadlocalstorage.cpp llthreadsafequeue.cpp @@ -228,6 +229,7 @@ set(llcommon_HEADER_FILES llstaticstringtable.h llstatsaccumulator.h llsys.h + lltempredirect.h llthread.h llthreadlocalstorage.h llthreadsafequeue.h diff --git a/indra/llcommon/llerror.cpp b/indra/llcommon/llerror.cpp index 457965b1fd..2ae2cb6cbc 100644 --- a/indra/llcommon/llerror.cpp +++ b/indra/llcommon/llerror.cpp @@ -39,8 +39,6 @@ #if !LL_WINDOWS # include # include -#else -# include #endif // !LL_WINDOWS #include #include "string.h" @@ -54,20 +52,7 @@ #include "llsingleton.h" #include "llstl.h" #include "lltimer.h" - -#if LL_WINDOWS -#define fhclose _close -#define fhdup _dup -#define fhdup2 _dup2 -#define fhfdopen _fdopen -#define fhfileno _fileno -#else -#define fhclose ::close -#define fhdup ::dup -#define fhdup2 ::dup2 -#define fhfdopen ::fdopen -#define fhfileno ::fileno -#endif +#include "lltempredirect.h" namespace LLError { @@ -80,7 +65,6 @@ namespace LLError LLSINGLETON(Settings); public: SettingsConfigPtr getSettingsConfig(); - ~Settings(); void reset(); SettingsStoragePtr saveAndReset(); @@ -90,7 +74,7 @@ namespace LLError private: SettingsConfigPtr mSettingsConfig; - int mDupStderr; + LLTempRedirect mRedirect; }; } // namespace LLError @@ -162,8 +146,7 @@ namespace { public: RecordToFile(const std::string& filename): mName(filename), - mFile(LLFile::fopen(filename, "a")), - mSavedStderr(LLError::Settings::instance().getDupStderr()) + mFile(LLFile::fopen(filename, "a")) { if (!mFile) { @@ -174,16 +157,13 @@ namespace { // We use a number of classic-C libraries, some of which write // log output to stderr. The trouble with that is that unless // you launch the viewer from a console, stderr output is - // lost. Redirect STDERR_FILENO to write into this log file. - fhdup2(fhfileno(mFile), fhfileno(stderr)); + // lost. Redirect stderr to write into this log file. + mRedirect = LLTempRedirect(mFile, stderr); } } ~RecordToFile() { - // restore stderr to its original fileno so any subsequent output - // to stderr goes to original stream - fhdup2(mSavedStderr, fhfileno(stderr)); mFile.close(); } @@ -214,7 +194,7 @@ namespace { private: const std::string mName; LLUniqueFile mFile; - int mSavedStderr; + LLTempRedirect mRedirect; }; @@ -225,7 +205,7 @@ namespace { mUseANSI(checkANSI()), // use duplicate stderr file handle so THIS output isn't affected // by our internal redirection of all (other) stderr output - mStderr(fhfdopen(LLError::Settings::instance().getDupStderr(), "a")) + mStderr(llfd::open(LLError::Settings::instance().getDupStderr(), "a")) { this->showMultiline(true); } @@ -276,7 +256,7 @@ namespace { // Check whether it's okay to use ANSI; if stderr is // a tty then we assume yes. Can be turned off with // the LL_NO_ANSI_COLOR env var. - return (0 != isatty(fhfileno(stderr))) && + return (0 != isatty(fileno(stderr))) && (NULL == getenv("LL_NO_ANSI_COLOR")); #endif // LL_LINUX return false; @@ -572,16 +552,8 @@ namespace LLError Settings::Settings(): mSettingsConfig(new SettingsConfig()), // duplicate stderr file handle right away - mDupStderr(fhdup(fhfileno(stderr))) - { - } - - Settings::~Settings() + mRedirect(NULL, stderr) { - // restore original stderr - fhdup2(mDupStderr, fhfileno(stderr)); - // and close the duplicate - fhclose(mDupStderr); } SettingsConfigPtr Settings::getSettingsConfig() @@ -611,7 +583,7 @@ namespace LLError int Settings::getDupStderr() const { - return mDupStderr; + return mRedirect.getOriginalTarget(); } bool is_available() diff --git a/indra/llcommon/lltempredirect.cpp b/indra/llcommon/lltempredirect.cpp new file mode 100644 index 0000000000..1ae3116b77 --- /dev/null +++ b/indra/llcommon/lltempredirect.cpp @@ -0,0 +1,138 @@ +/** + * @file lltempredirect.cpp + * @author Nat Goodspeed + * @date 2019-10-31 + * @brief Implementation for lltempredirect. + * + * $LicenseInfo:firstyear=2019&license=viewerlgpl$ + * Copyright (c) 2019, Linden Research, Inc. + * $/LicenseInfo$ + */ + +// Precompiled header +#include "linden_common.h" +// associated header +#include "lltempredirect.h" +// STL headers +// std headers +#if !LL_WINDOWS +# include +#else +# include +#endif // !LL_WINDOWS +// external library headers +// other Linden headers + +/***************************************************************************** +* llfd +*****************************************************************************/ +// We could restate the implementation of each of llfd::close(), etc., but +// this is way more succinct. +#if LL_WINDOWS +#define fhclose _close +#define fhdup _dup +#define fhdup2 _dup2 +#define fhfdopen _fdopen +#define fhfileno _fileno +#else +#define fhclose ::close +#define fhdup ::dup +#define fhdup2 ::dup2 +#define fhfdopen ::fdopen +#define fhfileno ::fileno +#endif + +int llfd::close(int fd) +{ + return fhclose(fd); +} + +int llfd::dup(int target) +{ + return fhdup(target); +} + +int llfd::dup2(int target, int reference) +{ + return fhdup2(target, reference); +} + +FILE* llfd::open(int fd, const char* mode) +{ + return fhfdopen(fd, mode); +} + +int llfd::fileno(FILE* stream) +{ + return fhfileno(stream); +} + +/***************************************************************************** +* LLTempRedirect +*****************************************************************************/ +LLTempRedirect::LLTempRedirect(): + mOrigTarget(-1), // -1 is an invalid file descriptor + mReference(-1) +{} + +LLTempRedirect::LLTempRedirect(FILE* target, FILE* reference): + LLTempRedirect((target? fhfileno(target) : -1), + (reference? fhfileno(reference) : -1)) +{} + +LLTempRedirect::LLTempRedirect(int target, int reference): + // capture a duplicate file descriptor for the file originally targeted by + // 'reference' + mOrigTarget((reference >= 0)? fhdup(reference) : -1), + mReference(reference) +{ + if (target >= 0 && reference >= 0) + { + // As promised, force 'reference' to refer to 'target'. This first + // implicitly closes 'reference', which is why we first capture a + // duplicate so the original target file stays open. + fhdup2(target, reference); + } +} + +LLTempRedirect::LLTempRedirect(LLTempRedirect&& other) +{ + mOrigTarget = other.mOrigTarget; + mReference = other.mReference; + // other LLTempRedirect must be in moved-from state so its destructor + // won't repeat the same operations as ours! + other.mOrigTarget = -1; + other.mReference = -1; +} + +LLTempRedirect::~LLTempRedirect() +{ + reset(); +} + +void LLTempRedirect::reset() +{ + // If this instance was default-constructed (or constructed with an + // invalid file descriptor), skip the following. + if (mOrigTarget >= 0) + { + // Restore mReference to point to mOrigTarget. This implicitly closes + // the duplicate created by our constructor of its 'target' file + // descriptor. + fhdup2(mOrigTarget, mReference); + // mOrigTarget has served its purpose + fhclose(mOrigTarget); + // assign these because reset() is also responsible for a "moved from" + // instance + mOrigTarget = -1; + mReference = -1; + } +} + +LLTempRedirect& LLTempRedirect::operator=(LLTempRedirect&& other) +{ + reset(); + std::swap(mOrigTarget, other.mOrigTarget); + std::swap(mReference, other.mReference); + return *this; +} diff --git a/indra/llcommon/lltempredirect.h b/indra/llcommon/lltempredirect.h new file mode 100644 index 0000000000..33e05dc06b --- /dev/null +++ b/indra/llcommon/lltempredirect.h @@ -0,0 +1,91 @@ +/** + * @file lltempredirect.h + * @author Nat Goodspeed + * @date 2019-10-31 + * @brief RAII low-level file-descriptor redirection + * + * $LicenseInfo:firstyear=2019&license=viewerlgpl$ + * Copyright (c) 2019, Linden Research, Inc. + * $/LicenseInfo$ + */ + +#if ! defined(LL_LLTEMPREDIRECT_H) +#define LL_LLTEMPREDIRECT_H + +// Functions in this namespace are intended to insulate the caller from the +// aggravating distinction between ::close() and Microsoft _close(). +namespace llfd +{ + +int close(int fd); +int dup(int target); +int dup2(int target, int reference); +FILE* open(int fd, const char* mode); +int fileno(FILE* stream); + +} // namespace llfd + +/** + * LLTempRedirect is an RAII class that performs file redirection on low-level + * file descriptors, expressed as ints. (Use llfd::fileno() to obtain the file + * descriptor from a classic-C FILE*. There is no portable way to obtain the + * file descriptor from a std::fstream.) + * + * Instantiate LLTempRedirect with a target file descriptor (e.g. for some + * open file) and a reference file descriptor (e.g. for stderr). From that + * point until the LLTempRedirect instance is destroyed, all OS-level writes + * to the reference file descriptor will be redirected to the target file. + * + * Because dup2() is used for redirection, the original passed target file + * descriptor remains open. If you want LLTempRedirect's destructor to close + * the target file, close() the target file descriptor after passing it to + * LLTempRedirect's constructor. + * + * LLTempRedirect's constructor saves the original target of the reference + * file descriptor. Its destructor restores the reference file descriptor to + * point once again to its original target. + */ +class LLTempRedirect +{ +public: + LLTempRedirect(); + /** + * For the lifespan of this LLTempRedirect instance, all writes to + * 'reference' will be redirected to 'target'. When this LLTempRedirect is + * destroyed, the original target for 'reference' will be restored. + * + * Pass 'target' as NULL if you simply want to save and restore + * 'reference' against possible redirection in the meantime. + */ + LLTempRedirect(FILE* target, FILE* reference); + /** + * For the lifespan of this LLTempRedirect instance, all writes to + * 'reference' will be redirected to 'target'. When this LLTempRedirect is + * destroyed, the original target for 'reference' will be restored. + * + * Pass 'target' as -1 if you simply want to save and restore + * 'reference' against possible redirection in the meantime. + */ + LLTempRedirect(int target, int reference); + LLTempRedirect(const LLTempRedirect&) = delete; + LLTempRedirect(LLTempRedirect&& other); + + ~LLTempRedirect(); + + LLTempRedirect& operator=(const LLTempRedirect&) = delete; + LLTempRedirect& operator=(LLTempRedirect&& other); + + /// returns (duplicate file descriptor for) the original target of the + /// 'reference' file descriptor passed to our constructor + int getOriginalTarget() const { return mOrigTarget; } + /// returns the original 'reference' file descriptor passed to our + /// constructor + int getReference() const { return mReference; } + +private: + void reset(); + + int mOrigTarget, mReference; +}; + +#endif /* ! defined(LL_LLTEMPREDIRECT_H) */ -- cgit v1.3 From 7ef10fe11c0221ae4ac1a46eae378aafc178296d Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Tue, 12 Nov 2019 14:47:13 -0500 Subject: DRTVWR-476: Don't test configuration.emptyMap(). LLSD::emptyMap() is a factory for an empty map instance, NOT a predicate on any particular instance. In fact checking configuration.isUndefined() and testing whether the map is empty are both subsumed by (! configuration). --- indra/llcommon/llerror.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'indra/llcommon/llerror.cpp') diff --git a/indra/llcommon/llerror.cpp b/indra/llcommon/llerror.cpp index 2ae2cb6cbc..9a475464f4 100644 --- a/indra/llcommon/llerror.cpp +++ b/indra/llcommon/llerror.cpp @@ -436,7 +436,7 @@ namespace return false; } - if (configuration.isUndefined() || !configuration.isMap() || configuration.emptyMap()) + if (! configuration || !configuration.isMap()) { LL_WARNS() << filename() << " missing, ill-formed, or simply undefined" " content; not changing configuration" -- cgit v1.3 From 950204a5d77c2681e369efdc45a7dc7b8348ee75 Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Tue, 12 Nov 2019 16:54:56 -0500 Subject: DRTVWR-476: Partially revert 978e09882565: undo using LLTempRedirect. But leave LLTempRedirect available in the code base. --- indra/llcommon/llerror.cpp | 48 ++++++++++++++++++++++++++++++++++++---------- 1 file changed, 38 insertions(+), 10 deletions(-) (limited to 'indra/llcommon/llerror.cpp') diff --git a/indra/llcommon/llerror.cpp b/indra/llcommon/llerror.cpp index 9a475464f4..d9f299b2d9 100644 --- a/indra/llcommon/llerror.cpp +++ b/indra/llcommon/llerror.cpp @@ -39,6 +39,8 @@ #if !LL_WINDOWS # include # include +#else +# include #endif // !LL_WINDOWS #include #include "string.h" @@ -52,7 +54,20 @@ #include "llsingleton.h" #include "llstl.h" #include "lltimer.h" -#include "lltempredirect.h" + +#if LL_WINDOWS +#define fhclose _close +#define fhdup _dup +#define fhdup2 _dup2 +#define fhfdopen _fdopen +#define fhfileno _fileno +#else +#define fhclose ::close +#define fhdup ::dup +#define fhdup2 ::dup2 +#define fhfdopen ::fdopen +#define fhfileno ::fileno +#endif namespace LLError { @@ -65,6 +80,7 @@ namespace LLError LLSINGLETON(Settings); public: SettingsConfigPtr getSettingsConfig(); + ~Settings(); void reset(); SettingsStoragePtr saveAndReset(); @@ -74,7 +90,7 @@ namespace LLError private: SettingsConfigPtr mSettingsConfig; - LLTempRedirect mRedirect; + int mDupStderr; }; } // namespace LLError @@ -146,7 +162,8 @@ namespace { public: RecordToFile(const std::string& filename): mName(filename), - mFile(LLFile::fopen(filename, "a")) + mFile(LLFile::fopen(filename, "a")), + mSavedStderr(LLError::Settings::instance().getDupStderr()) { if (!mFile) { @@ -157,13 +174,16 @@ namespace { // We use a number of classic-C libraries, some of which write // log output to stderr. The trouble with that is that unless // you launch the viewer from a console, stderr output is - // lost. Redirect stderr to write into this log file. - mRedirect = LLTempRedirect(mFile, stderr); + // lost. Redirect STDERR_FILENO to write into this log file. + fhdup2(fhfileno(mFile), fhfileno(stderr)); } } ~RecordToFile() { + // restore stderr to its original fileno so any subsequent output + // to stderr goes to original stream + fhdup2(mSavedStderr, fhfileno(stderr)); mFile.close(); } @@ -194,7 +214,7 @@ namespace { private: const std::string mName; LLUniqueFile mFile; - LLTempRedirect mRedirect; + int mSavedStderr; }; @@ -205,7 +225,7 @@ namespace { mUseANSI(checkANSI()), // use duplicate stderr file handle so THIS output isn't affected // by our internal redirection of all (other) stderr output - mStderr(llfd::open(LLError::Settings::instance().getDupStderr(), "a")) + mStderr(fhfdopen(LLError::Settings::instance().getDupStderr(), "a")) { this->showMultiline(true); } @@ -256,7 +276,7 @@ namespace { // Check whether it's okay to use ANSI; if stderr is // a tty then we assume yes. Can be turned off with // the LL_NO_ANSI_COLOR env var. - return (0 != isatty(fileno(stderr))) && + return (0 != isatty(fhfileno(stderr))) && (NULL == getenv("LL_NO_ANSI_COLOR")); #endif // LL_LINUX return false; @@ -552,8 +572,16 @@ namespace LLError Settings::Settings(): mSettingsConfig(new SettingsConfig()), // duplicate stderr file handle right away - mRedirect(NULL, stderr) + mDupStderr(fhdup(fhfileno(stderr))) + { + } + + Settings::~Settings() { + // restore original stderr + fhdup2(mDupStderr, fhfileno(stderr)); + // and close the duplicate + fhclose(mDupStderr); } SettingsConfigPtr Settings::getSettingsConfig() @@ -583,7 +611,7 @@ namespace LLError int Settings::getDupStderr() const { - return mRedirect.getOriginalTarget(); + return mDupStderr; } bool is_available() -- cgit v1.3 From d94e4613cace2e1f0215126b11c5c84375337d44 Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Tue, 12 Nov 2019 16:56:43 -0500 Subject: DRTVWR-476: Back out e66ec842b851: unrolling stderr redirection. --- indra/llcommon/llerror.cpp | 2 -- 1 file changed, 2 deletions(-) (limited to 'indra/llcommon/llerror.cpp') diff --git a/indra/llcommon/llerror.cpp b/indra/llcommon/llerror.cpp index d9f299b2d9..c0e1e443c4 100644 --- a/indra/llcommon/llerror.cpp +++ b/indra/llcommon/llerror.cpp @@ -39,8 +39,6 @@ #if !LL_WINDOWS # include # include -#else -# include #endif // !LL_WINDOWS #include #include "string.h" -- cgit v1.3 From 99d4ddc6687fff0fb93b16192c8f713766874bfe Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Tue, 12 Nov 2019 16:59:08 -0500 Subject: DRTVWR-476: Back out e913c05d43b6: unroll stderr redirection. --- indra/llcommon/llerror.cpp | 118 +++++++++++++++------------------------------ 1 file changed, 39 insertions(+), 79 deletions(-) (limited to 'indra/llcommon/llerror.cpp') diff --git a/indra/llcommon/llerror.cpp b/indra/llcommon/llerror.cpp index c0e1e443c4..188b76bbae 100644 --- a/indra/llcommon/llerror.cpp +++ b/indra/llcommon/llerror.cpp @@ -53,46 +53,6 @@ #include "llstl.h" #include "lltimer.h" -#if LL_WINDOWS -#define fhclose _close -#define fhdup _dup -#define fhdup2 _dup2 -#define fhfdopen _fdopen -#define fhfileno _fileno -#else -#define fhclose ::close -#define fhdup ::dup -#define fhdup2 ::dup2 -#define fhfdopen ::fdopen -#define fhfileno ::fileno -#endif - -namespace LLError -{ - - class SettingsConfig; - typedef LLPointer SettingsConfigPtr; - - class Settings : public LLSingleton - { - LLSINGLETON(Settings); - public: - SettingsConfigPtr getSettingsConfig(); - ~Settings(); - - void reset(); - SettingsStoragePtr saveAndReset(); - void restore(SettingsStoragePtr pSettingsStorage); - - int getDupStderr() const; - - private: - SettingsConfigPtr mSettingsConfig; - int mDupStderr; - }; - -} // namespace LLError - namespace { #if LL_WINDOWS void debugger_print(const std::string& s) @@ -160,8 +120,7 @@ namespace { public: RecordToFile(const std::string& filename): mName(filename), - mFile(LLFile::fopen(filename, "a")), - mSavedStderr(LLError::Settings::instance().getDupStderr()) + mFile(LLFile::fopen(filename, "a")) { if (!mFile) { @@ -169,19 +128,25 @@ namespace { } else { +#if LL_DARWIN || LL_LINUX // We use a number of classic-C libraries, some of which write // log output to stderr. The trouble with that is that unless // you launch the viewer from a console, stderr output is // lost. Redirect STDERR_FILENO to write into this log file. - fhdup2(fhfileno(mFile), fhfileno(stderr)); + // But first, save the original stream in case we want it later. + mSavedStderr = ::dup(STDERR_FILENO); + ::dup2(::fileno(mFile), STDERR_FILENO); +#endif } } ~RecordToFile() { +#if LL_DARWIN || LL_LINUX // restore stderr to its original fileno so any subsequent output // to stderr goes to original stream - fhdup2(mSavedStderr, fhfileno(stderr)); + ::dup2(mSavedStderr, STDERR_FILENO); +#endif mFile.close(); } @@ -201,8 +166,7 @@ namespace { virtual void recordMessage(LLError::ELevel level, const std::string& message) override { - ::fwrite(message.c_str(), sizeof(char), message.length(), mFile); - ::fputc('\n', mFile); + fwrite(message.c_str(), sizeof(char), message.length(), mFile); if (LLError::getAlwaysFlush()) { ::fflush(mFile); @@ -212,26 +176,22 @@ namespace { private: const std::string mName; LLUniqueFile mFile; - int mSavedStderr; + int mSavedStderr{0}; }; class RecordToStderr : public LLError::Recorder { public: - RecordToStderr(bool timestamp) : - mUseANSI(checkANSI()), - // use duplicate stderr file handle so THIS output isn't affected - // by our internal redirection of all (other) stderr output - mStderr(fhfdopen(LLError::Settings::instance().getDupStderr(), "a")) - { - this->showMultiline(true); - } - - virtual bool enabled() override + RecordToStderr(bool timestamp) : mUseANSI(checkANSI()) { - return LLError::getEnabledLogTypesMask() & 0x04; + this->showMultiline(true); } + + virtual bool enabled() override + { + return LLError::getEnabledLogTypesMask() & 0x04; + } virtual void recordMessage(LLError::ELevel level, const std::string& message) override @@ -254,18 +214,17 @@ namespace { break; } } - fprintf(mStderr, "%s\n", message.c_str()); + fprintf(stderr, "%s\n", message.c_str()); if (mUseANSI) colorANSI("0"); // reset } - + private: bool mUseANSI; - LLFILE* mStderr; void colorANSI(const std::string color) { // ANSI color code escape sequence - fprintf(mStderr, "\033[%sm", color.c_str() ); + fprintf(stderr, "\033[%sm", color.c_str() ); }; static bool checkANSI(void) @@ -274,7 +233,7 @@ namespace { // Check whether it's okay to use ANSI; if stderr is // a tty then we assume yes. Can be turned off with // the LL_NO_ANSI_COLOR env var. - return (0 != isatty(fhfileno(stderr))) && + return (0 != isatty(2)) && (NULL == getenv("LL_NO_ANSI_COLOR")); #endif // LL_LINUX return false; @@ -545,6 +504,22 @@ namespace LLError SettingsConfig(); }; + typedef LLPointer SettingsConfigPtr; + + class Settings : public LLSingleton + { + LLSINGLETON(Settings); + public: + SettingsConfigPtr getSettingsConfig(); + + void reset(); + SettingsStoragePtr saveAndReset(); + void restore(SettingsStoragePtr pSettingsStorage); + + private: + SettingsConfigPtr mSettingsConfig; + }; + SettingsConfig::SettingsConfig() : LLRefCount(), mDefaultLevel(LLError::LEVEL_DEBUG), @@ -568,18 +543,8 @@ namespace LLError } Settings::Settings(): - mSettingsConfig(new SettingsConfig()), - // duplicate stderr file handle right away - mDupStderr(fhdup(fhfileno(stderr))) - { - } - - Settings::~Settings() + mSettingsConfig(new SettingsConfig()) { - // restore original stderr - fhdup2(mDupStderr, fhfileno(stderr)); - // and close the duplicate - fhclose(mDupStderr); } SettingsConfigPtr Settings::getSettingsConfig() @@ -607,11 +572,6 @@ namespace LLError mSettingsConfig = newSettingsConfig; } - int Settings::getDupStderr() const - { - return mDupStderr; - } - bool is_available() { return Settings::instanceExists() && Globals::instanceExists(); -- cgit v1.3 From 7826683fa264add84ef7d87fae5f962d27471a19 Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Tue, 12 Nov 2019 17:02:11 -0500 Subject: DRTVWR-476: Back out 355d9db4a59f: unroll stderr redirection. --- indra/llcommon/llerror.cpp | 68 ++++++++++++++++++++-------------------------- 1 file changed, 30 insertions(+), 38 deletions(-) (limited to 'indra/llcommon/llerror.cpp') diff --git a/indra/llcommon/llerror.cpp b/indra/llcommon/llerror.cpp index 188b76bbae..ea0d06b93c 100644 --- a/indra/llcommon/llerror.cpp +++ b/indra/llcommon/llerror.cpp @@ -119,67 +119,59 @@ namespace { { public: RecordToFile(const std::string& filename): - mName(filename), - mFile(LLFile::fopen(filename, "a")) + mName(filename) { + mFile.open(filename.c_str(), std::ios_base::out | std::ios_base::app); if (!mFile) { - LL_WARNS() << "Error setting log file to " << filename << LL_ENDL; + LL_INFOS() << "Error setting log file to " << filename << LL_ENDL; } else { -#if LL_DARWIN || LL_LINUX - // We use a number of classic-C libraries, some of which write - // log output to stderr. The trouble with that is that unless - // you launch the viewer from a console, stderr output is - // lost. Redirect STDERR_FILENO to write into this log file. - // But first, save the original stream in case we want it later. - mSavedStderr = ::dup(STDERR_FILENO); - ::dup2(::fileno(mFile), STDERR_FILENO); -#endif + if (!LLError::getAlwaysFlush()) + { + mFile.sync_with_stdio(false); + } } } ~RecordToFile() { -#if LL_DARWIN || LL_LINUX - // restore stderr to its original fileno so any subsequent output - // to stderr goes to original stream - ::dup2(mSavedStderr, STDERR_FILENO); -#endif mFile.close(); } - virtual bool enabled() override - { + virtual bool enabled() override + { #ifdef LL_RELEASE_FOR_DOWNLOAD - return 1; + return 1; #else - return LLError::getEnabledLogTypesMask() & 0x02; + return LLError::getEnabledLogTypesMask() & 0x02; #endif - } - - bool okay() const { return bool(mFile); } + } + + bool okay() const { return mFile.good(); } - std::string getFilename() const { return mName; } + std::string getFilename() const { return mName; } - virtual void recordMessage(LLError::ELevel level, - const std::string& message) override - { - fwrite(message.c_str(), sizeof(char), message.length(), mFile); - if (LLError::getAlwaysFlush()) - { - ::fflush(mFile); - } - } + virtual void recordMessage(LLError::ELevel level, + const std::string& message) override + { + if (LLError::getAlwaysFlush()) + { + mFile << message << std::endl; + } + else + { + mFile << message << "\n"; + } + } private: const std::string mName; - LLUniqueFile mFile; - int mSavedStderr{0}; + llofstream mFile; }; - - + + class RecordToStderr : public LLError::Recorder { public: -- cgit v1.3 From cc9bdbcf19354c323c3f090949636267f54851e0 Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Mon, 18 Nov 2019 18:43:01 -0500 Subject: DRTVWR-476: Introduce LLStacktrace, a token to stream stack trace. LLStacktrace has no behavior except when you stream an instance to a std::ostream. Then it reports the current traceback at that point to the ostream. This bit of indirection is intended to avoid the boost/stacktrace.hpp header from being included everywhere. --- indra/llcommon/llerror.cpp | 245 +++++++++++++++++++++++---------------------- indra/llcommon/llerror.h | 54 +++++----- 2 files changed, 158 insertions(+), 141 deletions(-) (limited to 'indra/llcommon/llerror.cpp') diff --git a/indra/llcommon/llerror.cpp b/indra/llcommon/llerror.cpp index ea0d06b93c..1cb93d27f7 100644 --- a/indra/llcommon/llerror.cpp +++ b/indra/llcommon/llerror.cpp @@ -53,6 +53,13 @@ #include "llstl.h" #include "lltimer.h" +// On Mac, got: +// #error "Boost.Stacktrace requires `_Unwind_Backtrace` function. Define +// `_GNU_SOURCE` macro or `BOOST_STACKTRACE_GNU_SOURCE_NOT_REQUIRED` if +// _Unwind_Backtrace is available without `_GNU_SOURCE`." +#define BOOST_STACKTRACE_GNU_SOURCE_NOT_REQUIRED +#include + namespace { #if LL_WINDOWS void debugger_print(const std::string& s) @@ -1554,124 +1561,128 @@ namespace LLError S32 LLCallStacks::sIndex = 0 ; //static - void LLCallStacks::allocateStackBuffer() - { - if(sBuffer == NULL) - { - sBuffer = new char*[512] ; - sBuffer[0] = new char[512 * 128] ; - for(S32 i = 1 ; i < 512 ; i++) - { - sBuffer[i] = sBuffer[i-1] + 128 ; - } - sIndex = 0 ; - } - } - - void LLCallStacks::freeStackBuffer() - { - if(sBuffer != NULL) - { - delete [] sBuffer[0] ; - delete [] sBuffer ; - sBuffer = NULL ; - } - } - - //static - void LLCallStacks::push(const char* function, const int line) - { - LLMutexTrylock lock(getMutex(), 5); - if (!lock.isLocked()) - { - return; - } - - if(sBuffer == NULL) - { - allocateStackBuffer(); - } - - if(sIndex > 511) - { - clear() ; - } - - strcpy(sBuffer[sIndex], function) ; - sprintf(sBuffer[sIndex] + strlen(function), " line: %d ", line) ; - sIndex++ ; - - return ; - } + void LLCallStacks::allocateStackBuffer() + { + if(sBuffer == NULL) + { + sBuffer = new char*[512] ; + sBuffer[0] = new char[512 * 128] ; + for(S32 i = 1 ; i < 512 ; i++) + { + sBuffer[i] = sBuffer[i-1] + 128 ; + } + sIndex = 0 ; + } + } - //static - std::ostringstream* LLCallStacks::insert(const char* function, const int line) - { - std::ostringstream* _out = LLError::Log::out(); - *_out << function << " line " << line << " " ; - - return _out ; - } - - //static - void LLCallStacks::end(std::ostringstream* _out) - { - LLMutexTrylock lock(getMutex(), 5); - if (!lock.isLocked()) - { - return; - } - - if(sBuffer == NULL) - { - allocateStackBuffer(); - } - - if(sIndex > 511) - { - clear() ; - } - - LLError::Log::flush(_out, sBuffer[sIndex++]) ; - } - - //static - void LLCallStacks::print() - { - LLMutexTrylock lock(getMutex(), 5); - if (!lock.isLocked()) - { - return; - } - - if(sIndex > 0) - { - LL_INFOS() << " ************* PRINT OUT LL CALL STACKS ************* " << LL_ENDL; - while(sIndex > 0) - { - sIndex-- ; - LL_INFOS() << sBuffer[sIndex] << LL_ENDL; - } - LL_INFOS() << " *************** END OF LL CALL STACKS *************** " << LL_ENDL; - } - - if(sBuffer != NULL) - { - freeStackBuffer(); - } - } - - //static - void LLCallStacks::clear() - { - sIndex = 0 ; - } - - //static - void LLCallStacks::cleanup() - { - freeStackBuffer(); - } + void LLCallStacks::freeStackBuffer() + { + if(sBuffer != NULL) + { + delete [] sBuffer[0] ; + delete [] sBuffer ; + sBuffer = NULL ; + } + } + + //static + void LLCallStacks::push(const char* function, const int line) + { + LLMutexTrylock lock(getMutex(), 5); + if (!lock.isLocked()) + { + return; + } + + if(sBuffer == NULL) + { + allocateStackBuffer(); + } + + if(sIndex > 511) + { + clear() ; + } + + strcpy(sBuffer[sIndex], function) ; + sprintf(sBuffer[sIndex] + strlen(function), " line: %d ", line) ; + sIndex++ ; + + return ; + } + + //static + std::ostringstream* LLCallStacks::insert(const char* function, const int line) + { + std::ostringstream* _out = LLError::Log::out(); + *_out << function << " line " << line << " " ; + return _out ; + } + + //static + void LLCallStacks::end(std::ostringstream* _out) + { + LLMutexTrylock lock(getMutex(), 5); + if (!lock.isLocked()) + { + return; + } + + if(sBuffer == NULL) + { + allocateStackBuffer(); + } + + if(sIndex > 511) + { + clear() ; + } + + LLError::Log::flush(_out, sBuffer[sIndex++]) ; + } + + //static + void LLCallStacks::print() + { + LLMutexTrylock lock(getMutex(), 5); + if (!lock.isLocked()) + { + return; + } + + if(sIndex > 0) + { + LL_INFOS() << " ************* PRINT OUT LL CALL STACKS ************* " << LL_ENDL; + while(sIndex > 0) + { + sIndex-- ; + LL_INFOS() << sBuffer[sIndex] << LL_ENDL; + } + LL_INFOS() << " *************** END OF LL CALL STACKS *************** " << LL_ENDL; + } + + if(sBuffer != NULL) + { + freeStackBuffer(); + } + } + + //static + void LLCallStacks::clear() + { + sIndex = 0 ; + } + + //static + void LLCallStacks::cleanup() + { + freeStackBuffer(); + } + + std::ostream& operator<<(std::ostream& out, const LLStacktrace&) + { + return out << boost::stacktrace::stacktrace(); + } } bool debugLoggingEnabled(const std::string& tag) diff --git a/indra/llcommon/llerror.h b/indra/llcommon/llerror.h index 9613911531..48162eca9e 100644 --- a/indra/llcommon/llerror.h +++ b/indra/llcommon/llerror.h @@ -262,30 +262,36 @@ namespace LLError class LL_COMMON_API NoClassInfo { }; // used to indicate no class info known for logging - //LLCallStacks keeps track of call stacks and output the call stacks to log file - //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. - //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 - { - private: - static char** sBuffer ; - static S32 sIndex ; - - static void allocateStackBuffer(); - static void freeStackBuffer(); - - public: - static void push(const char* function, const int line) ; - static std::ostringstream* insert(const char* function, const int line) ; - static void print() ; - static void clear() ; - static void end(std::ostringstream* _out) ; - static void cleanup(); - }; + //LLCallStacks keeps track of call stacks and output the call stacks to log file + //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. + //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 + { + private: + static char** sBuffer ; + static S32 sIndex ; + + static void allocateStackBuffer(); + static void freeStackBuffer(); + + public: + static void push(const char* function, const int line) ; + static std::ostringstream* insert(const char* function, const int line) ; + static void print() ; + static void clear() ; + static void end(std::ostringstream* _out) ; + static void cleanup(); + }; + + // class which, when streamed, inserts the current stack trace + struct LLStacktrace + { + friend std::ostream& operator<<(std::ostream& out, const LLStacktrace&); + }; } //this is cheaper than llcallstacks if no need to output other variables to call stacks. -- cgit v1.3 From 962ccb4f01f220850fea35e32c3d92a718d35631 Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Thu, 2 Apr 2020 13:14:31 -0400 Subject: DRTVWR-476: Facilitate debugging test programs with logging. On Mac, even if you run a test program with --debug or set LOGTEST=DEBUG, it won't log to stderr if you're filtering build output or running the build in an emacs compile buffer. This is because, on Mac, a viewer launched by mouse rather than from the command line is passed a stderr stream that ultimately gets logged to the system Console. The shouldLogToStderr() function is intended to avoid spamming the Console with the (voluminous) viewer log output. It tests whether stderr isatty() and, if not, suppresses calling LLError::logToStderr(). This makes debugging test programs using log output trickier than necessary. Change shouldLogToStderr() to permit logging when either stderr isatty() or is a pipe. The original intention is preserved in that empirically, a viewer launched by mouse is passed a stderr stream identified as a character device rather than as a pipe. Also introduce SetEnv, a class that facilitates setting (e.g.) LOGTEST=DEBUG for specific test programs without setting it for all test programs in the build. Using the constructor for a static object means you can set environment variables before main() is entered, which is important because it's the main() function in test.cpp that acts on the LOGTEST and LOGFAIL environment variables. These changes make it unnecessary to retain the temporary change in test.cpp to force LOGTEST to DEBUG. --- indra/llcommon/llerror.cpp | 43 +++++++++++++++++++++--------- indra/test/setenv.h | 66 ++++++++++++++++++++++++++++++++++++++++++++++ indra/test/test.cpp | 3 --- 3 files changed, 96 insertions(+), 16 deletions(-) create mode 100644 indra/test/setenv.h (limited to 'indra/llcommon/llerror.cpp') diff --git a/indra/llcommon/llerror.cpp b/indra/llcommon/llerror.cpp index 1cb93d27f7..0daae96cca 100644 --- a/indra/llcommon/llerror.cpp +++ b/indra/llcommon/llerror.cpp @@ -39,6 +39,7 @@ #if !LL_WINDOWS # include # include +# include #endif // !LL_WINDOWS #include #include "string.h" @@ -654,22 +655,38 @@ namespace LLError namespace { - bool shouldLogToStderr() - { + bool shouldLogToStderr() + { #if LL_DARWIN - // On Mac OS X, stderr from apps launched from the Finder goes to the - // console log. It's generally considered bad form to spam too much - // there. - - // If stderr is a tty, assume the user launched from the command line or - // debugger and therefore wants to see stderr. Otherwise, assume we've - // been launched from the finder and shouldn't spam stderr. - return isatty(STDERR_FILENO); + // On Mac OS X, stderr from apps launched from the Finder goes to the + // console log. It's generally considered bad form to spam too much + // there. That scenario can be detected by noticing that stderr is a + // character device (S_IFCHR). + + // If stderr is a tty or a pipe, assume the user launched from the + // command line or debugger and therefore wants to see stderr. + if (isatty(STDERR_FILENO)) + return true; + // not a tty, but might still be a pipe -- check + struct stat st; + if (fstat(STDERR_FILENO, &st) < 0) + { + // capture errno right away, before engaging any other operations + auto errno_save = errno; + // this gets called during log-system setup -- can't log yet! + std::cerr << "shouldLogToStderr: fstat(" << STDERR_FILENO << ") failed, errno " + << errno_save << std::endl; + // if we can't tell, err on the safe side and don't write stderr + return false; + } + + // fstat() worked: return true only if stderr is a pipe + return ((st.st_mode & S_IFMT) == S_IFIFO); #else - return true; + return true; #endif - } - + } + bool stderrLogWantsTime() { #if LL_WINDOWS diff --git a/indra/test/setenv.h b/indra/test/setenv.h new file mode 100644 index 0000000000..ed2de9ccca --- /dev/null +++ b/indra/test/setenv.h @@ -0,0 +1,66 @@ +/** + * @file setenv.h + * @author Nat Goodspeed + * @date 2020-04-01 + * @brief Provide a way for a particular test program to alter the + * environment before entry to main(). + * + * $LicenseInfo:firstyear=2020&license=viewerlgpl$ + * Copyright (c) 2020, Linden Research, Inc. + * $/LicenseInfo$ + */ + +#if ! defined(LL_SETENV_H) +#define LL_SETENV_H + +#include // setenv() + +/** + * Our test.cpp main program responds to environment variables LOGTEST and + * LOGFAIL. But if you set (e.g.) LOGTEST=DEBUG before a viewer build, @em + * every test program in the build emits debug log output. This can be so + * voluminous as to slow down the build. + * + * With an integration test program, you can specifically build (e.g.) the + * INTEGRATION_TEST_llstring target, and set any environment variables you + * want for that. But with a unit test program, since executing the program is + * a side effect rather than an explicit target, specifically building (e.g.) + * PROJECT_lllogin_TEST_lllogin only builds the executable without running it. + * + * To set an environment variable for a particular test program, declare a + * static instance of SetEnv in its .cpp file. SetEnv's constructor takes + * pairs of strings, e.g. + * + * @code + * static SetEnv sLOGGING("LOGTEST", "INFO"); + * @endcode + * + * Declaring a static instance of SetEnv is important because that ensures + * that the environment variables are set before main() is entered, since it + * is main() that examines LOGTEST and LOGFAIL. + */ +struct SetEnv +{ + // degenerate constructor, terminate recursion + SetEnv() {} + + /** + * SetEnv() accepts an arbitrary number of pairs of strings: variable + * name, value, variable name, value ... Entering the constructor sets + * those variables in the process environment using Posix setenv(), + * overriding any previous value. If static SetEnv declarations in + * different translation units specify overlapping sets of variable names, + * it is indeterminate which instance will "win." + */ + template + SetEnv(VAR&& var, VAL&& val, ARGS&&... rest): + // constructor forwarding handles the tail of the list + SetEnv(std::forward(rest)...) + { + // set just the first (variable, value) pair + // 1 means override previous value if any + setenv(std::forward(var), std::forward(val), 1); + } +}; + +#endif /* ! defined(LL_SETENV_H) */ diff --git a/indra/test/test.cpp b/indra/test/test.cpp index ea54ba658e..87c4a8d8a3 100644 --- a/indra/test/test.cpp +++ b/indra/test/test.cpp @@ -544,9 +544,6 @@ int main(int argc, char **argv) // LOGTEST overrides default, but can be overridden by --debug. const char* LOGTEST = getenv("LOGTEST"); - // DELETE - LOGTEST = "DEBUG"; - // values used for options parsing apr_status_t apr_err; const char* opt_arg = NULL; -- cgit v1.3 From d8649dbb8a5a20753248923a25c13f729cadd99a Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Tue, 2 Jun 2020 16:44:22 -0400 Subject: SL-13361: Enable color processing on Windows 10 debug console. (cherry picked from commit 0b61150e698537a7e42a4cdae02496da500399d9) --- indra/llcommon/llerror.cpp | 5 ++--- indra/newview/llappviewerwin32.cpp | 21 ++++++++++++++------- 2 files changed, 16 insertions(+), 10 deletions(-) (limited to 'indra/llcommon/llerror.cpp') diff --git a/indra/llcommon/llerror.cpp b/indra/llcommon/llerror.cpp index 41c4ddc725..411412c883 100644 --- a/indra/llcommon/llerror.cpp +++ b/indra/llcommon/llerror.cpp @@ -40,6 +40,8 @@ # include # include # include +#else +# include #endif // !LL_WINDOWS #include #include "string.h" @@ -236,14 +238,11 @@ namespace { static bool checkANSI(void) { -#if LL_LINUX || LL_DARWIN // Check whether it's okay to use ANSI; if stderr is // a tty then we assume yes. Can be turned off with // the LL_NO_ANSI_COLOR env var. return (0 != isatty(2)) && (NULL == getenv("LL_NO_ANSI_COLOR")); -#endif // LL_LINUX - return FALSE; // works in a cygwin shell... ;) } }; diff --git a/indra/newview/llappviewerwin32.cpp b/indra/newview/llappviewerwin32.cpp index 1f66177c37..f0aa355342 100644 --- a/indra/newview/llappviewerwin32.cpp +++ b/indra/newview/llappviewerwin32.cpp @@ -500,6 +500,10 @@ void LLAppViewerWin32::disableWinErrorReporting() } const S32 MAX_CONSOLE_LINES = 500; +// Only defined in newer SDKs than we currently use +#ifndef ENABLE_VIRTUAL_TERMINAL_PROCESSING +#define ENABLE_VIRTUAL_TERMINAL_PROCESSING 4 +#endif namespace { @@ -507,13 +511,11 @@ FILE* set_stream(const char* which, DWORD handle_id, const char* mode); bool create_console() { - CONSOLE_SCREEN_BUFFER_INFO coninfo; - FILE *fp; - // allocate a console for this app const bool isConsoleAllocated = AllocConsole(); // set the screen buffer to be big enough to let us scroll text + CONSOLE_SCREEN_BUFFER_INFO coninfo; GetConsoleScreenBufferInfo(GetStdHandle(STD_OUTPUT_HANDLE), &coninfo); coninfo.dwSize.Y = MAX_CONSOLE_LINES; SetConsoleScreenBufferSize(GetStdHandle(STD_OUTPUT_HANDLE), coninfo.dwSize); @@ -542,19 +544,24 @@ bool create_console() return isConsoleAllocated; } -FILE* set_stream(const char* which, DWORD handle_id, const char* mode) +FILE* set_stream(const char* desc, DWORD handle_id, const char* mode) { - long l_std_handle = (long)GetStdHandle(handle_id); - int h_con_handle = _open_osfhandle(l_std_handle, _O_TEXT); + auto l_std_handle = GetStdHandle(handle_id); + int h_con_handle = _open_osfhandle(reinterpret_cast(l_std_handle), _O_TEXT); if (h_con_handle == -1) { - LL_WARNS() << "create_console() failed to open " << which << " handle" << LL_ENDL; + LL_WARNS() << "create_console() failed to open " << desc << " handle" << LL_ENDL; return nullptr; } else { FILE* fp = _fdopen( h_con_handle, mode ); setvbuf( fp, NULL, _IONBF, 0 ); + // Enable color processing on Windows 10 console windows. + DWORD dwMode = 0; + GetConsoleMode(l_std_handle, &dwMode); + dwMode |= ENABLE_VIRTUAL_TERMINAL_PROCESSING; + SetConsoleMode(l_std_handle, dwMode); return fp; } } -- cgit v1.3