Make JitDump work in ilc.exe

The JIT is not expected to make assumption about format of raw signatures to parse them.
Instead, it should depend on JIT-EE interface for parsing. This invariant was violates in
couple of places related to debug COMPlus_ method filters that caused them to crash under CoreRT.

Fixed the filters to use the proper abstraction instead.

diff --git a/src/inc/utilcode.h b/src/inc/utilcode.h
index fa0e1b1..097dfe7 100644 (file)
--- a/src/inc/utilcode.h
+++ b/src/inc/utilcode.h
@@ -4102,6 +4102,8 @@ public:
 // Class to parse a list of method names and then find a match
 //*****************************************************************************
 
+struct CORINFO_SIG_INFO;
+
 class MethodNamesListBase
 {
     struct MethodName
@@ -4114,6 +4116,8 @@ class MethodNamesListBase
 
     MethodName     *pNames;         // List of names
 
+    bool IsInList(LPCUTF8 methodName, LPCUTF8 className, int numArgs);
+
 public:
     void Init()
     {
@@ -4132,7 +4136,8 @@ public:
 
     void Insert(__in __in_z LPWSTR list);
 
-    bool IsInList(LPCUTF8 methodName, LPCUTF8 className, PCCOR_SIGNATURE sig);
+    bool IsInList(LPCUTF8 methodName, LPCUTF8 className, PCCOR_SIGNATURE sig = NULL);
+    bool IsInList(LPCUTF8 methodName, LPCUTF8 className, CORINFO_SIG_INFO* pSigInfo);
     bool IsEmpty()
     {
         LIMITED_METHOD_CONTRACT;
@@ -4247,7 +4252,8 @@ public:
         return m_list.IsEmpty();
     }
 
-    bool contains(LPCUTF8 methodName, LPCUTF8 className, PCCOR_SIGNATURE sig);
+    bool contains(LPCUTF8 methodName, LPCUTF8 className, PCCOR_SIGNATURE sig = NULL);
+    bool contains(LPCUTF8 methodName, LPCUTF8 className, CORINFO_SIG_INFO* pSigInfo);
 
     inline void ensureInit(const CLRConfig::ConfigStringInfo & info)
     {

diff --git a/src/jit/compiler.cpp b/src/jit/compiler.cpp
index a8b61a0..54f8073 100644 (file)
--- a/src/jit/compiler.cpp
+++ b/src/jit/compiler.cpp
@@ -1994,7 +1994,7 @@ void                Compiler::compInitOptions(unsigned compileFlags)
     }
 
 #ifdef ALT_JIT
-    if (pfAltJit->contains(info.compMethodName, info.compClassName, info.compMethodInfo->args.pSig))
+    if (pfAltJit->contains(info.compMethodName, info.compClassName, &info.compMethodInfo->args))
     {
         opts.altJit = true;
     }
@@ -2078,7 +2078,7 @@ void                Compiler::compInitOptions(unsigned compileFlags)
     {
         static ConfigMethodSet fJitImportBreak;
         fJitImportBreak.ensureInit(CLRConfig::INTERNAL_JitImportBreak);
-        if (fJitImportBreak.contains(info.compMethodName, info.compClassName, info.compMethodInfo->args.pSig))
+        if (fJitImportBreak.contains(info.compMethodName, info.compClassName, &info.compMethodInfo->args))
             assert(!"JitImportBreak reached");
     }
 
@@ -2113,7 +2113,7 @@ void                Compiler::compInitOptions(unsigned compileFlags)
             static ConfigMethodSet fNgenDump;
             fNgenDump.ensureInit(CLRConfig::INTERNAL_NgenDump);
 
-            if (fNgenDump.contains(info.compMethodName, info.compClassName, info.compMethodInfo->args.pSig))
+            if (fNgenDump.contains(info.compMethodName, info.compClassName, &info.compMethodInfo->args))
                 verboseDump = true;
 
             static ConfigDWORD fNgenHashDump;
@@ -2124,7 +2124,7 @@ void                Compiler::compInitOptions(unsigned compileFlags)
             static ConfigMethodSet fNgenDumpIR;
             fNgenDumpIR.ensureInit(CLRConfig::INTERNAL_NgenDumpIR);
 
-            if (fNgenDumpIR.contains(info.compMethodName, info.compClassName, info.compMethodInfo->args.pSig))
+            if (fNgenDumpIR.contains(info.compMethodName, info.compClassName, &info.compMethodInfo->args))
                 dumpIR = true;
 
             static ConfigDWORD fNgenHashDumpIR;
@@ -2143,7 +2143,7 @@ void                Compiler::compInitOptions(unsigned compileFlags)
             static ConfigMethodSet fJitDump;
             fJitDump.ensureInit(CLRConfig::INTERNAL_JitDump);
 
-            if (fJitDump.contains(info.compMethodName, info.compClassName, info.compMethodInfo->args.pSig))
+            if (fJitDump.contains(info.compMethodName, info.compClassName, &info.compMethodInfo->args))
                 verboseDump = true;
 
             static ConfigDWORD fJitHashDump;
@@ -2154,7 +2154,7 @@ void                Compiler::compInitOptions(unsigned compileFlags)
             static ConfigMethodSet fJitDumpIR;
             fJitDumpIR.ensureInit(CLRConfig::INTERNAL_JitDumpIR);
 
-            if (fJitDumpIR.contains(info.compMethodName, info.compClassName, info.compMethodInfo->args.pSig))
+            if (fJitDumpIR.contains(info.compMethodName, info.compClassName, &info.compMethodInfo->args))
                 dumpIR = true;
 
             static ConfigDWORD fJitHashDumpIR;
@@ -2538,24 +2538,24 @@ void                Compiler::compInitOptions(unsigned compileFlags)
 
             static ConfigMethodSet fNgenGCDump;
             fNgenGCDump.ensureInit(CLRConfig::INTERNAL_NgenGCDump);
-            if (fNgenGCDump.contains(info.compMethodName, info.compClassName, info.compMethodInfo->args.pSig))
+            if (fNgenGCDump.contains(info.compMethodName, info.compClassName, &info.compMethodInfo->args))
                 opts.dspGCtbls = true;
 
             static ConfigMethodSet fNgenDisasm;
             fNgenDisasm.ensureInit(CLRConfig::INTERNAL_NgenDisasm);
-            if (fNgenDisasm.contains(info.compMethodName, info.compClassName, info.compMethodInfo->args.pSig))
+            if (fNgenDisasm.contains(info.compMethodName, info.compClassName, &info.compMethodInfo->args))
                 opts.disAsm = true;
-            if (fNgenDisasm.contains("SPILLED", NULL, NULL))
+            if (fNgenDisasm.contains("SPILLED", NULL))
                 opts.disAsmSpilled = true;
 
             static ConfigMethodSet fNgenUnwindDump;
             fNgenUnwindDump.ensureInit(CLRConfig::INTERNAL_NgenUnwindDump);
-            if (fNgenUnwindDump.contains(info.compMethodName, info.compClassName, info.compMethodInfo->args.pSig))
+            if (fNgenUnwindDump.contains(info.compMethodName, info.compClassName, &info.compMethodInfo->args))
                 opts.dspUnwind = true;
 
             static ConfigMethodSet fNgenEHDump;
             fNgenEHDump.ensureInit(CLRConfig::INTERNAL_NgenEHDump);
-            if (fNgenEHDump.contains(info.compMethodName, info.compClassName, info.compMethodInfo->args.pSig))
+            if (fNgenEHDump.contains(info.compMethodName, info.compClassName, &info.compMethodInfo->args))
                 opts.dspEHTable = true;
         }
         else 
@@ -2566,32 +2566,32 @@ void                Compiler::compInitOptions(unsigned compileFlags)
 
             static ConfigMethodSet fJitGCDump;
             fJitGCDump.ensureInit(CLRConfig::INTERNAL_JitGCDump);
-            if (fJitGCDump.contains(info.compMethodName, info.compClassName, info.compMethodInfo->args.pSig))
+            if (fJitGCDump.contains(info.compMethodName, info.compClassName, &info.compMethodInfo->args))
                 opts.dspGCtbls = true;
 
             static ConfigMethodSet fJitDisasm;
             fJitDisasm.ensureInit(CLRConfig::INTERNAL_JitDisasm);
-            if (fJitDisasm.contains(info.compMethodName, info.compClassName, info.compMethodInfo->args.pSig))
+            if (fJitDisasm.contains(info.compMethodName, info.compClassName, &info.compMethodInfo->args))
                 opts.disAsm = true;
 
-            if (fJitDisasm.contains("SPILLED", NULL, NULL))
+            if (fJitDisasm.contains("SPILLED", NULL))
                 opts.disAsmSpilled = true;
 
             static ConfigMethodSet fJitUnwindDump;
             fJitUnwindDump.ensureInit(CLRConfig::INTERNAL_JitUnwindDump);
-            if (fJitUnwindDump.contains(info.compMethodName, info.compClassName, info.compMethodInfo->args.pSig))
+            if (fJitUnwindDump.contains(info.compMethodName, info.compClassName, &info.compMethodInfo->args))
                 opts.dspUnwind = true;
 
             static ConfigMethodSet fJitEHDump;
             fJitEHDump.ensureInit(CLRConfig::INTERNAL_JitEHDump);
-            if (fJitEHDump.contains(info.compMethodName, info.compClassName, info.compMethodInfo->args.pSig))
+            if (fJitEHDump.contains(info.compMethodName, info.compClassName, &info.compMethodInfo->args))
                 opts.dspEHTable = true;
         }
 
 #ifdef LATE_DISASM
         static ConfigMethodSet fJitLateDisasm;
         fJitLateDisasm.ensureInit(CLRConfig::INTERNAL_JitLateDisasm);
