From 97a2171ea88fe52060e1dfe3fb09d2c320e1bb10 Mon Sep 17 00:00:00 2001 From: Graham Madarasz Date: Tue, 4 Jun 2013 09:05:23 -0700 Subject: MAINT-2740 make use of OsOutputDebugString _DEBUG only to avoid interactions between Win 32-bit SEH and boost coroutine fiber stack handling --- indra/llcommon/llerror.cpp | 5 +---- indra/llcommon/llerror.h | 14 ++++++++++++++ indra/llcommon/llsys.cpp | 5 ++++- 3 files changed, 19 insertions(+), 5 deletions(-) (limited to 'indra/llcommon') diff --git a/indra/llcommon/llerror.cpp b/indra/llcommon/llerror.cpp index 9b0141eb76..5c8e6cca29 100755 --- a/indra/llcommon/llerror.cpp +++ b/indra/llcommon/llerror.cpp @@ -201,10 +201,7 @@ namespace { virtual void recordMessage(LLError::ELevel level, const std::string& message) { - llutf16string utf16str = - wstring_to_utf16str(utf8str_to_wstring(message)); - utf16str += '\n'; - OutputDebugString(utf16str.c_str()); + LL_WINDOWS_OUTPUT_DEBUG(wstring_to_utf16str(utf8str_to_wstring(message))); } }; #endif diff --git a/indra/llcommon/llerror.h b/indra/llcommon/llerror.h index b65b410153..08a5cd26df 100755 --- a/indra/llcommon/llerror.h +++ b/indra/llcommon/llerror.h @@ -283,6 +283,20 @@ typedef LLError::NoClassInfo _LL_CLASS_TO_LOG; #define LL_ENDL llendl #define LL_CONT (*_out) +// Short story: We don't want to enable this in release builds. +// +// Long story: ...because this call generates C++ exceptions +// which are handled and fine under the debugger, but instant death should they occur from +// within a coroutine's stackframe due to inherent limitations of Windows 32-bit SEH +// interacting with the fiber-based coroutine support used by boost. +// +// gmad BUG-2707/MAINT-2740 +#if LL_WINDOWS && defined(_DEBUG) + #define LL_WINDOWS_OUTPUT_DEBUG(a) OutputDebugString(utf8str_to_utf16str(a).c_str()), OutputDebugString("\n") +#else + #define LL_WINDOWS_OUTPUT_DEBUG(a) +#endif + /* Use this construct if you need to do computation in the middle of a message: diff --git a/indra/llcommon/llsys.cpp b/indra/llcommon/llsys.cpp index 57a6de9060..418c5763f8 100755 --- a/indra/llcommon/llsys.cpp +++ b/indra/llcommon/llsys.cpp @@ -1385,7 +1385,7 @@ public: return false; } // Congratulations, we've hit a new low. :-P - +#if _DEBUG LL_INFOS("FrameWatcher") << ' '; if (! prevSize) { @@ -1398,6 +1398,9 @@ public: } LL_CONT << std::fixed << std::setprecision(1) << framerate << '\n' << LLMemoryInfo() << LL_ENDL; +#else + (void)prevSize; +#endif return false; } -- cgit v1.2.3 From 2750e86beb8220baa62f978c7ccb391654eb955b Mon Sep 17 00:00:00 2001 From: Graham Madarasz Date: Tue, 4 Jun 2013 12:57:03 -0700 Subject: MAINT-2740 and MAINT-2672 rework after code review for 2740 fix and include 2672 fix needed for doing local integ tests --- indra/llcommon/llerror.cpp | 16 +++++++++++++++- indra/llcommon/llerror.h | 29 ++++++++++++++--------------- indra/llcommon/llsys.cpp | 5 +---- 3 files changed, 30 insertions(+), 20 deletions(-) (limited to 'indra/llcommon') diff --git a/indra/llcommon/llerror.cpp b/indra/llcommon/llerror.cpp index 5c8e6cca29..10b8b3105b 100755 --- a/indra/llcommon/llerror.cpp +++ b/indra/llcommon/llerror.cpp @@ -201,7 +201,7 @@ namespace { virtual void recordMessage(LLError::ELevel level, const std::string& message) { - LL_WINDOWS_OUTPUT_DEBUG(wstring_to_utf16str(utf8str_to_wstring(message))); + LL_WINDOWS_OUTPUT_DEBUG(wstring_to_utf16str(utf8str_to_wstring(message)).c_str()); } }; #endif @@ -1398,5 +1398,19 @@ namespace LLError { sIndex = 0 ; } + +#if LL_WINDOWS && !defined(LL_RELEASE_FOR_DOWNLOAD) + void LLOutputDebugUTF16(const unsigned short* s) + { + // Be careful not to enable this in non-debug builds as there are bad interactions between the + // exceptions thrown by this function and the handling of stacks in coroutine fibers. BUG-2707 + // + #if defined(_DEBUG) + OutputDebugString(s); + OutputDebugString(TEXT("\n")); + #endif + } +#endif + } diff --git a/indra/llcommon/llerror.h b/indra/llcommon/llerror.h index 08a5cd26df..1bc93b4def 100755 --- a/indra/llcommon/llerror.h +++ b/indra/llcommon/llerror.h @@ -34,7 +34,6 @@ #include "llerrorlegacy.h" #include "stdtypes.h" - /** Error Logging Facility Information for most users: @@ -199,8 +198,22 @@ namespace LLError static void clear() ; static void end(std::ostringstream* _out) ; }; + +#if LL_WINDOWS && !defined(LL_RELEASE_FOR_DOWNLOAD) + void LLOutputDebugUTF16(const unsigned short* s); +#endif + } +#if LL_WINDOWS && !defined(LL_RELEASE_FOR_DOWNLOAD) + // Macro accepting a wchar_t* for display in windows debugging console in debug builds only + // (wchar_t flavor chosen for maximal utility with unicode text debugging) + // + #define LL_WINDOWS_OUTPUT_DEBUG(a) LLError::LLOutputDebugUTF16((a)) +#else + #define LL_WINDOWS_OUTPUT_DEBUG(a) +#endif + //this is cheaper than llcallstacks if no need to output other variables to call stacks. #define llpushcallstacks LLError::LLCallStacks::push(__FUNCTION__, __LINE__) #define llcallstacks \ @@ -283,20 +296,6 @@ typedef LLError::NoClassInfo _LL_CLASS_TO_LOG; #define LL_ENDL llendl #define LL_CONT (*_out) -// Short story: We don't want to enable this in release builds. -// -// Long story: ...because this call generates C++ exceptions -// which are handled and fine under the debugger, but instant death should they occur from -// within a coroutine's stackframe due to inherent limitations of Windows 32-bit SEH -// interacting with the fiber-based coroutine support used by boost. -// -// gmad BUG-2707/MAINT-2740 -#if LL_WINDOWS && defined(_DEBUG) - #define LL_WINDOWS_OUTPUT_DEBUG(a) OutputDebugString(utf8str_to_utf16str(a).c_str()), OutputDebugString("\n") -#else - #define LL_WINDOWS_OUTPUT_DEBUG(a) -#endif - /* Use this construct if you need to do computation in the middle of a message: diff --git a/indra/llcommon/llsys.cpp b/indra/llcommon/llsys.cpp index 418c5763f8..57a6de9060 100755 --- a/indra/llcommon/llsys.cpp +++ b/indra/llcommon/llsys.cpp @@ -1385,7 +1385,7 @@ public: return false; } // Congratulations, we've hit a new low. :-P -#if _DEBUG + LL_INFOS("FrameWatcher") << ' '; if (! prevSize) { @@ -1398,9 +1398,6 @@ public: } LL_CONT << std::fixed << std::setprecision(1) << framerate << '\n' << LLMemoryInfo() << LL_ENDL; -#else - (void)prevSize; -#endif return false; } -- cgit v1.2.3 From 0ce3d2e1cc9ae70bc7f6146ce38c5dcc1af4a3c6 Mon Sep 17 00:00:00 2001 From: Graham Madarasz Date: Tue, 4 Jun 2013 14:17:34 -0700 Subject: MAINT-2740 convert to std::string based API for OutputDebug wrapper cleanliness after sage counsel (slight refrain) --- indra/llcommon/llerror.cpp | 8 +++++--- indra/llcommon/llerror.h | 4 ++-- 2 files changed, 7 insertions(+), 5 deletions(-) (limited to 'indra/llcommon') diff --git a/indra/llcommon/llerror.cpp b/indra/llcommon/llerror.cpp index 10b8b3105b..c2daa6902f 100755 --- a/indra/llcommon/llerror.cpp +++ b/indra/llcommon/llerror.cpp @@ -201,7 +201,7 @@ namespace { virtual void recordMessage(LLError::ELevel level, const std::string& message) { - LL_WINDOWS_OUTPUT_DEBUG(wstring_to_utf16str(utf8str_to_wstring(message)).c_str()); + LL_WINDOWS_OUTPUT_DEBUG(message); } }; #endif @@ -1400,13 +1400,15 @@ namespace LLError } #if LL_WINDOWS && !defined(LL_RELEASE_FOR_DOWNLOAD) - void LLOutputDebugUTF16(const unsigned short* s) + void LLOutputDebugUTF8(const std::string& s) { // Be careful not to enable this in non-debug builds as there are bad interactions between the // exceptions thrown by this function and the handling of stacks in coroutine fibers. BUG-2707 // #if defined(_DEBUG) - OutputDebugString(s); + // Need UTF16 for Unicode OutputDebugString + // + OutputDebugString(utf8str_to_utf16str(s).c_str()); OutputDebugString(TEXT("\n")); #endif } diff --git a/indra/llcommon/llerror.h b/indra/llcommon/llerror.h index 1bc93b4def..bf8f488aba 100755 --- a/indra/llcommon/llerror.h +++ b/indra/llcommon/llerror.h @@ -200,7 +200,7 @@ namespace LLError }; #if LL_WINDOWS && !defined(LL_RELEASE_FOR_DOWNLOAD) - void LLOutputDebugUTF16(const unsigned short* s); + void LLOutputDebugUTF8(const std::string& s); #endif } @@ -209,7 +209,7 @@ namespace LLError // Macro accepting a wchar_t* for display in windows debugging console in debug builds only // (wchar_t flavor chosen for maximal utility with unicode text debugging) // - #define LL_WINDOWS_OUTPUT_DEBUG(a) LLError::LLOutputDebugUTF16((a)) + #define LL_WINDOWS_OUTPUT_DEBUG(a) LLError::LLOutputDebugUTF8((a)) #else #define LL_WINDOWS_OUTPUT_DEBUG(a) #endif -- cgit v1.2.3 From a45860fd662f4b0f6c10bf102ebf6c0aac5127f3 Mon Sep 17 00:00:00 2001 From: Graham Madarasz Date: Tue, 4 Jun 2013 17:12:46 -0700 Subject: MAINT-2740 put debug output back with guard of debugger presence per Nat review suggestion --- indra/llcommon/llerror.cpp | 7 ++++--- indra/llcommon/llerror.h | 4 ++-- 2 files changed, 6 insertions(+), 5 deletions(-) (limited to 'indra/llcommon') diff --git a/indra/llcommon/llerror.cpp b/indra/llcommon/llerror.cpp index c2daa6902f..37ba097832 100755 --- a/indra/llcommon/llerror.cpp +++ b/indra/llcommon/llerror.cpp @@ -1399,18 +1399,19 @@ namespace LLError sIndex = 0 ; } -#if LL_WINDOWS && !defined(LL_RELEASE_FOR_DOWNLOAD) +#if LL_WINDOWS void LLOutputDebugUTF8(const std::string& s) { // Be careful not to enable this in non-debug builds as there are bad interactions between the // exceptions thrown by this function and the handling of stacks in coroutine fibers. BUG-2707 // - #if defined(_DEBUG) + if (IsDebuggerPresent()) + { // Need UTF16 for Unicode OutputDebugString // OutputDebugString(utf8str_to_utf16str(s).c_str()); OutputDebugString(TEXT("\n")); - #endif + } } #endif diff --git a/indra/llcommon/llerror.h b/indra/llcommon/llerror.h index bf8f488aba..a2a6993330 100755 --- a/indra/llcommon/llerror.h +++ b/indra/llcommon/llerror.h @@ -199,13 +199,13 @@ namespace LLError static void end(std::ostringstream* _out) ; }; -#if LL_WINDOWS && !defined(LL_RELEASE_FOR_DOWNLOAD) +#if LL_WINDOWS void LLOutputDebugUTF8(const std::string& s); #endif } -#if LL_WINDOWS && !defined(LL_RELEASE_FOR_DOWNLOAD) +#if LL_WINDOWS // Macro accepting a wchar_t* for display in windows debugging console in debug builds only // (wchar_t flavor chosen for maximal utility with unicode text debugging) // -- cgit v1.2.3 From 1fcf54094a915bbc7ec1d74a877699212714ae93 Mon Sep 17 00:00:00 2001 From: Graham Madarasz Date: Wed, 5 Jun 2013 07:24:31 -0700 Subject: BUG-2707 fix comment typos, remove unnecessary winmm_shim changes, and revert 2672 fix included only for local integ test --- indra/llcommon/llerror.h | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) (limited to 'indra/llcommon') diff --git a/indra/llcommon/llerror.h b/indra/llcommon/llerror.h index a2a6993330..cc9bfba818 100755 --- a/indra/llcommon/llerror.h +++ b/indra/llcommon/llerror.h @@ -206,10 +206,8 @@ namespace LLError } #if LL_WINDOWS - // Macro accepting a wchar_t* for display in windows debugging console in debug builds only - // (wchar_t flavor chosen for maximal utility with unicode text debugging) - // - #define LL_WINDOWS_OUTPUT_DEBUG(a) LLError::LLOutputDebugUTF8((a)) + // Macro accepting a const std::string& for display in windows debugging console in debug builds only + #define LL_WINDOWS_OUTPUT_DEBUG(a) LLError::LLOutputDebugUTF8(a) #else #define LL_WINDOWS_OUTPUT_DEBUG(a) #endif -- cgit v1.2.3 From bc6070bd197159dd81750d1e2e8af493864be818 Mon Sep 17 00:00:00 2001 From: Graham Madarasz Date: Wed, 5 Jun 2013 07:38:54 -0700 Subject: MAINT-2740 more comment grooming and NULL string guard --- indra/llcommon/llerror.cpp | 13 +++++++++---- indra/llcommon/llerror.h | 2 +- 2 files changed, 10 insertions(+), 5 deletions(-) (limited to 'indra/llcommon') diff --git a/indra/llcommon/llerror.cpp b/indra/llcommon/llerror.cpp index 37ba097832..d2af004cde 100755 --- a/indra/llcommon/llerror.cpp +++ b/indra/llcommon/llerror.cpp @@ -1402,15 +1402,20 @@ namespace LLError #if LL_WINDOWS void LLOutputDebugUTF8(const std::string& s) { - // Be careful not to enable this in non-debug builds as there are bad interactions between the - // exceptions thrown by this function and the handling of stacks in coroutine fibers. BUG-2707 + // Be careful when calling OutputDebugString as it throws DBG_PRINTEXCEPTION_C + // which works just fine under the windows debugger, but can cause users who + // have enabled SEHOP exception chain validation to crash due to interactions + // between the Win 32-bit exception handling and boost coroutine fiber stacks. BUG-2707 // if (IsDebuggerPresent()) { // Need UTF16 for Unicode OutputDebugString // - OutputDebugString(utf8str_to_utf16str(s).c_str()); - OutputDebugString(TEXT("\n")); + if (s.size()) + { + OutputDebugString(utf8str_to_utf16str(s).c_str()); + OutputDebugString(TEXT("\n")); + } } } #endif diff --git a/indra/llcommon/llerror.h b/indra/llcommon/llerror.h index cc9bfba818..0b723aeb5d 100755 --- a/indra/llcommon/llerror.h +++ b/indra/llcommon/llerror.h @@ -206,7 +206,7 @@ namespace LLError } #if LL_WINDOWS - // Macro accepting a const std::string& for display in windows debugging console in debug builds only + // Macro accepting a std::string for display in windows debugging console #define LL_WINDOWS_OUTPUT_DEBUG(a) LLError::LLOutputDebugUTF8(a) #else #define LL_WINDOWS_OUTPUT_DEBUG(a) -- cgit v1.2.3