diff --git a/docs/design/coreclr/botr/clr-abi.md b/docs/design/coreclr/botr/clr-abi.md index 14801dd8b6b897..1e5730f54d334d 100644 --- a/docs/design/coreclr/botr/clr-abi.md +++ b/docs/design/coreclr/botr/clr-abi.md @@ -696,7 +696,7 @@ The extra state created by the JIT for synchronized methods (lock taken flag) mu ## Generics -EnC is not supported for generic methods and methods on generic types. +EnC is supported for adding and editing generic methods and methods on generic types and generic methods on non-generic types. # System V x86_64 support diff --git a/docs/design/coreclr/botr/method-descriptor.md b/docs/design/coreclr/botr/method-descriptor.md index 35fa91c85263c9..496ecc792af6f3 100644 --- a/docs/design/coreclr/botr/method-descriptor.md +++ b/docs/design/coreclr/botr/method-descriptor.md @@ -206,10 +206,10 @@ For example, it may be a bad idea to use the temporary entry point to call the m The methods to get callable entry points from MethodDesc are: -- MethodDesc::GetSingleCallableAddrOfCode -- MethodDesc::GetMultiCallableAddrOfCode -- MethodDesc::GetSingleCallableAddrOfVirtualizedCode -- MethodDesc::GetMultiCallableAddrOfVirtualizedCode +- `MethodDesc::GetSingleCallableAddrOfCode` +- `MethodDesc::GetMultiCallableAddrOfCode` +- `MethodDesc::GetSingleCallableAddrOfVirtualizedCode` +- `MethodDesc::GetMultiCallableAddrOfVirtualizedCode` Types of precode ---------------- diff --git a/docs/design/coreclr/botr/shared-generics.md b/docs/design/coreclr/botr/shared-generics.md index 14ecfa1aa70cac..153ab429725d66 100644 --- a/docs/design/coreclr/botr/shared-generics.md +++ b/docs/design/coreclr/botr/shared-generics.md @@ -25,7 +25,7 @@ Without shared generics, the code for instantiations like `Method` or `M ret ``` -With shared generics, the canonical code will not have any hard-coded versions of the type handle of List, but instead looks up the exact type handle either through a call to a runtime helper API, or by loading it up from the *generic dictionary* of the instantiation of Method that is executing. The code would look more like the following: +With shared generics, the canonical code will not have any hard-coded versions of the type handle of `List`, but instead looks up the exact type handle either through a call to a runtime helper API, or by loading it up from the *generic dictionary* of the instantiation of `Method` that is executing. The code would look more like the following: ``` asm mov rcx, generic context // MethodDesc of Method or Method mov rcx, [rcx + offset of InstantiatedMethodDesc::m_pPerInstInfo] // This is the generic dictionary @@ -34,9 +34,9 @@ With shared generics, the canonical code will not have any hard-coded versions o ret ``` -The generic context in this example is the InstantiatedMethodDesc of `Method` or `Method`. The generic dictionary is a data structure used by shared generic code to fetch instantiation-specific information. It is basically an array where the entries are instantiation-specific type handles, method handles, field handles, method entry points, etc... The "PerInstInfo" fields on MethodTable and InstantiatedMethodDesc structures point at the generic dictionary structure for a generic type and method respectively. +The generic context in this example is the `InstantiatedMethodDesc` of `Method` or `Method`. The generic dictionary is a data structure used by shared generic code to fetch instantiation-specific information. It is basically an array where the entries are instantiation-specific type handles, method handles, field handles, method entry points, etc... The "PerInstInfo" fields on MethodTable and `InstantiatedMethodDesc` structures point at the generic dictionary structure for a generic type and method respectively. -In this example, the generic dictionary for Method will contain a slot with the type handle for type List, and the generic dictionary for Method will contain a slot with the type handle for type List. +In this example, the generic dictionary for `Method` will contain a slot with the type handle for type `List`, and the generic dictionary for `Method` will contain a slot with the type handle for type `List`. This feature is currently only supported for instantiations over reference types because they all have the same size/properties/layout/etc... For instantiations over primitive types or value types, the runtime will generate separate code bodies for each instantiation. @@ -89,7 +89,7 @@ Note that `AnotherDerivedClass` doesn't have a dictionary of its own given that ### Dictionary Slots -As described earlier, a generic dictionary is an array of multiple slots containing instantiation-specific information. When a dictionary is initially allocated for a certain generic type or method, all of its slots are initialized to NULL, and are lazily populated on demand as code executes (see: `Dictionary::PopulateEntry(...)`). +As described earlier, a generic dictionary is an array of multiple slots containing instantiation-specific information. When a dictionary is initially allocated for a certain generic type or method, all of its slots are initialized to `NULL`, and are lazily populated on demand as code executes (see: `Dictionary::PopulateEntry(...)`). The first N slots in an instantiation of N arguments are always going to be the type handles of the instantiation type arguments (this is kind of an optimization as well). The slots that follow contain instantiation-based information. @@ -97,17 +97,17 @@ For instance, here is an example of the contents of the generic dictionary for o | `Method's dictionary` | |--------------------------| -| slot[0]: TypeHandle(`string`) | -| slot[1]: Total dictionary size | -| slot[2]: TypeHandle(`List`) | -| slot[3]: NULL (not used) | -| slot[4]: NULL (not used) | +| `slot[0]: TypeHandle(string)` | +| `slot[1]: Total dictionary size` | +| `slot[2]: TypeHandle(List)` | +| `slot[3]: NULL (not used)` | +| `slot[4]: NULL (not used)` | *Note: the size slot is never used by generic code, and is part of the dynamic dictionary expansion feature. More on that below.* -When this dictionary is first allocated, only slot[0] is initialized because it contains the instantiation type arguments (and of course the size slot is also initialized with the dictionary expansion feature), but the rest of the slots (example slot[2]) are NULL, and get lazily populated with values if we ever hit a code path that attempts to use them. +When this dictionary is first allocated, only `slot[0]` is initialized because it contains the instantiation type arguments (and of course the size slot is also initialized with the dictionary expansion feature), but the rest of the slots (for example, `slot[2]`) are `NULL`, and get lazily populated with values if we ever hit a code path that attempts to use them. -When loading information from a slot that is still NULL, the generic code will call one of these runtime helper functions to populate the dictionary slot with a value: +When loading information from a slot that is still `NULL`, the generic code will call one of these runtime helper functions to populate the dictionary slot with a value: - `JIT_GenericHandleClass`: Used to lookup a value in a generic type dictionary. This helper is used by all instance methods on generic types. - `JIT_GenericHandleMethod`: Used to lookup a value in a generic method dictionary. This helper used by all generic methods, or non-generic static methods on generic types. @@ -117,9 +117,9 @@ When generating shared generic code, the JIT knows which slots to use for the va The `DictionaryLayout` structure is what tells the JIT which slot to use when performing a dictionary lookup. This `DictionaryLayout` structure has a couple of important properties: - It is shared across all compatible instantiations of a certain type of method. In other words, a dictionary layout is associated with the canonical instantiation of a type or a method. For instance, in our example above, `Method` and `Method` are compatible instantiations, each with their own **separate dictionaries**, however they all share the **same dictionary layout**, which is associated with the canonical instantiation `Method<__Canon>`. -- The dictionaries of generic types or methods have the same number of slots as their dictionary layouts. Note: historically before the introduction of the dynamic dictionary expansion feature, the generic dictionaries could be smaller than their layouts, meaning that for certain lookups, we had to use invoke some runtime helper APIs (slow path). +- The dictionaries of generic types or methods have the same number of slots as their dictionary layouts. Note: historically before the introduction of the dynamic dictionary expansion feature, the generic dictionaries could be smaller than their layouts, meaning that for certain lookups, we had to invoke some runtime helper APIs (slow path). -When a generic type or method is first created, its dictionary layout contains 'unassigned' slots. Assignments happen as part of code generation, whenever the JIT needs to emit a dictionary lookup sequence. This assignment happens during the calls to the `DictionaryLayout::FindToken(...)` APIs. Once a slot has been assigned, it becomes associated with a certain signature, which describes the kind of value that will go in every instantiated dictionary at that slot index. +When a generic type or method is first created, its dictionary layout contains 'unassigned' slots. Assignments happen as part of code generation, whenever the JIT needs to emit a dictionary lookup sequence. This assignment happens during calls to the `DictionaryLayout::FindToken(...)` APIs. Once a slot has been assigned, it becomes associated with a certain signature, which describes the kind of value that will go in every instantiated dictionary at that slot index. Given an input signature, slot assignment is performed with the following algorithm: diff --git a/docs/design/coreclr/botr/type-loader.md b/docs/design/coreclr/botr/type-loader.md index b2d5d6032bb4ca..e8ccf1181d60bb 100644 --- a/docs/design/coreclr/botr/type-loader.md +++ b/docs/design/coreclr/botr/type-loader.md @@ -306,14 +306,14 @@ type, they have the typical instantiation in mind. Example: public class A {} ``` -The C# `typeof(A<,,>)` compiles to ldtoken A\'3 which makes the +The C# `typeof(A<,,>)` compiles to ``ldtoken A`3`` which makes the runtime load ``A`3`` instantiated at `S` , `T` , `U`. **Canonical Instantiation** An instantiation where all generic arguments are `System.__Canon`. `System.__Canon` is an internal type defined -in **mscorlib** and its task is just to be well-known and different +in **corlib** and its task is just to be well-known and different from any other type which may be used as a generic argument. Types/methods with canonical instantiation are used as representatives of all instantiations and carry information shared by @@ -343,8 +343,7 @@ of all these types is the same. The figure illustrates this for `List` and `List`. The canonical `MethodTable` was created automatically before the first reference type instantiation was loaded and contains data which is hot but not -instantiation specific like non-virtual slots or -`RemotableMethodInfo`. Instantiations containing only value types +instantiation specific like non-virtual slots. Instantiations containing only value types are not shared and every such instantiated type gets its own unshared `EEClass`. diff --git a/src/coreclr/System.Private.CoreLib/src/System/Reflection/Metadata/MetadataUpdater.cs b/src/coreclr/System.Private.CoreLib/src/System/Reflection/Metadata/MetadataUpdater.cs index 26ef9ed252df4e..ff1c5b855032c4 100644 --- a/src/coreclr/System.Private.CoreLib/src/System/Reflection/Metadata/MetadataUpdater.cs +++ b/src/coreclr/System.Private.CoreLib/src/System/Reflection/Metadata/MetadataUpdater.cs @@ -54,7 +54,7 @@ public static void ApplyUpdate(Assembly assembly, ReadOnlySpan metadataDel /// /// Returns the metadata update capabilities. /// - internal static string GetCapabilities() => "Baseline AddMethodToExistingType AddStaticFieldToExistingType AddInstanceFieldToExistingType NewTypeDefinition ChangeCustomAttributes UpdateParameters"; + internal static string GetCapabilities() => "Baseline AddMethodToExistingType AddStaticFieldToExistingType AddInstanceFieldToExistingType NewTypeDefinition ChangeCustomAttributes UpdateParameters GenericUpdateMethod GenericAddMethodToExistingType GenericAddFieldToExistingType"; /// /// Returns true if the apply assembly update is enabled and available. diff --git a/src/coreclr/debug/daccess/dacdbiimpl.cpp b/src/coreclr/debug/daccess/dacdbiimpl.cpp index 2d546ece1ccec3..5be32be906bf1a 100644 --- a/src/coreclr/debug/daccess/dacdbiimpl.cpp +++ b/src/coreclr/debug/daccess/dacdbiimpl.cpp @@ -1728,8 +1728,7 @@ void DacDbiInterfaceImpl::CollectFields(TypeHandle thExact, // FieldDesc::GetExactDeclaringType to get at the correct field. This requires the exact // TypeHandle. EncApproxFieldDescIterator fdIterator(thApprox.GetMethodTable(), - ApproxFieldDescIterator::ALL_FIELDS, - FALSE); // don't fixup EnC (we can't, we're stopped) + ApproxFieldDescIterator::ALL_FIELDS); // don't fixup EnC (we can't, we're stopped) PTR_FieldDesc pCurrentFD; unsigned int index = 0; @@ -3873,8 +3872,7 @@ void DacDbiInterfaceImpl::GetCachedWinRTTypes( PTR_FieldDesc DacDbiInterfaceImpl::FindField(TypeHandle thApprox, mdFieldDef fldToken) { EncApproxFieldDescIterator fdIterator(thApprox.GetMethodTable(), - ApproxFieldDescIterator::ALL_FIELDS, - FALSE); // don't fixup EnC (we can't, we're stopped) + ApproxFieldDescIterator::ALL_FIELDS); // don't fixup EnC (we can't, we're stopped) PTR_FieldDesc pCurrentFD; diff --git a/src/coreclr/debug/ee/controller.cpp b/src/coreclr/debug/ee/controller.cpp index 12e486a773bb5e..266b860ebcfdcf 100644 --- a/src/coreclr/debug/ee/controller.cpp +++ b/src/coreclr/debug/ee/controller.cpp @@ -94,7 +94,7 @@ SharedPatchBypassBuffer* DebuggerControllerPatch::GetOrCreateSharedPatchBypassBu return m_pSharedPatchBypassBuffer; } -#endif // FEATURE_EMULATE_SINGLESTEP +#endif // !FEATURE_EMULATE_SINGLESTEP // @todo - remove all this splicing trash // This Sort/Splice stuff just reorders the patches within a particular chain such @@ -105,7 +105,7 @@ SharedPatchBypassBuffer* DebuggerControllerPatch::GetOrCreateSharedPatchBypassBu #if 1 void DebuggerPatchTable::SortPatchIntoPatchList(DebuggerControllerPatch **ppPatch) { - LOG((LF_CORDB, LL_EVERYTHING, "DPT::SPIPL called.\n")); + LOG((LF_CORDB, LL_EVERYTHING, "DPT::SPIPL ppPatch: %p, pPatch: %p \n", ppPatch, (*ppPatch))); #ifdef _DEBUG DebuggerControllerPatch *patchFirst = (DebuggerControllerPatch *) Find(Hash((*ppPatch)), Key((*ppPatch))); @@ -113,12 +113,11 @@ void DebuggerPatchTable::SortPatchIntoPatchList(DebuggerControllerPatch **ppPatc _ASSERTE((*ppPatch)->controller->GetDCType() != DEBUGGER_CONTROLLER_STATIC); #endif //_DEBUG DebuggerControllerPatch *patchNext = GetNextPatch((*ppPatch)); -LOG((LF_CORDB, LL_EVERYTHING, "DPT::SPIPL GetNextPatch passed\n")); + //List contains one, (sorted) element if (patchNext == NULL) { - LOG((LF_CORDB, LL_INFO10000, - "DPT::SPIPL: Patch 0x%x is a sorted singleton\n", (*ppPatch))); + LOG((LF_CORDB, LL_INFO10000, "DPT::SPIPL: %p single element\n", (*ppPatch))); return; } @@ -144,12 +143,12 @@ LOG((LF_CORDB, LL_EVERYTHING, "DPT::SPIPL GetNextPatch passed\n")); if (patchNext == GetNextPatch((*ppPatch))) { LOG((LF_CORDB, LL_INFO10000, - "DPT::SPIPL: Patch 0x%x is already sorted\n", (*ppPatch))); + "DPT::SPIPL: Patch %p is already sorted\n", (*ppPatch))); return; //already sorted } LOG((LF_CORDB, LL_INFO10000, - "DPT::SPIPL: Patch 0x%x will be moved \n", (*ppPatch))); + "DPT::SPIPL: Patch %p will be moved \n", (*ppPatch))); //remove it from the list SpliceOutOfList((*ppPatch)); @@ -162,7 +161,7 @@ LOG((LF_CORDB, LL_EVERYTHING, "DPT::SPIPL GetNextPatch passed\n")); SpliceInBackOf((*ppPatch), patchCur); LOG((LF_CORDB, LL_INFO10000, - "DPT::SPIPL: Patch 0x%x is now sorted\n", (*ppPatch))); + "DPT::SPIPL: Patch %p is now sorted\n", (*ppPatch))); } // This can leave the list empty, so don't do this unless you put @@ -470,7 +469,7 @@ DebuggerControllerPatch *DebuggerPatchTable::AddPatchForMethodDef(DebuggerContro DebuggerPatchKind kind, FramePointer fp, AppDomain *pAppDomain, - SIZE_T masterEnCVersion, + SIZE_T primaryEnCVersion, DebuggerJitInfo *dji) { CONTRACTL @@ -481,12 +480,8 @@ DebuggerControllerPatch *DebuggerPatchTable::AddPatchForMethodDef(DebuggerContro } CONTRACTL_END; - - - LOG( (LF_CORDB,LL_INFO10000,"DCP:AddPatchForMethodDef unbound " - "relative in methodDef 0x%x with dji 0x%x " - "controller:0x%x AD:0x%x\n", md, - dji, controller, pAppDomain)); + LOG( (LF_CORDB,LL_INFO10000,"DPT:APFMD 0x%x with dji %p, %s offset 0x%zx controller:%p AD:%p\n", + md, dji, (offsetIsIL ? "IL" : "native"), offset, controller, pAppDomain)); DebuggerFunctionKey key; @@ -494,15 +489,13 @@ DebuggerControllerPatch *DebuggerPatchTable::AddPatchForMethodDef(DebuggerContro key.md = md; // Get a new uninitialized patch object - DebuggerControllerPatch *patch = - (DebuggerControllerPatch *) Add(HashKey(&key)); + DebuggerControllerPatch *patch = (DebuggerControllerPatch *) Add(HashKey(&key)); if (patch == NULL) - { ThrowOutOfMemory(); - } + #ifndef FEATURE_EMULATE_SINGLESTEP patch->Initialize(); -#endif +#endif // !FEATURE_EMULATE_SINGLESTEP //initialize the patch data structure. InitializePRD(&(patch->opcode)); @@ -518,12 +511,12 @@ DebuggerControllerPatch *DebuggerPatchTable::AddPatchForMethodDef(DebuggerContro patch->refCount = 1; // AddRef() patch->fSaveOpcode = false; patch->pAppDomain = pAppDomain; - patch->pid = m_pid++; + patch->patchId = m_patchId++; - if (kind == PATCH_KIND_IL_MASTER) + if (kind == PATCH_KIND_IL_PRIMARY) { _ASSERTE(dji == NULL); - patch->encVersion = masterEnCVersion; + patch->encVersion = primaryEnCVersion; } else { @@ -531,32 +524,38 @@ DebuggerControllerPatch *DebuggerPatchTable::AddPatchForMethodDef(DebuggerContro } patch->kind = kind; - if (dji) - LOG((LF_CORDB,LL_INFO10000,"AddPatchForMethodDef w/ version 0x%04x, " - "pid:0x%x\n", dji->m_encVersion, patch->pid)); - else if (kind == PATCH_KIND_IL_MASTER) - LOG((LF_CORDB,LL_INFO10000,"AddPatchForMethodDef w/ version 0x%04x, " - "pid:0x%x\n", masterEnCVersion,patch->pid)); + if (dji != NULL) + { + LOG((LF_CORDB,LL_INFO10000,"DPT:APFMD w/ encVersion 0x%zx, patchId:0x%zx\n", + dji->m_encVersion, patch->patchId)); + } + else if (kind == PATCH_KIND_IL_PRIMARY) + { + LOG((LF_CORDB,LL_INFO10000,"DPT:APFMD w/ encVersion 0x%zx, patchId:0x%zx (primary)\n", + primaryEnCVersion, patch->patchId)); + } else - LOG((LF_CORDB,LL_INFO10000,"AddPatchForMethodDef w/ no dji or dmi, pid:0x%x\n",patch->pid)); - + { + LOG((LF_CORDB,LL_INFO10000,"DPT:APFMD w/ no dji or dmi, patchId:0x%zx\n", + patch->patchId)); + } // This patch is not yet bound or activated _ASSERTE( !patch->IsBound() ); _ASSERTE( !patch->IsActivated() ); - // The only kind of patch with IL offset is the IL master patch. - _ASSERTE(patch->IsILMasterPatch() || patch->offsetIsIL == FALSE); + // The only kind of patch with IL offset is the IL primary patch. + _ASSERTE(patch->IsILPrimaryPatch() || patch->offsetIsIL == FALSE); - // The only kind of patch that allows a MethodDescFilter is the IL master patch - _ASSERTE(patch->IsILMasterPatch() || patch->pMethodDescFilter == NULL); + // The only kind of patch that allows a MethodDescFilter is the IL primary patch + _ASSERTE(patch->IsILPrimaryPatch() || patch->pMethodDescFilter == NULL); // Zero is the only native offset that we allow to bind across different jitted // code bodies. There isn't any sensible meaning to binding at some other native offset. // Even if all the code bodies had an instruction that started at that offset there is // no guarantee those instructions represent a semantically equivalent point in the // method's execution. - _ASSERTE(!(patch->IsILMasterPatch() && !patch->offsetIsIL && patch->offset != 0)); + _ASSERTE(!(patch->IsILPrimaryPatch() && !patch->offsetIsIL && patch->offset != 0)); return patch; } @@ -572,7 +571,7 @@ DebuggerControllerPatch *DebuggerPatchTable::AddPatchForAddress(DebuggerControll FramePointer fp, AppDomain *pAppDomain, DebuggerJitInfo *dji, - SIZE_T pid, + SIZE_T patchId, TraceType traceType) { @@ -602,7 +601,7 @@ DebuggerControllerPatch *DebuggerPatchTable::AddPatchForAddress(DebuggerControll } #ifndef FEATURE_EMULATE_SINGLESTEP patch->Initialize(); -#endif +#endif // !FEATURE_EMULATE_SINGLESTEP // initialize the patch data structure InitializePRD(&(patch->opcode)); @@ -627,20 +626,22 @@ DebuggerControllerPatch *DebuggerPatchTable::AddPatchForAddress(DebuggerControll patch->refCount = 1; // AddRef() patch->fSaveOpcode = false; patch->pAppDomain = pAppDomain; - if (pid == DCP_PID_INVALID) - patch->pid = m_pid++; + if (patchId == DCP_PATCHID_INVALID) + patch->patchId = m_patchId++; else - patch->pid = pid; + patch->patchId = patchId; patch->dji = dji; patch->kind = kind; if (dji == NULL) - LOG((LF_CORDB,LL_INFO10000,"AddPatchForAddress w/ version with no dji, pid:0x%x\n", patch->pid)); + { + LOG((LF_CORDB,LL_INFO10000,"AddPatchForAddress w/ version with no dji, patchId:0x%zx\n", patch->patchId)); + } else { - LOG((LF_CORDB,LL_INFO10000,"AddPatchForAddress w/ version 0x%04x, " - "pid:0x%x\n", dji->m_methodInfo->GetCurrentEnCVersion(), patch->pid)); + LOG((LF_CORDB,LL_INFO10000,"AddPatchForAddress w/ version 0x%zx, " + "patchId:0x%zx\n", dji->m_methodInfo->GetCurrentEnCVersion(), patch->patchId)); _ASSERTE( fd==NULL || fd == dji->m_nativeCodeVersion.GetMethodDesc() ); } @@ -651,8 +652,8 @@ DebuggerControllerPatch *DebuggerPatchTable::AddPatchForAddress(DebuggerControll _ASSERTE( patch->IsBound() ); _ASSERTE( !patch->IsActivated() ); - // The only kind of patch with IL offset is the IL master patch. - _ASSERTE(patch->IsILMasterPatch() || patch->offsetIsIL == FALSE); + // The only kind of patch with IL offset is the IL primary patch. + _ASSERTE(patch->IsILPrimaryPatch() || patch->offsetIsIL == FALSE); return patch; } @@ -661,7 +662,7 @@ void DebuggerPatchTable::BindPatch(DebuggerControllerPatch *patch, CORDB_ADDRESS { _ASSERTE(patch != NULL); _ASSERTE(address != NULL); - _ASSERTE( !patch->IsILMasterPatch() ); + _ASSERTE( !patch->IsILPrimaryPatch() ); _ASSERTE(!patch->IsBound() ); //Since the actual patch doesn't move, we don't have to worry about @@ -684,7 +685,7 @@ void DebuggerPatchTable::BindPatch(DebuggerControllerPatch *patch, CORDB_ADDRESS void DebuggerPatchTable::UnbindPatch(DebuggerControllerPatch *patch) { _ASSERTE(patch != NULL); - _ASSERTE(patch->kind != PATCH_KIND_IL_MASTER); + _ASSERTE(patch->kind != PATCH_KIND_IL_PRIMARY); _ASSERTE(patch->IsBound() ); _ASSERTE(!patch->IsActivated() ); @@ -721,7 +722,7 @@ void DebuggerPatchTable::RemovePatch(DebuggerControllerPatch *patch) _ASSERTE( !patch->IsActivated() ); #ifndef FEATURE_EMULATE_SINGLESTEP patch->DoCleanup(); -#endif +#endif // !FEATURE_EMULATE_SINGLESTEP // // Because of the implementation of CHashTable, we can safely @@ -730,7 +731,6 @@ void DebuggerPatchTable::RemovePatch(DebuggerControllerPatch *patch) // implementation without considering this fact. // Delete(Hash(patch), (HASHENTRY *) patch); - } DebuggerControllerPatch *DebuggerPatchTable::GetNextPatch(DebuggerControllerPatch *prev) @@ -767,34 +767,25 @@ DebuggerControllerPatch *DebuggerPatchTable::GetNextPatch(DebuggerControllerPatc return NULL; } -#ifdef _DEBUG_PATCH_TABLE - // DEBUG An internal debugging routine, it iterates - // through the hashtable, stopping at every - // single entry, no matter what it's state. For this to - // compile, you're going to have to add friend status - // of this class to CHashTableAndData in - // to $\Com99\Src\inc\UtilCode.h +#ifdef _DEBUG void DebuggerPatchTable::CheckPatchTable() { if (NULL != m_pcEntries) { + LOG((LF_CORDB,LL_INFO1000, "DPT:CPT: %u\n", m_iEntries)); DebuggerControllerPatch *dcp; - int i = 0; - while (i++ opcode != 0 ) { - LOG((LF_CORDB,LL_INFO1000, "dcp->addr:0x%8x " - "mdMD:0x%8x, offset:0x%x, native:%d\n", - dcp->address, dcp->key.md, dcp->offset, - dcp->IsNativePatch())); + dcp->LogInstance(); } } } } - -#endif // _DEBUG_PATCH_TABLE +#endif // _DEBUG // Count how many patches are in the table. // Use for asserts @@ -851,7 +842,7 @@ void DebuggerController::EnsureUniqueThreadStarter(DebuggerThreadStarter * pNew) void DebuggerController::CancelOutstandingThreadStarter(Thread * pThread) { _ASSERTE(pThread != NULL); - LOG((LF_CORDB, LL_EVERYTHING, "DC:CancelOutstandingThreadStarter - checking on thread =0x%p\n", pThread)); + LOG((LF_CORDB, LL_EVERYTHING, "DC:CancelOutstandingThreadStarter - checking on thread=%p\n", pThread)); ControllerLockHolder lockController; DebuggerController * p = g_controllers; @@ -861,7 +852,7 @@ void DebuggerController::CancelOutstandingThreadStarter(Thread * pThread) { if (p->GetThread() == pThread) { - LOG((LF_CORDB, LL_EVERYTHING, "DC:CancelOutstandingThreadStarter, pThread=0x%p, Found=0x%p\n", p)); + LOG((LF_CORDB, LL_EVERYTHING, "DC:CancelOutstandingThreadStarter Found=%p\n", p)); // There's only 1 DTS per thread, so once we find it, we can quit. p->Delete(); @@ -962,7 +953,7 @@ DebuggerController::DebuggerController(Thread * pThread, AppDomain * pAppDomain) } CONTRACTL_END; - LOG((LF_CORDB, LL_INFO10000, "DC: 0x%x m_eventQueuedCount to 0 - DC::DC\n", this)); + LOG((LF_CORDB, LL_INFO10000, "DC::DC %p m_eventQueuedCount=%d\n", this, m_eventQueuedCount)); ControllerLockHolder lockController; { m_next = g_controllers; @@ -1053,15 +1044,13 @@ void DebuggerController::Delete() if (m_eventQueuedCount == 0) { - LOG((LF_CORDB|LF_ENC, LL_INFO100000, "DC::Delete: actual delete of this:0x%x!\n", this)); + LOG((LF_CORDB|LF_ENC, LL_INFO100000, "DC::Delete: actual delete of this: %p\n", this)); TRACE_FREE(this); DeleteInteropSafe(this); } else { - LOG((LF_CORDB|LF_ENC, LL_INFO100000, "DC::Delete: marked for " - "future delete of this:0x%x!\n", this)); - LOG((LF_CORDB|LF_ENC, LL_INFO10000, "DC:0x%x m_eventQueuedCount at 0x%x\n", + LOG((LF_CORDB|LF_ENC, LL_INFO100000, "DC::Delete: marked for future delete of this: %p, m_eventQueuedCount=%d\n", this, m_eventQueuedCount)); m_deleted = true; } @@ -1073,18 +1062,19 @@ void DebuggerController::DebuggerDetachClean() } //static -void DebuggerController::AddRef(DebuggerControllerPatch *patch) +void DebuggerController::AddRefPatch(DebuggerControllerPatch *patch) { + LOG((LF_CORDB, LL_INFO10000, "DC::ARP: patchId:0x%zx\n", patch->patchId)); patch->refCount++; } //static -void DebuggerController::Release(DebuggerControllerPatch *patch) +void DebuggerController::ReleasePatch(DebuggerControllerPatch *patch) { patch->refCount--; if (patch->refCount == 0) { - LOG((LF_CORDB, LL_INFO10000, "DCP::R: patch deleted, deactivating\n")); + LOG((LF_CORDB, LL_INFO10000, "DC::RP: patchId:0x%zx deleted, deactivating\n", patch->patchId)); DeactivatePatch(patch); GetPatchTable()->RemovePatch(patch); } @@ -1128,7 +1118,7 @@ void DebuggerController::DisableAll() { if (patch->controller == this) { - Release(patch); + ReleasePatch(patch); } } } @@ -1165,7 +1155,7 @@ void DebuggerController::Enqueue() LIMITED_METHOD_CONTRACT; m_eventQueuedCount++; - LOG((LF_CORDB, LL_INFO10000, "DC::Enq DC:0x%x m_eventQueuedCount at 0x%x\n", + LOG((LF_CORDB, LL_INFO10000, "DC::Enq DC: %p m_eventQueuedCount at 0x%x\n", this, m_eventQueuedCount)); } @@ -1183,7 +1173,7 @@ void DebuggerController::Dequeue() } CONTRACTL_END; - LOG((LF_CORDB, LL_INFO10000, "DC::Deq DC:0x%x m_eventQueuedCount at 0x%x\n", + LOG((LF_CORDB, LL_INFO10000, "DC::Deq DC: %p m_eventQueuedCount at 0x%x\n", this, m_eventQueuedCount)); if (--m_eventQueuedCount == 0) { @@ -1221,7 +1211,7 @@ void DebuggerController::Dequeue() // mapping with. Future calls will fail too. // returns false, *pFail = true bool DebuggerController::BindPatch(DebuggerControllerPatch *patch, - MethodDesc *fd, + MethodDesc *pMD, CORDB_ADDRESS_TYPE *startAddr) { CONTRACTL @@ -1235,8 +1225,11 @@ bool DebuggerController::BindPatch(DebuggerControllerPatch *patch, CONTRACTL_END; _ASSERTE(patch != NULL); - _ASSERTE(!patch->IsILMasterPatch()); - _ASSERTE(fd != NULL); + _ASSERTE(!patch->IsILPrimaryPatch()); + _ASSERTE(pMD != NULL); + + LOG((LF_CORDB,LL_INFO10000, "DC::BP: Patch %p (patchId:0x%zx) to %s::%s (pMD: %p) at %p\n", + patch, patch->patchId, pMD->m_pszDebugClassName, pMD->m_pszDebugMethodName, pMD, startAddr)); // // Translate patch to address, if it hasn't been already. @@ -1258,10 +1251,10 @@ bool DebuggerController::BindPatch(DebuggerControllerPatch *patch, { // Should not be trying to place patches on MethodDecs's for stubs. // These stubs will never get jitted. - CONSISTENCY_CHECK_MSGF(!fd->IsWrapperStub(), ("Can't place patch at stub md %p, %s::%s", - fd, fd->m_pszDebugClassName, fd->m_pszDebugMethodName)); + CONSISTENCY_CHECK_MSGF(!pMD->IsWrapperStub(), ("Can't place patch at stub md %p, %s::%s", + pMD, pMD->m_pszDebugClassName, pMD->m_pszDebugMethodName)); - startAddr = (CORDB_ADDRESS_TYPE *)g_pEEInterface->GetFunctionAddress(fd); + startAddr = (CORDB_ADDRESS_TYPE *)g_pEEInterface->GetFunctionAddress(pMD); // // Code is not available yet to patch. The prestub should // notify us when it is executed. @@ -1269,7 +1262,7 @@ bool DebuggerController::BindPatch(DebuggerControllerPatch *patch, if (startAddr == NULL) { LOG((LF_CORDB, LL_INFO10000, - "DC::BP:Patch at 0x%x not bindable yet.\n", patch->offset)); + "DC::BP: Patch at 0x%zx not bindable yet.\n", patch->offset)); return false; } @@ -1279,12 +1272,12 @@ bool DebuggerController::BindPatch(DebuggerControllerPatch *patch, _ASSERTE(!g_pEEInterface->IsStub((const BYTE *)startAddr)); // If we've jitted, map to a native offset. - DebuggerJitInfo *info = g_pDebugger->GetJitInfo(fd, (const BYTE *)startAddr); + DebuggerJitInfo *info = g_pDebugger->GetJitInfo(pMD, (const BYTE *)startAddr); #ifdef LOGGING if (info == NULL) { - LOG((LF_CORDB,LL_INFO10000, "DC::BindPa: For startAddr 0x%x, didn't find a DJI\n", startAddr)); + LOG((LF_CORDB,LL_INFO10000, "DC::BP: For startAddr %p, didn't find a DJI\n", startAddr)); } #endif //LOGGING if (info != NULL) @@ -1292,21 +1285,21 @@ bool DebuggerController::BindPatch(DebuggerControllerPatch *patch, // There is a strange case with prejitted code and unjitted trace patches. We can enter this function // with no DebuggerJitInfo created, then have the call just above this actually create the // DebuggerJitInfo, which causes JitComplete to be called, which causes all patches to be bound! If this - // happens, then we don't need to continue here (its already been done recursivley) and we don't need to - // re-active the patch, so we return false from right here. We can check this by seeing if we suddently + // happens, then we don't need to continue here (its already been done recursively) and we don't need to + // re-active the patch, so we return false from right here. We can check this by seeing if we suddenly // have the address in the patch set. if (patch->address != NULL) { - LOG((LF_CORDB,LL_INFO10000, "DC::BindPa: patch bound recursivley by GetJitInfo, bailing...\n")); + LOG((LF_CORDB,LL_INFO10000, "DC::BP: patch bound recursively by GetJitInfo, bailing...\n")); return false; } - LOG((LF_CORDB,LL_INFO10000, "DC::BindPa: For startAddr 0x%p, got DJI " - "0x%p, from 0x%p size: 0x%x\n", startAddr, info, info->m_addrOfCode, info->m_sizeOfCode)); + LOG((LF_CORDB,LL_INFO10000, "DC::BP: For startAddr %p, got DJI %p, from %p size: 0x%zu\n", + startAddr, info, info->m_addrOfCode, info->m_sizeOfCode)); } - LOG((LF_CORDB, LL_INFO10000, "DC::BP:Trying to bind patch in %s::%s version %d\n", - fd->m_pszDebugClassName, fd->m_pszDebugMethodName, info ? info->m_encVersion : (SIZE_T)-1)); + LOG((LF_CORDB, LL_INFO10000, "DC::BP: Trying to bind patch in %s::%s version %zu\n", + pMD->m_pszDebugClassName, pMD->m_pszDebugMethodName, info ? info->m_encVersion : (SIZE_T)-1)); _ASSERTE(g_patches != NULL); @@ -1314,7 +1307,7 @@ bool DebuggerController::BindPatch(DebuggerControllerPatch *patch, CodeRegionInfo::GetCodeRegionInfo(NULL, NULL, startAddr).OffsetToAddress(patch->offset); g_patches->BindPatch(patch, addr); - LOG((LF_CORDB, LL_INFO10000, "DC::BP:Binding patch at 0x%x(off:%x)\n", addr, patch->offset)); + LOG((LF_CORDB, LL_INFO10000, "DC::BP:Binding patch at %p (off:0x%zx)\n", addr, patch->offset)); return true; } @@ -1333,23 +1326,21 @@ bool DebuggerController::BindPatch(DebuggerControllerPatch *patch, // placed into the code-stream, false otherwise bool DebuggerController::ApplyPatch(DebuggerControllerPatch *patch) { - LOG((LF_CORDB, LL_INFO10000, "DC::ApplyPatch at addr 0x%p\n", - patch->address)); + _ASSERTE(patch != NULL); + + LOG((LF_CORDB, LL_INFO10000, "DC::ApplyPatch %p, patchId:0x%zx at addr %p\n", + patch, patch->patchId, patch->address)); // If we try to apply an already applied patch, we'll override our saved opcode // with the break opcode and end up getting a break in out patch bypass buffer. _ASSERTE(!patch->IsActivated() ); _ASSERTE(patch->IsBound()); - // Note we may be patching at certain "blessed" points in mscorwks. - // This is very dangerous b/c we can't be sure patch->Address is blessed or not. - - // // Apply the patch. // _ASSERTE(!(g_pConfig->GetGCStressLevel() & (EEConfig::GCSTRESS_INSTR_JIT|EEConfig::GCSTRESS_INSTR_NGEN)) - && "Debugger does not work with GCSTRESS 4"); + && "Debugger does not work with GCSTRESS 0x4 or 0x8"); if (patch->IsNativePatch()) { @@ -1390,7 +1381,8 @@ bool DebuggerController::ApplyPatch(DebuggerControllerPatch *patch) patch->opcode = CORDbgGetInstruction(patch->address); CORDbgInsertBreakpoint((CORDB_ADDRESS_TYPE *)patch->address); - LOG((LF_CORDB, LL_EVERYTHING, "Breakpoint was inserted at %p for opcode %x\n", patch->address, patch->opcode)); + LOG((LF_CORDB, LL_EVERYTHING, "DC::ApplyPatch Breakpoint was inserted at %p for opcode %x\n", + patch->address, patch->opcode)); #if !defined(HOST_OSX) || !defined(HOST_ARM64) if (!VirtualProtect(baseAddress, @@ -1455,11 +1447,12 @@ bool DebuggerController::ApplyPatch(DebuggerControllerPatch *patch) // Returns: true if the patch was unapplied, false otherwise bool DebuggerController::UnapplyPatch(DebuggerControllerPatch *patch) { + _ASSERTE(patch != NULL); _ASSERTE(patch->address != NULL); _ASSERTE(patch->IsActivated() ); - LOG((LF_CORDB,LL_INFO1000, "DC::UP unapply patch at addr 0x%p\n", - patch->address)); + LOG((LF_CORDB, LL_INFO1000, "DC::UnapplyPatch %p, patchId:0x%zx\n", + patch, patch->patchId)); if (patch->IsNativePatch()) { @@ -1563,54 +1556,16 @@ bool DebuggerController::UnapplyPatch(DebuggerControllerPatch *patch) return true; } -// void DebuggerController::UnapplyPatchAt() -// NO LOCKING -// UnapplyPatchAt removes the patch from a copy of the patched code. -// Like UnapplyPatch, except that we don't bother checking -// memory permissions, but instead replace the breakpoint instruction -// with the opcode at an arbitrary memory address. -void DebuggerController::UnapplyPatchAt(DebuggerControllerPatch *patch, - CORDB_ADDRESS_TYPE *address) -{ - _ASSERTE(patch->IsBound() ); - - if (patch->IsNativePatch()) - { - CORDbgSetInstruction((CORDB_ADDRESS_TYPE *)address, patch->opcode); - //note that we don't have to zero out opcode field - //since we're unapplying at something other than - //the original spot. We assert this is true: - _ASSERTE( patch->address != address ); - } - else - { - // - // !!! IL patch logic assumes reference encoding - // -// TODO: : determine if this is needed for AMD64 -#ifdef TARGET_X86 - _ASSERTE(*(unsigned short*)(address+1) == CEE_BREAK); - - *(unsigned short *) (address+1) - = (unsigned short) patch->opcode; - _ASSERTE( patch->address != address ); -#endif // this makes no sense on anything but X86 - } -} - // bool DebuggerController::IsPatched() Is there a patch at addr? // How: if fNative && the instruction at addr is the break // instruction for this platform. bool DebuggerController::IsPatched(CORDB_ADDRESS_TYPE *address, BOOL native) { LIMITED_METHOD_CONTRACT; - if (native) - { return AddressIsBreakpoint(address); - } - else - return false; + + return false; } // DWORD DebuggerController::GetPatchedOpcode() Gets the opcode @@ -1730,8 +1685,10 @@ BOOL DebuggerController::CheckGetPatchedOpcode(CORDB_ADDRESS_TYPE *address, _ASSERTE(patch->IsBound() ); _ASSERTE(!patch->IsActivated() ); - bool fApply = true; + LOG((LF_CORDB|LF_ENC,LL_INFO1000,"DC::ActivatePatch: patchId:0x%zx\n", patch->patchId)); + patch->LogInstance(); + bool fApply = true; // // See if we already have an active patch at this address. // @@ -1744,6 +1701,8 @@ BOOL DebuggerController::CheckGetPatchedOpcode(CORDB_ADDRESS_TYPE *address, // If we're going to skip activating 'patch' because 'p' already exists at the same address // then 'p' must be activated. We expect that all bound patches are activated. _ASSERTE( p->IsActivated() ); + LOG((LF_CORDB, LL_INFO10000, "DC::ActivatePatch: There is another patch at this address, no need to apply it.\n")); + p->LogInstance(); patch->opcode = p->opcode; fApply = false; break; @@ -1759,7 +1718,7 @@ BOOL DebuggerController::CheckGetPatchedOpcode(CORDB_ADDRESS_TYPE *address, ApplyPatch(patch); } - _ASSERTE(patch->IsActivated() ); + _ASSERTE(patch->IsActivated()); } // void DebuggerController::DeactivatePatch() Make sure that a @@ -1771,11 +1730,12 @@ BOOL DebuggerController::CheckGetPatchedOpcode(CORDB_ADDRESS_TYPE *address, void DebuggerController::DeactivatePatch(DebuggerControllerPatch *patch) { _ASSERTE(g_patches != NULL); + _ASSERTE(patch != NULL); - if( !patch->IsBound() ) { - // patch is not bound, nothing to do + LOG((LF_CORDB|LF_ENC,LL_INFO1000,"DC::DeactivatePatch: patchId:0x%zx\n", patch->patchId)); + patch->LogInstance(); + if( !patch->IsBound() ) return; - } // We expect that all bound patches are also activated. // One exception to this is if the shutdown thread killed another thread right after @@ -1795,6 +1755,8 @@ void DebuggerController::DeactivatePatch(DebuggerControllerPatch *patch) { // There is another patch at this address, so don't remove it // However, clear the patch data so that we no longer consider this particular patch activated + LOG((LF_CORDB, LL_INFO10000, "DC::DeactivatePatch: There is another patch at this address, don't unapply it.\n")); + p->LogInstance(); fUnapply = false; InitializePRD(&(patch->opcode)); break; @@ -1813,9 +1775,9 @@ void DebuggerController::DeactivatePatch(DebuggerControllerPatch *patch) // } -// AddILMasterPatch: record a patch on IL code but do not bind it or activate it. The master b.p. +// AddILPrimaryPatch: record a patch on IL code but do not bind it or activate it. The primary b.p. // is associated with a module/token pair. It is used later -// (e.g. in MapAndBindFunctionPatches) to create one or more "slave" +// (e.g. in MapAndBindFunctionPatches) to create one or more "replica" // breakpoints which are associated with particular MethodDescs/JitInfos. // // Rationale: For generic code a single IL patch (e.g a breakpoint) @@ -1825,14 +1787,14 @@ void DebuggerController::DeactivatePatch(DebuggerControllerPatch *patch) // // So we keep one patch which describes // the breakpoint but which is never actually bound or activated. -// This is then used to apply new "slave" patches to all copies of +// This is then used to apply new "replica" patches to all copies of // JITted code associated with the method. // -// In theory we could bind and apply the master patch when the +// In theory we could bind and apply the primary patch when the // code is known not to be generic (as used to happen to all breakpoint // patches in V1). However this seems like a premature // optimization. -DebuggerControllerPatch *DebuggerController::AddILMasterPatch(Module *module, +DebuggerControllerPatch *DebuggerController::AddILPrimaryPatch(Module *module, mdMethodDef md, MethodDesc *pMethodDescFilter, SIZE_T offset, @@ -1858,51 +1820,52 @@ DebuggerControllerPatch *DebuggerController::AddILMasterPatch(Module *module, pMethodDescFilter, offset, offsetIsIL, - PATCH_KIND_IL_MASTER, + PATCH_KIND_IL_PRIMARY, LEAF_MOST_FRAME, NULL, encVersion, NULL); LOG((LF_CORDB, LL_INFO10000, - "DC::AP: Added IL master patch 0x%p for mdTok 0x%x, desc 0x%p at %s offset %d encVersion %d\n", - patch, md, pMethodDescFilter, offsetIsIL ? "il" : "native", offset, encVersion)); + "DC::AP: Added IL primary patch %p for mdTok 0x%x, filter %p at %s offset 0x%zx encVersion %zx\n", + patch, md, pMethodDescFilter, (offsetIsIL ? "IL" : "native"), offset, encVersion)); return patch; } -// See notes above on AddILMasterPatch -BOOL DebuggerController::AddBindAndActivateILSlavePatch(DebuggerControllerPatch *master, +// See notes above on AddILPrimaryPatch +BOOL DebuggerController::AddBindAndActivateILReplicaPatch(DebuggerControllerPatch *primary, DebuggerJitInfo *dji) { _ASSERTE(g_patches != NULL); - _ASSERTE(master->IsILMasterPatch()); + _ASSERTE(primary->IsILPrimaryPatch()); _ASSERTE(dji != NULL); - BOOL result = FALSE; + BOOL result = FALSE; + MethodDesc* pMD = dji->m_nativeCodeVersion.GetMethodDesc(); - if (!master->offsetIsIL) + if (primary->offsetIsIL == 0) { // Zero is the only native offset that we allow to bind across different jitted // code bodies. - _ASSERTE(master->offset == 0); + _ASSERTE(primary->offset == 0); INDEBUG(BOOL fOk = ) - AddBindAndActivatePatchForMethodDesc(dji->m_nativeCodeVersion.GetMethodDesc(), dji, - 0, PATCH_KIND_IL_SLAVE, + AddBindAndActivatePatchForMethodDesc(pMD, dji, + 0, PATCH_KIND_IL_REPLICA, LEAF_MOST_FRAME, m_pAppDomain); _ASSERTE(fOk); result = TRUE; } else // bind by IL offset { - // Do not dereference the "master" pointer in the loop! The loop may add more patches, + // Do not dereference the "primary" pointer in the loop! The loop may add more patches, // causing the patch table to grow and move. - SIZE_T masterILOffset = master->offset; + SIZE_T primaryILOffset = primary->offset; // Loop through all the native offsets mapped to the given IL offset. On x86 the mapping // should be 1:1. On WIN64, because there are funclets, we have a 1:N mapping. DebuggerJitInfo::ILToNativeOffsetIterator it; - for (dji->InitILToNativeOffsetIterator(it, masterILOffset); !it.IsAtEnd(); it.Next()) + for (dji->InitILToNativeOffsetIterator(it, primaryILOffset); !it.IsAtEnd(); it.Next()) { BOOL fExact; SIZE_T offsetNative = it.Current(&fExact); @@ -1911,21 +1874,18 @@ BOOL DebuggerController::AddBindAndActivateILSlavePatch(DebuggerControllerPatch // at the beginning of a method that hasn't been jitted yet. In // that case it's possible that offset 0 has been optimized out, // but we still want to set the closest breakpoint to that. - if (!fExact && (masterILOffset != 0)) + if (!fExact && (primaryILOffset != 0)) { - LOG((LF_CORDB, LL_INFO10000, "DC::BP:Failed to bind patch at IL offset 0x%p in %s::%s\n", - masterILOffset, dji->m_nativeCodeVersion.GetMethodDesc()->m_pszDebugClassName, dji->m_nativeCodeVersion.GetMethodDesc()->m_pszDebugMethodName)); - + LOG((LF_CORDB, LL_INFO10000, "DC::BP:Failed to bind patch in %s::%s at IL offset 0x%zx, native offset 0x%zx\n", + pMD->m_pszDebugClassName, pMD->m_pszDebugMethodName, primaryILOffset, offsetNative)); continue; } - else - { - result = TRUE; - } + + result = TRUE; INDEBUG(BOOL fOk = ) - AddBindAndActivatePatchForMethodDesc(dji->m_nativeCodeVersion.GetMethodDesc(), dji, - offsetNative, PATCH_KIND_IL_SLAVE, + AddBindAndActivatePatchForMethodDesc(pMD, dji, + offsetNative, PATCH_KIND_IL_REPLICA, LEAF_MOST_FRAME, m_pAppDomain); _ASSERTE(fOk); } @@ -1945,8 +1905,8 @@ BOOL DebuggerController::AddBindAndActivateILSlavePatch(DebuggerControllerPatch // This routine will return FALSE only if we will _never_ be able to // place the patch in any native code corresponding to the given offset. // Otherwise it will: -// (a) record a "master" patch -// (b) apply as many slave patches as it can to existing copies of code +// (a) record a "primary" patch +// (b) apply as many replica patches as it can to existing copies of code // that have debugging information BOOL DebuggerController::AddILPatch(AppDomain * pAppDomain, Module *module, mdMethodDef md, @@ -1962,9 +1922,9 @@ BOOL DebuggerController::AddILPatch(AppDomain * pAppDomain, Module *module, BOOL fOk = FALSE; DebuggerMethodInfo *dmi = g_pDebugger->GetOrCreateMethodInfo(module, md); // throws - LOG((LF_CORDB,LL_INFO10000,"DC::AILP: dmi:0x%p, mdToken:0x%x, mdFilter:0x%p, " - "encVer:%zu, offset:0x%zx <- isIL:%d, Mod:0x%p\n", - dmi, md, pMethodDescFilter, encVersion, offset, offsetIsIL, module)); + LOG((LF_CORDB,LL_INFO10000,"DC::AILP: dmi:%p, mdToken:0x%x, mdFilter:%p, " + "encVer:%zu, offset:0x%zx <- isIL:%s, Mod:%p\n", + dmi, md, pMethodDescFilter, encVersion, offset, (offsetIsIL ? "true" : "false"), module)); if (dmi == NULL) { @@ -1975,15 +1935,15 @@ BOOL DebuggerController::AddILPatch(AppDomain * pAppDomain, Module *module, { // OK, we either have (a) no code at all or (b) we have both JIT information and code //. - // Either way, lay down the MasterPatch. + // Either way, lay down the PrimaryPatch. // // MapAndBindFunctionPatches will take care of any instantiations that haven't - // finished JITting, by making a copy of the master breakpoint. - DebuggerControllerPatch *master = AddILMasterPatch(module, md, pMethodDescFilter, offset, offsetIsIL, encVersion); + // finished JITting, by making a copy of the primary breakpoint. + DebuggerControllerPatch *primary = AddILPrimaryPatch(module, md, pMethodDescFilter, offset, offsetIsIL, encVersion); // We have to keep the index here instead of the pointer. The loop below adds more patches, // which may cause the patch table to grow and move. - ULONG masterIndex = g_patches->GetItemIndex((HASHENTRY*)master); + ULONG primaryIndex = g_patches->GetItemIndex((HASHENTRY*)primary); // Iterate through every existing NativeCodeBlob (with the same EnC version). // This includes generics + prejitted code. @@ -2012,14 +1972,14 @@ BOOL DebuggerController::AddILPatch(AppDomain * pAppDomain, Module *module, { fVersionMatch = TRUE; - master = (DebuggerControllerPatch *)g_patches->GetEntryPtr(masterIndex); + primary = (DebuggerControllerPatch *)g_patches->GetEntryPtr(primaryIndex); // If we're missing JIT info for any then // we won't have applied the bp to every instantiation. That should probably be reported // as a new kind of condition to the debugger, i.e. report "bp only partially applied". It would be // a shame to completely fail just because on instantiation is missing debug info: e.g. just because // one component hasn't been prejitted with debugging information. - fOk = (AddBindAndActivateILSlavePatch(master, dji) || fOk); + fOk = (AddBindAndActivateILReplicaPatch(primary, dji) || fOk); } it.Next(); } @@ -2028,6 +1988,7 @@ BOOL DebuggerController::AddILPatch(AppDomain * pAppDomain, Module *module, // because we don't have a matching version of the method, we need to return TRUE. if (fVersionMatch == FALSE) { + LOG((LF_CORDB,LL_INFO10000,"DC::AILP: No matching DebuggerJitInfo found\n")); fOk = TRUE; } } @@ -2106,17 +2067,15 @@ BOOL DebuggerController::AddBindAndActivatePatchForMethodDesc(MethodDesc *fd, MODE_ANY; // don't really care what mode we're in. PRECONDITION(ThisMaybeHelperThread()); - PRECONDITION(kind != PATCH_KIND_IL_MASTER); + PRECONDITION(kind != PATCH_KIND_IL_PRIMARY); } CONTRACTL_END; BOOL ok = FALSE; ControllerLockHolder ch; - LOG((LF_CORDB|LF_ENC,LL_INFO10000,"DC::AP: Add to %s::%s, at offs 0x%x " - "fp:0x%p AD:0x%p\n", fd->m_pszDebugClassName, - fd->m_pszDebugMethodName, - nativeOffset, fp.GetSPValue(), pAppDomain)); + LOG((LF_CORDB|LF_ENC,LL_INFO10000,"DC::ABAAPFMD: Add to %s::%s, at offs 0x%zx kind:%d fp:%p AD:%p\n", + fd->m_pszDebugClassName, fd->m_pszDebugMethodName, nativeOffset, kind, fp.GetSPValue(), pAppDomain)); DebuggerControllerPatch *patch = g_patches->AddPatchForMethodDef( this, @@ -2133,7 +2092,6 @@ BOOL DebuggerController::AddBindAndActivatePatchForMethodDesc(MethodDesc *fd, if (DebuggerController::BindPatch(patch, fd, NULL)) { - LOG((LF_CORDB|LF_ENC,LL_INFO1000,"BindPatch went fine, doing ActivatePatch\n")); DebuggerController::ActivatePatch(patch); ok = TRUE; } @@ -2171,7 +2129,7 @@ DebuggerControllerPatch *DebuggerController::AddAndActivateNativePatchForAddress fp, NULL, NULL, - DebuggerPatchTable::DCP_PID_INVALID, + DebuggerPatchTable::DCP_PATCHID_INVALID, traceType); ActivatePatch(patch); @@ -2229,7 +2187,7 @@ void DebuggerController::RemovePatchesFromModule(Module *pModule, AppDomain *pAp // we shouldn't be both hitting this patch AND // unloading the module it belongs to. _ASSERTE(!patch->IsTriggering()); - Release( patch ); + ReleasePatch( patch ); } } } @@ -2612,8 +2570,8 @@ DPOSS_ACTION DebuggerController::ScanForTriggers(CORDB_ADDRESS_TYPE *address, CONTRACT_VIOLATION(ThrowsViolation); - LOG((LF_CORDB, LL_INFO10000, "DC::SFT: starting scan for addr:0x%p" - " thread:0x%x\n", address, thread)); + LOG((LF_CORDB, LL_INFO10000, "DC::SFT: starting scan for addr:%p thread:%p\n", + address, thread)); _ASSERTE( pTpr != NULL ); DebuggerControllerPatch *patch = NULL; @@ -2651,7 +2609,7 @@ DPOSS_ACTION DebuggerController::ScanForTriggers(CORDB_ADDRESS_TYPE *address, DebuggerControllerPatch *patchNext = g_patches->GetNextPatch(patch); - LOG((LF_CORDB, LL_INFO10000, "DC::SFT: patch 0x%x, patchNext 0x%x\n", patch, patchNext)); + LOG((LF_CORDB, LL_INFO10000, "DC::SFT: patch:%p, patchNext:%p\n", patch, patchNext)); // Annoyingly, TriggerPatch may add patches, which may cause // the patch table to move, which may, in turn, invalidate @@ -2666,7 +2624,7 @@ DPOSS_ACTION DebuggerController::ScanForTriggers(CORDB_ADDRESS_TYPE *address, if (MatchPatch(thread, context, patch)) { LOG((LF_CORDB, LL_INFO10000, "DC::SFT: patch matched\n")); - AddRef(patch); + AddRefPatch(patch); // We are hitting a patch at a virtual trace call target, so let's trigger trace call here. if (patch->trace.GetTraceType() == TRACE_ENTRY_STUB) @@ -2717,9 +2675,9 @@ DPOSS_ACTION DebuggerController::ScanForTriggers(CORDB_ADDRESS_TYPE *address, if (patchNext != NULL) iEventNext = g_patches->GetItemIndex((HASHENTRY *)patchNext); - // Note that Release() actually removes the patch if its ref count + // Note that ReleasePatch() actually removes the patch if its ref count // reaches 0 after the release. - Release(patch); + ReleasePatch(patch); } if (tpr == TPR_IGNORE_AND_STOP || @@ -2829,55 +2787,58 @@ DPOSS_ACTION DebuggerController::ScanForTriggers(CORDB_ADDRESS_TYPE *address, // Significant speed increase from single dereference, I bet :) (*pTpr) = tpr; - LOG((LF_CORDB, LL_INFO10000, "DC::SFT returning 0x%x as used\n",used)); + LOG((LF_CORDB, LL_INFO10000, "DC::SFT: returning 0x%x as used\n",used)); return used; } #ifdef EnC_SUPPORTED -DebuggerControllerPatch *DebuggerController::IsXXXPatched(const BYTE *PC, - DEBUGGER_CONTROLLER_TYPE dct) -{ - _ASSERTE(g_patches != NULL); - - DebuggerControllerPatch *patch = g_patches->GetPatch((CORDB_ADDRESS_TYPE *)PC); - - while(patch != NULL && - (int)patch->controller->GetDCType() <= (int)dct) - { - if (patch->IsNativePatch() && - patch->controller->GetDCType()==dct) - { - return patch; - } - patch = g_patches->GetNextPatch(patch); - } - - return NULL; -} - // This function will check for an EnC patch at the given address and return // it if one is there, otherwise it will return NULL. -DebuggerControllerPatch *DebuggerController::GetEnCPatch(const BYTE *address) + DebuggerControllerPatch *DebuggerController::GetEnCPatch(const BYTE *address) { _ASSERTE(address); + LOG((LF_CORDB|LF_ENC,LL_INFO1000,"DC:GEnCP at %p\n", address)); if( g_pEEInterface->IsManagedNativeCode(address) ) { + LOG((LF_CORDB|LF_ENC,LL_INFO1000,"DC:GEnCP address is managed code\n")); DebuggerJitInfo *dji = g_pDebugger->GetJitInfoFromAddr((TADDR) address); if (dji == NULL) return NULL; + dji->LogInstance(); + // we can have two types of patches - one in code where the IL has been updated to trigger // the switch and the other in the code we've switched to in order to trigger FunctionRemapComplete // callback. If version == default then can't be the latter, but otherwise if haven't handled the // remap for this function yet is certainly the latter. - if (! dji->m_encBreakpointsApplied && - (dji->m_encVersion == CorDB_DEFAULT_ENC_FUNCTION_VERSION)) + if (! dji->m_encBreakpointsApplied + && (dji->m_encVersion == CorDB_DEFAULT_ENC_FUNCTION_VERSION)) { return NULL; } } - return IsXXXPatched(address, DEBUGGER_CONTROLLER_ENC); + + // Look for EnC patch + DebuggerControllerPatch *patch = g_patches->GetPatch((CORDB_ADDRESS_TYPE *)address); + LOG((LF_CORDB|LF_ENC,LL_INFO1000,"DC:GEnCP Searching, beginning patch: %p\n", patch)); + + while (patch != NULL) + { + // Patches are ordered by DEBUGGER_CONTROLLER_TYPE value + DEBUGGER_CONTROLLER_TYPE dct = patch->controller->GetDCType(); + if ((int)dct > (int)DEBUGGER_CONTROLLER_ENC) + break; + + if (dct == DEBUGGER_CONTROLLER_ENC + && patch->IsNativePatch()) + { + return patch; + } + patch = g_patches->GetNextPatch(patch); + } + + return NULL; } #endif //EnC_SUPPORTED @@ -2935,8 +2896,7 @@ DPOSS_ACTION DebuggerController::DispatchPatchOrSingleStep(Thread *thread, CONTE // debugger wants to remap the function at this point, then we'll call ResumeInUpdatedFunction and // not return, otherwise we will just continue with regular patch-handling logic dcpEnCOriginal = GetEnCPatch(dac_cast(GetIP(context))); - - if (dcpEnCOriginal) + if (dcpEnCOriginal != NULL) { LOG((LF_CORDB|LF_ENC,LL_INFO10000, "DC::DPOSS EnC short-circuit\n")); TP_RESULT tpres = @@ -3416,7 +3376,7 @@ void DebuggerController::EnableUnwind(FramePointer fp) CONTRACTL_END; ASSERT(m_thread != NULL); - LOG((LF_CORDB,LL_EVERYTHING,"DC:EU EnableUnwind at 0x%x\n", fp.GetSPValue())); + LOG((LF_CORDB,LL_EVERYTHING,"DC:EU EnableUnwind at %p\n", fp.GetSPValue())); ControllerLockHolder lockController; m_unwindFP = fp; @@ -4210,15 +4170,16 @@ bool DebuggerController::DispatchNativeException(EXCEPTION_RECORD *pException, // In most cases it is an error to nest, however in the patch-skipping logic we must // copy an unknown amount of code into another buffer and it occasionally triggers - // an AV. This heuristic should filter that case out. See DDB 198093. + // an AV. This heuristic should filter that case out. // Ensure we perform this exception nesting filtering even before the call to // DebuggerController::DispatchExceptionHook, otherwise the nesting will continue when // a contract check is triggered in DispatchExceptionHook and another BP exception is - // raised. See Dev11 66058. - if ((pOldContext != NULL) && pCurThread->AVInRuntimeImplOkay() && - pException->ExceptionCode == STATUS_ACCESS_VIOLATION) + // raised. + if (pOldContext != NULL + && pCurThread->AVInRuntimeImplOkay() + && pException->ExceptionCode == STATUS_ACCESS_VIOLATION) { - STRESS_LOG1(LF_CORDB, LL_INFO100, "DC::DNE Nested Access Violation at 0x%p is being ignored\n", + STRESS_LOG1(LF_CORDB, LL_INFO100, "DC::DNE Nested Access Violation at %p is being ignored\n", pException->ExceptionAddress); return false; } @@ -4340,7 +4301,7 @@ bool DebuggerController::DispatchNativeException(EXCEPTION_RECORD *pException, #ifdef FEATURE_EMULATE_SINGLESTEP if (pCurThread->IsSingleStepEnabled()) pCurThread->ApplySingleStep(pContext); -#endif +#endif // FEATURE_EMULATE_SINGLESTEP FireEtwDebugExceptionProcessingEnd(); @@ -4531,8 +4492,7 @@ DebuggerPatchSkip::DebuggerPatchSkip(Thread *thread, if (context ==(T_CONTEXT*) &c) thread->SetThreadContext(&c); - - LOG((LF_CORDB, LL_INFO10000, "DPS::DPS Bypass at 0x%p for opcode %p \n", patchBypassRX, patch->opcode)); + LOG((LF_CORDB, LL_INFO10000, "DPS::DPS Bypass at %p for opcode 0x%zx \n", patchBypassRX, patch->opcode)); // // Turn on single step (if the platform supports it) so we can @@ -4542,7 +4502,7 @@ DebuggerPatchSkip::DebuggerPatchSkip(Thread *thread, EnableSingleStep(); -#endif // FEATURE_EMULATE_SINGLESTEP +#endif // !FEATURE_EMULATE_SINGLESTEP EnableExceptionHook(); } @@ -4552,7 +4512,7 @@ DebuggerPatchSkip::~DebuggerPatchSkip() #ifndef FEATURE_EMULATE_SINGLESTEP _ASSERTE(m_pSharedPatchBypassBuffer); m_pSharedPatchBypassBuffer->Release(); -#endif +#endif // !FEATURE_EMULATE_SINGLESTEP } void DebuggerPatchSkip::DebuggerDetachClean() @@ -4587,7 +4547,7 @@ void DebuggerPatchSkip::DebuggerDetachClean() SetIP(context, (PCODE)((BYTE *)GetIP(context) - (patchBypass - (BYTE *)m_address))); } } -#endif +#endif // !FEATURE_EMULATE_SINGLESTEP } @@ -4979,7 +4939,6 @@ DebuggerBreakpoint::DebuggerBreakpoint(Module *module, if (native && !nativeCodeBindAllVersions) { (*pSucceed) = AddBindAndActivateNativeManagedPatch(nativeMethodDesc, nativeJITInfo, offset, LEAF_MOST_FRAME, pAppDomain); - return; } else { @@ -5198,7 +5157,7 @@ bool DebuggerStepper::IsRangeAppropriate(ControllerStackInfo *info) bool DebuggerStepper::IsInRange(SIZE_T ip, COR_DEBUG_STEP_RANGE *range, SIZE_T rangeCount, ControllerStackInfo *pInfo) { - LOG((LF_CORDB,LL_INFO10000,"DS::IIR: off=0x%p\n", ip)); + LOG((LF_CORDB,LL_INFO10000,"DS::IIR: off=%zx\n", ip)); if (range == NULL) { @@ -5218,15 +5177,13 @@ bool DebuggerStepper::IsInRange(SIZE_T ip, COR_DEBUG_STEP_RANGE *range, SIZE_T r while (r < rEnd) { SIZE_T endOffset = r->endOffset ? r->endOffset : ~0; - LOG((LF_CORDB,LL_INFO100000,"DS::IIR: so=0x%x, eo=0x%x\n", + LOG((LF_CORDB,LL_INFO100000,"DS::IIR: so=0x%x, eo=0x%zx\n", r->startOffset, endOffset)); if (ip >= r->startOffset && ip < endOffset) { - LOG((LF_CORDB,LL_INFO1000,"DS::IIR:this:0x%p Found native offset " - "0x%x to be in the range" - "[0x%x, 0x%x), index 0x%x\n\n", this, ip, r->startOffset, - endOffset, ((r-range)/sizeof(COR_DEBUG_STEP_RANGE *)) )); + LOG((LF_CORDB,LL_INFO1000,"DS::IIR: this:%p, Found native offset 0x%zx to be in the range [0x%x, 0x%zx), index 0x%zx\n", + this, ip, r->startOffset, endOffset, ((r-range)/sizeof(COR_DEBUG_STEP_RANGE *)) )); return true; } @@ -5637,7 +5594,6 @@ bool DebuggerStepper::TrapStepInHelper( StubManager::DbgBeginLog((TADDR) ipNext, (TADDR) ipCallTarget); #endif - if (TrapStepInto(pInfo, ipCallTarget, &td)) { // If we placed a patch, see if we need to update our step-reason @@ -5653,22 +5609,19 @@ bool DebuggerStepper::TrapStepInHelper( CodeRegionInfo code = CodeRegionInfo::GetCodeRegionInfo(pDJI, md); if (code.AddressToOffset((const BYTE *)td.GetAddress()) == 0) { - - LOG((LF_CORDB,LL_INFO1000,"\tDS::TS 0x%x m_reason = STEP_CALL" - "@ip0x%x\n", this, (BYTE*)GetControlPC(&(pInfo->m_activeFrame.registers)))); + LOG((LF_CORDB,LL_INFO1000,"DS::TS %p m_reason = STEP_CALL @ip%p\n", + this, (BYTE*)GetControlPC(&(pInfo->m_activeFrame.registers)))); m_reason = STEP_CALL; } else { - LOG((LF_CORDB, LL_INFO1000, "Didn't step: md:0x%x" - "td.type:%s td.address:0x%p, hot code address:0x%p\n", - md, GetTType(td.GetTraceType()), td.GetAddress(), - code.getAddrOfHotCode())); + LOG((LF_CORDB, LL_INFO1000, "Didn't step: md:%p td.type:%s td.address:%p, hot code address:%p\n", + md, GetTType(td.GetTraceType()), td.GetAddress(), code.getAddrOfHotCode())); } } else { - LOG((LF_CORDB,LL_INFO10000,"DS::TS else 0x%x m_reason = STEP_CALL\n", + LOG((LF_CORDB,LL_INFO10000,"DS::TS else %p m_reason = STEP_CALL\n", this)); m_reason = STEP_CALL; } @@ -5767,7 +5720,7 @@ static bool IsTailCallThatReturns(const BYTE * ip, ControllerStackInfo* info) // on the method that called the current frame's method. bool DebuggerStepper::TrapStep(ControllerStackInfo *info, bool in) { - LOG((LF_CORDB,LL_INFO10000,"DS::TS: this:0x%x\n", this)); + LOG((LF_CORDB,LL_INFO10000,"DS::TS: this:%p\n", this)); if (!info->m_activeFrame.managed) { // @@ -6733,10 +6686,8 @@ bool DebuggerStepper::SetRangesFromIL(DebuggerJitInfo *dji, COR_DEBUG_STEP_RANGE if (dji != NULL) { - LOG((LF_CORDB,LL_INFO10000,"DeSt::St: For code md=0x%p, got DJI 0x%p, from 0x%p to 0x%p\n", - fd, - dji, dji->m_addrOfCode, (ULONG)dji->m_addrOfCode - + (ULONG)dji->m_sizeOfCode)); + LOG((LF_CORDB,LL_INFO10000,"DeSt::St: For code md=%p, got DJI %p, from %p to %p\n", + fd, dji, dji->m_addrOfCode, dji->m_addrOfCode + dji->m_sizeOfCode)); // // Map ranges to native offsets for jitted code @@ -6889,7 +6840,7 @@ bool DebuggerStepper::Step(FramePointer fp, bool in, COR_DEBUG_STEP_RANGE *ranges, SIZE_T rangeCount, bool rangeIL) { - LOG((LF_CORDB, LL_INFO1000, "DeSt:Step this:0x%x ", this)); + LOG((LF_CORDB, LL_INFO1000, "DeSt:Step this:%p ", this)); if (rangeCount>0) LOG((LF_CORDB,LL_INFO10000," start,end[0]:(0x%x,0x%x)\n", ranges[0].startOffset, ranges[0].endOffset)); @@ -8269,7 +8220,7 @@ bool DebuggerJMCStepper::DetectHandleInterceptors(ControllerStackInfo * info) DebuggerThreadStarter::DebuggerThreadStarter(Thread *thread) : DebuggerController(thread, NULL) { - LOG((LF_CORDB, LL_INFO1000, "DTS::DTS: this:0x%x Thread:0x%x\n", + LOG((LF_CORDB, LL_INFO1000, "DTS::DTS: this: %p Thread: %p\n", this, thread)); // Check to make sure we only have 1 ThreadStarter on a given thread. (Inspired by NDPWhidbey issue 16888) @@ -8720,17 +8671,34 @@ DebuggerEnCBreakpoint::DebuggerEnCBreakpoint(SIZE_T offset, DebuggerJitInfo *jitInfo, DebuggerEnCBreakpoint::TriggerType fTriggerType, AppDomain *pAppDomain) - : DebuggerController(NULL, pAppDomain), - m_jitInfo(jitInfo), - m_fTriggerType(fTriggerType) + : DebuggerController(NULL, pAppDomain), + m_jitInfo(jitInfo), + m_fTriggerType(fTriggerType) { - _ASSERTE( jitInfo != NULL ); - // Add and activate the specified patch - AddBindAndActivateNativeManagedPatch(jitInfo->m_nativeCodeVersion.GetMethodDesc(), jitInfo, offset, LEAF_MOST_FRAME, pAppDomain); - LOG((LF_ENC,LL_INFO1000, "DEnCBPDEnCBP::adding %s patch!\n", - fTriggerType == REMAP_PENDING ? "remap pending" : "remap complete")); -} + _ASSERTE( m_jitInfo != NULL ); + BOOL success; + MethodDesc* pMD = m_jitInfo->m_nativeCodeVersion.GetMethodDesc(); + if (m_fTriggerType == DebuggerEnCBreakpoint::REMAP_COMPLETE) + { + success = AddBindAndActivateNativeManagedPatch(pMD, m_jitInfo, offset, LEAF_MOST_FRAME, pAppDomain); + } + else + { + _ASSERTE(m_fTriggerType == DebuggerEnCBreakpoint::REMAP_PENDING); + + // Add and activate the specified patch + Module* module = m_jitInfo->m_pLoaderModule; + mdMethodDef tkMethod = pMD->GetMemberDef(); + SIZE_T encVersion = m_jitInfo->m_encVersion; + success = AddILPatch(pAppDomain, module, tkMethod, NULL, encVersion, offset, TRUE); + } + + _ASSERTE(success != FALSE); + + LOG((LF_ENC,LL_INFO1000, "DEnCBP::DEnCBP adding %s patch to 0x%x encVersion: %zx\n", + fTriggerType == REMAP_PENDING ? "ResumePending" : "ResumeComplete", pMD->GetMemberDef(), m_jitInfo->m_encVersion)); +} //--------------------------------------------------------------------------------------- // @@ -8753,26 +8721,9 @@ TP_RESULT DebuggerEnCBreakpoint::TriggerPatch(DebuggerControllerPatch *patch, { _ASSERTE(HasLock()); - Module *module = patch->key.module; - mdMethodDef md = patch->key.md; - SIZE_T offset = patch->offset; - - // Map the current native offset back to the IL offset in the old - // function. This will be mapped to the new native offset within - // ResumeInUpdatedFunction - CorDebugMappingResult map; - DWORD which; - SIZE_T currentIP = (SIZE_T)m_jitInfo->MapNativeOffsetToIL(offset, - &map, &which); - - // We only lay DebuggerEnCBreakpoints at sequence points - _ASSERTE(map == MAPPING_EXACT); - LOG((LF_ENC, LL_ALWAYS, - "DEnCBP::TP: triggered E&C %s breakpoint: tid=0x%x, module=0x%08x, " - "method def=0x%08x, version=%d, native offset=0x%x, IL offset=0x%x\n this=0x%x\n", - m_fTriggerType == REMAP_PENDING ? "ResumePending" : "ResumeComplete", - thread, module, md, m_jitInfo->m_encVersion, offset, currentIP, this)); + "DEnCBP::TP: Triggered EnC %s breakpoint: tid=%p, this=%p\n", + m_fTriggerType == REMAP_PENDING ? "ResumePending" : "ResumeComplete", thread, this)); // If this is a REMAP_COMPLETE patch, then dispatch the RemapComplete callback if (m_fTriggerType == REMAP_COMPLETE) @@ -8788,25 +8739,43 @@ TP_RESULT DebuggerEnCBreakpoint::TriggerPatch(DebuggerControllerPatch *patch, return TPR_IGNORE; } - _ASSERTE(patch->IsManagedPatch()); - - // Grab the MethodDesc for this function. - _ASSERTE(module != NULL); - - // GENERICS: @todo generics. This should be replaced by a similar loop - // over the DJIs for the DMI as in BindPatch up above. - MethodDesc *pFD = g_pEEInterface->FindLoadedMethodRefOrDef(module, md); + // Map the current native offset back to the IL offset in the old + // function. This will be mapped to the new native offset within + // ResumeInUpdatedFunction + CorDebugMappingResult map; + DWORD which; + SIZE_T currentIL = (SIZE_T)m_jitInfo->MapNativeOffsetToIL(patch->offset, &map, &which); - _ASSERTE(pFD != NULL); + // We only lay DebuggerEnCBreakpoints at sequence points + _ASSERTE(map == MAPPING_EXACT); + _ASSERTE(patch->IsManagedPatch()); - LOG((LF_ENC, LL_ALWAYS, - "DEnCBP::TP: in %s::%s\n", pFD->m_pszDebugClassName,pFD->m_pszDebugMethodName)); + Module *module = patch->key.module; + mdMethodDef md = patch->key.md; + LOG((LF_ENC, LL_INFO10000, + "DEnCBP::TP: methodDef=0x%08x, encVersion=%zx, IL offset=0x%zx\n", + md, m_jitInfo->m_encVersion, currentIL)); + patch->LogInstance(); // Grab the jit info for the original copy of the method, which is // what we are executing right now. - DebuggerJitInfo *pJitInfo = m_jitInfo; - _ASSERTE(pJitInfo); - _ASSERTE(pJitInfo->m_nativeCodeVersion.GetMethodDesc() == pFD); + DebuggerJitInfo* pJitInfo = m_jitInfo; + // Grab the MethodDesc for this function. + MethodDesc* pMD = pJitInfo->m_nativeCodeVersion.GetMethodDesc(); + + _ASSERTE(pJitInfo != NULL); + _ASSERTE(pMD != NULL); + LOG((LF_ENC, LL_INFO10000, + "DEnCBP::TP: DJI: %p pMD: %p (%s::%s)\n", pJitInfo, pMD, pMD->m_pszDebugClassName, pMD->m_pszDebugMethodName)); + +#if defined(_DEBUG) + { + // Either the current method matches what we are executing or it is + // the typical definition for a generic method (i.e., typical). + MethodDesc* loadedMD = g_pEEInterface->FindLoadedMethodRefOrDef(module, md); + _ASSERTE(loadedMD == pMD || loadedMD->IsTypicalMethodDefinition()); + } +#endif // _DEBUG // Grab the context for this thread. This is the context that was // passed to COMPlusFrameHandler. @@ -8819,13 +8788,13 @@ TP_RESULT DebuggerEnCBreakpoint::TriggerPatch(DebuggerControllerPatch *patch, // Release the controller lock for the rest of this method CrstBase::UnsafeCrstInverseHolder inverseLock(&g_criticalSection); - // resumeIP is the native offset in the new version of the method the debugger wants + // resumeIL is the IL offset in the new version of the method the debugger wants // to resume to. We'll pass the address of this variable over to the right-side // and if it modifies the contents while we're stopped dispatching the RemapOpportunity, // then we know it wants a remap. // This form of side-channel communication seems like an error-prone workaround. Ideally the - // remap IP (if any) would just be returned in a response event. - SIZE_T resumeIP = (SIZE_T) -1; + // remap IL (if any) would just be returned in a response event. + SIZE_T resumeIL = (SIZE_T) -1; // Debugging code to enable a break after N RemapOpportunities #ifdef _DEBUG @@ -8842,29 +8811,29 @@ TP_RESULT DebuggerEnCBreakpoint::TriggerPatch(DebuggerControllerPatch *patch, } #endif - // Send an event to the RS to call the RemapOpportunity callback, passing the address of resumeIP. - // If the debugger responds with a call to RemapFunction, the supplied IP will be copied into resumeIP + // Send an event to the RS to call the RemapOpportunity callback, passing the address of resumeIL. + // If the debugger responds with a call to RemapFunction, the supplied IP will be copied into resumeIL // and we will know to update the context and resume the function at the new IP. Otherwise we just do // nothing and try again on next RemapFunction breakpoint - g_pDebugger->LockAndSendEnCRemapEvent(pJitInfo, currentIP, &resumeIP); + g_pDebugger->LockAndSendEnCRemapEvent(pJitInfo, currentIL, &resumeIL); LOG((LF_ENC, LL_ALWAYS, - "DEnCBP::TP: resume IL offset is 0x%x\n", resumeIP)); + "DEnCBP::TP: resume IL is %zx\n", resumeIL)); // Has the debugger requested a remap? - if (resumeIP != (SIZE_T) -1) + if (resumeIL != (SIZE_T) -1) { // This will jit the function, update the context, and resume execution at the new location. g_pEEInterface->ResumeInUpdatedFunction(pModule, - pFD, + pMD, (void*)pJitInfo, - resumeIP, + resumeIL, pContext); _ASSERTE(!"Returned from ResumeInUpdatedFunction!"); } - LOG((LF_CORDB, LL_ALWAYS, "DEnCB::TP: We've returned from ResumeInUpd" - "atedFunction, we're going to skip the EnC patch ####\n")); + LOG((LF_CORDB, LL_ALWAYS, "DEnCB::TP: We've returned from ResumeInUpdatedFunction" + "we're going to skip the EnC patchId:0x%zx\n", patch->patchId)); // We're returning then we'll have to re-get this lock. Be careful that we haven't kept any controller/patches // in the caller. They can move when we unlock, so when we release the lock and reget it here, things might have @@ -8886,7 +8855,7 @@ TP_RESULT DebuggerEnCBreakpoint::HandleRemapComplete(DebuggerControllerPatch *pa Thread *thread, TRIGGER_WHY tyWhy) { - LOG((LF_ENC, LL_ALWAYS, "DEnCBP::HRC: HandleRemapComplete\n")); + LOG((LF_ENC, LL_ALWAYS, "DEnCBP::HRC: HandleRemapComplete: %p\n", this)); // Debugging code to enable a break after N RemapCompletes #ifdef _DEBUG @@ -8903,13 +8872,19 @@ TP_RESULT DebuggerEnCBreakpoint::HandleRemapComplete(DebuggerControllerPatch *pa #endif _ASSERTE(HasLock()); - bool fApplied = m_jitInfo->m_encBreakpointsApplied; + MethodDesc* pMD = m_jitInfo->m_nativeCodeVersion.GetMethodDesc(); + _ASSERTE(pMD != NULL); + LOG((LF_ENC, LL_INFO10000, "DEnCBP::HRC: Applied: %s, pMD: %p (%s::%s)\n", + (fApplied ? "true" : "false"), pMD, pMD->m_pszDebugClassName, pMD->m_pszDebugMethodName)); + // Need to delete this before unlock below so if any other thread come in after the unlock // they won't handle this patch. Delete(); - // We just deleted ourselves. Can't access anything any instances after this point. + // + // NOTE: We just deleted ourselves. Can't access anything any instances after this point. + // // if have somehow updated this function before we resume into it then just bail if (fApplied) @@ -8918,9 +8893,14 @@ TP_RESULT DebuggerEnCBreakpoint::HandleRemapComplete(DebuggerControllerPatch *pa return TPR_IGNORE_AND_STOP; } - // GENERICS: @todo generics. This should be replaced by a similar loop - // over the DJIs for the DMI as in BindPatch up above. - MethodDesc *pFD = g_pEEInterface->FindLoadedMethodRefOrDef(patch->key.module, patch->key.md); +#if defined(_DEBUG) + { + // Either the current method matches what we are executing or it is + // the typical definition for a generic method (i.e., typical). + MethodDesc* loadedMD = g_pEEInterface->FindLoadedMethodRefOrDef(patch->key.module, patch->key.md); + _ASSERTE(loadedMD == pMD || loadedMD->IsTypicalMethodDefinition()); + } +#endif // _DEBUG LOG((LF_ENC, LL_ALWAYS, "DEnCBP::HRC: unlocking controller\n")); @@ -8929,7 +8909,7 @@ TP_RESULT DebuggerEnCBreakpoint::HandleRemapComplete(DebuggerControllerPatch *pa LOG((LF_ENC, LL_ALWAYS, "DEnCBP::HRC: sending RemapCompleteEvent\n")); - g_pDebugger->LockAndSendEnCRemapCompleteEvent(pFD); + g_pDebugger->LockAndSendEnCRemapCompleteEvent(pMD); // We're returning then we'll have to re-get this lock. Be careful that we haven't kept any controller/patches // in the caller. They can move when we unlock, so when we release the lock and reget it here, things might have diff --git a/src/coreclr/debug/ee/controller.h b/src/coreclr/debug/ee/controller.h index 27666a6eed6d8b..0b8a32927c326d 100644 --- a/src/coreclr/debug/ee/controller.h +++ b/src/coreclr/debug/ee/controller.h @@ -331,23 +331,23 @@ struct DebuggerFunctionKey1 typedef DebuggerFunctionKey1 UNALIGNED DebuggerFunctionKey; -// IL Master: Breakpoints on IL code may need to be applied to multiple +// IL Primary: Breakpoints on IL code may need to be applied to multiple // copies of code. Historically generics was the only way IL code was JITTed // multiple times but more recently the CodeVersionManager and tiered compilation // provide more open-ended mechanisms to have multiple native code bodies derived // from a single IL method body. -// The "master" is a patch we keep to record the IL offset or native offset, and -// is used to create new "slave"patches. For native offsets only offset 0 is allowed +// The "primary" is a patch we keep to record the IL offset or native offset, and +// is used to create new "replica" patches. For native offsets only offset 0 is allowed // because that is the only one that we think would have a consistent semantic // meaning across different code bodies. // There can also be multiple IL bodies for the same method given EnC or ReJIT. -// A given master breakpoint is tightly bound to one particular IL body determined +// A given primary breakpoint is tightly bound to one particular IL body determined // by encVersion. ReJIT + breakpoints isn't currently supported. // // -// IL Slave: The slaves created from Master patches. If the master used an IL offset -// then the slave also initially has an IL offset that will later become a native offset. -// If the master uses a native offset (0) then the slave will also have a native offset (0). +// IL Replica: The replicas created from Primary patches. If the primary used an IL offset +// then the replica also initially has an IL offset that will later become a native offset. +// If the primary uses a native offset (0) then the replica will also have a native offset (0). // These patches always resolve to addresses in jitted code. // // @@ -360,7 +360,7 @@ typedef DebuggerFunctionKey1 UNALIGNED DebuggerFunctionKey; // // NativeUnmanaged: A patch applied to any kind of native code. -enum DebuggerPatchKind { PATCH_KIND_IL_MASTER, PATCH_KIND_IL_SLAVE, PATCH_KIND_NATIVE_MANAGED, PATCH_KIND_NATIVE_UNMANAGED }; +enum DebuggerPatchKind { PATCH_KIND_IL_PRIMARY, PATCH_KIND_IL_REPLICA, PATCH_KIND_NATIVE_MANAGED, PATCH_KIND_NATIVE_UNMANAGED }; // struct DebuggerControllerPatch: An entry in the patch (hash) table, // this should contain all the info that's needed over the course of a @@ -369,7 +369,8 @@ enum DebuggerPatchKind { PATCH_KIND_IL_MASTER, PATCH_KIND_IL_SLAVE, PATCH_KIND_N // FREEHASHENTRY entry: Three ULONGs, this is required // by the underlying hashtable implementation // DWORD opcode: A nonzero opcode && address field means that -// the patch has been applied to something. +// the patch has been applied to something. This may not be a full +// opcode, it is possible it is a partial opcode. // A patch with a zero'd opcode field means that the patch isn't // actually tracking a valid break opcode. See DebuggerPatchTable // for more details. @@ -405,7 +406,7 @@ enum DebuggerPatchKind { PATCH_KIND_IL_MASTER, PATCH_KIND_IL_SLAVE, PATCH_KIND_N // the method (and version) that this patch is applied to. This field may // also have the value DebuggerJitInfo::DMI_VERSION_INVALID -// SIZE_T pid: Within a given patch table, all patches have a +// SIZE_T patchId: Within a given patch table, all patches have a // semi-unique ID. There should be one and only 1 patch for a given // {pid,nVersion} tuple, thus ensuring that we don't duplicate // patches from multiple, previous versions. @@ -427,40 +428,41 @@ struct DebuggerControllerPatch SIZE_T offset; PTR_CORDB_ADDRESS_TYPE address; FramePointer fp; - PRD_TYPE opcode; //this name will probably change because it is a misnomer + PRD_TYPE opcode; // See description above. BOOL fSaveOpcode; - PRD_TYPE opcodeSaved;//also a misnomer + PRD_TYPE opcodeSaved; BOOL offsetIsIL; TraceDestination trace; - MethodDesc* pMethodDescFilter; // used for IL Master patches that should only bind to jitted + MethodDesc* pMethodDescFilter; // used for IL Primary patches that should only bind to jitted // code versions for a single generic instantiation private: + DebuggerPatchKind kind; int refCount; union { - SIZE_T encVersion; // used for Master patches, to record which EnC version this Master applies to - DebuggerJitInfo *dji; // used for Slave and native patches, though only when tracking JIT Info + SIZE_T encVersion; // used for Primary patches, to record which EnC version this Primary applies to + DebuggerJitInfo *dji; // used for Replica and native patches, though only when tracking JIT Info }; #ifndef FEATURE_EMULATE_SINGLESTEP // this is shared among all the skippers for this controller. see the comments // right before the definition of SharedPatchBypassBuffer for lifetime info. SharedPatchBypassBuffer* m_pSharedPatchBypassBuffer; -#endif // TARGET_ARM +#endif // !FEATURE_EMULATE_SINGLESTEP public: - SIZE_T pid; + SIZE_T patchId; AppDomain *pAppDomain; BOOL IsNativePatch(); BOOL IsManagedPatch(); - BOOL IsILMasterPatch(); - BOOL IsILSlavePatch(); + BOOL IsILPrimaryPatch(); + BOOL IsILReplicaPatch(); DebuggerPatchKind GetKind(); // A patch has DJI if it was created with it or if it has been mapped to a // function that has been jitted while JIT tracking was on. It does not - // necessarily mean the patch is bound. ILMaster patches never have DJIs. + // necessarily mean the patch is bound. ILPrimary patches never have DJIs. // Patches will never have DJIs if we are not tracking JIT information. // // Patches can also be unbound, e.g. in UnbindFunctionPatches. Any DJI gets cleared @@ -469,27 +471,30 @@ struct DebuggerControllerPatch // we don't skip the patch when we get new code. BOOL HasDJI() { - return (!IsILMasterPatch() && dji != NULL); + return (!IsILPrimaryPatch() && dji != NULL); } DebuggerJitInfo *GetDJI() { - _ASSERTE(!IsILMasterPatch()); + _ASSERTE(!IsILPrimaryPatch()); return dji; } + // Determine if the patch is related to EnC remap logic. + BOOL IsEnCRemapPatch(); + // These tell us which EnC version a patch relates to. They are used // to determine if we are mapping a patch to a new version. // BOOL HasEnCVersion() { - return (IsILMasterPatch() || HasDJI()); + return (IsILPrimaryPatch() || HasDJI()); } SIZE_T GetEnCVersion() { _ASSERTE(HasEnCVersion()); - return (IsILMasterPatch() ? encVersion : (HasDJI() ? GetDJI()->m_encVersion : CorDB_DEFAULT_ENC_FUNCTION_VERSION)); + return (IsILPrimaryPatch() ? encVersion : (HasDJI() ? GetDJI()->m_encVersion : CorDB_DEFAULT_ENC_FUNCTION_VERSION)); } // We set the DJI explicitly after mapping a patch @@ -498,7 +503,7 @@ struct DebuggerControllerPatch // should not skip the patch. void SetDJI(DebuggerJitInfo *newDJI) { - _ASSERTE(!IsILMasterPatch()); + _ASSERTE(!IsILPrimaryPatch()); dji = newDJI; } @@ -516,8 +521,8 @@ struct DebuggerControllerPatch return FALSE; } - // IL Master patches are never bound. - _ASSERTE( !IsILMasterPatch() ); + // IL Primary patches are never bound. + _ASSERTE( !IsILPrimaryPatch() ); return TRUE; } @@ -562,10 +567,29 @@ struct DebuggerControllerPatch if (m_pSharedPatchBypassBuffer != NULL) m_pSharedPatchBypassBuffer->Release(); } -#endif // TARGET_ARM +#endif // !FEATURE_EMULATE_SINGLESTEP -private: - DebuggerPatchKind kind; + void LogInstance() + { + LOG((LF_CORDB, LL_INFO10000, " DCP: %p\n" + " patchId: 0x%zx\n" + " offset: 0x%zx\n" + " address: %p\n" + " offsetIsIL: %s\n" + " refCount: %d\n" + " kind: %d\n" + " IsBound: %s\n" + " IsNativePatch: %s\n" + " IsManagedPatch: %s\n" + " IsILPrimaryPatch: %s\n" + " IsILReplicaPatch: %s\n", + this, patchId, offset, address, (offsetIsIL ? "true" : "false"), refCount, GetKind(), + (IsBound() ? "true" : "false"), + (IsNativePatch() ? "true" : "false"), + (IsManagedPatch() ? "true" : "false"), + (IsILPrimaryPatch() ? "true" : "false"), + (IsILReplicaPatch() ? "true" : "false"))); + } }; typedef DPTR(DebuggerControllerPatch) PTR_DebuggerControllerPatch; @@ -625,7 +649,7 @@ class DebuggerPatchTable : private CHashTableAndData private: //incremented so that we can get DPT-wide unique PIDs. // pid = Patch ID. - SIZE_T m_pid; + SIZE_T m_patchId; // Given a patch, retrieves the correct key. The return value of this function is passed to Cmp(), Find(), etc. SIZE_T Key(DebuggerControllerPatch *patch) { @@ -700,8 +724,8 @@ class DebuggerPatchTable : private CHashTableAndData //Public Members public: enum { - DCP_PID_INVALID, - DCP_PID_FIRST_VALID, + DCP_PATCHID_INVALID, + DCP_PATCHID_FIRST_VALID, }; #ifndef DACCESS_COMPILE @@ -712,7 +736,7 @@ class DebuggerPatchTable : private CHashTableAndData { WRAPPER_NO_CONTRACT; - m_pid = DCP_PID_FIRST_VALID; + m_patchId = DCP_PATCHID_FIRST_VALID; SUPPRESS_ALLOCATION_ASSERTS_IN_THIS_SCOPE; return NewInit(17, sizeof(DebuggerControllerPatch), 101); @@ -722,7 +746,7 @@ class DebuggerPatchTable : private CHashTableAndData // GetNextPatch from this patch) are either sorted or NULL, take the given // patch (which should be the first patch in the chain). This // is called by AddPatch to make sure that the order of the - // patches is what we want for things like E&C, DePatchSkips,etc. + // patches is what we want for things like EnC, DePatchSkips,etc. void SortPatchIntoPatchList(DebuggerControllerPatch **ppPatch); void SpliceOutOfList(DebuggerControllerPatch *patch); @@ -742,7 +766,7 @@ class DebuggerPatchTable : private CHashTableAndData DebuggerPatchKind kind, FramePointer fp, AppDomain *pAppDomain, - SIZE_T masterEnCVersion, + SIZE_T primaryEnCVersion, DebuggerJitInfo *dji); DebuggerControllerPatch *AddPatchForAddress(DebuggerController *controller, @@ -753,7 +777,7 @@ class DebuggerPatchTable : private CHashTableAndData FramePointer fp, AppDomain *pAppDomain, DebuggerJitInfo *dji = NULL, - SIZE_T pid = DCP_PID_INVALID, + SIZE_T patchId = DCP_PATCHID_INVALID, TraceType traceType = DPT_DEFAULT_TRACE_TYPE); // Set the native address for this patch. @@ -859,21 +883,17 @@ class DebuggerPatchTable : private CHashTableAndData return ItemIndex(p); } -#ifdef _DEBUG_PATCH_TABLE +#ifdef _DEBUG public: // DEBUG An internal debugging routine, it iterates // through the hashtable, stopping at every - // single entry, no matter what it's state. For this to - // compile, you're going to have to add friend status - // of this class to CHashTableAndData in - // to $\Com99\Src\inc\UtilCode.h + // single entry, no matter what it's state. void CheckPatchTable(); -#endif // _DEBUG_PATCH_TABLE +#endif // _DEBUG // Count how many patches are in the table. // Use for asserts int GetNumberOfPatches(); - }; typedef VPTR(class DebuggerPatchTable) PTR_DebuggerPatchTable; @@ -909,8 +929,6 @@ enum DEBUGGER_CONTROLLER_TYPE { DEBUGGER_CONTROLLER_THREAD_STARTER, DEBUGGER_CONTROLLER_ENC, - DEBUGGER_CONTROLLER_ENC_PATCH_TO_SKIP, // At any one address, - // There can be only one! DEBUGGER_CONTROLLER_PATCH_SKIP, DEBUGGER_CONTROLLER_BREAKPOINT, DEBUGGER_CONTROLLER_STEPPER, @@ -1007,8 +1025,7 @@ inline void VerifyExecutableAddress(const BYTE* address) // DebuggerController: DebuggerController serves // both as a static class that dispatches exceptions coming from the -// EE, and as an abstract base class for the five classes that derrive -// from it. +// EE, and as an abstract base class for other debugger concepts. class DebuggerController { VPTR_BASE_CONCRETE_VTABLE_CLASS(DebuggerController); @@ -1084,9 +1101,6 @@ class DebuggerController static bool ModuleHasPatches( Module* pModule ); #if EnC_SUPPORTED - static DebuggerControllerPatch *IsXXXPatched(const BYTE *eip, - DEBUGGER_CONTROLLER_TYPE dct); - static DebuggerControllerPatch *GetEnCPatch(const BYTE *address); #endif //EnC_SUPPORTED @@ -1129,8 +1143,8 @@ class DebuggerController // destroys the thread before it fires, then we'd still have an active DTS. static void CancelOutstandingThreadStarter(Thread * pThread); - static void AddRef(DebuggerControllerPatch *patch); - static void Release(DebuggerControllerPatch *patch); + static void AddRefPatch(DebuggerControllerPatch *patch); + static void ReleasePatch(DebuggerControllerPatch *patch); private: @@ -1177,7 +1191,6 @@ class DebuggerController CORDB_ADDRESS_TYPE *startAddr); static bool ApplyPatch(DebuggerControllerPatch *patch); static bool UnapplyPatch(DebuggerControllerPatch *patch); - static void UnapplyPatchAt(DebuggerControllerPatch *patch, CORDB_ADDRESS_TYPE *address); static bool IsPatched(CORDB_ADDRESS_TYPE *address, BOOL native); static void ActivatePatch(DebuggerControllerPatch *patch); @@ -1238,17 +1251,6 @@ class DebuggerController Thread *GetThread() { return m_thread; } - // This one should be made private - BOOL AddBindAndActivateILSlavePatch(DebuggerControllerPatch *master, - DebuggerJitInfo *dji); - - BOOL AddILPatch(AppDomain * pAppDomain, Module *module, - mdMethodDef md, - MethodDesc* pMethodFilter, - SIZE_T encVersion, // what encVersion does this apply to? - SIZE_T offset, - BOOL offsetIsIL); - // The next two are very similar. Both work on offsets, // but one takes a "patch id". I don't think these are really needed: the // controller itself can act as the id of the patch. @@ -1314,17 +1316,27 @@ class DebuggerController void Enqueue(); void Dequeue(); - private: + protected: // Helper function that is called on each virtual trace call target to set a trace patch static void PatchTargetVisitor(TADDR pVirtualTraceCallTarget, VOID* pUserData); - DebuggerControllerPatch *AddILMasterPatch(Module *module, + DebuggerControllerPatch *AddILPrimaryPatch(Module *module, mdMethodDef md, MethodDesc *pMethodDescFilter, SIZE_T offset, BOOL offsetIsIL, SIZE_T encVersion); + BOOL AddBindAndActivateILReplicaPatch(DebuggerControllerPatch *primary, + DebuggerJitInfo *dji); + + BOOL AddILPatch(AppDomain * pAppDomain, Module *module, + mdMethodDef md, + MethodDesc* pMethodFilter, + SIZE_T encVersion, // what encVersion does this apply to? + SIZE_T offset, + BOOL offsetIsIL); + BOOL AddBindAndActivatePatchForMethodDesc(MethodDesc *fd, DebuggerJitInfo *dji, SIZE_T nativeOffset, @@ -1332,7 +1344,6 @@ class DebuggerController FramePointer fp, AppDomain *pAppDomain); - protected: // @@ -1482,7 +1493,7 @@ class DebuggerPatchSkip : public DebuggerController BYTE* patchBypass = m_pSharedPatchBypassBuffer->PatchBypass; return (CORDB_ADDRESS_TYPE *)patchBypass; } -#endif // TARGET_ARM +#endif // !FEATURE_EMULATE_SINGLESTEP }; /* ------------------------------------------------------------------------- * @@ -1911,25 +1922,25 @@ class DebuggerContinuableExceptionBreakpoint : public DebuggerController // DebuggerEnCBreakpoint - used by edit and continue to support remapping // // When a method is updated, we make no immediate attempt to remap any existing execution -// of the old method. Instead we mine the old method with EnC breakpoints, and prompt the -// debugger whenever one is hit, giving it the opportunity to request a remap to the +// of the previous version. Instead we set EnC breakpoints in the previous version, and +// prompt the debugger whenever one is hit, giving it the opportunity to request a remap to the // latest version of the method. // // Over long debugging sessions which make many edits to large methods, we can create // a large number of these breakpoints. We currently make no attempt to reclaim the -// code or patch overhead for old methods. Ideally we'd be able to detect when there are -// no outstanding references to the old method version and clean up after it. At the -// very least, we could remove all but the first patch when there are no outstanding +// code or patch overhead for previous versions. Ideally we'd be able to detect when there are +// no outstanding references to older method versions and clean up. At the very least, we +// could remove all but the first patch when there are no outstanding // frames for a specific version of an edited method. // -class DebuggerEnCBreakpoint : public DebuggerController +class DebuggerEnCBreakpoint final : public DebuggerController { public: // We have two types of EnC breakpoints. The first is the one we - // sprinkle through old code to let us know when execution is occurring - // in a function that now has a new version. The second is when we've - // actually resumed excecution into a remapped function and we need - // to then notify the debugger. + // sprinkle through previous code versions to let us know when execution + // is occurring in a function that now has a new version. + // The second is when we've actually resumed excecution into a remapped + // function and we need to then notify the debugger. enum TriggerType {REMAP_PENDING, REMAP_COMPLETE}; // Create and activate an EnC breakpoint at the specified native offset @@ -1938,13 +1949,13 @@ class DebuggerEnCBreakpoint : public DebuggerController TriggerType fTriggerType, AppDomain *pAppDomain); - virtual DEBUGGER_CONTROLLER_TYPE GetDCType( void ) + virtual DEBUGGER_CONTROLLER_TYPE GetDCType( void ) override { return DEBUGGER_CONTROLLER_ENC; } private: TP_RESULT TriggerPatch(DebuggerControllerPatch *patch, Thread *thread, - TRIGGER_WHY tyWhy); + TRIGGER_WHY tyWhy) override; TP_RESULT HandleRemapComplete(DebuggerControllerPatch *patch, Thread *thread, diff --git a/src/coreclr/debug/ee/controller.inl b/src/coreclr/debug/ee/controller.inl index 059e9b255ece9e..3f660d8a42feac 100644 --- a/src/coreclr/debug/ee/controller.inl +++ b/src/coreclr/debug/ee/controller.inl @@ -27,29 +27,32 @@ inline DebuggerPatchKind DebuggerControllerPatch::GetKind() { return kind; } -inline BOOL DebuggerControllerPatch::IsILMasterPatch() + +inline BOOL DebuggerControllerPatch::IsILPrimaryPatch() { LIMITED_METHOD_CONTRACT; - - return (kind == PATCH_KIND_IL_MASTER); + return (kind == PATCH_KIND_IL_PRIMARY); } -inline BOOL DebuggerControllerPatch::IsILSlavePatch() +inline BOOL DebuggerControllerPatch::IsILReplicaPatch() { LIMITED_METHOD_CONTRACT; - - return (kind == PATCH_KIND_IL_SLAVE); + return (kind == PATCH_KIND_IL_REPLICA); } inline BOOL DebuggerControllerPatch::IsManagedPatch() { - return (IsILMasterPatch() || IsILSlavePatch() || kind == PATCH_KIND_NATIVE_MANAGED); - + return (IsILPrimaryPatch() || IsILReplicaPatch() || kind == PATCH_KIND_NATIVE_MANAGED); } + inline BOOL DebuggerControllerPatch::IsNativePatch() { - return (kind == PATCH_KIND_NATIVE_MANAGED || kind == PATCH_KIND_NATIVE_UNMANAGED || (IsILSlavePatch() && !offsetIsIL)); + return (kind == PATCH_KIND_NATIVE_MANAGED || kind == PATCH_KIND_NATIVE_UNMANAGED || (IsILReplicaPatch() && !offsetIsIL)); +} +inline BOOL DebuggerControllerPatch::IsEnCRemapPatch() +{ + return (controller->GetDCType() == DEBUGGER_CONTROLLER_ENC); } #endif // CONTROLLER_INL_ diff --git a/src/coreclr/debug/ee/debugger.cpp b/src/coreclr/debug/ee/debugger.cpp index 7603524319670a..aa433342287331 100644 --- a/src/coreclr/debug/ee/debugger.cpp +++ b/src/coreclr/debug/ee/debugger.cpp @@ -2465,10 +2465,6 @@ DebuggerMethodInfo *Debugger::CreateMethodInfo(Module *module, mdMethodDef md) } - - - - /****************************************************************************** // void Debugger::JITComplete(): JITComplete is called by // the jit interface when the JIT completes, successfully or not. @@ -2496,9 +2492,8 @@ void Debugger::JITComplete(NativeCodeVersion nativeCodeVersion, TADDR newAddress MethodDesc* fd = nativeCodeVersion.GetMethodDesc(); - LOG((LF_CORDB, LL_INFO100000, "D::JITComplete: md:0x%p (%s::%s), address:0x%p.\n", - fd, fd->m_pszDebugClassName, fd->m_pszDebugMethodName, - newAddress)); + LOG((LF_CORDB, LL_INFO100000, "D::JITComplete: md:%p (%s::%s), address:%p.\n", + fd, fd->m_pszDebugClassName, fd->m_pszDebugMethodName, newAddress)); #ifdef TARGET_ARM newAddress = newAddress|THUMB_CODE; @@ -2521,7 +2516,7 @@ void Debugger::JITComplete(NativeCodeVersion nativeCodeVersion, TADDR newAddress goto Exit; } BOOL jiWasCreated = FALSE; - DebuggerJitInfo * ji = dmi->CreateInitAndAddJitInfo(nativeCodeVersion, newAddress, &jiWasCreated); + DebuggerJitInfo * dji = dmi->CreateInitAndAddJitInfo(nativeCodeVersion, newAddress, &jiWasCreated); if (!jiWasCreated) { // we've already been notified about this code, no work remains. @@ -2529,19 +2524,17 @@ void Debugger::JITComplete(NativeCodeVersion nativeCodeVersion, TADDR newAddress // method on two threads. When this occurs both threads will // return the same code pointer and this callback is invoked // multiple times. - LOG((LF_CORDB, LL_INFO1000000, "D::JITComplete: md:0x%p (%s::%s), address:0x%p. Already created\n", - fd, fd->m_pszDebugClassName, fd->m_pszDebugMethodName, - newAddress)); + LOG((LF_CORDB, LL_INFO1000000, "D::JITComplete: md:%p (%s::%s), address:%p. Already created\n", + fd, fd->m_pszDebugClassName, fd->m_pszDebugMethodName, newAddress)); goto Exit; } - LOG((LF_CORDB, LL_INFO1000000, "D::JITComplete: md:0x%p (%s::%s), address:0x%p. Created ji:0x%p\n", - fd, fd->m_pszDebugClassName, fd->m_pszDebugMethodName, - newAddress, ji)); + LOG((LF_CORDB, LL_INFO1000000, "D::JITComplete: md:0x%p (%s::%s), address:%p. Created dji:%p\n", + fd, fd->m_pszDebugClassName, fd->m_pszDebugMethodName, newAddress, dji)); // Bind any IL patches to the newly jitted native code. HRESULT hr; - hr = MapAndBindFunctionPatches(ji, fd, (CORDB_ADDRESS_TYPE *)newAddress); + hr = MapAndBindFunctionPatches(dji, fd, (CORDB_ADDRESS_TYPE *)newAddress); _ASSERTE(SUCCEEDED(hr)); } @@ -2639,7 +2632,7 @@ DebuggerJitInfo *Debugger::GetJitInfoFromAddr(TADDR addr) // Get a DJI for a Native MD (MD for a native function). // In the EnC scenario, the MethodDesc refers to the most recent method. // This is very dangerous since there may be multiple versions alive at the same time. -// This will give back the wrong DJI if we're lookikng for a stale method desc. +// This will give back the wrong DJI if we're looking for a stale method desc. // @todo - can a caller possibly use this correctly? DebuggerJitInfo *Debugger::GetLatestJitInfoFromMethodDesc(MethodDesc * pMethodDesc) { @@ -4450,7 +4443,7 @@ HRESULT Debugger::GetVariablesFromOffset(MethodDesc *pMD, memset( rgVal1, 0, sizeof(SIZE_T)*uRgValSize); memset( rgVal2, 0, sizeof(SIZE_T)*uRgValSize); - LOG((LF_CORDB|LF_ENC, LL_INFO10000, "D::GVFO: %s::%s, infoCount:0x%x, from:0x%p\n", + LOG((LF_CORDB|LF_ENC, LL_INFO10000, "D::GVFO: %s::%s, infoCount:0x%x, from: %p\n", pMD->m_pszDebugClassName, pMD->m_pszDebugMethodName, varNativeInfoCount, @@ -4475,9 +4468,7 @@ HRESULT Debugger::GetVariablesFromOffset(MethodDesc *pMD, hr = S_OK; - LOG((LF_CORDB|LF_ENC, - LL_INFO10000, - "D::GVFO rgVal1 0x%X, rgVal2 0x%X\n", + LOG((LF_CORDB|LF_ENC, LL_INFO10000, "D::GVFO rgVal1 %p, rgVal2 %p\n", rgVal1, rgVal2)); @@ -4508,7 +4499,7 @@ HRESULT Debugger::GetVariablesFromOffset(MethodDesc *pMD, (varNativeInfo[i].endOffset < offsetFrom) || (varNativeInfo[i].loc.vlType == ICorDebugInfo::VLT_INVALID)) { - LOG((LF_CORDB|LF_ENC,LL_INFO10000, "D::GVFO [%2d] invalid\n", i)); + LOG((LF_CORDB|LF_ENC,LL_INFO10000, "D::GVFO [%2u] invalid\n", i)); continue; } @@ -4530,7 +4521,7 @@ HRESULT Debugger::GetVariablesFromOffset(MethodDesc *pMD, BIT64_ARG(cbClass)); LOG((LF_CORDB|LF_ENC,LL_INFO10000, - "D::GVFO [%2d] varnum %d, nonVC type %x, addr %8.8x: %8.8x;%8.8x\n", + "D::GVFO [%2u] varnum %u, nonVC type %x, addr %p: %zx;%zx\n", i, varNativeInfo[i].varNumber, varNativeInfo[i].loc.vlType, @@ -4565,7 +4556,7 @@ HRESULT Debugger::GetVariablesFromOffset(MethodDesc *pMD, cValueClasses++; #ifdef _DEBUG LOG((LF_CORDB|LF_ENC,LL_INFO10000, - "D::GVFO [%2d] varnum %d, VC len %d, addr %8.8x, sample: %8.8x%8.8x\n", + "D::GVFO [%2u] varnum %d, VC len %d, addr %p, sample: %8.8x%8.8x\n", i, varNativeInfo[i].varNumber, cbClass, @@ -4574,7 +4565,7 @@ HRESULT Debugger::GetVariablesFromOffset(MethodDesc *pMD, #endif } - LOG((LF_CORDB|LF_ENC, LL_INFO10000, "D::GVFO: returning %8.8x\n", hr)); + LOG((LF_CORDB|LF_ENC, LL_INFO10000, "D::GVFO: returning 0x%x\n", hr)); if (SUCCEEDED(hr)) { (*rgpVCs) = rgpValueClasses; @@ -4734,15 +4725,15 @@ HRESULT Debugger::SetVariablesAtOffset(MethodDesc *pMD, return hr; } -BOOL IsDuplicatePatch(SIZE_T *rgEntries, - ULONG cEntries, - SIZE_T Entry ) +static BOOL IsDuplicatePatch(SIZE_T_UNORDERED_ARRAY* unorderedArray, SIZE_T Entry) { LIMITED_METHOD_CONTRACT; - for( ULONG i = 0; i < cEntries;i++) + SIZE_T* entries = unorderedArray->Table(); + ULONG count = unorderedArray->Count(); + for (ULONG i = 0; i < count; ++i) { - if (rgEntries[i] == Entry) + if (entries[i] == Entry) return TRUE; } return FALSE; @@ -4761,7 +4752,6 @@ BOOL IsDuplicatePatch(SIZE_T *rgEntries, // // Parameters: // djiNew - this is the DJI created in D::JitComplete. -// If djiNew == NULL iff we aren't tracking debug-info. // fd - the method desc that we're binding too. // addrOfCode - address of the native blob of code we just jitted // @@ -4780,7 +4770,8 @@ HRESULT Debugger::MapAndBindFunctionPatches(DebuggerJitInfo *djiNew, { THROWS; CALLED_IN_DEBUGGERDATALOCK_HOLDER_SCOPE_MAY_GC_TRIGGERS_CONTRACT; - PRECONDITION(!djiNew || djiNew->m_nativeCodeVersion.GetMethodDesc() == fd); + PRECONDITION(djiNew != NULL); + PRECONDITION(djiNew->m_nativeCodeVersion.GetMethodDesc() == fd); } CONTRACTL_END; @@ -4791,8 +4782,8 @@ HRESULT Debugger::MapAndBindFunctionPatches(DebuggerJitInfo *djiNew, Module *pModule = g_pEEInterface->MethodDescGetModule(fd); mdMethodDef md = fd->GetMemberDef(); - LOG((LF_CORDB,LL_INFO10000,"D::MABFP: All BPs will be mapped to " - "Ver:0x%04x (DJI:0x%p)\n", djiNew?djiNew->m_methodInfo->GetCurrentEnCVersion():0, djiNew)); + LOG((LF_CORDB,LL_INFO10000,"D::MABFP: All BPs will be mapped to encVersion: %zx (DJI:%p)\n", + djiNew->m_methodInfo->GetCurrentEnCVersion(), djiNew)); // We need to traverse the patch list while under the controller lock (small lock). // But we can only send BreakpointSetErrors while under the debugger lock (big lock). @@ -4800,7 +4791,6 @@ HRESULT Debugger::MapAndBindFunctionPatches(DebuggerJitInfo *djiNew, // and then send the whole list when under the big lock. PATCH_UNORDERED_ARRAY listUnbindablePatches; - // First lock the patch table so it doesn't move while we're // examining it. LOG((LF_CORDB,LL_INFO10000, "D::MABFP: About to lock patch table\n")); @@ -4815,15 +4805,14 @@ HRESULT Debugger::MapAndBindFunctionPatches(DebuggerJitInfo *djiNew, dcp != NULL; dcp = pPatchTable->GetNextPatch( &hf )) { - - LOG((LF_CORDB, LL_INFO10000, "D::MABFP: got patch 0x%p\n", dcp)); + LOG((LF_CORDB, LL_INFO10000, "D::MABFP: Got patch %p, patchId:0x%zx\n", dcp, dcp->patchId)); // Only copy over breakpoints that are in this method // Ideally we'd have a per-method index since there can be a lot of patches // when the EnCBreakpoint patches are included. if (dcp->key.module != pModule || dcp->key.md != md) { - LOG((LF_CORDB, LL_INFO10000, "Patch not in this method\n")); + LOG((LF_CORDB, LL_INFO10000, "D::MABFP: Patch not in this method\n")); continue; } @@ -4831,16 +4820,15 @@ HRESULT Debugger::MapAndBindFunctionPatches(DebuggerJitInfo *djiNew, // elsewhere. if(dcp->pMethodDescFilter != NULL && dcp->pMethodDescFilter != djiNew->m_nativeCodeVersion.GetMethodDesc()) { - LOG((LF_CORDB, LL_INFO10000, "Patch not in this generic instance, filter 0x%p\n", dcp->pMethodDescFilter)); + LOG((LF_CORDB, LL_INFO10000, "D::MABFP: Patch not in this generic instance, filter %p\n", dcp->pMethodDescFilter)); continue; } - - // Do not copy over slave breakpoint patches. Instead place a new slave - // based off the master. - if (dcp->IsILSlavePatch()) + // Do not copy over replica breakpoint patches. Instead place a new replica + // based off the primary. + if (dcp->IsILReplicaPatch()) { - LOG((LF_CORDB, LL_INFO10000, "Not copying over slave breakpoint patch\n")); + LOG((LF_CORDB, LL_INFO10000, "D::MABFP: Not copying over replica breakpoint patch\n")); continue; } @@ -4848,16 +4836,17 @@ HRESULT Debugger::MapAndBindFunctionPatches(DebuggerJitInfo *djiNew, // Eg. It may be bound to a different generic method instantiation. if (dcp->IsBound()) { - LOG((LF_CORDB, LL_INFO10000, "Skipping already bound patch\n")); + LOG((LF_CORDB, LL_INFO10000, "D::MABFP: Skipping already bound patch\n")); continue; } - // Only apply breakpoint patches that are for this version. - // If the patch doesn't have a particular EnCVersion available from its data then + // Only apply breakpoint and EnC remap patches that are for this version. + // If the patches doesn't have a particular EnCVersion available from its data then // we're (probably) not tracking JIT info. - if (dcp->IsBreakpointPatch() && dcp->HasEnCVersion() && djiNew && dcp->GetEnCVersion() != djiNew->m_encVersion) + if ((dcp->IsBreakpointPatch() || dcp->IsEnCRemapPatch()) + && dcp->HasEnCVersion() && dcp->GetEnCVersion() != djiNew->m_encVersion) { - LOG((LF_CORDB, LL_INFO10000, "Not applying breakpoint patch to new version\n")); + LOG((LF_CORDB, LL_INFO10000, "Not applying breakpoint or EnC remap patch to new version\n")); continue; } @@ -4867,98 +4856,61 @@ HRESULT Debugger::MapAndBindFunctionPatches(DebuggerJitInfo *djiNew, // This is to signal that we should not skip here. // under exactly what scenarios (EnC, code pitching etc.) will this apply?... // can't we be a little clearer about why we don't want to bind the patch in this arcane situation? - if (dcp->HasDJI() && !dcp->IsBreakpointPatch() && !dcp->IsStepperPatch()) + if (dcp->HasDJI() && !dcp->IsBreakpointPatch() && !dcp->IsStepperPatch()) { - LOG((LF_CORDB, LL_INFO10000, "Neither stepper nor BP but we have valid a DJI (i.e. the DJI hasn't been deleted as part of the Unbind/MovedCode/Rebind mess)! - getting next patch!\n")); + LOG((LF_CORDB, LL_INFO10000, "D::MABFP: Neither stepper nor BP but we have valid a DJI (i.e. the DJI hasn't been deleted as part of the Unbind/MovedCode/Rebind mess)! - getting next patch!\n")); continue; } - // Now check if we're tracking JIT info or not - if (djiNew == NULL) - { - // This means we put a patch in a method w/ no debug info. - _ASSERTE(dcp->IsBreakpointPatch() || - dcp->IsStepperPatch() || - dcp->controller->GetDCType() == DEBUGGER_CONTROLLER_THREAD_STARTER); - - // W/o Debug-info, We can only patch native offsets, and only at the start of the method (native offset 0). - // Why can't we patch other native offsets?? - // Maybe b/c we don't know if we're patching - // in the middle of an instruction. Though that's not a - // strict requirement. - // We can't even do a IL-offset 0 because that's after the prolog and w/o the debug-info, - // we don't know where the prolog ends. - // Failing this assert is arguably an API misusage - the debugger should have enabled - // jit-tracking if they wanted to put bps at offsets other than native:0. - if (dcp->IsNativePatch() && (dcp->offset == 0)) - { - DebuggerController::g_patches->BindPatch(dcp, addrOfCode); - DebuggerController::ActivatePatch(dcp); - } - else - { - // IF a debugger calls EnableJitDebugging(true, ...) in the module-load callback, - // we should never get here. - *(listUnbindablePatches.AppendThrowing()) = dcp; - } + pidInCaseTableMoves = dcp->patchId; + // If we've already mapped this one to the current version, + // don't map it again. + LOG((LF_CORDB,LL_INFO10000,"D::MABFP: Checking if 0x%zx is a dup... ", + pidInCaseTableMoves)); + + if ( IsDuplicatePatch(GetBPMappingDuplicates(), pidInCaseTableMoves) ) + { + LOG((LF_CORDB,LL_INFO10000,"it is!\n")); + continue; } - else + LOG((LF_CORDB,LL_INFO10000,"nope, applying.\n")); + dcp->LogInstance(); + + // Attempt mapping from patch to new version of code, and + // we don't care if it turns out that there isn't a mapping. + // @todo-postponed: EnC: Make sure that this doesn't cause + // the patch-table to shift. + hr = MapPatchToDJI( dcp, djiNew ); + if (CORDBG_E_CODE_NOT_AVAILABLE == hr ) { - pidInCaseTableMoves = dcp->pid; - - // If we've already mapped this one to the current version, - // don't map it again. - LOG((LF_CORDB,LL_INFO10000,"D::MABFP: Checking if 0x%x is a dup...", - pidInCaseTableMoves)); - - if ( IsDuplicatePatch(GetBPMappingDuplicates()->Table(), - GetBPMappingDuplicates()->Count(), - pidInCaseTableMoves) ) - { - LOG((LF_CORDB,LL_INFO10000,"it is!\n")); - continue; - } - LOG((LF_CORDB,LL_INFO10000,"nope!\n")); - - // Attempt mapping from patch to new version of code, and - // we don't care if it turns out that there isn't a mapping. - // @todo-postponed: EnC: Make sure that this doesn't cause - // the patch-table to shift. - hr = MapPatchToDJI( dcp, djiNew ); - if (CORDBG_E_CODE_NOT_AVAILABLE == hr ) - { - *(listUnbindablePatches.AppendThrowing()) = dcp; - hr = S_OK; - } - - if (FAILED(hr)) - break; + *(listUnbindablePatches.AppendThrowing()) = dcp; + hr = S_OK; + } - //Remember the patch id to prevent duplication later - pidTableEntry = GetBPMappingDuplicates()->Append(); - if (NULL == pidTableEntry) - { - hr = E_OUTOFMEMORY; - break; - } + if (FAILED(hr)) + break; - *pidTableEntry = pidInCaseTableMoves; - LOG((LF_CORDB,LL_INFO10000,"D::MABFP Adding 0x%x to list of " - "already mapped patches\n", pidInCaseTableMoves)); + //Remember the patch id to prevent duplication later + pidTableEntry = GetBPMappingDuplicates()->Append(); + if (NULL == pidTableEntry) + { + hr = E_OUTOFMEMORY; + break; } + + *pidTableEntry = pidInCaseTableMoves; + LOG((LF_CORDB,LL_INFO10000,"D::MABFP Adding 0x%zx to list of already mapped patches\n", + pidInCaseTableMoves)); } // unlock controller lock before sending events. } LOG((LF_CORDB,LL_INFO10000, "D::MABFP: Unlocked patch table\n")); - // Now send any Breakpoint bind error events. if (listUnbindablePatches.Count() > 0) - { LockAndSendBreakpointSetError(&listUnbindablePatches); - } return hr; } @@ -4971,7 +4923,7 @@ HRESULT Debugger::MapAndBindFunctionPatches(DebuggerJitInfo *djiNew, // available, and thus no patch was placed. The caller may or may // not care. ******************************************************************************/ -HRESULT Debugger::MapPatchToDJI( DebuggerControllerPatch *dcp,DebuggerJitInfo *djiTo) +HRESULT Debugger::MapPatchToDJI(DebuggerControllerPatch *dcp, DebuggerJitInfo *djiTo) { CONTRACTL { @@ -4993,14 +4945,10 @@ HRESULT Debugger::MapPatchToDJI( DebuggerControllerPatch *dcp,DebuggerJitInfo *d } #endif - LOG((LF_CORDB, LL_EVERYTHING, "Calling MapPatchToDJI\n")); + LOG((LF_CORDB, LL_EVERYTHING, "D::MPTDJI Begin %p, patchId:0x%zx\n", dcp, dcp->patchId)); // We shouldn't have been asked to map an already bound patch _ASSERTE( !dcp->IsBound() ); - if ( dcp->IsBound() ) - { - return S_OK; - } // If the patch has no DJI then we're doing a UnbindFunctionPatches/RebindFunctionPatches. Either // way, we simply want the most recent version. In the absence of EnC we should have djiCur == djiTo. @@ -5011,20 +4959,20 @@ HRESULT Debugger::MapPatchToDJI( DebuggerControllerPatch *dcp,DebuggerJitInfo *d // decays into BindFunctionPatch's BindPatch function if (djiCur->m_encVersion == djiTo->m_encVersion) { - // If the patch is a "master" then make a new "slave" patch instead of - // binding the old one. This is to stop us mucking with the master breakpoint patch + // If the patch is a "primary" then make a new "replica" patch instead of + // binding the old one. This is to stop us mucking with the primary breakpoint patch // which we may need to bind several times for generic code. - if (dcp->IsILMasterPatch()) + if (dcp->IsILPrimaryPatch()) { - LOG((LF_CORDB, LL_EVERYTHING, "Add, Bind, Activate new patch from master patch\n")); - if (dcp->controller->AddBindAndActivateILSlavePatch(dcp, djiTo)) + LOG((LF_CORDB, LL_EVERYTHING, "D::MPTDJI Add, Bind, Activate new patch from primary patch\n")); + if (dcp->controller->AddBindAndActivateILReplicaPatch(dcp, djiTo)) { - LOG((LF_CORDB, LL_INFO1000, "Add, Bind Activate went fine!\n" )); + LOG((LF_CORDB, LL_INFO1000, "D::MPTDJI Applied from primary patch!\n" )); return S_OK; } else { - LOG((LF_CORDB, LL_INFO1000, "Didn't work for some reason!\n")); + LOG((LF_CORDB, LL_INFO1000, "D::MPTDJI Didn't work for some reason!\n")); // Caller can track this HR and send error. return CORDBG_E_CODE_NOT_AVAILABLE; @@ -5040,16 +4988,16 @@ HRESULT Debugger::MapPatchToDJI( DebuggerControllerPatch *dcp,DebuggerJitInfo *d // We have an unbound native patch (eg. for PatchTrace), lets try to bind and activate it dcp->SetDJI(djiTo); - LOG((LF_CORDB, LL_EVERYTHING, "trying to bind patch... could be problem\n")); + LOG((LF_CORDB, LL_EVERYTHING, "D::MPTDJI trying to bind patch... could be problem\n")); if (DebuggerController::BindPatch(dcp, djiTo->m_nativeCodeVersion.GetMethodDesc(), NULL)) { DebuggerController::ActivatePatch(dcp); - LOG((LF_CORDB, LL_INFO1000, "Application went fine!\n" )); + LOG((LF_CORDB, LL_INFO1000, "D::MPTDJI Binding went fine!\n" )); return S_OK; } else { - LOG((LF_CORDB, LL_INFO1000, "Didn't apply for some reason!\n")); + LOG((LF_CORDB, LL_INFO1000, "D::MPTDJI Binding failed for some reason!\n")); // Caller can track this HR and send error. return CORDBG_E_CODE_NOT_AVAILABLE; @@ -5178,9 +5126,9 @@ void Debugger::SendSyncCompleteIPCEvent(bool isEESuspendedForGC) DebuggerModule * Debugger::LookupOrCreateModule(DomainAssembly * pDomainAssembly) { _ASSERTE(pDomainAssembly != NULL); - LOG((LF_CORDB, LL_INFO1000, "D::LOCM df=0x%x\n", pDomainAssembly)); + LOG((LF_CORDB, LL_INFO1000, "D::LOCM df=%p\n", pDomainAssembly)); DebuggerModule * pDModule = LookupOrCreateModule(pDomainAssembly->GetModule(), pDomainAssembly->GetAppDomain()); - LOG((LF_CORDB, LL_INFO1000, "D::LOCM m=0x%x ad=0x%x -> dm=0x%x\n", pDomainAssembly->GetModule(), pDomainAssembly->GetAppDomain(), pDModule)); + LOG((LF_CORDB, LL_INFO1000, "D::LOCM m=%p ad=%p -> dm=%p\n", pDomainAssembly->GetModule(), pDomainAssembly->GetAppDomain(), pDModule)); _ASSERTE(pDModule != NULL); _ASSERTE(pDModule->GetDomainAssembly() == pDomainAssembly); @@ -5225,7 +5173,7 @@ DebuggerModule* Debugger::LookupOrCreateModule(Module* pModule, AppDomain *pAppD } CONTRACTL_END; - LOG((LF_CORDB, LL_INFO1000, "D::LOCM m=0x%x ad=0x%x\n", pModule, pAppDomain)); + LOG((LF_CORDB, LL_INFO1000, "D::LOCM m=%p ad=%p\n", pModule, pAppDomain)); // DebuggerModules are relative to a specific AppDomain so we should always be looking up a module / // AppDomain pair. @@ -5253,7 +5201,7 @@ DebuggerModule* Debugger::LookupOrCreateModule(Module* pModule, AppDomain *pAppD // If it doesn't exist, create it. if (dmod == NULL) { - LOG((LF_CORDB, LL_INFO1000, "D::LOCM dmod for m=0x%x ad=0x%x not found, creating.\n", pModule, pAppDomain)); + LOG((LF_CORDB, LL_INFO1000, "D::LOCM dmod for m=%p ad=%p not found, creating.\n", pModule, pAppDomain)); HRESULT hr = S_OK; EX_TRY { @@ -5268,7 +5216,7 @@ DebuggerModule* Debugger::LookupOrCreateModule(Module* pModule, AppDomain *pAppD // The module must be in the AppDomain that was requested _ASSERTE( (dmod == NULL) || (dmod->GetAppDomain() == pAppDomain) ); - LOG((LF_CORDB, LL_INFO1000, "D::LOCM m=0x%x ad=0x%x -> dm=0x%x(Mod=0x%x, DomFile=0x%x, AD=0x%x)\n", + LOG((LF_CORDB, LL_INFO1000, "D::LOCM m=%p ad=%p -> dm=%p(Mod=%p, DomFile=%p, AD=%p)\n", pModule, pAppDomain, dmod, dmod->GetRuntimeModule(), dmod->GetDomainAssembly(), dmod->GetAppDomain())); return dmod; } @@ -6204,7 +6152,7 @@ void Debugger::LockAndSendEnCRemapEvent(DebuggerJitInfo * dji, SIZE_T currentIP, if (CORDBUnrecoverableError(this)) return; - MethodDesc * pFD = dji->m_nativeCodeVersion.GetMethodDesc(); + MethodDesc * pMD = dji->m_nativeCodeVersion.GetMethodDesc(); // Note that the debugger lock is reentrant, so we may or may not hold it already. Thread *thread = g_pEEInterface->GetThread(); @@ -6221,21 +6169,18 @@ void Debugger::LockAndSendEnCRemapEvent(DebuggerJitInfo * dji, SIZE_T currentIP, ipce->EnCRemap.resumeVersionNumber = dji->m_methodInfo->GetCurrentEnCVersion();; ipce->EnCRemap.currentILOffset = currentIP; ipce->EnCRemap.resumeILOffset = resumeIP; - ipce->EnCRemap.funcMetadataToken = pFD->GetMemberDef(); + ipce->EnCRemap.funcMetadataToken = pMD->GetMemberDef(); - LOG((LF_CORDB, LL_INFO10000, "D::LASEnCRE: token 0x%x, from version %d to %d\n", + LOG((LF_CORDB, LL_INFO10000, "D::LASEnCRE: methodDef 0x%x, from version %zx to %zx\n", ipce->EnCRemap.funcMetadataToken, ipce->EnCRemap.currentVersionNumber, ipce->EnCRemap.resumeVersionNumber)); - Module *pRuntimeModule = pFD->GetModule(); + Module *pRuntimeModule = pMD->GetModule(); DebuggerModule * pDModule = LookupOrCreateModule(pRuntimeModule, thread->GetDomain()); ipce->EnCRemap.vmDomainAssembly.SetRawPtr((pDModule ? pDModule->GetDomainAssembly() : NULL)); - LOG((LF_CORDB, LL_INFO10000, "D::LASEnCRE: %s::%s " - "dmod:0x%x, methodDef:0x%x \n", - pFD->m_pszDebugClassName, pFD->m_pszDebugMethodName, - pDModule, - ipce->EnCRemap.funcMetadataToken)); + LOG((LF_CORDB, LL_INFO10000, "D::LASEnCRE: %s::%s dmod:%p\n", + pMD->m_pszDebugClassName, pMD->m_pszDebugMethodName, pDModule)); // IPC event is now initialized, so we can send it over. SendSimpleIPCEventAndBlock(); @@ -6244,12 +6189,11 @@ void Debugger::LockAndSendEnCRemapEvent(DebuggerJitInfo * dji, SIZE_T currentIP, SENDIPCEVENT_END; LOG((LF_CORDB, LL_INFO10000, "D::LASEnCRE: done\n")); - } // Send the RemapComplete event and block until the debugger Continues -// pFD - specifies the method in which we've remapped into -void Debugger::LockAndSendEnCRemapCompleteEvent(MethodDesc *pFD) +// pMD - specifies the method in which we've remapped into +void Debugger::LockAndSendEnCRemapCompleteEvent(MethodDesc *pMD) { CONTRACTL { @@ -6272,7 +6216,7 @@ void Debugger::LockAndSendEnCRemapCompleteEvent(MethodDesc *pFD) // Ensure the DJI for the latest version of this method has been pre-created. // It's not clear whether this is necessary or not, but it shouldn't hurt since // we're going to need to create it anyway since we'll be debugging inside it. - DebuggerJitInfo *dji = g_pDebugger->GetLatestJitInfoFromMethodDesc(pFD); + DebuggerJitInfo *dji = g_pDebugger->GetLatestJitInfoFromMethodDesc(pMD); (void)dji; //prevent "unused variable" error from GCC _ASSERTE( dji != NULL ); } @@ -6292,18 +6236,15 @@ void Debugger::LockAndSendEnCRemapCompleteEvent(MethodDesc *pFD) thread, thread->GetDomain()); + ipce->EnCRemapComplete.funcMetadataToken = pMD->GetMemberDef(); - ipce->EnCRemapComplete.funcMetadataToken = pFD->GetMemberDef(); - - Module *pRuntimeModule = pFD->GetModule(); + Module *pRuntimeModule = pMD->GetModule(); DebuggerModule * pDModule = LookupOrCreateModule(pRuntimeModule, thread->GetDomain()); ipce->EnCRemapComplete.vmDomainAssembly.SetRawPtr((pDModule ? pDModule->GetDomainAssembly() : NULL)); - - LOG((LF_CORDB, LL_INFO10000, "D::LASEnCRC: %s::%s " - "dmod:0x%x, methodDef:0x%x \n", - pFD->m_pszDebugClassName, pFD->m_pszDebugMethodName, + LOG((LF_CORDB, LL_INFO10000, "D::LASEnCRE: %s::%s dmod:%p, methodDef:0x%08x \n", + pMD->m_pszDebugClassName, pMD->m_pszDebugMethodName, pDModule, ipce->EnCRemap.funcMetadataToken)); @@ -6313,7 +6254,7 @@ void Debugger::LockAndSendEnCRemapCompleteEvent(MethodDesc *pFD) // This will block on the continue SENDIPCEVENT_END; - LOG((LF_CORDB, LL_INFO10000, "D::LASEnCRC: done\n")); + LOG((LF_CORDB, LL_INFO10000, "D::LASEnCRE: done\n")); } // @@ -6335,7 +6276,8 @@ void Debugger::SendEnCUpdateEvent(DebuggerIPCEventType eventType, } CONTRACTL_END; - LOG((LF_CORDB, LL_INFO10000, "D::LASEnCUFE:\n")); + LOG((LF_CORDB, LL_INFO10000, "D::SEnCUE: type: 0x%x module: %p tkMember: 0x%08x tkType: 0x%08x, ver: %zu\n", + eventType, pModule, memberToken, classToken, enCVersion)); _ASSERTE(eventType == DB_IPCE_ENC_UPDATE_FUNCTION || eventType == DB_IPCE_ENC_ADD_FUNCTION || @@ -6368,8 +6310,7 @@ void Debugger::SendEnCUpdateEvent(DebuggerIPCEventType eventType, m_pRCThread->SendIPCEvent(); - LOG((LF_CORDB, LL_INFO10000, "D::LASEnCUE: done\n")); - + LOG((LF_CORDB, LL_INFO10000, "D::SEnCUE: done\n")); } @@ -6391,10 +6332,9 @@ void Debugger::LockAndSendBreakpointSetError(PATCH_UNORDERED_ARRAY * listUnbinda if (CORDBUnrecoverableError(this)) return; - ULONG count = listUnbindablePatches->Count(); _ASSERTE(count > 0); // must send at least 1 event. - + LOG((LF_CORDB, LL_INFO10000, "D::LASBSE: Failed to bind %u patch(s)\n", count)); Thread *thread = g_pEEInterface->GetThread(); // Note that the debugger lock is reentrant, so we may or may not hold it already. @@ -6402,7 +6342,7 @@ void Debugger::LockAndSendBreakpointSetError(PATCH_UNORDERED_ARRAY * listUnbinda DebuggerIPCEvent* ipce = m_pRCThread->GetIPCEventSendBuffer(); - for(ULONG i = 0; i < count; i++) + for(ULONG i = 0; i < count; i++) { DebuggerControllerPatch *patch = listUnbindablePatches->Table()[i]; _ASSERTE(patch != NULL); @@ -6411,11 +6351,10 @@ void Debugger::LockAndSendBreakpointSetError(PATCH_UNORDERED_ARRAY * listUnbinda DebuggerController *controller = patch->controller; if (controller->GetDCType() != DEBUGGER_CONTROLLER_BREAKPOINT) - { continue; - } - LOG((LF_CORDB, LL_INFO10000, "D::LASBSE:\n")); + LOG((LF_CORDB, LL_INFO10000, "D::LASBSE: Breakpoint binding failure (%u/%u) %p type: %u\n", + i+1, count, controller, controller->GetDCType())); // Send a breakpoint set error event to the Right Side. InitIPCEvent(ipce, DB_IPCE_BREAKPOINT_SET_ERROR, thread, thread->GetDomain()); @@ -9411,12 +9350,8 @@ void Debugger::UnloadAssembly(DomainAssembly * pDomainAssembly) // This will block on the continue SENDIPCEVENT_END; - } - - - // // LoadModule is called when a Runtime thread loads a new module and a debugger // is attached. This also includes when a domain-neutral module is "loaded" into @@ -10593,13 +10528,12 @@ bool Debugger::HandleIPCEvent(DebuggerIPCEvent * pEvent) case DB_IPCE_STEP: { - LOG((LF_CORDB,LL_INFO10000, "D::HIPCE: stepIn:0x%x frmTok:0x%x" - "StepIn:0x%x RangeIL:0x%x RangeCount:0x%x MapStop:0x%x " - "InterceptStop:0x%x AppD:0x%x\n", - pEvent->StepData.stepIn, + LOG((LF_CORDB,LL_INFO10000, "D::HIPCE: frame SP:%p " + "StepIn:%s RangeIL:%s RangeCount:%u MapStop:0x%x " + "InterceptStop:0x%x AppD:%p\n", pEvent->StepData.frameToken.GetSPValue(), - pEvent->StepData.stepIn, - pEvent->StepData.rangeIL, + (pEvent->StepData.stepIn ? "true" : "false"), + (pEvent->StepData.rangeIL ? "true" : "false"), pEvent->StepData.rangeCount, pEvent->StepData.rgfMappingStop, pEvent->StepData.rgfInterceptStop, @@ -10861,8 +10795,8 @@ bool Debugger::HandleIPCEvent(DebuggerIPCEvent * pEvent) _ASSERTE(pDebuggerModule != NULL); LOG((LF_CORDB, LL_INFO10000, - "D::HIPCE: class load flag is %d for module 0x%p\n", - pEvent->SetClassLoad.flag, + "D::HIPCE: class load flag is %s for module %p\n", + (pEvent->SetClassLoad.flag ? "true" : "false"), pDebuggerModule)); pDebuggerModule->EnableClassLoadCallbacks((BOOL)pEvent->SetClassLoad.flag); @@ -11178,7 +11112,7 @@ bool Debugger::HandleIPCEvent(DebuggerIPCEvent * pEvent) case DB_IPCE_FUNC_EVAL_ABORT: { - LOG((LF_CORDB, LL_INFO1000, "D::HIPCE: Got FuncEvalAbort for pDE:%08x\n", + LOG((LF_CORDB, LL_INFO1000, "D::HIPCE: Got FuncEvalAbort for pDE:%p\n", pEvent->FuncEvalAbort.debuggerEvalKey.UnWrap())); // This is a synchronous event (reply required) @@ -11194,7 +11128,7 @@ bool Debugger::HandleIPCEvent(DebuggerIPCEvent * pEvent) case DB_IPCE_FUNC_EVAL_RUDE_ABORT: { - LOG((LF_CORDB, LL_INFO1000, "D::HIPCE: Got FuncEvalRudeAbort for pDE:%08x\n", + LOG((LF_CORDB, LL_INFO1000, "D::HIPCE: Got FuncEvalRudeAbort for pDE:%p\n", pEvent->FuncEvalRudeAbort.debuggerEvalKey.UnWrap())); // This is a synchronous event (reply required) @@ -11565,21 +11499,21 @@ HRESULT Debugger::GetAndSendInterceptCommand(DebuggerIPCEvent *event) if (DbgIsSpecialILOffset(pJitInfo->GetSequenceMap()[i].ilOffset)) { LOG((LF_CORDB, LL_INFO10000, - "D::HIPCE: not placing breakpoint at special offset 0x%x\n", startOffset)); + "D::HIPCE: not placing breakpoint at special offset 0x%zx\n", startOffset)); continue; } if ((i >= 1) && (startOffset == pJitInfo->GetSequenceMap()[i-1].nativeStartOffset)) { LOG((LF_CORDB, LL_INFO10000, - "D::HIPCE: not placing redundant breakpoint at duplicate offset 0x%x\n", startOffset)); + "D::HIPCE: not placing redundant breakpoint at duplicate offset 0x%zx\n", startOffset)); continue; } if (startOffset > relOffset) { LOG((LF_CORDB, LL_INFO10000, - "D::HIPCE: Stopping scan for breakpoint at offset 0x%x\n", startOffset)); + "D::HIPCE: Stopping scan for breakpoint at offset 0x%zx\n", startOffset)); continue; } @@ -11587,8 +11521,8 @@ HRESULT Debugger::GetAndSendInterceptCommand(DebuggerIPCEvent *event) if (!(src & ICorDebugInfo::STACK_EMPTY)) { - LOG((LF_CORDB, LL_INFO10000, "D::HIPCE: not placing E&C breakpoint at offset " - "0x%x b/c not STACK_EMPTY:it's 0x%x\n", startOffset, src)); + LOG((LF_CORDB, LL_INFO10000, "D::HIPCE: not placing EnC breakpoint at offset " + "0x%zx b/c not STACK_EMPTY:it's 0x%x\n", startOffset, src)); continue; } @@ -11599,7 +11533,7 @@ HRESULT Debugger::GetAndSendInterceptCommand(DebuggerIPCEvent *event) #endif // FEATURE_EH_FUNCLETS ) { - LOG((LF_CORDB, LL_INFO10000, "D::HIPCE: updating breakpoint at native offset 0x%x\n", + LOG((LF_CORDB, LL_INFO10000, "D::HIPCE: updating breakpoint at native offset 0x%zx\n", startOffset)); foundOffset = startOffset; #if defined(FEATURE_EH_FUNCLETS) @@ -12214,7 +12148,7 @@ HRESULT Debugger::SendReleaseBuffer(DebuggerRCThread* rcThread, void *pBuffer) } CONTRACTL_END; - LOG((LF_CORDB,LL_INFO10000, "D::SRB for buffer 0x%x\n", pBuffer)); + LOG((LF_CORDB,LL_INFO10000, "D::SRB: Buffer %p\n", pBuffer)); // This is a synchronous event (reply required) DebuggerIPCEvent* event = rcThread->GetIPCEventReceiveBuffer(); @@ -12248,7 +12182,7 @@ HRESULT Debugger::ReleaseRemoteBuffer(void *pBuffer, bool removeFromBlobList) } CONTRACTL_END; - LOG((LF_CORDB, LL_EVERYTHING, "D::RRB: Releasing RS-alloc'd buffer 0x%x\n", pBuffer)); + LOG((LF_CORDB, LL_EVERYTHING, "D::RRB: Releasing RS-alloc'd buffer %p\n", pBuffer)); // Remove the buffer from the blob list if necessary. if (removeFromBlobList) @@ -12684,7 +12618,7 @@ EnCSequencePointHelper::EnCSequencePointHelper(DebuggerJitInfo *pJitInfo) if (DbgIsSpecialILOffset(pJitInfo->GetSequenceMap()[i].ilOffset)) { LOG((LF_ENC, LL_INFO10000, - "D::UF: not placing E&C breakpoint at special offset 0x%x (IL: 0x%x)\n", + "D::UF: not placing EnC breakpoint at special offset 0x%zx (IL: 0x%x)\n", offset, m_pJitInfo->GetSequenceMap()[i].ilOffset)); continue; } @@ -12693,8 +12627,7 @@ EnCSequencePointHelper::EnCSequencePointHelper(DebuggerJitInfo *pJitInfo) if (i >=1 && offset == pJitInfo->GetSequenceMap()[i-1].nativeStartOffset) { LOG((LF_ENC, LL_INFO10000, - "D::UF: not placing redundant E&C " - "breakpoint at duplicate offset 0x%x (IL: 0x%x)\n", + "D::UF: not placing redundant EnC breakpoint at duplicate offset 0x%zx (IL: 0x%x)\n", offset, m_pJitInfo->GetSequenceMap()[i].ilOffset)); continue; } @@ -12705,19 +12638,17 @@ EnCSequencePointHelper::EnCSequencePointHelper(DebuggerJitInfo *pJitInfo) if (!(pJitInfo->GetSequenceMap()[i].source & ICorDebugInfo::STACK_EMPTY)) { LOG((LF_ENC, LL_INFO10000, - "D::UF: not placing E&C breakpoint at offset " - "0x%x (IL: 0x%x) b/c not STACK_EMPTY:it's 0x%x\n", offset, - m_pJitInfo->GetSequenceMap()[i].ilOffset, pJitInfo->GetSequenceMap()[i].source)); + "D::UF: not placing EnC breakpoint at offset 0x%zx (IL: 0x%x) b/c not STACK_EMPTY:it's 0x%x\n", + offset, m_pJitInfo->GetSequenceMap()[i].ilOffset, pJitInfo->GetSequenceMap()[i].source)); continue; } // So far this sequence point looks good, so store it's native offset so we can get // EH information about it from the EE. LOG((LF_ENC, LL_INFO10000, - "D::UF: possibly placing E&C breakpoint at offset " - "0x%x (IL: 0x%x)\n", offset, m_pJitInfo->GetSequenceMap()[i].ilOffset)); + "D::UF: possibly placing EnC breakpoint at offset 0x%zx (IL: 0x%x)\n", + offset, m_pJitInfo->GetSequenceMap()[i].ilOffset)); m_pOffsetToHandlerInfo[i].offset = m_pJitInfo->GetSequenceMap()[i].nativeStartOffset; - } // Ask the EE to fill in the isInFilterOrHandler bit for the native offsets we're interested in @@ -12729,15 +12660,11 @@ EnCSequencePointHelper::~EnCSequencePointHelper() { CONTRACTL { - THROWS; + NOTHROW; GC_NOTRIGGER; } CONTRACTL_END; - - if (m_pOffsetToHandlerInfo) - { - delete[] m_pOffsetToHandlerInfo; - } + delete[] m_pOffsetToHandlerInfo; } // @@ -12772,7 +12699,7 @@ BOOL EnCSequencePointHelper::ShouldSetRemapBreakpoint(unsigned int offsetIndex) if (m_pOffsetToHandlerInfo[offsetIndex].isInFilterOrHandler) { LOG((LF_ENC, LL_INFO10000, - "D::UF: not placing E&C breakpoint in filter/handler at offset 0x%x\n", + "D::UF: not placing EnC breakpoint in filter/handler at offset 0x%zx\n", m_pOffsetToHandlerInfo[offsetIndex].offset)); return FALSE; } @@ -12799,8 +12726,8 @@ HRESULT Debugger::UpdateFunction(MethodDesc* pMD, SIZE_T encVersion) } CONTRACTL_END; - LOG((LF_CORDB, LL_INFO10000, "D::UF: updating " - "%s::%s to version %d\n", pMD->m_pszDebugClassName, pMD->m_pszDebugMethodName, encVersion)); + LOG((LF_CORDB, LL_INFO10000, "D::UF: updating %s::%s to encVersion %zx\n", + pMD->m_pszDebugClassName, pMD->m_pszDebugMethodName, encVersion)); // tell the RS that this function has been updated so that it can create new CorDBFunction Module *pModule = g_pEEInterface->MethodDescGetModule(pMD); @@ -12828,11 +12755,12 @@ HRESULT Debugger::UpdateFunction(MethodDesc* pMD, SIZE_T encVersion) // So this call will get the most recent old function. DebuggerJitInfo *pJitInfo = GetLatestJitInfoFromMethodDesc(pMD); - if (pJitInfo == NULL ) + // We only place the patches if we have jit info for this + // function, i.e., its already been jitted. Otherwise, the EE will + // pickup the new method on the next JIT anyway. + if (pJitInfo == NULL) { - LOG((LF_CORDB,LL_INFO10000,"Unable to get DJI by recently " - "D::UF: JITted version number (it hasn't been jitted yet)," - "which is fine\n")); + LOG((LF_CORDB,LL_INFO10000,"D::UF: JITted version number (it hasn't been jitted yet), which is fine\n")); return S_OK; } @@ -12849,14 +12777,11 @@ HRESULT Debugger::UpdateFunction(MethodDesc* pMD, SIZE_T encVersion) LOG((LF_CORDB,LL_INFO10000,"D::UF: Applying breakpoints\n")); - // We only place the patches if we have jit info for this - // function, i.e., its already been jitted. Otherwise, the EE will - // pickup the new method on the next JIT anyway. - EnCSequencePointHelper sequencePointHelper(pJitInfo); // For each offset in the IL->Native map, set a new EnC breakpoint on the // ones that we know could be remap points. + PTR_DebuggerILToNativeMap seqMap = pJitInfo->GetSequenceMap(); for (unsigned int i = 0; i < pJitInfo->GetSequenceMapCount(); i++) { // Skip if this isn't a valid remap point (eg. is in an exception handler) @@ -12865,11 +12790,9 @@ HRESULT Debugger::UpdateFunction(MethodDesc* pMD, SIZE_T encVersion) continue; } - SIZE_T offset = pJitInfo->GetSequenceMap()[i].nativeStartOffset; - - LOG((LF_CORDB, LL_INFO10000, - "D::UF: placing E&C breakpoint at native offset 0x%x\n", - offset)); + SIZE_T offset = seqMap[i].ilOffset; + LOG((LF_CORDB, LL_INFO10000, "D::UF: placing EnC breakpoint at offset 0x%x (IL: 0x%x)\n", + seqMap[i].nativeStartOffset, seqMap[i].ilOffset)); DebuggerEnCBreakpoint *bp; @@ -12984,7 +12907,7 @@ HRESULT Debugger::AddField(FieldDesc* pFD, SIZE_T encVersion) CONTRACTL_END; LOG((LF_CORDB, LL_INFO10000, "D::AFld: adding " - "%8.8d::%8.8d to version %d\n", pFD->GetApproxEnclosingMethodTable()->GetCl(), pFD->GetMemberDef(), encVersion)); + "TypeDef: 0x%08x FieldDef: 0x%08x to version %zx\n", pFD->GetApproxEnclosingMethodTable()->GetCl(), pFD->GetMemberDef(), encVersion)); // tell the RS that this field has been added so that it can update it's structures SendEnCUpdateEvent( DB_IPCE_ENC_ADD_FIELD, @@ -13013,29 +12936,21 @@ HRESULT Debugger::RemapComplete(MethodDesc* pMD, TADDR addr, SIZE_T nativeOffset { THROWS; GC_TRIGGERS_FROM_GETJITINFO; + PRECONDITION(pMD != NULL); + PRECONDITION(addr != NULL); } CONTRACTL_END; - _ASSERTE(pMD != NULL); - _ASSERTE(addr != NULL); - - LOG((LF_CORDB, LL_INFO10000, "D::RC: installed remap complete patch for " - "%s::%s to version %d\n", pMD->m_pszDebugClassName, pMD->m_pszDebugMethodName)); - - DebuggerMethodInfo *dmi = GetOrCreateMethodInfo(pMD->GetModule(), pMD->GetMemberDef()); - - if (dmi == NULL) - { - return E_OUTOFMEMORY; - } - DebuggerJitInfo *pJitInfo = GetJitInfo(pMD, (const BYTE *) addr); - if (pJitInfo == NULL) { _ASSERTE(!"Debugger doesn't handle OOM"); return E_OUTOFMEMORY; } + + LOG((LF_CORDB, LL_INFO10000, "D::RC: installed remap complete patch for %s::%s to encVersion %zx\n", + pMD->m_pszDebugClassName, pMD->m_pszDebugMethodName, pJitInfo->m_encVersion)); + _ASSERTE(pJitInfo->m_addrOfCode + nativeOffset == addr); DebuggerEnCBreakpoint *bp; @@ -13049,6 +12964,7 @@ HRESULT Debugger::RemapComplete(MethodDesc* pMD, TADDR addr, SIZE_T nativeOffset (AppDomain *)pMD->GetModule()->GetDomain()); if (bp == NULL) { + _ASSERTE(!"Debugger doesn't handle OOM"); return E_OUTOFMEMORY; } @@ -13083,10 +12999,8 @@ HRESULT Debugger::MapILInfoToCurrentNative(MethodDesc *pMD, _ASSERTE(HasLazyData()); // only used for EnC, should have already inited. - - LOG((LF_CORDB, LL_INFO1000000, "D::MILITCN: %s::%s ilOff:0x%x, " - ", natFnx:0x%x dji:0x%x\n", pMD->m_pszDebugClassName, - pMD->m_pszDebugMethodName, ilOffset, nativeFnxStart)); + LOG((LF_CORDB, LL_INFO1000000, "D::MILITCN: %s::%s ilOff:0x%zx, natFnx:%p\n", + pMD->m_pszDebugClassName, pMD->m_pszDebugMethodName, ilOffset, nativeFnxStart)); *nativeOffset = 0; DebuggerJitInfo *djiTo = GetJitInfo( pMD, (const BYTE *)nativeFnxStart); diff --git a/src/coreclr/debug/ee/debugger.h b/src/coreclr/debug/ee/debugger.h index 55aa6c6df7e27a..a116c802e1ae35 100644 --- a/src/coreclr/debug/ee/debugger.h +++ b/src/coreclr/debug/ee/debugger.h @@ -689,7 +689,7 @@ class DebuggerRCThread _ASSERTE(m_pDCB != NULL); // In case this turns into a continuation event GetRCThreadSendBuffer()->next = NULL; - LOG((LF_CORDB,LL_EVERYTHING, "GIPCESBuffer: got event 0x%x\n", GetRCThreadSendBuffer())); + LOG((LF_CORDB,LL_EVERYTHING, "GIPCESBuffer: got event %p\n", GetRCThreadSendBuffer())); return GetRCThreadSendBuffer(); } @@ -1250,7 +1250,7 @@ class CodeRegionInfo MethodDesc * md = NULL, PTR_CORDB_ADDRESS_TYPE addr = PTR_NULL); - // Fills in the CodeRegoinInfo fields from the start address. + // Fills in the CodeRegionInfo fields from the start address. void InitializeFromStartAddress(PCODE addr) { CONTRACTL @@ -1436,6 +1436,27 @@ class DebuggerJitInfo #endif public: + void LogInstance() + { +#ifdef LOGGING + const char* encState = "not enabled"; +#ifdef EnC_SUPPORTED + encState = m_encBreakpointsApplied ? "true" : "false"; +#endif //EnC_SUPPORTED + LOG((LF_CORDB, LL_INFO10000, " DJI: %p\n" + " m_jitComplete: %s\n" + " m_encBreakpointsApplied: %s\n" + " m_methodInfo: %p\n" + " m_addrOfCode: %p\n" + " m_sizeOfCode: 0x%zx\n" + " m_lastIL: 0x%x\n" + " m_sequenceMapCount: %u\n" + " m_callsiteMapCount: %u\n", + this, (m_jitComplete ? "true" : "false"), encState, + m_methodInfo, m_addrOfCode, m_sizeOfCode, m_lastIL, m_sequenceMapCount, m_callsiteMapCount)); +#endif //LOGGING + } + unsigned int GetSequenceMapCount() { SUPPORTS_DAC; diff --git a/src/coreclr/debug/ee/functioninfo.cpp b/src/coreclr/debug/ee/functioninfo.cpp index cf94eab8f0458e..5602dc2580681e 100644 --- a/src/coreclr/debug/ee/functioninfo.cpp +++ b/src/coreclr/debug/ee/functioninfo.cpp @@ -383,8 +383,8 @@ DebuggerJitInfo::NativeOffset DebuggerJitInfo::MapILOffsetToNative(DebuggerJitIn { #endif // FEATURE_EH_FUNCLETS PREFIX_ASSUME( map != NULL ); - LOG((LF_CORDB, LL_INFO10000, "DJI::MILOTN: ilOff 0x%x to nat 0x%x exact:0x%x (Entry IL Off:0x%x)\n", - ilOffset.m_ilOffset, map->nativeStartOffset, resultOffset.m_fExact, map->ilOffset)); + LOG((LF_CORDB, LL_INFO10000, "DJI::MILOTN: ilOffset 0x%zx to nat 0x%x exact:%s (Entry IL Off:0x%x)\n", + ilOffset.m_ilOffset, map->nativeStartOffset, (resultOffset.m_fExact ? "true" : "false"), map->ilOffset)); resultOffset.m_nativeOffset = map->nativeStartOffset; @@ -654,7 +654,7 @@ SIZE_T DebuggerJitInfo::MapILOffsetToNativeForSetIP(SIZE_T offsetILTo, int funcl // calls MapILOffsetToNative for the startOffset (putting the // result into start), and the endOffset (putting the result into end). // SIZE_T startOffset: IL offset from beginning of function. -// SIZE_T endOffset: IL offset from beginngin of function, +// SIZE_T endOffset: IL offset from beginning of function, // or zero to indicate that the end of the function should be used. // DebuggerILToNativeMap **start: Contains start & end // native offsets that correspond to startOffset. Set to NULL if @@ -892,13 +892,9 @@ void DebuggerJitInfo::LazyInitBounds() PRECONDITION(!g_pDebugger->HasDebuggerDataLock()); } CONTRACTL_END; - LOG((LF_CORDB, LL_EVERYTHING, "DJI::LazyInitBounds: this=0x%p m_fAttemptInit %s\n", this, m_fAttemptInit == true ? "true": "false")); - // Only attempt lazy-init once if (m_fAttemptInit) - { return; - } EX_TRY { @@ -927,7 +923,8 @@ void DebuggerJitInfo::LazyInitBounds() &cMap, &pMap, &cVars, &pVars); - LOG((LF_CORDB,LL_EVERYTHING, "DJI::LazyInitBounds: this=0x%p GetBoundariesAndVars success=0x%x\n", this, fSuccess)); + LOG((LF_CORDB,LL_EVERYTHING, "DJI::LazyInitBounds: this=%p GetBoundariesAndVars success=%s\n", + this, (fSuccess ? "true" : "false"))); // SetBoundaries uses the CodeVersionManager, need to take it now for lock ordering reasons CodeVersionManager::LockHolder codeVersioningLockHolder; @@ -951,7 +948,7 @@ void DebuggerJitInfo::LazyInitBounds() } EX_CATCH { - LOG((LF_CORDB,LL_WARNING, "DJI::LazyInitBounds: this=0x%x Exception was thrown and caught\n", this)); + LOG((LF_CORDB,LL_WARNING, "DJI::LazyInitBounds: this=%p Exception was thrown and caught\n", this)); // Just catch the exception. The DJI maps may or may-not be initialized, // but they should still be in a consistent state, so we should be ok. } @@ -1016,7 +1013,7 @@ void DebuggerJitInfo::SetBoundaries(ULONG32 cMap, ICorDebugInfo::OffsetMapping * } CONTRACTL_END; - LOG((LF_CORDB,LL_EVERYTHING, "DJI::SetBoundaries: this=0x%p cMap=0x%x pMap=0x%p\n", this, cMap, pMap)); + LOG((LF_CORDB,LL_EVERYTHING, "DJI::sB: this=%p cMap=%u pMap=%p\n", this, cMap, pMap)); _ASSERTE((cMap == 0) == (pMap == NULL)); _ASSERTE(m_sequenceMap == NULL); @@ -1043,7 +1040,7 @@ void DebuggerJitInfo::SetBoundaries(ULONG32 cMap, ICorDebugInfo::OffsetMapping * // like the DebuggerJitInfo's. // m_sequenceMap = (DebuggerILToNativeMap *)new (interopsafe) DebuggerILToNativeMap[cMap]; - LOG((LF_CORDB,LL_EVERYTHING, "DJI::SetBoundaries: this=0x%p m_sequenceMap=0x%x\n", this, m_sequenceMap)); + LOG((LF_CORDB,LL_EVERYTHING, "DJI::sB: this=%p m_sequenceMap=%p\n", this, m_sequenceMap)); _ASSERTE(m_sequenceMap != NULL); // we'll throw on null m_sequenceMapCount = cMap; @@ -1186,42 +1183,46 @@ void DebuggerJitInfo::SetBoundaries(ULONG32 cMap, ICorDebugInfo::OffsetMapping * m_callsiteMap = m_sequenceMap + m_sequenceMapCount; m_callsiteMapCount -= m_sequenceMapCount; - LOG((LF_CORDB, LL_INFO100000, "DJI::SetBoundaries: this=0x%p boundary count is %d (%d callsites)\n", + LOG((LF_CORDB, LL_INFO100000, "DJI::sB: this=%p boundary count is %u (%u callsites)\n", this, m_sequenceMapCount, m_callsiteMapCount)); #ifdef LOGGING - for (unsigned int count = 0; count < m_sequenceMapCount + m_callsiteMapCount; count++) - { - if( m_sequenceMap[count].ilOffset == - (ULONG) ICorDebugInfo::PROLOG ) - LOG((LF_CORDB, LL_INFO1000000, - "D::sB: PROLOG --> 0x%08x -- 0x%08x", - m_sequenceMap[count].nativeStartOffset, - m_sequenceMap[count].nativeEndOffset)); - else if ( m_sequenceMap[count].ilOffset == - (ULONG) ICorDebugInfo::EPILOG ) - LOG((LF_CORDB, LL_INFO1000000, - "D::sB: EPILOG --> 0x%08x -- 0x%08x", - m_sequenceMap[count].nativeStartOffset, - m_sequenceMap[count].nativeEndOffset)); - else if ( m_sequenceMap[count].ilOffset == - (ULONG) ICorDebugInfo::NO_MAPPING ) - LOG((LF_CORDB, LL_INFO1000000, - "D::sB: NO MAP --> 0x%08x -- 0x%08x", - m_sequenceMap[count].nativeStartOffset, - m_sequenceMap[count].nativeEndOffset)); - else - LOG((LF_CORDB, LL_INFO1000000, - "D::sB: 0x%04x (Real:0x%04x) --> 0x%08x -- 0x%08x", - m_sequenceMap[count].ilOffset, - m_methodInfo->TranslateToInstIL(&mapping, - m_sequenceMap[count].ilOffset, - bOriginalToInstrumented), - m_sequenceMap[count].nativeStartOffset, - m_sequenceMap[count].nativeEndOffset)); - - LOG((LF_CORDB, LL_INFO1000000, " Src:0x%x\n", m_sequenceMap[count].source)); + for (unsigned count = 0; count < m_sequenceMapCount + m_callsiteMapCount; count++) + { + const DebuggerILToNativeMap& entry = m_sequenceMap[count]; + switch (entry.ilOffset) + { + case (ULONG) ICorDebugInfo::PROLOG: + LOG((LF_CORDB, LL_INFO1000000, + "DJI::sB: PROLOG --> 0x%08x -- 0x%08x", + entry.nativeStartOffset, + entry.nativeEndOffset)); + break; + case (ULONG) ICorDebugInfo::EPILOG: + LOG((LF_CORDB, LL_INFO1000000, + "DJI::sB: EPILOG --> 0x%08x -- 0x%08x", + entry.nativeStartOffset, + entry.nativeEndOffset)); + break; + case (ULONG) ICorDebugInfo::NO_MAPPING: + LOG((LF_CORDB, LL_INFO1000000, + "DJI::sB: NO MAP --> 0x%08x -- 0x%08x", + entry.nativeStartOffset, + entry.nativeEndOffset)); + break; + default: + LOG((LF_CORDB, LL_INFO1000000, + "DJI::sB: 0x%04x (Real:0x%04x) --> 0x%08x -- 0x%08x", + entry.ilOffset, + m_methodInfo->TranslateToInstIL(&mapping, + entry.ilOffset, + bOriginalToInstrumented), + entry.nativeStartOffset, + entry.nativeEndOffset)); + break; + } + LOG((LF_CORDB, LL_INFO1000000, " Src:0x%x\n", entry.source)); } #endif //LOGGING } @@ -1246,17 +1247,18 @@ void DebuggerJitInfo::Init(TADDR newAddress) this->InitFuncletAddress(); #endif // FEATURE_EH_FUNCLETS - LOG((LF_CORDB,LL_INFO10000,"De::JITCo:Got DJI 0x%p(V %d)," - "Hot section from 0x%p to 0x%p " - "Cold section from 0x%p to 0x%p " - "varCount=%d seqCount=%d\n", + LOG((LF_CORDB,LL_INFO10000,"De::JITCo:Got DJI %p (encVersion: %zx)," + "Hot section from %p to %p " + "Cold section from %p to %p " + "Code from %p to %p " + "varCount=%u seqCount=%u\n", this, this->m_encVersion, this->m_codeRegionInfo.getAddrOfHotCode(), this->m_codeRegionInfo.getAddrOfHotCode() + this->m_codeRegionInfo.getSizeOfHotCode(), this->m_codeRegionInfo.getAddrOfColdCode(), this->m_codeRegionInfo.getAddrOfColdCode() + this->m_codeRegionInfo.getSizeOfColdCode(), - (ULONG)this->m_addrOfCode, - (ULONG)this->m_addrOfCode+(ULONG)this->m_sizeOfCode, + this->m_addrOfCode, + this->m_addrOfCode+(ULONG)this->m_sizeOfCode, this->GetVarNativeInfoCount(), this->GetSequenceMapCount())); @@ -1717,7 +1719,7 @@ DebuggerJitInfo *DebuggerMethodInfo::CreateInitAndAddJitInfo(NativeCodeVersion n // We've now added a new DJI into the table and released the lock. Thus any other thread // can come and use our DJI. Good thing we inited the DJI _before_ adding it to the table. - LOG((LF_CORDB,LL_INFO10000,"DMI:CAAJI: new head of dji list:0x%p\n", m_latestJitInfo)); + LOG((LF_CORDB,LL_INFO10000,"DMI:CAAJI: new head of dji list: %p\n", m_latestJitInfo)); return dji; } @@ -1983,7 +1985,7 @@ void DebuggerMethodInfo::IterateAllDJIs(AppDomain * pAppDomain, Module * pLoader _ASSERTE(pEnum != NULL); _ASSERTE(pAppDomain != NULL || pMethodDescFilter != NULL); - // Esnure we have DJIs for everything. + // Ensure we have DJIs for everything. CreateDJIsForNativeBlobs(pAppDomain, pLoaderModuleFilter, pMethodDescFilter); pEnum->m_pCurrent = m_latestJitInfo; @@ -2076,7 +2078,7 @@ void DebuggerMethodInfo::CreateDJIsForMethodDesc(MethodDesc * pMethodDesc) } CONTRACTL_END; - LOG((LF_CORDB, LL_INFO10000, "DMI::CDJIFMD pMethodDesc:0x%p\n", pMethodDesc)); + LOG((LF_CORDB, LL_INFO10000, "DMI::CDJIFMD pMethodDesc:%p\n", pMethodDesc)); // The debugger doesn't track Lightweight-codegen methods b/c they have no metadata. if (pMethodDesc->IsDynamicMethod()) @@ -2099,17 +2101,17 @@ void DebuggerMethodInfo::CreateDJIsForMethodDesc(MethodDesc * pMethodDesc) // Some versions may not be compiled yet - skip those for now // if they compile later the JitCompiled callback will add a DJI to our cache at that time PCODE codeAddr = itr->GetNativeCode(); - LOG((LF_CORDB, LL_INFO10000, "DMI::CDJIFMD (%d) Native code for DJI - 0x%p\n", ++count, codeAddr)); + LOG((LF_CORDB, LL_INFO10000, "DMI::CDJIFMD (%d) Native code for DJI - %p\n", ++count, codeAddr)); if (codeAddr) { // The DJI may already be populated in the cache, if so CreateInitAndAdd is // a no-op and that is fine. BOOL unusedDjiWasCreated; CreateInitAndAddJitInfo(*itr, codeAddr, &unusedDjiWasCreated); - LOG((LF_CORDB, LL_INFO10000, "DMI::CDJIFMD Was DJI created? 0x%d\n", unusedDjiWasCreated)); + LOG((LF_CORDB, LL_INFO10000, "DMI::CDJIFMD Was DJI created? %s\n", (unusedDjiWasCreated ? "true" : "false"))); } } - LOG((LF_CORDB, LL_INFO10000, "DMI::CDJIFMD NativeCodeVersion total %d for md=0x%p\n", count, pMethodDesc)); + LOG((LF_CORDB, LL_INFO10000, "DMI::CDJIFMD NativeCodeVersion total %d for md=%p\n", count, pMethodDesc)); } #else // We just ask for the DJI to ensure that it's lazily created. @@ -2179,8 +2181,8 @@ HRESULT DebuggerMethodInfoTable::AddMethodInfo(Module *pModule, } CONTRACTL_END; - LOG((LF_CORDB, LL_INFO1000, "DMIT::AMI Adding dmi:0x%x Mod:0x%x tok:" - "0x%x nVer:0x%x\n", mi, pModule, token, mi->GetCurrentEnCVersion())); + LOG((LF_CORDB, LL_INFO1000, "DMIT::AMI: Adding dmi:%p Mod:%p tok:0x%08x nVer:0x%zx\n", + mi, pModule, token, mi->GetCurrentEnCVersion())); _ASSERTE(mi != NULL); @@ -2197,18 +2199,18 @@ HRESULT DebuggerMethodInfoTable::AddMethodInfo(Module *pModule, DebuggerMethodInfoEntry *dmie = (DebuggerMethodInfoEntry *) Add(HASH(&dmik)); - if (dmie != NULL) + if (dmie == NULL) { - dmie->key.pModule = pModule; - dmie->key.token = token; - dmie->mi = mi; - - LOG((LF_CORDB, LL_INFO1000, "DMIT::AJI: mod:0x%x tok:0%x ", - pModule, token)); - return S_OK; + ThrowOutOfMemory(); + return E_OUTOFMEMORY; } - ThrowOutOfMemory(); + dmie->key.pModule = pModule; + dmie->key.token = token; + dmie->mi = mi; + + LOG((LF_CORDB, LL_INFO1000, "DMIT::AMI: mod:%p tok:0x%08x\n", + pModule, token)); return S_OK; } @@ -2227,8 +2229,8 @@ HRESULT DebuggerMethodInfoTable::OverwriteMethodInfo(Module *pModule, } CONTRACTL_END; - LOG((LF_CORDB, LL_INFO1000, "DMIT::OJI: dmi:0x%x mod:0x%x tok:0x%x\n", mi, - pModule, token)); + LOG((LF_CORDB, LL_INFO1000, "DMIT::OMI: dmi:%p mod:%p tok:0x%08x\n", + mi, pModule, token)); _ASSERTE(g_pDebugger->HasDebuggerDataLock()); @@ -2240,14 +2242,13 @@ HRESULT DebuggerMethodInfoTable::OverwriteMethodInfo(Module *pModule, = (DebuggerMethodInfoEntry *) Find(HASH(&dmik), KEY(&dmik)); if (entry != NULL) { - if ( (fOnlyIfNull && - entry->mi == NULL) || - !fOnlyIfNull) + if ( (fOnlyIfNull && entry->mi == NULL) + || !fOnlyIfNull) { entry->mi = mi; - LOG((LF_CORDB, LL_INFO1000, "DMIT::OJI: mod:0x%x tok:0x%x remap" - "nVer:0x%x\n", pModule, token, entry->nVersionLastRemapped)); + LOG((LF_CORDB, LL_INFO1000, "DMIT::OMI: mod:%p tok:0x%08x remap nVer:0x%zx\n", + pModule, token, entry->nVersionLastRemapped)); return S_OK; } } @@ -2380,7 +2381,7 @@ DebuggerMethodInfo *DebuggerMethodInfoTable::GetMethodInfo(Module *pModule, mdMe } else { - LOG((LF_CORDB, LL_INFO1000, "DMI::GMI: for methodDef 0x%x, got 0x%p prev:0x%p\n", + LOG((LF_CORDB, LL_INFO1000, "DMI::GMI: for methodDef 0x%x, DMI=%p prev DMI=%p\n", token, entry->mi, (entry->mi?entry->mi->m_prevMethodInfo:0))); return entry->mi; } diff --git a/src/coreclr/debug/ee/rcthread.cpp b/src/coreclr/debug/ee/rcthread.cpp index e7aa9f2b067ce4..b20c46559f734f 100644 --- a/src/coreclr/debug/ee/rcthread.cpp +++ b/src/coreclr/debug/ee/rcthread.cpp @@ -1595,7 +1595,7 @@ bool DebuggerRCThread::IsRCThreadReady() // leaving the threadid still non-0. So check the actual thread object // and make sure it's still around. int ret = WaitForSingleObject(m_thread, 0); - LOG((LF_CORDB, LL_EVERYTHING, "DRCT::IsReady - wait(0x%p)=%d, GetLastError() = %d\n", m_thread, ret, GetLastError())); + LOG((LF_CORDB, LL_EVERYTHING, "DRCT::IsReady - wait(%p)=0x%x, GetLastError() = 0x%x\n", m_thread, ret, GetLastError())); if (ret != WAIT_TIMEOUT) { diff --git a/src/coreclr/inc/corinfo.h b/src/coreclr/inc/corinfo.h index 0b50a047b3f66e..66a4e46063fab7 100644 --- a/src/coreclr/inc/corinfo.h +++ b/src/coreclr/inc/corinfo.h @@ -519,7 +519,7 @@ enum CorInfoHelpFunc CORINFO_HELP_SETFIELDDOUBLE, CORINFO_HELP_GETFIELDADDR, - + CORINFO_HELP_GETSTATICFIELDADDR, CORINFO_HELP_GETSTATICFIELDADDR_TLS, // Helper for PE TLS fields // There are a variety of specialized helpers for accessing static fields. The JIT should use diff --git a/src/coreclr/inc/jiteeversionguid.h b/src/coreclr/inc/jiteeversionguid.h index 7279f9fb99fd73..69d98159f440ee 100644 --- a/src/coreclr/inc/jiteeversionguid.h +++ b/src/coreclr/inc/jiteeversionguid.h @@ -43,11 +43,11 @@ typedef const GUID *LPCGUID; #define GUID_DEFINED #endif // !GUID_DEFINED -constexpr GUID JITEEVersionIdentifier = { /* d5079702-9480-4e42-a720-6f38a0d9413d */ - 0xd5079702, - 0x9480, - 0x4e42, - {0xa7, 0x20, 0x6f, 0x38, 0xa0, 0xd9, 0x41, 0x3d} +constexpr GUID JITEEVersionIdentifier = { /* 7925c4a8-129f-48ef-b48a-262d60ef84b0 */ + 0x7925c4a8, + 0x129f, + 0x48ef, + { 0xb4, 0x8a, 0x26, 0x2d, 0x60, 0xef, 0x84, 0xb0 } }; ////////////////////////////////////////////////////////////////////////////////////////////////////////// diff --git a/src/coreclr/inc/jithelpers.h b/src/coreclr/inc/jithelpers.h index 7193c124d602e7..30f8fe38ae4501 100644 --- a/src/coreclr/inc/jithelpers.h +++ b/src/coreclr/inc/jithelpers.h @@ -173,7 +173,7 @@ JITHELPER(CORINFO_HELP_SETFIELDDOUBLE, JIT_SetFieldDouble,CORINFO_HELP_SIG_8_STACK) JITHELPER(CORINFO_HELP_GETFIELDADDR, JIT_GetFieldAddr,CORINFO_HELP_SIG_REG_ONLY) - + JITHELPER(CORINFO_HELP_GETSTATICFIELDADDR, JIT_GetStaticFieldAddr,CORINFO_HELP_SIG_REG_ONLY) JITHELPER(CORINFO_HELP_GETSTATICFIELDADDR_TLS, NULL, CORINFO_HELP_SIG_CANNOT_USE_ALIGN_STUB) JITHELPER(CORINFO_HELP_GETGENERICS_GCSTATIC_BASE, JIT_GetGenericsGCStaticBase,CORINFO_HELP_SIG_REG_ONLY) diff --git a/src/coreclr/tools/Common/JitInterface/CorInfoHelpFunc.cs b/src/coreclr/tools/Common/JitInterface/CorInfoHelpFunc.cs index e4cc6a13c3e174..2c7ba7aec14e9f 100644 --- a/src/coreclr/tools/Common/JitInterface/CorInfoHelpFunc.cs +++ b/src/coreclr/tools/Common/JitInterface/CorInfoHelpFunc.cs @@ -161,7 +161,7 @@ which is the right helper to use to allocate an object of a given type. */ CORINFO_HELP_SETFIELDDOUBLE, CORINFO_HELP_GETFIELDADDR, - + CORINFO_HELP_GETSTATICFIELDADDR, CORINFO_HELP_GETSTATICFIELDADDR_TLS, // Helper for PE TLS fields // There are a variety of specialized helpers for accessing static fields. The JIT should use diff --git a/src/coreclr/utilcode/log.cpp b/src/coreclr/utilcode/log.cpp index 79b9a9f50bdfd9..29c001f7d0e19b 100644 --- a/src/coreclr/utilcode/log.cpp +++ b/src/coreclr/utilcode/log.cpp @@ -327,8 +327,8 @@ VOID LogSpewAlwaysValist(const char *fmt, va_list args) needsPrefix = (fmt[strlen(fmt)-1] == '\n'); - int cCountWritten = _vsnprintf_s(&pBuffer[buflen], BUFFERSIZE-buflen, _TRUNCATE, fmt, args ); - pBuffer[BUFFERSIZE-1] = 0; + int cCountWritten = _vsnprintf_s(&pBuffer[buflen], BUFFERSIZE - buflen - 1, _TRUNCATE, fmt, args ); + pBuffer[BUFFERSIZE - 1] = 0; if (cCountWritten < 0) { buflen = BUFFERSIZE - 1; } else { diff --git a/src/coreclr/vm/ceeload.cpp b/src/coreclr/vm/ceeload.cpp index f9b466a19c8625..ec1e5b083fde69 100644 --- a/src/coreclr/vm/ceeload.cpp +++ b/src/coreclr/vm/ceeload.cpp @@ -545,6 +545,25 @@ void Module::SetDebuggerInfoBits(DebuggerAssemblyControlFlags newBits) } #ifndef DACCESS_COMPILE +static BOOL IsEditAndContinueCapable(Assembly *pAssembly, PEAssembly *pPEAssembly) +{ + CONTRACTL + { + NOTHROW; + GC_NOTRIGGER; + MODE_ANY; + SUPPORTS_DAC; + } + CONTRACTL_END; + + _ASSERTE(pAssembly != NULL && pPEAssembly != NULL); + + // Some modules are never EnC-capable + return ! (pAssembly->GetDebuggerInfoBits() & DACF_ALLOW_JIT_OPTS || + pPEAssembly->IsSystem() || + pPEAssembly->IsDynamic()); +} + /* static */ Module *Module::Create(Assembly *pAssembly, PEAssembly *pPEAssembly, AllocMemTracker *pamTracker) { @@ -566,7 +585,7 @@ Module *Module::Create(Assembly *pAssembly, PEAssembly *pPEAssembly, AllocMemTra // Create the module #ifdef EnC_SUPPORTED - if (IsEditAndContinueCapable(pAssembly, pPEAssembly)) + if (::IsEditAndContinueCapable(pAssembly, pPEAssembly)) { // if file is EnCCapable, always create an EnC-module, but EnC won't necessarily be enabled. // Debugger enables this by calling SetJITCompilerFlags on LoadModule callback. @@ -598,7 +617,7 @@ void Module::ApplyMetaData() } CONTRACTL_END; - LOG((LF_CLASSLOADER, LL_INFO100, "Module::ApplyNewMetaData %x\n", this)); + LOG((LF_CLASSLOADER, LL_INFO100, "Module::ApplyNewMetaData this:%p\n", this)); HRESULT hr = S_OK; ULONG ulCount; @@ -834,26 +853,6 @@ MethodTable *Module::GetGlobalMethodTable() #endif // !DACCESS_COMPILE -/*static*/ -BOOL Module::IsEditAndContinueCapable(Assembly *pAssembly, PEAssembly *pPEAssembly) -{ - CONTRACTL - { - NOTHROW; - GC_NOTRIGGER; - MODE_ANY; - SUPPORTS_DAC; - } - CONTRACTL_END; - - _ASSERTE(pAssembly != NULL && pPEAssembly != NULL); - - // Some modules are never EnC-capable - return ! (pAssembly->GetDebuggerInfoBits() & DACF_ALLOW_JIT_OPTS || - pPEAssembly->IsSystem() || - pPEAssembly->IsDynamic()); -} - BOOL Module::IsManifest() { WRAPPER_NO_CONTRACT; @@ -5187,6 +5186,23 @@ void Module::EnumMemoryRegions(CLRDataEnumMemoryFlags flags, } ECall::EnumFCallMethods(); + +#ifdef EnC_SUPPORTED + m_ClassList.EnumMemoryRegions(); + + DPTR(PTR_EnCEEClassData) classData = m_ClassList.Table(); + DPTR(PTR_EnCEEClassData) classLast = classData + m_ClassList.Count(); + + while (classData.IsValid() && classData < classLast) + { + if ((*classData).IsValid()) + { + (*classData)->EnumMemoryRegions(flags); + } + + classData++; + } +#endif // EnC_SUPPORTED } FieldDesc *Module::LookupFieldDef(mdFieldDef token) diff --git a/src/coreclr/vm/ceeload.h b/src/coreclr/vm/ceeload.h index b2a31352746e0f..a14916258ec7dd 100644 --- a/src/coreclr/vm/ceeload.h +++ b/src/coreclr/vm/ceeload.h @@ -62,12 +62,15 @@ class Module; class SString; class Pending; class MethodTable; -class AppDomain; class DynamicMethodTable; class CodeVersionManager; class TieredCompilationManager; class JITInlineTrackingMap; +#ifdef EnC_SUPPORTED +class EnCEEClassData; +#endif // EnC_SUPPORTED + // Hash table parameter of available classes (name -> module/class) hash #define AVAILABLE_CLASSES_HASH_BUCKETS 1024 #define AVAILABLE_CLASSES_HASH_BUCKETS_COLLECTIBLE 128 @@ -936,9 +939,12 @@ class Module : public ModuleBase BOOL IsPEFile() const { WRAPPER_NO_CONTRACT; return !GetPEAssembly()->IsDynamic(); } BOOL IsReflection() const { WRAPPER_NO_CONTRACT; SUPPORTS_DAC; return GetPEAssembly()->IsDynamic(); } + BOOL IsSystem() { WRAPPER_NO_CONTRACT; SUPPORTS_DAC; return m_pPEAssembly->IsSystem(); } // Returns true iff the debugger can see this module. BOOL IsVisibleToDebugger(); + virtual BOOL IsEditAndContinueCapable() const { return FALSE; } + BOOL IsEditAndContinueEnabled() { LIMITED_METHOD_CONTRACT; @@ -947,21 +953,22 @@ class Module : public ModuleBase return (m_dwTransientFlags & IS_EDIT_AND_CONTINUE) != 0; } - virtual BOOL IsEditAndContinueCapable() const { return FALSE; } - - BOOL IsSystem() { WRAPPER_NO_CONTRACT; SUPPORTS_DAC; return m_pPEAssembly->IsSystem(); } - - static BOOL IsEditAndContinueCapable(Assembly *pAssembly, PEAssembly *file); +#ifdef EnC_SUPPORTED + // Holds a table of EnCEEClassData object for classes in this module that have been modified + CUnorderedArray m_ClassList; +#endif // EnC_SUPPORTED +private: void EnableEditAndContinue() { LIMITED_METHOD_CONTRACT; SUPPORTS_DAC; _ASSERTE(IsEditAndContinueCapable()); - LOG((LF_ENC, LL_INFO100, "EnableEditAndContinue: this:0x%x, %s\n", this, GetDebugName())); + LOG((LF_ENC, LL_INFO100, "M:EnableEditAndContinue: this:%p, %s\n", this, GetDebugName())); m_dwTransientFlags |= IS_EDIT_AND_CONTINUE; } +public: BOOL IsTenured() { LIMITED_METHOD_CONTRACT; diff --git a/src/coreclr/vm/class.cpp b/src/coreclr/vm/class.cpp index 4e8ca18fa09f6b..4a178e819b601c 100644 --- a/src/coreclr/vm/class.cpp +++ b/src/coreclr/vm/class.cpp @@ -84,12 +84,6 @@ void EEClass::Destruct(MethodTable * pOwningMT) } CONTRACTL_END - -#ifdef _DEBUG - _ASSERTE(!IsDestroyed()); - SetDestroyed(); -#endif - #ifdef PROFILING_SUPPORTED // If profiling, then notify the class is getting unloaded. { @@ -257,8 +251,7 @@ VOID EEClass::FixupFieldDescForEnC(MethodTable * pMT, EnCFieldDesc *pFD, mdField CONTRACTL { THROWS; - MODE_COOPERATIVE; - WRAPPER(GC_TRIGGERS); + GC_TRIGGERS; INJECT_FAULT(COMPlusThrowOM();); } CONTRACTL_END @@ -274,7 +267,7 @@ VOID EEClass::FixupFieldDescForEnC(MethodTable * pMT, EnCFieldDesc *pFD, mdField { szFieldName = "Invalid FieldDef record"; } - LOG((LF_ENC, LL_INFO100, "EEClass::FixupFieldDescForEnC %08x %s\n", fieldDef, szFieldName)); + LOG((LF_ENC, LL_INFO100, "EEClass::FixupFieldDescForEnC '%s' (0x%08x)\n", szFieldName, fieldDef)); } #endif //LOGGING @@ -291,17 +284,21 @@ VOID EEClass::FixupFieldDescForEnC(MethodTable * pMT, EnCFieldDesc *pFD, mdField // once this function is out of scope. ACQUIRE_STACKING_ALLOCATOR(pStackingAllocator); + // Collect the attributes for the field + mdToken fieldDefs[1] = { fieldDef }; + DWORD fieldAttrs[1]; + IfFailThrow(pImport->GetFieldDefProps(fieldDefs[0], &fieldAttrs[0])); + MethodTableBuilder::bmtMetaDataInfo bmtMetaData; - bmtMetaData.cFields = 1; - bmtMetaData.pFields = (mdToken*)_alloca(sizeof(mdToken)); - bmtMetaData.pFields[0] = fieldDef; - bmtMetaData.pFieldAttrs = (DWORD*)_alloca(sizeof(DWORD)); - IfFailThrow(pImport->GetFieldDefProps(fieldDef, &bmtMetaData.pFieldAttrs[0])); + bmtMetaData.cFields = ARRAY_SIZE(fieldDefs); + bmtMetaData.pFields = fieldDefs; + bmtMetaData.pFieldAttrs = fieldAttrs; - MethodTableBuilder::bmtMethAndFieldDescs bmtMFDescs; // We need to alloc the memory, but don't have to fill it in. InitializeFieldDescs // will copy pFD (1st arg) into here. - bmtMFDescs.ppFieldDescList = (FieldDesc**)_alloca(sizeof(FieldDesc*)); + FieldDesc* fieldDescs[1]; + MethodTableBuilder::bmtMethAndFieldDescs bmtMFDescs; + bmtMFDescs.ppFieldDescList = fieldDescs; MethodTableBuilder::bmtFieldPlacement bmtFP; @@ -329,37 +326,31 @@ VOID EEClass::FixupFieldDescForEnC(MethodTable * pMT, EnCFieldDesc *pFD, mdField } else { + _ASSERTE(!pMT->IsValueType()); bmtEnumFields.dwNumInstanceFields = 1; } // We shouldn't have to fill this in b/c we're not allowed to EnC value classes, or // anything else with layout info associated with it. - LayoutRawFieldInfo *pLayoutRawFieldInfos = (LayoutRawFieldInfo*)_alloca((2) * sizeof(LayoutRawFieldInfo)); + // Provide 2, 1 placeholder and 1 for the actual field - see BuildMethodTableThrowing(). + LayoutRawFieldInfo layoutRawFieldInfos[2]; // If not NULL, it means there are some by-value fields, and this contains an entry for each instance or static field, // which is NULL if not a by value field, and points to the EEClass of the field if a by value field. Instance fields // come first, statics come second. - MethodTable **pByValueClassCache = NULL; - - EEClass * pClass = pMT->GetClass(); - - // InitializeFieldDescs are going to change these numbers to something wrong, - // even though we already have the right numbers. Save & restore after. - WORD wNumInstanceFields = pMT->GetNumInstanceFields(); - WORD wNumStaticFields = pMT->GetNumStaticFields(); - unsigned totalDeclaredFieldSize = 0; + MethodTable** pByValueClassCache = NULL; AllocMemTracker dummyAmTracker; - BaseDomain * pDomain = pMT->GetDomain(); + EEClass* pClass = pMT->GetClass(); MethodTableBuilder builder(pMT, pClass, pStackingAllocator, &dummyAmTracker); + TypeHandle thisTH(pMT); + SigTypeContext typeContext(thisTH); MethodTableBuilder::bmtGenericsInfo genericsInfo; - - OBJECTREF pThrowable = NULL; - GCPROTECT_BEGIN(pThrowable); + genericsInfo.typeContext = typeContext; builder.SetBMTData(pMT->GetLoaderAllocator(), &bmtError, @@ -377,11 +368,11 @@ VOID EEClass::FixupFieldDescForEnC(MethodTable * pMT, EnCFieldDesc *pFD, mdField &genericsInfo, &bmtEnumFields); - EX_TRY { GCX_PREEMP(); + unsigned totalDeclaredFieldSize = 0; builder.InitializeFieldDescs(pFD, - pLayoutRawFieldInfos, + layoutRawFieldInfos, &bmtInternal, &genericsInfo, &bmtMetaData, @@ -392,29 +383,9 @@ VOID EEClass::FixupFieldDescForEnC(MethodTable * pMT, EnCFieldDesc *pFD, mdField &bmtFP, &totalDeclaredFieldSize); } - EX_CATCH_THROWABLE(&pThrowable); dummyAmTracker.SuppressRelease(); - // Restore now - pClass->SetNumInstanceFields(wNumInstanceFields); - pClass->SetNumStaticFields(wNumStaticFields); - - // PERF: For now, we turn off the fast equality check for valuetypes when a - // a field is modified by EnC. Consider doing a check and setting the bit only when - // necessary. - if (pMT->IsValueType()) - { - pClass->SetIsNotTightlyPacked(); - } - - if (pThrowable != NULL) - { - COMPlusThrow(pThrowable); - } - - GCPROTECT_END(); - pFD->SetMethodTable(pMT); // We set this when we first created the FieldDesc, but initializing the FieldDesc @@ -435,16 +406,19 @@ VOID EEClass::FixupFieldDescForEnC(MethodTable * pMT, EnCFieldDesc *pFD, mdField // Here we just create the FieldDesc and link it to the class. The actual storage will // be created lazily on demand. // -HRESULT EEClass::AddField(MethodTable * pMT, mdFieldDef fieldDef, EnCFieldDesc **ppNewFD) +HRESULT EEClass::AddField(MethodTable* pMT, mdFieldDef fieldDef, FieldDesc** ppNewFD) { CONTRACTL { THROWS; GC_NOTRIGGER; MODE_COOPERATIVE; + PRECONDITION(pMT != NULL); + PRECONDITION(ppNewFD != NULL); } CONTRACTL_END; + HRESULT hr; Module * pModule = pMT->GetModule(); IMDInternalImport *pImport = pModule->GetMDImport(); @@ -456,7 +430,7 @@ HRESULT EEClass::AddField(MethodTable * pMT, mdFieldDef fieldDef, EnCFieldDesc * { szFieldName = "Invalid FieldDef record"; } - LOG((LF_ENC, LL_INFO100, "EEClass::AddField %s\n", szFieldName)); + LOG((LF_ENC, LL_INFO100, "EEClass::AddField '%s' tok:0x%08x\n", szFieldName, fieldDef)); } #endif //LOGGING @@ -472,6 +446,101 @@ HRESULT EEClass::AddField(MethodTable * pMT, mdFieldDef fieldDef, EnCFieldDesc * DWORD dwFieldAttrs; IfFailThrow(pImport->GetFieldDefProps(fieldDef, &dwFieldAttrs)); + FieldDesc* pNewFD; + if (FAILED(hr = AddFieldDesc(pMT, fieldDef, dwFieldAttrs, &pNewFD))) + { + LOG((LF_ENC, LL_INFO100, "EEClass::AddField failed: 0x%08x\n", hr)); + return hr; + } + + // Store the FieldDesc into the module's field list + // This should not be done for instantiated types. Only fields on the + // open type are added to the module directly. This check is a + // consequence of calling AddField() for EnC static fields on generics. + if (!pMT->HasInstantiation()) + { + pModule->EnsureFieldDefCanBeStored(fieldDef); + pModule->EnsuredStoreFieldDef(fieldDef, pNewFD); + } + LOG((LF_ENC, LL_INFO100, "EEClass::AddField Added pFD:%p for token 0x%08x\n", + pNewFD, fieldDef)); + + // If the type is generic, then we need to update all existing instantiated types + if (pMT->IsGenericTypeDefinition()) + { + LOG((LF_ENC, LL_INFO100, "EEClass::AddField Looking for existing instantiations in all assemblies\n")); + + PTR_AppDomain pDomain = AppDomain::GetCurrentDomain(); + AppDomain::AssemblyIterator appIt = pDomain->IterateAssembliesEx((AssemblyIterationFlags)(kIncludeLoaded | kIncludeExecution)); + + bool isStaticField = !!pNewFD->IsStatic(); + CollectibleAssemblyHolder pDomainAssembly; + while (appIt.Next(pDomainAssembly.This()) && SUCCEEDED(hr)) + { + Module* pMod = pDomainAssembly->GetModule(); + LOG((LF_ENC, LL_INFO100, "EEClass::AddField Checking: %s mod:%p\n", pMod->GetDebugName(), pMod)); + + EETypeHashTable* paramTypes = pMod->GetAvailableParamTypes(); + EETypeHashTable::Iterator it(paramTypes); + EETypeHashEntry* pEntry; + while (paramTypes->FindNext(&it, &pEntry)) + { + TypeHandle th = pEntry->GetTypeHandle(); + if (th.IsTypeDesc()) + continue; + + // For instance fields we only update instantiations of the generic MethodTable we updated above. + // For static fields we update the the canonical version and instantiations. + MethodTable* pMTMaybe = th.AsMethodTable(); + if ((!isStaticField && !pMTMaybe->IsCanonicalMethodTable()) + || !pMT->HasSameTypeDefAs(pMTMaybe)) + { + continue; + } + + FieldDesc* pNewFDUnused; + if (FAILED(AddFieldDesc(pMTMaybe, fieldDef, dwFieldAttrs, &pNewFDUnused))) + { + LOG((LF_ENC, LL_INFO100, "EEClass::AddField failed: 0x%08x\n", hr)); + EEPOLICY_HANDLE_FATAL_ERROR_WITH_MESSAGE(COR_E_FAILFAST, + W("Failed to add field to existing instantiated type instance")); + return E_FAIL; + } + } + } + } + + // Success, return the new FieldDesc + *ppNewFD = pNewFD; + + return S_OK; +} + +//--------------------------------------------------------------------------------------- +// +// AddFieldDesc - called when a new FieldDesc needs to be created and added for EnC +// +HRESULT EEClass::AddFieldDesc( + MethodTable* pMT, + mdMethodDef fieldDef, + DWORD dwFieldAttrs, + FieldDesc** ppNewFD) +{ + CONTRACTL + { + THROWS; + GC_NOTRIGGER; + MODE_COOPERATIVE; + PRECONDITION(pMT != NULL); + PRECONDITION(ppNewFD != NULL); + } + CONTRACTL_END; + + LOG((LF_ENC, LL_INFO100, "EEClass::AddFieldDesc pMT:%p, %s <- tok:0x%08x attrs:%u\n", + pMT, pMT->debug_m_szClassName, fieldDef, dwFieldAttrs)); + + Module* pModule = pMT->GetModule(); + IMDInternalImport* pImport = pModule->GetMDImport(); LoaderAllocator* pAllocator = pMT->GetLoaderAllocator(); // Here we allocate a FieldDesc and set just enough info to be able to fix it up later @@ -489,26 +558,26 @@ HRESULT EEClass::AddField(MethodTable * pMT, mdFieldDef fieldDef, EnCFieldDesc * // Get the EnCEEClassData for this class // Don't adjust EEClass stats b/c EnC fields shouldn't touch EE data structures. // We'll just update our private EnC structures instead. - EnCEEClassData *pEnCClass = ((EditAndContinueModule*)pModule)->GetEnCEEClassData(pMT); - if (! pEnCClass) + _ASSERTE(pModule->IsEditAndContinueEnabled()); + EnCEEClassData* pEnCClass = ((EditAndContinueModule*)pModule)->GetEnCEEClassData(pMT); + if (!pEnCClass) return E_FAIL; // Add the field element to the list of added fields for this class pEnCClass->AddField(pAddedField); - - // Store the FieldDesc into the module's field list - { - CONTRACT_VIOLATION(ThrowsViolation); // B#25680 (Fix Enc violations): Must handle OOM's from Ensure - pModule->EnsureFieldDefCanBeStored(fieldDef); - } - pModule->EnsuredStoreFieldDef(fieldDef, pNewFD); pNewFD->SetMethodTable(pMT); + // Record that we are adding a new static field. Static generic fields + // are added for currently non-instantiated types during type construction. + // We want to limit the cost of making the check at that time so we use + // a bit on the EEClass to indicate we've added a static field and it should + // be checked. + if (IsFdStatic(dwFieldAttrs)) + pMT->GetClass()->SetHasEnCStaticFields(); + // Success, return the new FieldDesc - if (ppNewFD) - { - *ppNewFD = pNewFD; - } + *ppNewFD = pNewFD; + return S_OK; } @@ -517,20 +586,25 @@ HRESULT EEClass::AddField(MethodTable * pMT, mdFieldDef fieldDef, EnCFieldDesc * // AddMethod - called when a new method is added by EnC // // The method has already been added to the metadata with token methodDef. -// Create a new MethodDesc for the method. +// Create a new MethodDesc for the method, add to the associated EEClass and +// update any existing Generic instantiations if the MethodTable represents a +// generic type. // -HRESULT EEClass::AddMethod(MethodTable * pMT, mdMethodDef methodDef, RVA newRVA, MethodDesc **ppMethod) +HRESULT EEClass::AddMethod(MethodTable* pMT, mdMethodDef methodDef, MethodDesc** ppMethod) { CONTRACTL { THROWS; GC_NOTRIGGER; MODE_COOPERATIVE; + PRECONDITION(pMT != NULL); + PRECONDITION(methodDef != mdTokenNil); } CONTRACTL_END; - Module * pModule = pMT->GetModule(); - IMDInternalImport *pImport = pModule->GetMDImport(); + HRESULT hr; + Module* pModule = pMT->GetModule(); + IMDInternalImport* pImport = pModule->GetMDImport(); #ifdef LOGGING if (LoggingEnabled()) @@ -540,26 +614,23 @@ HRESULT EEClass::AddMethod(MethodTable * pMT, mdMethodDef methodDef, RVA newRVA, { szMethodName = "Invalid MethodDef record"; } - LOG((LF_ENC, LL_INFO100, "EEClass::AddMethod %s\n", szMethodName)); + LOG((LF_ENC, LL_INFO100, "EEClass::AddMethod '%s' tok:0x%08x\n", szMethodName, methodDef)); } #endif //LOGGING DWORD dwDescrOffset; DWORD dwImplFlags; - HRESULT hr = S_OK; - if (FAILED(pImport->GetMethodImplProps(methodDef, &dwDescrOffset, &dwImplFlags))) - { return COR_E_BADIMAGEFORMAT; - } DWORD dwMemberAttrs; - IfFailThrow(pImport->GetMethodDefProps(methodDef, &dwMemberAttrs)); + if (FAILED(pImport->GetMethodDefProps(methodDef, &dwMemberAttrs))) + return COR_E_BADIMAGEFORMAT; // Refuse to add other special cases - if (IsReallyMdPinvokeImpl(dwMemberAttrs) || - (pMT->IsInterface() && !IsMdStatic(dwMemberAttrs)) || - IsMiRuntime(dwImplFlags)) + if (IsReallyMdPinvokeImpl(dwMemberAttrs) + || (pMT->IsInterface() && !IsMdStatic(dwMemberAttrs)) + || IsMiRuntime(dwImplFlags)) { _ASSERTE(! "**Error** EEClass::AddMethod only IL private non-virtual methods are supported"); LOG((LF_ENC, LL_INFO100, "**Error** EEClass::AddMethod only IL private non-virtual methods are supported\n")); @@ -569,7 +640,7 @@ HRESULT EEClass::AddMethod(MethodTable * pMT, mdMethodDef methodDef, RVA newRVA, #ifdef _DEBUG // Validate that this methodDef correctly has a parent typeDef mdTypeDef parentTypeDef; - if (FAILED(hr = pImport->GetParentToken(methodDef, &parentTypeDef))) + if (FAILED(pImport->GetParentToken(methodDef, &parentTypeDef))) { _ASSERTE(! "**Error** EEClass::AddMethod parent token not found"); LOG((LF_ENC, LL_INFO100, "**Error** EEClass::AddMethod parent token not found\n")); @@ -577,75 +648,173 @@ HRESULT EEClass::AddMethod(MethodTable * pMT, mdMethodDef methodDef, RVA newRVA, } #endif // _DEBUG - EEClass * pClass = pMT->GetClass(); + MethodDesc* pNewMD; + if (FAILED(hr = AddMethodDesc(pMT, methodDef, dwImplFlags, dwMemberAttrs, &pNewMD))) + { + LOG((LF_ENC, LL_INFO100, "EEClass::AddMethod failed: 0x%08x\n", hr)); + return hr; + } - // @todo: OOM: InitMethodDesc will allocate loaderheap memory but leak it - // on failure. This AllocMemTracker should be replaced with a real one. - AllocMemTracker dummyAmTracker; + // Store the new MethodDesc into the collection for this class + pModule->EnsureMethodDefCanBeStored(methodDef); + pModule->EnsuredStoreMethodDef(methodDef, pNewMD); + + LOG((LF_ENC, LL_INFO100, "EEClass::AddMethod Added pMD:%p for token 0x%08x\n", + pNewMD, methodDef)); + + // If the type is generic, then we need to update all existing instantiated types + if (pMT->IsGenericTypeDefinition()) + { + LOG((LF_ENC, LL_INFO100, "EEClass::AddMethod Looking for existing instantiations in all assemblies\n")); + + PTR_AppDomain pDomain = AppDomain::GetCurrentDomain(); + AppDomain::AssemblyIterator appIt = pDomain->IterateAssembliesEx((AssemblyIterationFlags)(kIncludeLoaded | kIncludeExecution)); + + CollectibleAssemblyHolder pDomainAssembly; + while (appIt.Next(pDomainAssembly.This()) && SUCCEEDED(hr)) + { + Module* pMod = pDomainAssembly->GetModule(); + LOG((LF_ENC, LL_INFO100, "EEClass::AddMethod Checking: %s mod:%p\n", pMod->GetDebugName(), pMod)); + + EETypeHashTable* paramTypes = pMod->GetAvailableParamTypes(); + EETypeHashTable::Iterator it(paramTypes); + EETypeHashEntry* pEntry; + while (paramTypes->FindNext(&it, &pEntry)) + { + TypeHandle th = pEntry->GetTypeHandle(); + if (th.IsTypeDesc()) + continue; + + // Only update instantiations of the generic MethodTable we updated above. + MethodTable* pMTMaybe = th.AsMethodTable(); + if (!pMTMaybe->IsCanonicalMethodTable() || !pMT->HasSameTypeDefAs(pMTMaybe)) + { + continue; + } + + MethodDesc* pNewMDUnused; + if (FAILED(AddMethodDesc(pMTMaybe, methodDef, dwImplFlags, dwMemberAttrs, &pNewMDUnused))) + { + LOG((LF_ENC, LL_INFO100, "EEClass::AddMethod failed: 0x%08x\n", hr)); + EEPOLICY_HANDLE_FATAL_ERROR_WITH_MESSAGE(COR_E_FAILFAST, + W("Failed to add method to existing instantiated type instance")); + return E_FAIL; + } + } + } + } + + // Success - return the new MethodDesc + if (ppMethod) + *ppMethod = pNewMD; + + return S_OK; +} + +//--------------------------------------------------------------------------------------- +// +// AddMethodDesc - called when a new MethodDesc needs to be created and added for EnC +// +HRESULT EEClass::AddMethodDesc( + MethodTable* pMT, + mdMethodDef methodDef, + DWORD dwImplFlags, + DWORD dwMemberAttrs, + MethodDesc** ppNewMD) +{ + CONTRACTL + { + THROWS; + GC_NOTRIGGER; + MODE_COOPERATIVE; + PRECONDITION(pMT != NULL); + PRECONDITION(methodDef != mdTokenNil); + PRECONDITION(ppNewMD != NULL); + } + CONTRACTL_END; + + LOG((LF_ENC, LL_INFO100, "EEClass::AddMethodDesc pMT:%p, %s <- tok:0x%08x flags:%u attrs:%u\n", + pMT, pMT->debug_m_szClassName, methodDef, dwImplFlags, dwMemberAttrs)); + + HRESULT hr; + Module* pModule = pMT->GetModule(); + IMDInternalImport* pImport = pModule->GetMDImport(); + + // Check if signature is generic. + ULONG sigLen; + PCCOR_SIGNATURE sig; + if (FAILED(hr = pImport->GetSigOfMethodDef(methodDef, &sigLen, &sig))) + return hr; + uint32_t callConv = CorSigUncompressData(sig); + DWORD classification = (callConv & IMAGE_CEE_CS_CALLCONV_GENERIC) + ? mcInstantiated + : mcIL; LoaderAllocator* pAllocator = pMT->GetLoaderAllocator(); - DWORD classification = mcIL; + // [TODO] OOM: InitMethodDesc will allocate loaderheap memory but leak it + // on failure. This AllocMemTracker should be replaced with a real one. + AllocMemTracker dummyAmTracker; - // Create a new MethodDescChunk to hold the new MethodDesc - // Create the chunk somewhere we'll know is within range of the VTable + // Create a new MethodDescChunk to hold the new MethodDesc. + // Create the chunk somewhere we'll know is within range of the VTable. MethodDescChunk *pChunk = MethodDescChunk::CreateChunk(pAllocator->GetHighFrequencyHeap(), - 1, // methodDescCount - classification, - TRUE /* fNonVtableSlot */, - TRUE /* fNativeCodeSlot */, - pMT, - &dummyAmTracker); + 1, // methodDescCount + classification, + TRUE, // fNonVtableSlot + TRUE, // fNativeCodeSlot + pMT, + &dummyAmTracker); // Get the new MethodDesc (Note: The method desc memory is zero initialized) - MethodDesc *pNewMD = pChunk->GetFirstMethodDesc(); - + MethodDesc* pNewMD = pChunk->GetFirstMethodDesc(); - // Initialize the new MethodDesc + EEClass* pClass = pMT->GetClass(); - // This method runs on a debugger thread. Debugger threads do not have Thread object that caches StackingAllocator. - // Use a local StackingAllocator instead. + // This method runs on a debugger thread. Debugger threads do not have Thread object + // that caches StackingAllocator, use a local StackingAllocator instead. StackingAllocator stackingAllocator; MethodTableBuilder::bmtInternalInfo bmtInternal; - bmtInternal.pModule = pMT->GetModule(); + bmtInternal.pModule = pModule; bmtInternal.pInternalImport = NULL; bmtInternal.pParentMT = NULL; MethodTableBuilder builder(pMT, - pClass, - &stackingAllocator, - &dummyAmTracker); + pClass, + &stackingAllocator, + &dummyAmTracker); builder.SetBMTData(pMT->GetLoaderAllocator(), - NULL, - NULL, - NULL, - NULL, - NULL, - NULL, - NULL, - NULL, - NULL, - &bmtInternal); + NULL, + NULL, + NULL, + NULL, + NULL, + NULL, + NULL, + NULL, + NULL, + &bmtInternal); + // Initialize the new MethodDesc EX_TRY { - INDEBUG(LPCSTR debug_szFieldName); - INDEBUG(if (FAILED(pImport->GetNameOfMethodDef(methodDef, &debug_szFieldName))) { debug_szFieldName = "Invalid MethodDef record"; }); + INDEBUG(LPCSTR debug_szMethodName); + INDEBUG(if (FAILED(pImport->GetNameOfMethodDef(methodDef, &debug_szMethodName))) { debug_szMethodName = "Invalid MethodDef record"; }); builder.InitMethodDesc(pNewMD, - classification, - methodDef, - dwImplFlags, - dwMemberAttrs, - TRUE, // fEnC - newRVA, - pImport, - NULL - COMMA_INDEBUG(debug_szFieldName) - COMMA_INDEBUG(pMT->GetDebugClassName()) - COMMA_INDEBUG(NULL) - ); + classification, + methodDef, + dwImplFlags, + dwMemberAttrs, + TRUE, // fEnC + 0, // RVA - non-zero only for NDirect + pImport, + NULL + COMMA_INDEBUG(debug_szMethodName) + COMMA_INDEBUG(pMT->GetDebugClassName()) + COMMA_INDEBUG(NULL) + ); pNewMD->SetTemporaryEntryPoint(pAllocator, &dummyAmTracker); @@ -665,18 +834,8 @@ HRESULT EEClass::AddMethod(MethodTable * pMT, mdMethodDef methodDef, RVA newRVA, pClass->AddChunk(pChunk); - // Store the new MethodDesc into the collection for this class - pModule->EnsureMethodDefCanBeStored(methodDef); - pModule->EnsuredStoreMethodDef(methodDef, pNewMD); - - LOG((LF_ENC, LL_INFO100, "EEClass::AddMethod new methoddesc %p for token %p\n", pNewMD, methodDef)); + *ppNewMD = pNewMD; - // Success - return the new MethodDesc - _ASSERTE( SUCCEEDED(hr) ); - if (ppMethod) - { - *ppMethod = pNewMD; - } return S_OK; } @@ -936,6 +1095,81 @@ ClassLoader::LoadExactParentAndInterfacesTransitively(MethodTable *pMT) #endif //_DEBUG } // ClassLoader::LoadExactParentAndInterfacesTransitively +namespace +{ +#ifdef EnC_SUPPORTED + void CreateAllEnCStaticFields(MethodTable* pMT, MethodTable* pMTCanon, EditAndContinueModule* pModule) + { + CONTRACTL + { + STANDARD_VM_CHECK; + PRECONDITION(CheckPointer(pMT)); + PRECONDITION(pMT->HasInstantiation()); + PRECONDITION(CheckPointer(pMTCanon)); + PRECONDITION(pMTCanon->IsCanonicalMethodTable()); + PRECONDITION(CheckPointer(pModule)); + } + CONTRACTL_END; + + LOG((LF_ENC, LL_INFO100, "CreateAllEnCStaticFields: pMT:%p pMTCanon:%p\n", pMT, pMTCanon)); + +#ifdef _DEBUG + // Sanity check there is relevant EnC data. + EnCEEClassData* pEnCClass = pModule->GetEnCEEClassData(pMTCanon); + _ASSERTE(pEnCClass != NULL && pEnCClass->GetAddedStaticFields() > 0); +#endif // _DEBUG + + // Iterate over the Canonical MethodTable and see if there are any EnC static fields + // we need to add to the current MethodTable. + EncApproxFieldDescIterator canonFieldIter( + pMTCanon, + ApproxFieldDescIterator::STATIC_FIELDS, + (EncApproxFieldDescIterator::FixUpEncFields | EncApproxFieldDescIterator::OnlyEncFields)); + PTR_FieldDesc pCanonFD; + while ((pCanonFD = canonFieldIter.Next()) != NULL) + { + mdFieldDef canonTok = pCanonFD->GetMemberDef(); + + // Check if the current MethodTable already has an entry for + // this FieldDesc. + bool shouldAdd = true; + EncApproxFieldDescIterator mtFieldIter( + pMT, + ApproxFieldDescIterator::STATIC_FIELDS, + (EncApproxFieldDescIterator::FixUpEncFields | EncApproxFieldDescIterator::OnlyEncFields)); + PTR_FieldDesc pFD; + while ((pFD = mtFieldIter.Next()) != NULL) + { + mdFieldDef tok = pFD->GetMemberDef(); + if (tok == canonTok) + { + shouldAdd = false; + break; + } + } + + // The FieldDesc already exists, no need to add. + if (!shouldAdd) + continue; + + LOG((LF_ENC, LL_INFO100, "CreateAllEnCStaticFields: Must add pCanonFD:%p\n", pCanonFD)); + + { + GCX_COOP(); + PTR_FieldDesc pNewFD; + HRESULT hr = EEClass::AddField(pMT, canonTok, &pNewFD); + if (FAILED(hr)) + { + LOG((LF_ENC, LL_INFO100, "CreateAllEnCStaticFields failed: 0x%08x\n", hr)); + EEPOLICY_HANDLE_FATAL_ERROR_WITH_MESSAGE(COR_E_FAILFAST, + W("Failed to add static field to instantiated type instance")); + } + } + } + } +#endif // EnC_SUPPORTED +} + // CLASS_LOAD_EXACTPARENTS phase of loading: // * Load the base class at exact instantiation // * Recurse LoadExactParents up parent hierarchy @@ -943,7 +1177,7 @@ ClassLoader::LoadExactParentAndInterfacesTransitively(MethodTable *pMT) // * Fixup vtable // /*static*/ -void ClassLoader::LoadExactParents(MethodTable *pMT) +void ClassLoader::LoadExactParents(MethodTable* pMT) { CONTRACT_VOID { @@ -966,6 +1200,23 @@ void ClassLoader::LoadExactParents(MethodTable *pMT) PropagateCovariantReturnMethodImplSlots(pMT); +#ifdef EnC_SUPPORTED + // Generics for EnC - create static FieldDescs. + // Instance FieldDescs don't need to be created here because they + // are added during type load by reading the updated metadata tables. + if (pMT->HasInstantiation()) + { + // Check if the MethodTable has any EnC static fields + PTR_MethodTable pMTCanon = pMT->GetCanonicalMethodTable(); + if (pMTCanon->GetClass()->HasEnCStaticFields()) + { + Module* pModule = pMT->GetModule(); + if (pModule->IsEditAndContinueEnabled()) + CreateAllEnCStaticFields(pMT, pMTCanon, (EditAndContinueModule*)pModule); + } + } +#endif // EnC_SUPPORTED + // We can now mark this type as having exact parents pMT->SetHasExactParent(); diff --git a/src/coreclr/vm/class.h b/src/coreclr/vm/class.h index 8515765826a483..fe2990b64d5ade 100644 --- a/src/coreclr/vm/class.h +++ b/src/coreclr/vm/class.h @@ -780,10 +780,24 @@ class EEClass // DO NOT CREATE A NEW EEClass USING NEW! #ifdef EnC_SUPPORTED // Add a new method to an already loaded type for EnC - static HRESULT AddMethod(MethodTable * pMT, mdMethodDef methodDef, RVA newRVA, MethodDesc **ppMethod); - + static HRESULT AddMethod(MethodTable* pMT, mdMethodDef methodDef, MethodDesc** ppMethod); +private: + static HRESULT AddMethodDesc( + MethodTable* pMT, + mdMethodDef methodDef, + DWORD dwImplFlags, + DWORD dwMemberAttrs, + MethodDesc** ppNewMD); +public: // Add a new field to an already loaded type for EnC - static HRESULT AddField(MethodTable * pMT, mdFieldDef fieldDesc, EnCFieldDesc **pAddedField); + static HRESULT AddField(MethodTable* pMT, mdFieldDef fieldDesc, FieldDesc** pAddedField); +private: + static HRESULT AddFieldDesc( + MethodTable* pMT, + mdMethodDef fieldDef, + DWORD dwFieldAttrs, + FieldDesc** ppNewFD); +public: static VOID FixupFieldDescForEnC(MethodTable * pMT, EnCFieldDesc *pFD, mdFieldDef fieldDef); #endif // EnC_SUPPORTED @@ -1182,14 +1196,6 @@ class EEClass // DO NOT CREATE A NEW EEClass USING NEW! return (m_VMFlags & VMFLAG_COVARIANTOVERRIDE); } -#ifdef _DEBUG - inline DWORD IsDestroyed() - { - LIMITED_METHOD_CONTRACT; - return (m_wAuxFlags & AUXFLAG_DESTROYED); - } -#endif - inline DWORD IsUnsafeValueClass() { LIMITED_METHOD_CONTRACT; @@ -1237,13 +1243,6 @@ class EEClass // DO NOT CREATE A NEW EEClass USING NEW! LIMITED_METHOD_CONTRACT; m_VMFlags |= VMFLAG_HAS_CUSTOM_FIELD_ALIGNMENT; } -#ifdef _DEBUG - inline void SetDestroyed() - { - LIMITED_METHOD_CONTRACT; - m_wAuxFlags |= AUXFLAG_DESTROYED; - } -#endif inline void SetHasFixedAddressVTStatics() { LIMITED_METHOD_CONTRACT; @@ -1320,6 +1319,19 @@ class EEClass // DO NOT CREATE A NEW EEClass USING NEW! m_VMFlags |= VMFLAG_DELEGATE; } +#ifdef EnC_SUPPORTED + inline BOOL HasEnCStaticFields() + { + LIMITED_METHOD_CONTRACT; + return m_VMFlags & VMFLAG_ENC_STATIC_FIELDS; + } + inline void SetHasEnCStaticFields() + { + LIMITED_METHOD_CONTRACT; + m_VMFlags |= VMFLAG_ENC_STATIC_FIELDS; + } +#endif // EnC_SUPPORTED + BOOL HasFixedAddressVTStatics() { LIMITED_METHOD_CONTRACT; @@ -1431,9 +1443,9 @@ class EEClass // DO NOT CREATE A NEW EEClass USING NEW! BOOL HasExplicitSize(); BOOL IsAutoLayoutOrHasAutoLayoutField(); - + // Only accurate on non-auto layout types - BOOL IsInt128OrHasInt128Fields(); + BOOL IsInt128OrHasInt128Fields(); static void GetBestFitMapping(MethodTable * pMT, BOOL *pfBestFitMapping, BOOL *pfThrowOnUnmappableChar); @@ -1664,13 +1676,6 @@ class EEClass // DO NOT CREATE A NEW EEClass USING NEW! SigPointer sp, CorGenericParamAttr position); -#if defined(_DEBUG) -public: - enum{ - AUXFLAG_DESTROYED = 0x00000008, // The Destruct() method has already been called on this class - }; -#endif // defined(_DEBUG) - //------------------------------------------------------------- // CONCRETE DATA LAYOUT // @@ -1691,7 +1696,6 @@ class EEClass // DO NOT CREATE A NEW EEClass USING NEW! // sets of flags - a full field is used. Please avoid adding such members if possible. //------------------------------------------------------------- - // @TODO: needed for asm code in cgenx86.cpp. Can this enum be private? // // Flags for m_VMFlags // @@ -1703,7 +1707,11 @@ class EEClass // DO NOT CREATE A NEW EEClass USING NEW! #endif VMFLAG_DELEGATE = 0x00000002, - // VMFLAG_UNUSED = 0x0000001c, +#ifdef EnC_SUPPORTED + VMFLAG_ENC_STATIC_FIELDS = 0x00000004, +#endif // EnC_SUPPORTED + + // VMFLAG_UNUSED = 0x00000018, VMFLAG_FIXED_ADDRESS_VT_STATICS = 0x00000020, // Value type Statics in this class will be pinned VMFLAG_HASLAYOUT = 0x00000040, @@ -2131,7 +2139,7 @@ inline BOOL EEClass::IsAutoLayoutOrHasAutoLayoutField() inline BOOL EEClass::IsInt128OrHasInt128Fields() { - // The name of this type is a slight misnomer as it doesn't detect Int128 fields on + // The name of this type is a slight misnomer as it doesn't detect Int128 fields on // auto layout types, but since we only need this for interop scenarios, it works out. LIMITED_METHOD_CONTRACT; // If this type is not auto @@ -2180,13 +2188,6 @@ PCODE TheVarargNDirectStub(BOOL hasRetBuffArg); #define METH_NAME_CACHE_SIZE 5 #define MAX_MISSES 3 -#ifdef EnC_SUPPORTED - -struct EnCAddedFieldElement; - -#endif // EnC_SUPPORTED - - // -------------------------------------------------------------------------------------------- // For generic instantiations the FieldDescs stored for instance // fields are approximate, not exact, i.e. they are representatives owned by diff --git a/src/coreclr/vm/codeversion.cpp b/src/coreclr/vm/codeversion.cpp index 6a5db21db37f69..55fbb5cdd1e2ea 100644 --- a/src/coreclr/vm/codeversion.cpp +++ b/src/coreclr/vm/codeversion.cpp @@ -1955,10 +1955,7 @@ HRESULT CodeVersionManager::EnumerateClosedMethodDescs( return E_OUTOFMEMORY; } *ppMD = pMD; - } - if (!pMD->HasClassOrMethodInstantiation()) - { // not generic, we're done for this method return S_OK; } diff --git a/src/coreclr/vm/crst.cpp b/src/coreclr/vm/crst.cpp index ea4c3376c89cf5..3eacfc29f76a15 100644 --- a/src/coreclr/vm/crst.cpp +++ b/src/coreclr/vm/crst.cpp @@ -92,7 +92,7 @@ void CrstBase::Destroy() DeleteCriticalSection(&m_criticalsection); } - LOG((LF_SYNC, INFO3, "Deleting 0x%x\n", this)); + LOG((LF_SYNC, INFO3, "CrstBase::Destroy %p\n", this)); #ifdef _DEBUG DebugDestroy(); #endif @@ -616,7 +616,7 @@ void CrstBase::DebugInit(CrstType crstType, CrstFlags flags) // @todo - Any Crst w/ CRST_DEBUGGER_THREAD must be on a special blessed list. Check that here. - LOG((LF_SYNC, INFO3, "ConstructCrst with this:0x%x\n", this)); + LOG((LF_SYNC, INFO3, "CrstBase::DebugInit %p\n", this)); for (int i = 0; i < crstDebugInfoCount; i++) { diff --git a/src/coreclr/vm/encee.cpp b/src/coreclr/vm/encee.cpp index 9b83cd39d92407..56500271314b57 100644 --- a/src/coreclr/vm/encee.cpp +++ b/src/coreclr/vm/encee.cpp @@ -15,6 +15,7 @@ #include "eeconfig.h" #include "excep.h" #include "stackwalk.h" +#include "methoditer.h" #ifdef DACCESS_COMPILE #include "../debug/daccess/gcinterface.dac.h" @@ -211,14 +212,14 @@ HRESULT EditAndContinueModule::ApplyEditAndContinue( mdToken token; while (pIMDInternalImportENC->EnumNext(&enumENC, &token)) { - STRESS_LOG3(LF_ENC, LL_INFO100, "EACM::AEAC: updated token %08x; type %08x; rid %08x\n", token, TypeFromToken(token), RidFromToken(token)); + STRESS_LOG1(LF_ENC, LL_INFO100, "EACM::AEAC: updated token 0x%08x\n", token); switch (TypeFromToken(token)) { case mdtMethodDef: // MethodDef token - update/add a method - LOG((LF_ENC, LL_INFO10000, "EACM::AEAC: Found method %08x\n", token)); + LOG((LF_ENC, LL_INFO10000, "EACM::AEAC: Found method 0x%08x\n", token)); ULONG dwMethodRVA; DWORD dwMethodFlags; @@ -251,7 +252,7 @@ HRESULT EditAndContinueModule::ApplyEditAndContinue( case mdtFieldDef: // FieldDef token - add a new field - LOG((LF_ENC, LL_INFO10000, "EACM::AEAC: Found field %08x\n", token)); + LOG((LF_ENC, LL_INFO10000, "EACM::AEAC: Found field 0x%08x\n", token)); if (LookupFieldDef(token)) { @@ -317,7 +318,8 @@ HRESULT EditAndContinueModule::UpdateMethod(MethodDesc *pMethod) // Notify the JIT that we've got new IL for this method // This will ensure that all new calls to the method will go to the new version. // The runtime does this by never backpatching the methodtable slots in EnC-enabled modules. - LOG((LF_ENC, LL_INFO100000, "EACM::UM: Updating function %s to version %d\n", pMethod->m_pszDebugMethodName, m_applyChangesCount)); + LOG((LF_ENC, LL_INFO100000, "EACM::UM: Updating function %s::%s to version %d\n", + pMethod->m_pszDebugClassName, pMethod->m_pszDebugMethodName, m_applyChangesCount)); // Reset any flags relevant to the old code // @@ -325,7 +327,30 @@ HRESULT EditAndContinueModule::UpdateMethod(MethodDesc *pMethod) // to the Method's code must be to the call/jmp blob immediately in front of the // MethodDesc itself. See MethodDesc::InEnCEnabledModule() // - pMethod->ResetCodeEntryPointForEnC(); + if (!pMethod->HasClassOrMethodInstantiation()) + { + // Not a method impacted by generics, so this is the MethodDesc to use. + pMethod->ResetCodeEntryPointForEnC(); + } + else + { + // Generics are involved so we need to search for all related MethodDescs. + Module* module = pMethod->GetLoaderModule(); + AppDomain* appDomain = module->GetDomain()->AsAppDomain(); + mdMethodDef tkMethod = pMethod->GetMemberDef(); + + LoadedMethodDescIterator it( + appDomain, + module, + tkMethod, + AssemblyIterationFlags(kIncludeLoaded | kIncludeExecution)); + CollectibleAssemblyHolder pDomainAssembly; + while (it.Next(pDomainAssembly.This())) + { + MethodDesc* pMD = it.Current(); + pMD->ResetCodeEntryPointForEnC(); + } + } return S_OK; } @@ -383,7 +408,7 @@ HRESULT EditAndContinueModule::AddMethod(mdMethodDef token) // Add the method to the runtime's Class data structures LOG((LF_ENC, LL_INFO100000, "EACM::AM: Adding function %08x to type %08x\n", token, parentTypeDef)); MethodDesc *pMethod = NULL; - hr = EEClass::AddMethod(pParentType, token, 0, &pMethod); + hr = EEClass::AddMethod(pParentType, token, &pMethod); if (FAILED(hr)) { @@ -439,7 +464,7 @@ HRESULT EditAndContinueModule::AddField(mdFieldDef token) if (FAILED(hr)) { - LOG((LF_ENC, LL_INFO100, "**Error** EnCModule::AF can't find parent token for field token %08x\n", token)); + LOG((LF_ENC, LL_INFO100, "**Error** EnCModule::AF can't find parent token for field token 0x%08x\n", token)); return E_FAIL; } @@ -451,18 +476,18 @@ HRESULT EditAndContinueModule::AddField(mdFieldDef token) MethodTable * pParentType = LookupTypeDef(parentTypeDef).AsMethodTable(); if (pParentType == NULL) { - LOG((LF_ENC, LL_INFO100, "EnCModule::AF class %08x not loaded (field %08x), our work is done\n", parentTypeDef, token)); + LOG((LF_ENC, LL_INFO100, "EnCModule::AF class 0x%08x not loaded (field 0x%08x), our work is done\n", parentTypeDef, token)); return S_OK; } - // Create a new EnCFieldDesc for the field and add it to the class - LOG((LF_ENC, LL_INFO100000, "EACM::AM: Adding field %08x to type %08x\n", token, parentTypeDef)); - EnCFieldDesc *pField; + // Create a new FieldDesc for the field and add it to the class + LOG((LF_ENC, LL_INFO100000, "EACM::AF: Adding field 0x%08x to type 0x%08x\n", token, parentTypeDef)); + FieldDesc *pField; hr = EEClass::AddField(pParentType, token, &pField); if (FAILED(hr)) { - LOG((LF_ENC, LL_INFO100000, "**Error** EACM::AF: Failed to add field %08x to EE with hr %08x\n", token, hr)); + LOG((LF_ENC, LL_INFO100000, "**Error** EACM::AF: Failed to add field 0x%08x to EE with hr 0x%08x\n", token, hr)); return hr; } @@ -508,8 +533,8 @@ PCODE EditAndContinueModule::JitUpdatedFunction( MethodDesc *pMD, } CONTRACTL_END; - LOG((LF_ENC, LL_INFO100, "EnCModule::JitUpdatedFunction for %s\n", - pMD->m_pszDebugMethodName)); + LOG((LF_ENC, LL_INFO100, "EnCModule::JitUpdatedFunction for %s::%s\n", + pMD->m_pszDebugClassName, pMD->m_pszDebugMethodName)); PCODE jittedCode = NULL; @@ -543,15 +568,15 @@ PCODE EditAndContinueModule::JitUpdatedFunction( MethodDesc *pMD, // get the code address (may jit the fcn if not already jitted) EX_TRY { - if (!pMD->IsPointingToNativeCode()) + if (pMD->IsPointingToNativeCode()) { - GCX_PREEMP(); - pMD->DoPrestub(NULL); - LOG((LF_ENC, LL_INFO100, "EnCModule::ResumeInUpdatedFunction JIT successful\n")); + LOG((LF_ENC, LL_INFO100, "EnCModule::ResumeInUpdatedFunction %p already JITed\n", pMD)); } else { - LOG((LF_ENC, LL_INFO100, "EnCModule::ResumeInUpdatedFunction function already JITed\n")); + GCX_PREEMP(); + pMD->DoPrestub(NULL); + LOG((LF_ENC, LL_INFO100, "EnCModule::ResumeInUpdatedFunction JIT of %p successful\n", pMD)); } jittedCode = pMD->GetNativeCode(); } EX_CATCH { @@ -611,8 +636,8 @@ HRESULT EditAndContinueModule::ResumeInUpdatedFunction( #if defined(TARGET_ARM) || defined(TARGET_LOONGARCH64) || defined(TARGET_RISCV64) return E_NOTIMPL; #else - LOG((LF_ENC, LL_INFO100, "EnCModule::ResumeInUpdatedFunction for %s at IL offset 0x%x, ", - pMD->m_pszDebugMethodName, newILOffset)); + LOG((LF_ENC, LL_INFO100, "EnCModule::ResumeInUpdatedFunction for %s::%s at IL offset 0x%zx\n", + pMD->m_pszDebugClassName, pMD->m_pszDebugMethodName, newILOffset)); #ifdef _DEBUG BOOL shouldBreak = CLRConfig::GetConfigValue( @@ -622,8 +647,6 @@ HRESULT EditAndContinueModule::ResumeInUpdatedFunction( } #endif - HRESULT hr = E_FAIL; - // JIT-compile the updated version of the method PCODE jittedCode = JitUpdatedFunction(pMD, pOrigContext); if ( jittedCode == NULL ) @@ -706,12 +729,10 @@ HRESULT EditAndContinueModule::ResumeInUpdatedFunction( LOG((LF_ENC, LL_ERROR, "**Error** EnCModule::ResumeInUpdatedFunction returned from ResumeAtJit")); _ASSERTE(!"Should not return from FixContextAndResume()"); - hr = E_FAIL; - // If we fail for any reason we have already potentially trashed with new locals and we have also unwound any // Win32 handlers on the stack so cannot ever return from this function. EEPOLICY_HANDLE_FATAL_ERROR(CORDBG_E_ENC_INTERNAL_ERROR); - return hr; + return E_FAIL; #endif // #if defined(TARGET_ARM) || defined(TARGET_LOONGARCH64) } @@ -1015,8 +1036,9 @@ PTR_EnCEEClassData EditAndContinueModule::GetEnCEEClassData(MethodTable * pMT, B _ASSERTE(getOnly == TRUE); #endif // DACCESS_COMPILE - DPTR(PTR_EnCEEClassData) ppData = m_ClassList.Table(); - DPTR(PTR_EnCEEClassData) ppLast = ppData + m_ClassList.Count(); + Module* loaderModule = pMT->GetLoaderModule(); + DPTR(PTR_EnCEEClassData) ppData = loaderModule->m_ClassList.Table(); + DPTR(PTR_EnCEEClassData) ppLast = ppData + loaderModule->m_ClassList.Count(); // Look for an existing entry for the specified class while (ppData < ppLast) @@ -1037,7 +1059,7 @@ PTR_EnCEEClassData EditAndContinueModule::GetEnCEEClassData(MethodTable * pMT, B // Create a new entry and add it to the end our our table EnCEEClassData *pNewData = (EnCEEClassData*)(void*)pMT->GetLoaderAllocator()->GetLowFrequencyHeap()->AllocMem_NoThrow(S_SIZE_T(sizeof(EnCEEClassData))); pNewData->Init(pMT); - ppData = m_ClassList.Append(); + ppData = loaderModule->m_ClassList.Append(); if (!ppData) return NULL; *ppData = pNewData; @@ -1604,36 +1626,6 @@ EnCEEClassData::EnumMemoryRegions(CLRDataEnumMemoryFlags flags) elt = elt->m_next; } } - -void -EditAndContinueModule::EnumMemoryRegions(CLRDataEnumMemoryFlags flags, - bool enumThis) -{ - SUPPORTS_DAC; - - if (enumThis) - { - DAC_ENUM_VTHIS(); - } - - Module::EnumMemoryRegions(flags, false); - - m_ClassList.EnumMemoryRegions(); - - DPTR(PTR_EnCEEClassData) classData = m_ClassList.Table(); - DPTR(PTR_EnCEEClassData) classLast = classData + m_ClassList.Count(); - - while (classData.IsValid() && classData < classLast) - { - if ((*classData).IsValid()) - { - (*classData)->EnumMemoryRegions(flags); - } - - classData++; - } -} - #endif // #ifdef DACCESS_COMPILE @@ -1644,12 +1636,12 @@ EditAndContinueModule::EnumMemoryRegions(CLRDataEnumMemoryFlags flags, // pMT - MethodTable indicating the type of interest // iteratorType - one of the ApproxFieldDescIterator::IteratorType values specifying which fields // are of interest. -// fixupEnC - if true, then any partially-initialized EnC FieldDescs will be fixed up to be complete -// initialized FieldDescs as they are returned by Next(). This may load types and do -// other things to trigger a GC. +// flags - See EncApproxFieldDescIterator::Flags. // -EncApproxFieldDescIterator::EncApproxFieldDescIterator(MethodTable *pMT, int iteratorType, BOOL fixupEnC) : - m_nonEnCIter( pMT, iteratorType ) +EncApproxFieldDescIterator::EncApproxFieldDescIterator(MethodTable *pMT, int iteratorType, uint32_t flags) + : m_nonEnCIter( pMT, iteratorType ) + , m_flags( flags ) + , m_encFieldsReturned( 0 ) { CONTRACTL { @@ -1659,22 +1651,19 @@ EncApproxFieldDescIterator::EncApproxFieldDescIterator(MethodTable *pMT, int ite } CONTRACTL_END - m_fixupEnC = fixupEnC; - #ifndef DACCESS_COMPILE // can't fixup for EnC on the debugger thread - _ASSERTE((g_pDebugInterface->GetRCThreadId() != GetCurrentThreadId()) || fixupEnC == FALSE); + _ASSERTE((g_pDebugInterface->GetRCThreadId() != GetCurrentThreadId()) || !(m_flags & FixUpEncFields)); #endif m_pCurrListElem = NULL; m_encClassData = NULL; - m_encFieldsReturned = 0; // If this is an EnC module, then grab a pointer to the EnC data if( pMT->GetModule()->IsEditAndContinueEnabled() ) { PTR_EditAndContinueModule encMod = PTR_EditAndContinueModule(pMT->GetModule()); - m_encClassData = encMod->GetEnCEEClassData( pMT, TRUE); + m_encClassData = encMod->GetEnCEEClassData(pMT, TRUE /* getOnly */); } } @@ -1684,14 +1673,15 @@ PTR_FieldDesc EncApproxFieldDescIterator::Next() CONTRACTL { NOTHROW; - if (m_fixupEnC) {GC_TRIGGERS;} else {GC_NOTRIGGER;} + if (m_flags & FixUpEncFields) {GC_TRIGGERS;} else {GC_NOTRIGGER;} FORBID_FAULT; SUPPORTS_DAC; } CONTRACTL_END - // If we still have non-EnC fields to look at, return one of them - if( m_nonEnCIter.CountRemaining() > 0 ) + // If we still have non-EnC fields to look at and the caller didn't + // request only EnC fields, return one of them + if ( !(m_flags & OnlyEncFields) && m_nonEnCIter.CountRemaining() > 0 ) { _ASSERTE( m_encFieldsReturned == 0 ); return m_nonEnCIter.Next(); @@ -1699,7 +1689,7 @@ PTR_FieldDesc EncApproxFieldDescIterator::Next() // Get the next EnC field Desc if any PTR_EnCFieldDesc pFD = NextEnC(); - if( pFD == NULL ) + if ( pFD == NULL ) { // No more fields return NULL; @@ -1707,7 +1697,7 @@ PTR_FieldDesc EncApproxFieldDescIterator::Next() #ifndef DACCESS_COMPILE // Fixup the fieldDesc if requested and necessary - if ( m_fixupEnC && (pFD->NeedsFixup()) ) + if ((m_flags & FixUpEncFields) && (pFD->NeedsFixup())) { // if we get an OOM during fixup, the field will just not get fixed up EX_TRY @@ -1745,7 +1735,11 @@ int EncApproxFieldDescIterator::Count() } CONTRACTL_END - int count = m_nonEnCIter.Count(); + int count = 0; + + // Check if the caller is only interested in EnC FieldDescs. + if (!(m_flags & OnlyEncFields)) + count = m_nonEnCIter.Count(); // If this module doesn't have any EnC data then there aren't any EnC fields if (m_encClassData == NULL) diff --git a/src/coreclr/vm/encee.h b/src/coreclr/vm/encee.h index bb18c99ec465a9..ae3ed4a0158be7 100644 --- a/src/coreclr/vm/encee.h +++ b/src/coreclr/vm/encee.h @@ -63,7 +63,7 @@ class EnCFieldDesc : public FieldDesc VOID Fixup(mdFieldDef token) { WRAPPER_NO_CONTRACT; - EEClass::FixupFieldDescForEnC(GetEnclosingMethodTable(), this, token); + EEClass::FixupFieldDescForEnC(GetApproxEnclosingMethodTable(), this, token); m_bNeedsFixup = FALSE; } @@ -195,9 +195,6 @@ class EditAndContinueModule : public Module // function itself has been edited. int m_applyChangesCount; - // Holds a table of EnCEEClassData object for classes in this module that have been modified - CUnorderedArray m_ClassList; - #ifndef DACCESS_COMPILE // Return the minimum permissable address for new IL to be stored at // This can't be less than the current load address because then we'd @@ -266,7 +263,6 @@ class EditAndContinueModule : public Module PTR_CBYTE ResolveOrAllocateField(OBJECTREF thisPointer, EnCFieldDesc * pFD); - // Get class-specific EnC data for a class in this module // Note: For DAC build, getOnly must be TRUE PTR_EnCEEClassData GetEnCEEClassData(MethodTable * pMT, BOOL getOnly = FALSE); @@ -276,11 +272,6 @@ class EditAndContinueModule : public Module { return m_applyChangesCount; } - -#ifdef DACCESS_COMPILE - virtual void EnumMemoryRegions(CLRDataEnumMemoryFlags flags, - bool enumThis); -#endif }; // Information about an instance field value added by EnC @@ -397,12 +388,26 @@ class EnCSyncBlockInfo // ApproxFieldDescIterator because none of our clients need it. But it would // be easy to add this using the data from m_classData // -class EncApproxFieldDescIterator +class EncApproxFieldDescIterator final { public: + enum Flags + { + None, + + // If set, then any partially-initialized EnC FieldDescs will be fixed up + // to be a completely initialized FieldDescs as they are returned by Next(). + // This may load types and do other things to trigger a GC. + // If an EnC FieldDesc is not fixed up, an assert will fire prior to return. + FixUpEncFields, + + // Only iterate over EnC FieldDescs + OnlyEncFields, + }; + #ifdef EnC_SUPPORTED // Create and initialize the iterator - EncApproxFieldDescIterator(MethodTable *pMT, int iteratorType, BOOL fixupEnC); + EncApproxFieldDescIterator(MethodTable *pMT, int iteratorType, uint32_t flags = None); // Get the next fieldDesc (either EnC or non-EnC) PTR_FieldDesc Next(); @@ -410,7 +415,7 @@ class EncApproxFieldDescIterator int Count(); #else // Non-EnC version - simple wrapper - EncApproxFieldDescIterator(MethodTable *pMT, int iteratorType, BOOL fixupEnC) : + EncApproxFieldDescIterator(MethodTable *pMT, int iteratorType, uint32_t flags = None) : m_nonEnCIter( pMT, iteratorType ) {} PTR_FieldDesc Next() { WRAPPER_NO_CONTRACT; return m_nonEnCIter.Next(); } @@ -434,11 +439,10 @@ class EncApproxFieldDescIterator // Return the next available EnC FieldDesc or NULL when done PTR_EnCFieldDesc NextEnC(); - // True if our client wants us to fixup any EnC fieldDescs before handing them back - BOOL m_fixupEnC; + uint32_t m_flags; // A count of how many EnC fields have been returned so far - int m_encFieldsReturned; + int32_t m_encFieldsReturned; // The current pointer into one of the EnC field lists when enumerating EnC fields PTR_EnCAddedFieldElement m_pCurrListElem; diff --git a/src/coreclr/vm/field.h b/src/coreclr/vm/field.h index 8e4f8539a3d138..5297c7b24ef483 100644 --- a/src/coreclr/vm/field.h +++ b/src/coreclr/vm/field.h @@ -334,7 +334,7 @@ class FieldDesc void* GetInstanceAddress(OBJECTREF o); - // Get the address of a field within object 'o' + // Get the address of a field within object 'o' PTR_VOID GetAddress(PTR_VOID o); PTR_VOID GetAddressNoThrowNoGC(PTR_VOID o); diff --git a/src/coreclr/vm/genericdict.cpp b/src/coreclr/vm/genericdict.cpp index 52645f0d92cf50..2a79009f4cbd78 100644 --- a/src/coreclr/vm/genericdict.cpp +++ b/src/coreclr/vm/genericdict.cpp @@ -1161,6 +1161,7 @@ Dictionary::PopulateEntry( } IfFailThrow(ptr.SkipExactlyOne()); + // Computed by MethodTable::GetIndexForFieldDesc(). uint32_t fieldIndex; IfFailThrow(ptr.GetData(&fieldIndex)); diff --git a/src/coreclr/vm/generics.cpp b/src/coreclr/vm/generics.cpp index 1ccf8128445a1a..ed095d20fdcd00 100644 --- a/src/coreclr/vm/generics.cpp +++ b/src/coreclr/vm/generics.cpp @@ -487,7 +487,6 @@ ClassLoader::CreateTypeHandleForNonCanonicalGenericInstantiation( pMT->Debug_SetHasInjectedInterfaceDuplicates(); #endif // _DEBUG - // This logic is identical to logic in class.cpp. Factor these out. // No need to generate IDs for open types. However // we still leave the optional member in the MethodTable holding the value -1 for the ID. if (fHasGenericsStaticsInfo) diff --git a/src/coreclr/vm/genmeth.cpp b/src/coreclr/vm/genmeth.cpp index e7687194c6507c..a1d7af7c038bc5 100644 --- a/src/coreclr/vm/genmeth.cpp +++ b/src/coreclr/vm/genmeth.cpp @@ -1356,7 +1356,7 @@ MethodDesc * MethodDesc::FindOrCreateTypicalSharedInstantiation(BOOL allowCreate allowCreate)); } -//@GENERICSVER: Set the typical (ie. formal) instantiation +//@GENERICSVER: Set up the typical instance (i.e., non-instantiated) void InstantiatedMethodDesc::SetupGenericMethodDefinition(IMDInternalImport* pIMDII, LoaderAllocator* pAllocator, AllocMemTracker* pamTracker, @@ -1380,7 +1380,7 @@ void InstantiatedMethodDesc::SetupGenericMethodDefinition(IMDInternalImport* pIM //@GENERICSVER: allocate space for and initialize the typical instantiation //we share the typical instantiation among all instantiations by placing it in the generic method desc - LOG((LF_JIT, LL_INFO10000, "GENERICSVER: Initializing typical method instantiation with type handles\n")); + LOG((LF_JIT, LL_INFO10000, "IMD::SGMD: Initializing typical MethodDesc this:%p\n", this)); mdGenericParam tkTyPar; HENUMInternalHolder hEnumTyPars(pIMDII); hEnumTyPars.EnumInit(mdtGenericParam, tok); @@ -1431,7 +1431,8 @@ void InstantiatedMethodDesc::SetupGenericMethodDefinition(IMDInternalImport* pIM pInstDest[i] = TypeHandle(pTypeVarTypeDesc); } } - LOG((LF_JIT, LL_INFO10000, "GENERICSVER: Initialized typical method instantiation with %d type handles\n",numTyPars)); + LOG((LF_JIT, LL_INFO10000, "IMD::SGMD: Initialized typical MethodDesc. type handles: %u\n", + numTyPars)); } void InstantiatedMethodDesc::SetupWrapperStubWithInstantiations(MethodDesc* wrappedMD,DWORD numGenericArgs, TypeHandle *pInst) diff --git a/src/coreclr/vm/jithelpers.cpp b/src/coreclr/vm/jithelpers.cpp index c36e50d77d2fd7..a6654618a95417 100644 --- a/src/coreclr/vm/jithelpers.cpp +++ b/src/coreclr/vm/jithelpers.cpp @@ -746,9 +746,9 @@ HCIMPLEND //======================================================================== /*********************************************************************/ -// Returns the address of the field in the object (This is an interior -// pointer and the caller has to use it appropriately). obj can be -// either a reference or a byref +// Returns the address of the instance field in the object (This is an interior +// pointer and the caller has to use it appropriately) or a static field. +// obj can be either a reference or a byref HCIMPL2(void*, JIT_GetFieldAddr_Framed, Object *obj, FieldDesc* pFD) { CONTRACTL { @@ -761,10 +761,9 @@ HCIMPL2(void*, JIT_GetFieldAddr_Framed, Object *obj, FieldDesc* pFD) HELPER_METHOD_FRAME_BEGIN_RET_1(objRef); - if (objRef == NULL) + if (!pFD->IsStatic() && objRef == NULL) COMPlusThrow(kNullReferenceException); - fldAddr = pFD->GetAddress(OBJECTREFToObject(objRef)); HELPER_METHOD_FRAME_END(); @@ -792,6 +791,25 @@ HCIMPL2(void*, JIT_GetFieldAddr, Object *obj, FieldDesc* pFD) HCIMPLEND #include +#include +HCIMPL1(void*, JIT_GetStaticFieldAddr, FieldDesc* pFD) +{ + CONTRACTL { + FCALL_CHECK; + PRECONDITION(CheckPointer(pFD)); + } CONTRACTL_END; + + // [TODO] Only handling EnC for now + _ASSERTE(pFD->IsEnCNew()); + + { + ENDFORBIDGC(); + return HCCALL2(JIT_GetFieldAddr_Framed, NULL, pFD); + } +} +HCIMPLEND +#include + /*********************************************************************/ #define HCallAssert(cache, target) // suppressed to avoid ambiguous cast errors caused by use of template template diff --git a/src/coreclr/vm/jitinterface.cpp b/src/coreclr/vm/jitinterface.cpp index 32d338a0da6b44..c7130d80419bac 100644 --- a/src/coreclr/vm/jitinterface.cpp +++ b/src/coreclr/vm/jitinterface.cpp @@ -980,8 +980,7 @@ void CEEInfo::resolveToken(/* IN, OUT */ CORINFO_RESOLVED_TOKEN * pResolvedToken EnsureActive(th, pMD); } } - else - if (pFD != NULL) + else if (pFD != NULL) { if ((tkType != mdtFieldDef) && (tkType != mdtMemberRef)) ThrowBadTokenException(pResolvedToken); @@ -1021,7 +1020,7 @@ void CEEInfo::resolveToken(/* IN, OUT */ CORINFO_RESOLVED_TOKEN * pResolvedToken } else { - unsigned metaTOK = pResolvedToken->token; + mdToken metaTOK = pResolvedToken->token; Module * pModule = (Module *)pResolvedToken->tokenScope; switch (TypeFromToken(metaTOK)) @@ -1533,9 +1532,18 @@ void CEEInfo::getFieldInfo (CORINFO_RESOLVED_TOKEN * pResolvedToken, if (pFieldMT->IsSharedByGenericInstantiations()) { - fieldAccessor = CORINFO_FIELD_STATIC_GENERICS_STATIC_HELPER; + if (pField->IsEnCNew()) + { + fieldAccessor = CORINFO_FIELD_STATIC_ADDR_HELPER; - pResult->helper = getGenericStaticsHelper(pField); + pResult->helper = CORINFO_HELP_GETSTATICFIELDADDR; + } + else + { + fieldAccessor = CORINFO_FIELD_STATIC_GENERICS_STATIC_HELPER; + + pResult->helper = getGenericStaticsHelper(pField); + } } else if (pFieldMT->GetModule()->IsSystem() && (flags & CORINFO_ACCESS_GET) && diff --git a/src/coreclr/vm/memberload.cpp b/src/coreclr/vm/memberload.cpp index 446370949771fe..57c9072db3a4b4 100644 --- a/src/coreclr/vm/memberload.cpp +++ b/src/coreclr/vm/memberload.cpp @@ -239,8 +239,7 @@ void MemberLoader::GetDescFromMemberRef(ModuleBase * pModule, { fMissingMethod = TRUE; } - else - if (pMethodDef->HasClassOrMethodInstantiation()) + else if (pMethodDef->HasClassOrMethodInstantiation()) { // A memberref to a varargs method must not find a MethodDesc that is generic (as varargs methods may not be implemented on generics) fMissingMethod = TRUE; @@ -653,24 +652,24 @@ MethodDesc* MemberLoader::GetMethodDescFromMethodDef(Module *pModule, //--------------------------------------------------------------------------------------- // FieldDesc* MemberLoader::GetFieldDescFromFieldDef(Module *pModule, - mdToken FieldDef, + mdToken fieldDef, BOOL strictMetadataChecks) { CONTRACTL { THROWS; GC_TRIGGERS; - PRECONDITION(TypeFromToken(FieldDef) == mdtFieldDef); + PRECONDITION(TypeFromToken(fieldDef) == mdtFieldDef); } CONTRACTL_END; // In lookup table? - FieldDesc * pFD = pModule->LookupFieldDef(FieldDef); + FieldDesc* pFD = pModule->LookupFieldDef(fieldDef); if (!pFD) { // No, so do it the long way mdTypeDef typeDef; - IfFailThrow(pModule->GetMDImport()->GetParentToken(FieldDef, &typeDef)); + IfFailThrow(pModule->GetMDImport()->GetParentToken(fieldDef, &typeDef)); // Load the class - that should set the desc in the rid map // Field defs to generic things resolve to the formal instantiation @@ -684,11 +683,11 @@ FieldDesc* MemberLoader::GetFieldDescFromFieldDef(Module *pModule, strictMetadataChecks ? ClassLoader::FailIfUninstDefOrRef : ClassLoader::PermitUninstDefOrRef); - pFD = pModule->LookupFieldDef(FieldDef); + pFD = pModule->LookupFieldDef(fieldDef); if (pFD == NULL) { LPCUTF8 szMember; - if (FAILED(pModule->GetMDImport()->GetNameOfFieldDef(FieldDef, &szMember))) + if (FAILED(pModule->GetMDImport()->GetNameOfFieldDef(fieldDef, &szMember))) { szMember = "Invalid FieldDef record"; } @@ -707,7 +706,7 @@ FieldDesc* MemberLoader::GetFieldDescFromFieldDef(Module *pModule, if (pEnCFD->NeedsFixup()) { GCX_COOP(); - pEnCFD->Fixup(FieldDef); + pEnCFD->Fixup(fieldDef); } } #endif // EnC_SUPPORTED @@ -817,14 +816,14 @@ MethodDesc * MemberLoader::GetMethodDescFromMethodSpec(Module * pModule, CQuickBytes qbGenericMethodArgs; - mdMemberRef GenericMemberRef; + mdToken genericMember; PCCOR_SIGNATURE pSig; ULONG cSig; IMDInternalImport * pInternalImport = pModule->GetMDImport(); // Get the member def/ref and instantiation signature - IfFailThrow(pInternalImport->GetMethodSpecProps(MethodSpec, &GenericMemberRef, &pSig, &cSig)); + IfFailThrow(pInternalImport->GetMethodSpecProps(MethodSpec, &genericMember, &pSig, &cSig)); if (ppMethodSig != NULL) { @@ -861,15 +860,15 @@ MethodDesc * MemberLoader::GetMethodDescFromMethodSpec(Module * pModule, MethodDesc * pMD = NULL; FieldDesc * pFD = NULL; - switch (TypeFromToken(GenericMemberRef)) + switch (TypeFromToken(genericMember)) { case mdtMethodDef: - pMD = GetMethodDescFromMethodDef(pModule, GenericMemberRef, strictMetadataChecks); + pMD = GetMethodDescFromMethodDef(pModule, genericMember, strictMetadataChecks); *ppTH = pMD->GetMethodTable(); break; case mdtMemberRef: - GetDescFromMemberRef(pModule, GenericMemberRef, &pMD, &pFD, pTypeContext, strictMetadataChecks, ppTH, + GetDescFromMemberRef(pModule, genericMember, &pMD, &pFD, pTypeContext, strictMetadataChecks, ppTH, actualTypeRequired, ppTypeSig, pcbTypeSig); if (pMD == NULL) @@ -1031,7 +1030,7 @@ BOOL MemberLoader::FM_ShouldSkipMethod(DWORD dwAttrs, FM_Flags flags) // but with generics, we need to have a properly set up Substitution, so that // we have a correct set of types to compare with. The idea is that either the current // EEClass matches up with the methoddesc, or a parent EEClass will match up. -BOOL CompareMethodSigWithCorrectSubstitution( +static BOOL CompareMethodSigWithCorrectSubstitution( PCCOR_SIGNATURE pSignature, DWORD cSignature, ModuleBase* pModule, @@ -1078,14 +1077,13 @@ BOOL CompareMethodSigWithCorrectSubstitution( // signature is defined. MethodDesc * MemberLoader::FindMethod( - MethodTable * pMT, + MethodTable* pMT, LPCUTF8 pszName, PCCOR_SIGNATURE pSignature, DWORD cSignature, ModuleBase* pModule, FM_Flags flags, // = FM_Default const Substitution *pDefSubst) // = NULL { - CONTRACT (MethodDesc *) { THROWS; GC_TRIGGERS; @@ -1093,9 +1091,15 @@ MemberLoader::FindMethod( MODE_ANY; } CONTRACT_END; + LOG((LF_LOADER, LL_INFO10000, "ML::FM pMT:%p for %s sig:%p sigLen:%u\n", + pMT, pszName, pSignature, cSignature)); + // Retrieve the right comparison function to use. UTF8StringCompareFuncPtr StrCompFunc = FM_GetStrCompFunc(flags); + const bool canSkipMethod = FM_PossibleToSkipMethod(flags); + const bool ignoreName = (flags & FM_IgnoreName) != 0; + // Statistically it's most likely for a method to be found in non-vtable portion of this class's members, then in the // vtable of this class's declared members, then in the inherited portion of the vtable, so we search backwards. @@ -1115,18 +1119,21 @@ MemberLoader::FindMethod( for (; it.IsValid(); it.Prev()) { MethodDesc *pCurDeclMD = it.GetDeclMethodDesc(); + + LOG((LF_LOADER, LL_INFO100000, "ML::FM Considering %s::%s, pMD:%p\n", + pCurDeclMD->m_pszDebugClassName, pCurDeclMD->m_pszDebugMethodName, pCurDeclMD)); + #ifdef _DEBUG MethodTable *pCurDeclMT = pCurDeclMD->GetMethodTable(); CONSISTENCY_CHECK(!pMT->IsInterface() || pCurDeclMT == pMT->GetCanonicalMethodTable()); #endif - if (FM_PossibleToSkipMethod(flags) && FM_ShouldSkipMethod(pCurDeclMD->GetAttrs(), flags)) + if (canSkipMethod && FM_ShouldSkipMethod(pCurDeclMD->GetAttrs(), flags)) { continue; } - if ((flags & FM_IgnoreName) != 0 - || StrCompFunc(pszName, pCurDeclMD->GetNameThrowing()) == 0) + if (ignoreName || StrCompFunc(pszName, pCurDeclMD->GetNameThrowing()) == 0) { if (CompareMethodSigWithCorrectSubstitution(pSignature, cSignature, pModule, pCurDeclMD, pDefSubst, pMT)) { @@ -1135,7 +1142,6 @@ MemberLoader::FindMethod( } } - // No inheritance on value types or interfaces if (pMT->IsValueType() || pMT->IsInterface()) { @@ -1146,30 +1152,63 @@ MemberLoader::FindMethod( //@todo: This routine might be factored slightly to improve perf. CONSISTENCY_CHECK(pMT->CheckLoadLevel(CLASS_LOAD_APPROXPARENTS)); - MethodTable *pParentMT = pMT->GetParentMethodTable(); + MethodDesc* md = NULL; + MethodTable* pParentMT = pMT->GetParentMethodTable(); if (pParentMT != NULL) { Substitution subst2 = pMT->GetSubstitutionForParent(pDefSubst); - MethodDesc *md = MemberLoader::FindMethod(pParentMT, - pszName, pSignature, cSignature, pModule, flags, &subst2); + md = MemberLoader::FindMethod(pParentMT, pszName, pSignature, cSignature, pModule, flags, &subst2); // Don't inherit constructors from parent classes. It is important to forbid this, // because the JIT needs to get the class handle from the memberRef, and when the // constructor is inherited, the JIT will get the class handle for the parent class - // (and not allocate enough space, etc.). See bug #50035 for details. - if (md) + // (and not allocate enough space, etc.). + if (md != NULL + && IsMdInstanceInitializer(md->GetAttrs(), pszName)) { - if (IsMdInstanceInitializer(md->GetAttrs(), pszName)) + md = NULL; + } + } + +#ifdef EnC_SUPPORTED + // In the event the method wasn't found and the current module has + // EnC enabled, try the slow path and go through all available methods. + if (md == NULL + && pMT->GetModule()->IsEditAndContinueEnabled()) + { + LOG((LF_LOADER, LL_INFO100000, "ML::FM Falling back to EnC slow path\n")); + + MethodTable::IntroducedMethodIterator itMethods(pMT, FALSE); + for (; itMethods.IsValid(); itMethods.Next()) + { + MethodDesc* pCurDeclMD = itMethods.GetMethodDesc(); + +#ifdef _DEBUG + MethodTable *pCurDeclMT = pCurDeclMD->GetMethodTable(); + CONSISTENCY_CHECK(!pMT->IsInterface() || pCurDeclMT == pMT->GetCanonicalMethodTable()); +#endif + + if (canSkipMethod && FM_ShouldSkipMethod(pCurDeclMD->GetAttrs(), flags)) { - md = NULL; + continue; } - } - RETURN md; + LOG((LF_LOADER, LL_INFO100000, "ML::FM EnC - Considering %s::%s, pMD:%p\n", + pCurDeclMD->m_pszDebugClassName, pCurDeclMD->m_pszDebugMethodName, pCurDeclMD)); + + if (ignoreName || StrCompFunc(pszName, pCurDeclMD->GetNameThrowing()) == 0) + { + if (CompareMethodSigWithCorrectSubstitution(pSignature, cSignature, pModule, pCurDeclMD, pDefSubst, pMT)) + { + RETURN pCurDeclMD; + } + } + } } +#endif // EnC_SUPPORTED - RETURN NULL; + RETURN md; } //******************************************************************************* @@ -1460,7 +1499,7 @@ MemberLoader::FindConstructor(MethodTable * pMT, PCCOR_SIGNATURE pSignature,DWOR #endif // DACCESS_COMPILE FieldDesc * -MemberLoader::FindField(MethodTable * pMT, LPCUTF8 pszName, PCCOR_SIGNATURE pSignature, DWORD cSignature, ModuleBase* pModule, BOOL bCaseSensitive) +MemberLoader::FindField(MethodTable* pMT, LPCUTF8 pszName, PCCOR_SIGNATURE pSignature, DWORD cSignature, ModuleBase* pModule, BOOL bCaseSensitive) { CONTRACTL { @@ -1471,52 +1510,49 @@ MemberLoader::FindField(MethodTable * pMT, LPCUTF8 pszName, PCCOR_SIGNATURE pSig } CONTRACTL_END - DWORD i; - DWORD dwFieldDescsToScan; - IMDInternalImport *pInternalImport = pMT->GetMDImport(); // All explicitly declared fields in this class will have the same scope + LOG((LF_LOADER, LL_INFO100000, "ML::FF '%s' in pModule:%p pMT:%p, %s\n", + pszName, pModule, pMT, pMT->GetDebugClassName())); + // Array classes don't have fields, and don't have metadata + if (pMT->IsArray()) + return NULL; + + IMDInternalImport *pInternalImport = pMT->GetMDImport(); // All explicitly declared fields in this class will have the same scope CONSISTENCY_CHECK(pMT->CheckLoadLevel(CLASS_LOAD_APPROXPARENTS)); // Retrieve the right comparison function to use. UTF8StringCompareFuncPtr StrCompFunc = bCaseSensitive ? strcmp : stricmpUTF8; - // Array classes don't have fields, and don't have metadata - if (pMT->IsArray()) - return NULL; - EEClass * pClass = pMT->GetClass(); MethodTable *pParentMT = pMT->GetParentMethodTable(); // Scan the FieldDescs of this class - if (pParentMT != NULL) - dwFieldDescsToScan = pClass->GetNumInstanceFields() - pParentMT->GetNumInstanceFields() + pClass->GetNumStaticFields(); - else - dwFieldDescsToScan = pClass->GetNumInstanceFields() + pClass->GetNumStaticFields(); + DWORD fieldDescCount = (pParentMT != NULL) + ? pClass->GetNumInstanceFields() - pParentMT->GetNumInstanceFields() + pClass->GetNumStaticFields() + : pClass->GetNumInstanceFields() + pClass->GetNumStaticFields(); PTR_FieldDesc pFieldDescList = pClass->GetFieldDescList(); - for (i = 0; i < dwFieldDescsToScan; i++) + LPCUTF8 szMemberName; + mdFieldDef mdField; + for (DWORD i = 0; i < fieldDescCount; i++) { - LPCUTF8 szMemberName; FieldDesc * pFD = &pFieldDescList[i]; PREFIX_ASSUME(pFD!=NULL); - mdFieldDef mdField = pFD->GetMemberDef(); // Check is valid FieldDesc, and not some random memory INDEBUGIMPL(pFD->GetApproxEnclosingMethodTable()->SanityCheck()); + mdField = pFD->GetMemberDef(); IfFailThrow(pInternalImport->GetNameOfFieldDef(mdField, &szMemberName)); if (StrCompFunc(szMemberName, pszName) != 0) - { continue; - } if (pSignature != NULL) { PCCOR_SIGNATURE pMemberSig; DWORD cMemberSig; - IfFailThrow(pInternalImport->GetSigOfFieldDef(mdField, &cMemberSig, &pMemberSig)); if (!MetaSig::CompareFieldSigs( @@ -1526,7 +1562,7 @@ MemberLoader::FindField(MethodTable * pMT, LPCUTF8 pszName, PCCOR_SIGNATURE pSig pSignature, cSignature, pModule)) - { + { continue; } } @@ -1534,5 +1570,52 @@ MemberLoader::FindField(MethodTable * pMT, LPCUTF8 pszName, PCCOR_SIGNATURE pSig return pFD; } +#if defined(EnC_SUPPORTED) && !defined(DACCESS_COMPILE) + if (pModule != NULL + && pModule->IsFullModule() + && ((Module*)pModule)->IsEditAndContinueEnabled()) + { + LOG((LF_LOADER, LL_INFO100000, "ML::FF Falling back to EnC slow path\n")); + + // We may not have the full FieldDesc info at ApplyEnC time because we don't + // have a thread so can't do things like load classes (due to possible exceptions) + EncApproxFieldDescIterator fdIterator( + pMT, + ApproxFieldDescIterator::ALL_FIELDS, + (EncApproxFieldDescIterator::FixUpEncFields | EncApproxFieldDescIterator::OnlyEncFields)); + PTR_FieldDesc pCurrentFD; + while ((pCurrentFD = fdIterator.Next()) != NULL) + { + // Check is valid FieldDesc, and not some random memory + INDEBUGIMPL(pCurrentFD->GetApproxEnclosingMethodTable()->SanityCheck()); + + mdField = pCurrentFD->GetMemberDef(); + IfFailThrow(pInternalImport->GetNameOfFieldDef(mdField, &szMemberName)); + + if (StrCompFunc(szMemberName, pszName) != 0) + continue; + + if (pSignature != NULL) + { + PCCOR_SIGNATURE pMemberSig; + DWORD cMemberSig; + IfFailThrow(pInternalImport->GetSigOfFieldDef(mdField, &cMemberSig, &pMemberSig)); + + if (!MetaSig::CompareFieldSigs( + pMemberSig, + cMemberSig, + pMT->GetModule(), + pSignature, + cSignature, + pModule)) + { + continue; + } + } + return pCurrentFD; + } + } +#endif // defined(EnC_SUPPORTED) && !defined(DACCESS_COMPILE) + return NULL; } diff --git a/src/coreclr/vm/method.cpp b/src/coreclr/vm/method.cpp index c09056a0121fcc..3e59a08cb47ace 100644 --- a/src/coreclr/vm/method.cpp +++ b/src/coreclr/vm/method.cpp @@ -918,9 +918,10 @@ PCODE MethodDesc::GetNativeCode() { // When profiler is enabled, profiler may ask to rejit a code even though we // we have ngen code for this MethodDesc. (See MethodDesc::DoPrestub). - // This means that *GetAddrOfNativeCodeSlot() - // is not stable. It can turn from non-zero to zero. - PCODE pCode = *GetAddrOfNativeCodeSlot(); + // This means that *ppCode is not stable. It can turn from non-zero to zero. + PTR_PCODE ppCode = GetAddrOfNativeCodeSlot(); + PCODE pCode = *ppCode; + #ifdef TARGET_ARM if (pCode != NULL) pCode |= THUMB_CODE; @@ -1548,12 +1549,17 @@ MethodDesc* MethodDesc::LoadTypicalMethodDefinition() #ifndef DACCESS_COMPILE if (HasClassOrMethodInstantiation()) { - MethodTable *pMT = GetMethodTable(); + MethodTable* pMT = GetMethodTable(); if (!pMT->IsTypicalTypeDefinition()) - pMT = ClassLoader::LoadTypeDefThrowing(pMT->GetModule(), - pMT->GetCl(), - ClassLoader::ThrowIfNotFound, - ClassLoader::PermitUninstDefOrRef).GetMethodTable(); + { + MethodTable* pMTTypical = ClassLoader::LoadTypeDefThrowing(pMT->GetModule(), + pMT->GetCl(), + ClassLoader::ThrowIfNotFound, + ClassLoader::PermitUninstDefOrRef).GetMethodTable(); + LOG((LF_CLASSLOADER, LL_INFO100000, "MD:LTMD: pMT:%p => pMTTypical:%p\n", + pMT, pMTTypical)); + pMT = pMTTypical; + } CONSISTENCY_CHECK(TypeHandle(pMT).CheckFullyLoaded()); MethodDesc *resultMD = pMT->GetParallelMethodDesc(this); PREFIX_ASSUME(resultMD != NULL); @@ -1562,7 +1568,9 @@ MethodDesc* MethodDesc::LoadTypicalMethodDefinition() } else #endif // !DACCESS_COMPILE + { RETURN(this); + } } //******************************************************************************* @@ -3244,6 +3252,8 @@ void MethodDesc::ResetCodeEntryPointForEnC() _ASSERTE(!IsVersionableWithPrecode()); _ASSERTE(!MayHaveEntryPointSlotsToBackpatch()); + LOG((LF_ENC, LL_INFO100000, "MD::RCEPFENC: this:%p - %s::%s - HasPrecode():%s, HasNativeCodeSlot():%s\n", + this, m_pszDebugClassName, m_pszDebugMethodName, (HasPrecode() ? "true" : "false"), (HasNativeCodeSlot() ? "true" : "false"))); if (HasPrecode()) { GetPrecode()->ResetTargetInterlocked(); @@ -3251,7 +3261,11 @@ void MethodDesc::ResetCodeEntryPointForEnC() if (HasNativeCodeSlot()) { - *GetAddrOfNativeCodeSlot() = NULL; + PTR_PCODE ppCode = GetAddrOfNativeCodeSlot(); + PCODE pCode = *ppCode; + LOG((LF_CORDB, LL_INFO1000000, "MD::RCEPFENC: %p -> %p\n", + ppCode, pCode)); + *ppCode = NULL; } } diff --git a/src/coreclr/vm/method.hpp b/src/coreclr/vm/method.hpp index d27a7eb0e9d533..e59c2dd71dc357 100644 --- a/src/coreclr/vm/method.hpp +++ b/src/coreclr/vm/method.hpp @@ -3205,10 +3205,10 @@ class ComPlusCallMethodDesc : public MethodDesc // come in four flavours, discriminated by the // low order bits of the first field: // -// 00 --> GenericMethodDefinition -// 01 --> UnsharedMethodInstantiation -// 10 --> SharedMethodInstantiation -// 11 --> WrapperStubWithInstantiations - and unboxing or instantiating stub +// 001 --> GenericMethodDefinition +// 010 --> UnsharedMethodInstantiation +// 011 --> SharedMethodInstantiation +// 100 --> WrapperStubWithInstantiations - and unboxing or instantiating stub // // A SharedMethodInstantiation descriptor extends MethodDesc // with a pointer to dictionary layout and a representative instantiation. @@ -3358,15 +3358,6 @@ class InstantiatedMethodDesc final : public MethodDesc void SetupWrapperStubWithInstantiations(MethodDesc* wrappedMD,DWORD numGenericArgs, TypeHandle *pGenericMethodInst); private: - enum - { - KindMask = 0x07, - GenericMethodDefinition = 0x00, - UnsharedMethodInstantiation = 0x01, - SharedMethodInstantiation = 0x02, - WrapperStubWithInstantiations = 0x03, - }; - friend class MethodDesc; // this fields are currently accessed by MethodDesc::Save/Restore etc. union { PTR_DictionaryLayout m_pDictLayout; //SharedMethodInstantiation @@ -3387,6 +3378,14 @@ class InstantiatedMethodDesc final : public MethodDesc PTR_Dictionary m_pPerInstInfo; //SHARED private: + enum + { + KindMask = 0x07, + GenericMethodDefinition = 0x01, + UnsharedMethodInstantiation = 0x02, + SharedMethodInstantiation = 0x03, + WrapperStubWithInstantiations = 0x04, + }; WORD m_wFlags2; WORD m_wNumGenericArgs; diff --git a/src/coreclr/vm/methodtable.cpp b/src/coreclr/vm/methodtable.cpp index 55413bb818e921..a5391fb8b4f763 100644 --- a/src/coreclr/vm/methodtable.cpp +++ b/src/coreclr/vm/methodtable.cpp @@ -2081,11 +2081,40 @@ MethodDesc *MethodTable::GetMethodDescForInterfaceMethod(TypeHandle ownerType, M } #endif // DACCESS_COMPILE +const DWORD EnCFieldIndex = 0x10000000; + //========================================================================================== PTR_FieldDesc MethodTable::GetFieldDescByIndex(DWORD fieldIndex) { LIMITED_METHOD_CONTRACT; + // Check if the field index is for an EnC field lookup. + // See GetIndexForFieldDesc() for when this is applied and why. + if ((fieldIndex & EnCFieldIndex) == EnCFieldIndex) + { + DWORD rid = fieldIndex & ~EnCFieldIndex; + LOG((LF_ENC, LL_INFO100, "MT:GFDBI: rid:0x%08x\n", rid)); + + mdFieldDef tokenToFind = TokenFromRid(rid, mdtFieldDef); + EncApproxFieldDescIterator fdIterator( + this, + ApproxFieldDescIterator::ALL_FIELDS, + (EncApproxFieldDescIterator::FixUpEncFields | EncApproxFieldDescIterator::OnlyEncFields)); + PTR_FieldDesc pField; + while ((pField = fdIterator.Next()) != NULL) + { + mdFieldDef token = pField->GetMemberDef(); + if (tokenToFind == token) + { + LOG((LF_ENC, LL_INFO100, "MT:GFDBI: Found pField:%p\n", pField)); + return pField; + } + } + + LOG((LF_ENC, LL_INFO100, "MT:GFDBI: Failed to find rid:0x%08x\n", rid)); + return NULL; + } + if (HasGenericsStaticsInfo() && fieldIndex >= GetNumIntroducedInstanceFields()) { @@ -2102,6 +2131,19 @@ DWORD MethodTable::GetIndexForFieldDesc(FieldDesc *pField) { LIMITED_METHOD_CONTRACT; + // EnC methods are not in a location where computing an index through + // pointer arithmetic is possible. Instead we use the RID and a high + // bit that is ECMA encodable (that is, < 0x1fffffff) and also doesn't + // conflict with any other RID (that is, > 0x00ffffff). + // See FieldDescSlot usage in the JIT interface. + if (pField->IsEnCNew()) + { + mdFieldDef tok = pField->GetMemberDef(); + DWORD rid = RidFromToken(tok); + LOG((LF_ENC, LL_INFO100, "MT:GIFFD: pField:%p rid:0x%08x\n", pField, rid)); + return rid | EnCFieldIndex; + } + if (pField->IsStatic() && HasGenericsStaticsInfo()) { FieldDesc *pStaticFields = GetGenericsStaticFieldDescs(); @@ -8301,7 +8343,7 @@ void MethodTable::MethodIterator::Init(MethodTable *pMTDecl, MethodTable *pMTImp PRECONDITION(CheckPointer(pMTImpl)); } CONTRACTL_END; - LOG((LF_LOADER, LL_INFO10000, "SD: MT::MethodIterator created for %s.\n", pMTDecl->GetDebugClassName())); + LOG((LF_LOADER, LL_INFO10000, "MT::MethodIterator created for %s.\n", pMTDecl->GetDebugClassName())); m_pMethodData = MethodTable::GetMethodData(pMTDecl, pMTImpl); CONSISTENCY_CHECK(CheckPointer(m_pMethodData)); @@ -8715,7 +8757,45 @@ MethodTable * MethodTable::GetRestoredSlotMT(DWORD slotNumber) } //========================================================================================== -MethodDesc * MethodTable::GetParallelMethodDesc(MethodDesc * pDefMD) +namespace +{ + // Methods added by EnC cannot be looked up by slot since + // they have none, see EEClass::AddMethodDesc(). We must perform + // a slow lookup instead of using the fast slot lookup path. + MethodDesc* GetParallelMethodDescForEnC(MethodTable* pMT, MethodDesc* pDefMD) + { + CONTRACTL + { + NOTHROW; + GC_NOTRIGGER; + MODE_ANY; + PRECONDITION(pMT != NULL); + PRECONDITION(pDefMD != NULL); + PRECONDITION(pDefMD->IsEnCAddedMethod()); + PRECONDITION(pDefMD->GetSlot() == MethodTable::NO_SLOT); + } + CONTRACTL_END; + + mdMethodDef tkMethod = pDefMD->GetMemberDef(); + Module* mod = pDefMD->GetModule(); + LOG((LF_ENC, LL_INFO100, "GPMDENC: pMT:%p tok:0x%08x mod:%p\n", pMT, tkMethod, mod)); + + MethodTable::IntroducedMethodIterator it(pMT); + for (; it.IsValid(); it.Next()) + { + MethodDesc* pMD = it.GetMethodDesc(); + if (pMD->GetMemberDef() == tkMethod + && pMD->GetModule() == mod) + { + return pMD; + } + } + LOG((LF_ENC, LL_INFO10000, "GPMDENC: Not found\n")); + return NULL; + } +} + +MethodDesc* MethodTable::GetParallelMethodDesc(MethodDesc* pDefMD) { CONTRACTL { @@ -8724,6 +8804,12 @@ MethodDesc * MethodTable::GetParallelMethodDesc(MethodDesc * pDefMD) MODE_ANY; } CONTRACTL_END; + +#ifdef EnC_SUPPORTED + if (pDefMD->IsEnCAddedMethod()) + return GetParallelMethodDescForEnC(this, pDefMD); +#endif // EnC_SUPPORTED + return GetMethodDescForSlot(pDefMD->GetSlot()); } diff --git a/src/coreclr/vm/methodtable.h b/src/coreclr/vm/methodtable.h index fbda486d729aee..acffd1b6141c7c 100644 --- a/src/coreclr/vm/methodtable.h +++ b/src/coreclr/vm/methodtable.h @@ -3554,20 +3554,14 @@ public : } private: - /* - * This stuff must be first in the struct and should fit on a cache line - don't move it. Used by the GC. - */ - // struct - // { - // Low WORD is component size for array and string types (HasComponentSize() returns true). // Used for flags otherwise. DWORD m_dwFlags; // Base size of instance of this class when allocated on the heap DWORD m_BaseSize; - // } + // See WFLAGS2_ENUM for values. WORD m_wFlags2; // Class token if it fits into 16-bits. If this is (WORD)-1, the class token is stored in the TokenOverflow optional member. @@ -3583,7 +3577,7 @@ public : PTR_MethodTable m_pParentMethodTable; - PTR_Module m_pLoaderModule; // LoaderModule. It is equal to the ZapModule in ngened images + PTR_Module m_pLoaderModule; PTR_MethodTableWriteableData m_pWriteableData; diff --git a/src/coreclr/vm/methodtablebuilder.cpp b/src/coreclr/vm/methodtablebuilder.cpp index bd867cddbacb68..147b2221fad472 100644 --- a/src/coreclr/vm/methodtablebuilder.cpp +++ b/src/coreclr/vm/methodtablebuilder.cpp @@ -1902,7 +1902,6 @@ MethodTableBuilder::BuildMethodTableThrowing( // if there are context or thread static set the info in the method table optional members // - // Check for the RemotingProxy Attribute // structs with GC pointers MUST be pointer sized aligned because the GC assumes it if (IsValueClass() && pMT->ContainsPointers() && (bmtFP->NumInstanceFieldBytes % TARGET_POINTER_SIZE != 0)) { @@ -4118,7 +4117,7 @@ VOID MethodTableBuilder::InitializeFieldDescs(FieldDesc *pFieldDescList, &bmtGenerics->typeContext, ClassLoader::LoadTypes, CLASS_LOAD_APPROXPARENTS, - TRUE, NULL, NULL, NULL, + TRUE, NULL, NULL, NULL, &recursiveControl).GetMethodTable(); } } @@ -4477,9 +4476,12 @@ VOID MethodTableBuilder::InitializeFieldDescs(FieldDesc *pFieldDescList, BuildMethodTableThrowException(IDS_EE_TOOMANYFIELDS); } - GetHalfBakedClass()->SetNumInstanceFields((WORD)dwNumInstanceFields); - GetHalfBakedClass()->SetNumStaticFields((WORD)dwNumStaticFields); - GetHalfBakedClass()->SetNumThreadStaticFields((WORD)dwNumThreadStaticFields); + if (!isEnCField) + { + GetHalfBakedClass()->SetNumInstanceFields((WORD)dwNumInstanceFields); + GetHalfBakedClass()->SetNumStaticFields((WORD)dwNumStaticFields); + GetHalfBakedClass()->SetNumThreadStaticFields((WORD)dwNumThreadStaticFields); + } if (bmtFP->fHasFixedAddressValueTypes) { @@ -6075,7 +6077,7 @@ MethodTableBuilder::InitMethodDesc( } CONTRACTL_END; - LOG((LF_CORDB, LL_EVERYTHING, "MTB::IMD: pNewMD:%p (%u) EnC: %s tok:%x (%s::%s)\n", + LOG((LF_CORDB, LL_EVERYTHING, "MTB::IMD: pNewMD:%p (%u) EnC: %s tok:0x%08x (%s::%s)\n", pNewMD, Classification, (fEnC ? "true" : "false"), tok, pszDebugClassName, pszDebugMethodName)); // Now we know the classification we can perform any classification specific initialization. diff --git a/src/coreclr/vm/prestub.cpp b/src/coreclr/vm/prestub.cpp index 3425f944bf47cf..29965fd0477a28 100644 --- a/src/coreclr/vm/prestub.cpp +++ b/src/coreclr/vm/prestub.cpp @@ -650,7 +650,7 @@ PCODE MethodDesc::JitCompileCode(PrepareCodeConfig* pConfig) STANDARD_VM_CONTRACT; LOG((LF_JIT, LL_INFO1000000, - "JitCompileCode(" FMT_ADDR ", %s) for %s:%s\n", + "JitCompileCode(" FMT_ADDR ", ILStub: %s) for %s::%s\n", DBG_ADDR(this), IsILStub() ? " TRUE" : "FALSE", GetMethodTable()->GetDebugClassName(), diff --git a/src/coreclr/vm/threadsuspend.cpp b/src/coreclr/vm/threadsuspend.cpp index ffff61764fc97c..e7414425ece2d9 100644 --- a/src/coreclr/vm/threadsuspend.cpp +++ b/src/coreclr/vm/threadsuspend.cpp @@ -2404,7 +2404,7 @@ void Thread::RareEnablePreemptiveGC() } } - STRESS_LOG0(LF_SYNC, LL_INFO100000, " RareEnablePreemptiveGC: leaving.\n"); + STRESS_LOG0(LF_SYNC, LL_INFO100000, "RareEnablePreemptiveGC: leaving.\n"); } // Called when we are passing through a safe point in CommonTripThread or