-        if (fJitLateDisasm.contains(info.compMethodName, info.compClassName, info.compMethodInfo->args.pSig))
+        if (fJitLateDisasm.contains(info.compMethodName, info.compClassName, &info.compMethodInfo->args))
             opts.doLateDisasm = true;
 #endif // LATE_DISASM
 
@@ -2649,7 +2649,7 @@ void                Compiler::compInitOptions(unsigned compileFlags)
 
     static ConfigMethodSet fJitBreak;
     fJitBreak.ensureInit(CLRConfig::INTERNAL_JitBreak);
-    if (fJitBreak.contains(info.compMethodName, info.compClassName, info.compMethodInfo->args.pSig))
+    if (fJitBreak.contains(info.compMethodName, info.compClassName, &info.compMethodInfo->args))
         assert(!"JitBreak reached");
 
     static ConfigDWORD fJitHashBreak;
@@ -2660,8 +2660,8 @@ void                Compiler::compInitOptions(unsigned compileFlags)
     static ConfigMethodSet fJitDebugBreak;
     fJitDebugBreak.ensureInit(CLRConfig::INTERNAL_JitDebugBreak);
     if (verbose ||
-        fJitDebugBreak.contains(info.compMethodName, info.compClassName, info.compMethodInfo->args.pSig) ||
-        fJitBreak.contains(info.compMethodName, info.compClassName, info.compMethodInfo->args.pSig))
+        fJitDebugBreak.contains(info.compMethodName, info.compClassName, &info.compMethodInfo->args) ||
+        fJitBreak.contains(info.compMethodName, info.compClassName, &info.compMethodInfo->args))
     {
         compDebugBreak = true;
     }
