Cleanup unnecessary code in exception formatting (dotnet/coreclr#24797)

* Cleanup unnecessary code in exception formatting

* Delete fNeedFileInfo argument for GetStackTrace

Fixes dotnet/coreclr#8694

Commit migrated from https://github.com/dotnet/coreclr/commit/156240d59b8acc277504fe43f148fe53e63f40bc

diff --git a/src/coreclr/src/System.Private.CoreLib/src/System/Exception.CoreCLR.cs b/src/coreclr/src/System.Private.CoreLib/src/System/Exception.CoreCLR.cs
index a569e01..7597a5b 100644 (file)
--- a/src/coreclr/src/System.Private.CoreLib/src/System/Exception.CoreCLR.cs
+++ b/src/coreclr/src/System.Private.CoreLib/src/System/Exception.CoreCLR.cs
@@ -162,42 +162,27 @@ namespace System
         {
             get
             {
-                // By default attempt to include file and line number info
-                return GetStackTrace(true);
-            }
-        }
+                string? stackTraceString = _stackTraceString;
+                string? remoteStackTraceString = _remoteStackTraceString;
 
-        // Computes and returns the stack trace as a string
-        // Attempts to get source file and line number information if needFileInfo
-        // is true.  Note that this requires FileIOPermission(PathDiscovery), and so
-        // will usually fail in CoreCLR.  To avoid the demand and resulting
-        // SecurityException we can explicitly not even try to get fileinfo.
-        private string? GetStackTrace(bool needFileInfo)
-        {
-            string? stackTraceString = _stackTraceString;
-            string? remoteStackTraceString = _remoteStackTraceString;
+                // if no stack trace, try to get one
+                if (stackTraceString != null)
+                {
+                    return remoteStackTraceString + stackTraceString;
+                }
+                if (_stackTrace == null)
+                {
+                    return remoteStackTraceString;
+                }
 
-            // if no stack trace, try to get one
-            if (stackTraceString != null)
-            {
-                return remoteStackTraceString + stackTraceString;
+                return remoteStackTraceString + GetStackTrace(this);
             }
-            if (_stackTrace == null)
-            {
-                return remoteStackTraceString;
-            }
-
-            // Obtain the stack trace string. Note that since GetStackTrace
-            // will add the path to the source file if the PDB is present:
-            // we need to make sure we don't store the stack trace string in
-            // the _stackTraceString member variable.
-            return remoteStackTraceString + GetStackTrace(needFileInfo, this);
         }
 
-        private static string GetStackTrace(bool needFileInfo, Exception e)
+        private static string GetStackTrace(Exception e)
         {
             // Do not include a trailing newline for backwards compatibility
-            return new StackTrace(e, needFileInfo).ToString(System.Diagnostics.StackTrace.TraceFormat.Normal);
+            return new StackTrace(e, fNeedFileInfo: true).ToString(System.Diagnostics.StackTrace.TraceFormat.Normal);
         }
 
         private string? CreateSourceName()
@@ -244,30 +229,11 @@ namespace System
         //  copy the stack trace to _remoteStackTraceString.
         internal void InternalPreserveStackTrace()
         {
-            string? tmpStackTraceString;
-
-#if FEATURE_APPX
-            if (ApplicationModel.IsUap)
-            {
-                // Call our internal GetStackTrace in AppX so we can parse the result should
-                // we need to strip file/line info from it to make it PII-free. Calling the
-                // public and overridable StackTrace getter here was probably not intended.
-                tmpStackTraceString = GetStackTrace(true);
-
-                // Make sure that the _source field is initialized if Source is not overriden.
-                // We want it to contain the original faulting point.
-                string? source = Source;
-            }
-            else
-#else // FEATURE_APPX
-            // Preinitialize _source on CoreSystem as well. The legacy behavior is not ideal and
-            // we keep it for back compat but we can afford to make the change on the Phone.
+            // Make sure that the _source field is initialized if Source is not overriden.
+            // We want it to contain the original faulting point.
             string? source = Source;
-#endif // FEATURE_APPX
-            {
-                // Call the StackTrace getter in classic for compat.
-                tmpStackTraceString = StackTrace;
-            }
+
+            string? tmpStackTraceString = StackTrace;
 
             if (tmpStackTraceString != null && tmpStackTraceString.Length > 0)
             {
@@ -426,7 +392,7 @@ namespace System
 
                 if (stackTraceString == null && _stackTrace != null)
                 {
-                    stackTraceString = GetStackTrace(true, this);
+                    stackTraceString = GetStackTrace(this);
                 }
 
                 return stackTraceString;

diff --git a/src/coreclr/src/vm/debughelp.cpp b/src/coreclr/src/vm/debughelp.cpp
index 0df43d9..ce54fef 100644 (file)
--- a/src/coreclr/src/vm/debughelp.cpp
+++ b/src/coreclr/src/vm/debughelp.cpp
@@ -773,40 +773,19 @@ void PrintException(OBJECTREF pObjectRef)
 
     GCPROTECT_BEGIN(pObjectRef);
 
-    if (!IsException(pObjectRef->GetMethodTable()))
-    {
-         printf("Specified object is not an exception object.\n");
-    }
-    else
-    {
-        MethodDescCallSite toString(METHOD__EXCEPTION__TO_STRING, &pObjectRef);
+    MethodDescCallSite toString(METHOD__OBJECT__TO_STRING, &pObjectRef);
 
-        ARG_SLOT arg[1] = {
-            ObjToArgSlot(pObjectRef)
-        };
+    ARG_SLOT arg[1] = {
+        ObjToArgSlot(pObjectRef)
+    };
 
-        STRINGREF str = toString.Call_RetSTRINGREF(arg);
+    STRINGREF str = toString.Call_RetSTRINGREF(arg);
 
-        if(str->GetBuffer() != NULL)
-        {
-            WszOutputDebugString(str->GetBuffer());
-        }
-    }
-
-    GCPROTECT_END();
-}
-
-void PrintException(UINT_PTR pObject)
-{
-    CONTRACTL
+    if(str->GetBuffer() != NULL)
     {
-        NOTHROW;
-        GC_NOTRIGGER;
+        WszOutputDebugString(str->GetBuffer());
     }
-    CONTRACTL_END;
 
-    OBJECTREF pObjectRef = NULL;
-    GCPROTECT_BEGIN(pObjectRef);
     GCPROTECT_END();
 }
 

diff --git a/src/coreclr/src/vm/excep.cpp b/src/coreclr/src/vm/excep.cpp
index e86fbf9..156a8e7 100644 (file)
--- a/src/coreclr/src/vm/excep.cpp
+++ b/src/coreclr/src/vm/excep.cpp
@@ -339,23 +339,13 @@ STRINGREF GetExceptionMessage(OBJECTREF throwable)
     if (throwable == NULL)
         return NULL;
 
-    // Assume we're calling Exception.ToString() ...
-    BinderMethodID sigID = METHOD__EXCEPTION__TO_STRING;
-
-    // ... but if it isn't an exception, call Object.ToString().
-    _ASSERTE(IsException(throwable->GetMethodTable()));        // what is the pathway here?
-    if (!IsException(throwable->GetMethodTable()))
-    {
-        sigID = METHOD__OBJECT__TO_STRING;
-    }
-
     // Return value.
     STRINGREF pString = NULL;
 
     GCPROTECT_BEGIN(throwable);
 
-    // Get the MethodDesc on which we'll call.
-    MethodDescCallSite toString(sigID, &throwable);
+    // Call Object.ToString(). Note that exceptions do not have to inherit from System.Exception
+    MethodDescCallSite toString(METHOD__OBJECT__TO_STRING, &throwable);
 
     // Make the call.
     ARG_SLOT arg[1] = {ObjToArgSlot(throwable)};
@@ -497,12 +487,8 @@ void ExceptionPreserveStackTrace(   // No return.
     {
         LOG((LF_EH, LL_INFO1000, "ExceptionPreserveStackTrace called\n"));
 
-        // We're calling Exception.InternalPreserveStackTrace() ...
-        BinderMethodID sigID = METHOD__EXCEPTION__INTERNAL_PRESERVE_STACK_TRACE;
-
-
-        // Get the MethodDesc on which we'll call.
-        MethodDescCallSite preserveStackTrace(sigID, &throwable);
+        // Call Exception.InternalPreserveStackTrace() ...
+        MethodDescCallSite preserveStackTrace(METHOD__EXCEPTION__INTERNAL_PRESERVE_STACK_TRACE, &throwable);
 
         // Make the call.
         ARG_SLOT arg[1] = {ObjToArgSlot(throwable)};

diff --git a/src/coreclr/src/vm/mscorlib.h b/src/coreclr/src/vm/mscorlib.h
index 13ffd95..442a249 100644 (file)
--- a/src/coreclr/src/vm/mscorlib.h
+++ b/src/coreclr/src/vm/mscorlib.h
@@ -357,7 +357,6 @@ DEFINE_METHOD(EXCEPTION,            GET_CLASS_NAME,         GetClassName,
 DEFINE_PROPERTY(EXCEPTION,          MESSAGE,                Message,                    Str)
 DEFINE_PROPERTY(EXCEPTION,          SOURCE,                 Source,                     Str)
 DEFINE_PROPERTY(EXCEPTION,          HELP_LINK,              HelpLink,                   Str)
-DEFINE_METHOD(EXCEPTION,            TO_STRING,              ToString,                   IM_RetStr)
 DEFINE_METHOD(EXCEPTION,            INTERNAL_PRESERVE_STACK_TRACE, InternalPreserveStackTrace, IM_RetVoid)
 #ifdef FEATURE_COMINTEROP
 DEFINE_METHOD(EXCEPTION,            ADD_EXCEPTION_DATA_FOR_RESTRICTED_ERROR_INFO, AddExceptionDataForRestrictedErrorInfo, IM_Str_Str_Str_Obj_Bool_RetVoid)

diff --git a/src/libraries/System.Private.CoreLib/src/System/Exception.cs b/src/libraries/System.Private.CoreLib/src/System/Exception.cs
index 521ade0..2f07ca2 100644 (file)
--- a/src/libraries/System.Private.CoreLib/src/System/Exception.cs
+++ b/src/libraries/System.Private.CoreLib/src/System/Exception.cs
@@ -155,7 +155,7 @@ namespace System
                 "   " + SR.Exception_EndOfInnerExceptionStack;
             }
 
-            string? stackTrace = GetStackTrace(needFileInfo: true);
+            string? stackTrace = StackTrace;
             if (stackTrace != null)
             {
                 s += Environment.NewLine + stackTrace;