From fa6c8b08e6b716570a9263715aa95bb5d2185714 Mon Sep 17 00:00:00 2001
From: Theodore Tsirpanis <teo@tsirpanis.gr>
Date: Fri, 12 Jul 2024 20:48:43 +0300
Subject: [PATCH] Ignore type resolution failures when checking for attributes
 in `StackTrace.ToString`. (#104530)

* Guard the call to `IsDefined(typeof(CompilerGeneratedAttribute))` from throwing.

An `IsDefinedSafe` method was added that returns false if checking for the attribute throws, and other usages in the file were updated as well. This also allows us to remove a big try catch block.

* Guard getting the `StateMachineAttribute`s from throwing.
---
 .../src/System/Diagnostics/StackTrace.cs      | 62 ++++++++++++-------
 1 file changed, 41 insertions(+), 21 deletions(-)

diff --git a/src/libraries/System.Private.CoreLib/src/System/Diagnostics/StackTrace.cs b/src/libraries/System.Private.CoreLib/src/System/Diagnostics/StackTrace.cs
index 5c5f8d374cc..8def9abf8a7 100644
--- a/src/libraries/System.Private.CoreLib/src/System/Diagnostics/StackTrace.cs
+++ b/src/libraries/System.Private.CoreLib/src/System/Diagnostics/StackTrace.cs
@@ -240,7 +240,7 @@ namespace System.Diagnostics
                     Type? declaringType = mb.DeclaringType;
                     string methodName = mb.Name;
                     bool methodChanged = false;
-                    if (declaringType != null && declaringType.IsDefined(typeof(CompilerGeneratedAttribute), inherit: false))
+                    if (declaringType != null && IsDefinedSafe(declaringType, typeof(CompilerGeneratedAttribute), inherit: false))
                     {
                         isAsync = declaringType.IsAssignableTo(typeof(IAsyncStateMachine));
                         if (isAsync || declaringType.IsAssignableTo(typeof(IEnumerator)))
@@ -384,33 +384,53 @@ namespace System.Diagnostics
                 return false;
             }
 
-            try
+            if (IsDefinedSafe(mb, typeof(StackTraceHiddenAttribute), inherit: false))
             {
-                if (mb.IsDefined(typeof(StackTraceHiddenAttribute), inherit: false))
-                {
-                    // Don't show where StackTraceHidden is applied to the method.
-                    return false;
-                }
-
-                Type? declaringType = mb.DeclaringType;
-                // Methods don't always have containing types, for example dynamic RefEmit generated methods.
-                if (declaringType != null &&
-                    declaringType.IsDefined(typeof(StackTraceHiddenAttribute), inherit: false))
-                {
-                    // Don't show where StackTraceHidden is applied to the containing Type of the method.
-                    return false;
-                }
+                // Don't show where StackTraceHidden is applied to the method.
+                return false;
             }
-            catch
+
+            Type? declaringType = mb.DeclaringType;
+            // Methods don't always have containing types, for example dynamic RefEmit generated methods.
+            if (declaringType != null &&
+                IsDefinedSafe(declaringType, typeof(StackTraceHiddenAttribute), inherit: false))
             {
-                // Getting the StackTraceHiddenAttribute has failed, behave as if it was not present.
-                // One of the reasons can be that the method mb or its declaring type use attributes
-                // defined in an assembly that is missing.
+                // Don't show where StackTraceHidden is applied to the containing Type of the method.
+                return false;
             }
 
             return true;
         }
 
+        private static bool IsDefinedSafe(MemberInfo memberInfo, Type attributeType, bool inherit)
+        {
+            try
+            {
+                return memberInfo.IsDefined(attributeType, inherit);
+            }
+            catch
+            {
+                // Checking for the attribute has failed, behave as if it was not present. One of
+                // the reasons can be that the member has attributes defined in an assembly that
+                // is missing.
+                return false;
+            }
+        }
+
+        private static Attribute[] GetCustomAttributesSafe(MemberInfo memberInfo, Type attributeType, bool inherit)
+        {
+            try
+            {
+                return Attribute.GetCustomAttributes(memberInfo, attributeType, inherit);
+            }
+            catch
+            {
+                // Getting the attributes has failed, return an empty array. One of the reasons
+                // can be that the member has attributes defined in an assembly that is missing.
+                return [];
+            }
+        }
+
         private static bool TryResolveStateMachineMethod(ref MethodBase method, out Type declaringType)
         {
             Debug.Assert(method != null);
@@ -438,7 +458,7 @@ namespace System.Diagnostics
 
             foreach (MethodInfo candidateMethod in methods)
             {
-                StateMachineAttribute[]? attributes = (StateMachineAttribute[])Attribute.GetCustomAttributes(candidateMethod, typeof(StateMachineAttribute), inherit: false);
+                StateMachineAttribute[]? attributes = (StateMachineAttribute[])GetCustomAttributesSafe(candidateMethod, typeof(StateMachineAttribute), inherit: false);
                 if (attributes == null)
                 {
                     continue;