@@ -2754,7 +2754,7 @@ void                Compiler::compInitOptions(unsigned compileFlags)
 #endif // DEBUGGING_SUPPORT
 
 #ifdef  LATE_DISASM
-    codeGen->getDisAssembler().disOpenForLateDisAsm(info.compMethodName, info.compClassName, info.compMethodInfo->args.pSig);
+    codeGen->getDisAssembler().disOpenForLateDisAsm(info.compMethodName, info.compClassName, &info.compMethodInfo->args);
 #endif
 
     //-------------------------------------------------------------------------
@@ -2793,19 +2793,19 @@ void                Compiler::compInitOptions(unsigned compileFlags)
         // splitting in the zapper.
         static ConfigMethodSet fJitForceProcedureSplitting;
         fJitForceProcedureSplitting.ensureInit(CLRConfig::INTERNAL_JitForceProcedureSplitting);
-        if (fJitForceProcedureSplitting.contains(info.compMethodName, info.compClassName, info.compMethodInfo->args.pSig))
+        if (fJitForceProcedureSplitting.contains(info.compMethodName, info.compClassName, &info.compMethodInfo->args))
             opts.compProcedureSplitting = true;
 
         // JitNoProcedureSplitting will always disable procedure splitting.
         static ConfigMethodSet fJitNoProcedureSplitting;
         fJitNoProcedureSplitting.ensureInit(CLRConfig::INTERNAL_JitNoProcedureSplitting);
-        if (fJitNoProcedureSplitting.contains(info.compMethodName, info.compClassName, info.compMethodInfo->args.pSig))
+        if (fJitNoProcedureSplitting.contains(info.compMethodName, info.compClassName, &info.compMethodInfo->args))
             opts.compProcedureSplitting = false;
         //
         // JitNoProcedureSplittingEH will disable procedure splitting in functions with EH.
         static ConfigMethodSet fJitNoProcedureSplittingEH;
         fJitNoProcedureSplittingEH.ensureInit(CLRConfig::INTERNAL_JitNoProcedureSplittingEH);
-        if (fJitNoProcedureSplittingEH.contains(info.compMethodName, info.compClassName, info.compMethodInfo->args.pSig))
+        if (fJitNoProcedureSplittingEH.contains(info.compMethodName, info.compClassName, &info.compMethodInfo->args))
             opts.compProcedureSplittingEH = false;
 #endif
     }
@@ -2931,7 +2931,7 @@ bool Compiler::compJitHaltMethod()
 
     static ConfigMethodSet fJitHalt;
     fJitHalt.ensureInit(CLRConfig::INTERNAL_JitHalt);
-    if (fJitHalt.contains(info.compMethodName, info.compClassName, info.compMethodInfo->args.pSig))
+    if (fJitHalt.contains(info.compMethodName, info.compClassName, &info.compMethodInfo->args))
     {
         return true;
     }
@@ -2977,7 +2977,7 @@ bool            Compiler::compStressCompile(compStressArea  stressArea,
     static ConfigMethodSet fJitStressOnly;
     fJitStressOnly.ensureInit(CLRConfig::INTERNAL_JitStressOnly);
     if (!fJitStressOnly.isEmpty() && 
-        !fJitStressOnly.contains(info.compMethodName, info.compClassName, info.compMethodInfo->args.pSig))
+        !fJitStressOnly.contains(info.compMethodName, info.compClassName, &info.compMethodInfo->args))
     {
         return false;
     }     
@@ -4123,12 +4123,12 @@ bool Compiler::skipMethod()
 
     static ConfigMethodSet fJitExclude;
     fJitExclude.ensureInit(CLRConfig::INTERNAL_JitExclude);
-    if (fJitExclude.contains(info.compMethodName, info.compClassName, info.compMethodInfo->args.pSig))
+    if (fJitExclude.contains(info.compMethodName, info.compClassName, &info.compMethodInfo->args))
         return true;
 
     static ConfigMethodSet fJitInclude;
     fJitInclude.ensureInit(CLRConfig::INTERNAL_JitInclude);
-    if (!fJitInclude.isEmpty() && !fJitInclude.contains(info.compMethodName, info.compClassName, info.compMethodInfo->args.pSig))
+    if (!fJitInclude.isEmpty() && !fJitInclude.contains(info.compMethodName, info.compClassName, &info.compMethodInfo->args))
         return true;
 
     return false;

diff --git a/src/utilcode/util.cpp b/src/utilcode/util.cpp
index d7d3a9f..4642a4c 100644 (file)
--- a/src/utilcode/util.cpp
+++ b/src/utilcode/util.cpp
@@ -18,6 +18,7 @@
 #include "loaderheap.h"
 #include "sigparser.h"
 #include "cor.h"
+#include "corinfo.h"
 
 #ifndef FEATURE_CORECLR
 #include "metahost.h"
@@ -1302,7 +1303,7 @@ bool ConfigMethodSet::contains(LPCUTF8 methodName, LPCUTF8 className, PCCOR_SIGN
         NOTHROW;
     }
     CONTRACTL_END;
-    
+
     _ASSERTE(m_inited == 1);
 
     if (m_list.IsEmpty())
@@ -1311,6 +1312,22 @@ bool ConfigMethodSet::contains(LPCUTF8 methodName, LPCUTF8 className, PCCOR_SIGN
 }
 
 /**************************************************************************/
+bool ConfigMethodSet::contains(LPCUTF8 methodName, LPCUTF8 className, CORINFO_SIG_INFO* pSigInfo)
+{
+    CONTRACTL
+    {
+        NOTHROW;
+    }
+    CONTRACTL_END;
+
+    _ASSERTE(m_inited == 1);
+
+    if (m_list.IsEmpty())
+        return false;
+    return(m_list.IsInList(methodName, className, pSigInfo));
+}
+
+/**************************************************************************/
 void ConfigDWORD::init_DontUse_(__in_z LPCWSTR keyName, DWORD defaultVal)
 {
     CONTRACTL
@@ -1648,20 +1665,50 @@ bool MethodNamesListBase::IsInList(LPCUTF8 methName, LPCUTF8 clsName, PCCOR_SIGN
         NOTHROW;
     }
     CONTRACTL_END;
-    
-    ULONG numArgs = -1;
+
+    int numArgs = -1;
     if (sig != NULL)
     {
         sig++;      // Skip calling convention
         numArgs = CorSigUncompressData(sig);  
     }
 
+    return IsInList(methName, clsName, numArgs);
+}
+
+/**************************************************************/
+bool MethodNamesListBase::IsInList(LPCUTF8 methName, LPCUTF8 clsName, CORINFO_SIG_INFO* pSigInfo)
+{
+    CONTRACTL
+    {
+        NOTHROW;
+    }
+    CONTRACTL_END;
+
+    int numArgs = -1;
+    if (pSigInfo != NULL)
+    {
+        numArgs = pSigInfo->numArgs;
+    }
+
+    return IsInList(methName, clsName, numArgs);
+}
+
+/**************************************************************/
+bool MethodNamesListBase::IsInList(LPCUTF8 methName, LPCUTF8 clsName, int numArgs) 
+{
+    CONTRACTL
+    {
+        NOTHROW;
+    }
+    CONTRACTL_END;
+
     // Try to match all the entries in the list
 
     for(MethodName * pName = pNames; pName; pName = pName->next)
     {
         // If numArgs is valid, check for mismatch
-        if (pName->numArgs != -1 && (ULONG)pName->numArgs != numArgs)
+        if (pName->numArgs != -1 && pName->numArgs != numArgs)
             continue;
 
         // If methodName is valid, check for mismatch

diff --git a/src/vm/securitymeta.cpp b/src/vm/securitymeta.cpp
index 9ab6dbe..5dd7997 100644 (file)
--- a/src/vm/securitymeta.cpp
+++ b/src/vm/securitymeta.cpp
@@ -67,7 +67,7 @@ void FieldSecurityDescriptor::VerifyDataComputed()
     // now.
     static ConfigMethodSet fieldTransparencyBreak;
     fieldTransparencyBreak.ensureInit(CLRConfig::INTERNAL_Security_TransparencyFieldBreak);
-    if (fieldTransparencyBreak.contains(m_pFD->GetName(), m_pFD->GetApproxEnclosingMethodTable()->GetDebugClassName(), NULL))
+    if (fieldTransparencyBreak.contains(m_pFD->GetName(), m_pFD->GetApproxEnclosingMethodTable()->GetDebugClassName()))
     {
         DebugBreak();
     }
@@ -296,7 +296,7 @@ void MethodSecurityDescriptor::ComputeCriticalTransparentInfo()
     // now.
     static ConfigMethodSet methodTransparencyBreak;
     methodTransparencyBreak.ensureInit(CLRConfig::INTERNAL_Security_TransparencyMethodBreak);
-    if (methodTransparencyBreak.contains(m_pMD->GetName(), pMT->GetDebugClassName(), NULL))
+    if (methodTransparencyBreak.contains(m_pMD->GetName(), pMT->GetDebugClassName()))
     {
         DebugBreak();
     }