Skip to content

Commit

Permalink
Add support for reporting byrefs to RVA static fields of collectible …
Browse files Browse the repository at this point in the history
…assemblies (dotnet#40346)

- Keep track of all RVA static field locations
  - For assemblies loaded from PE files, use a range that is the entire PE range
  - For assemblies dynamically created, use piecemeal ranges for each individual RVA static field
- Report byref references via the GcReportLoaderAllocator mechanism in PromoteCarefully

- Add a test to cover this scenario, and thread statics
  - disable test on Mono, as it doesn't pass there yet
  • Loading branch information
davidwrighton authored and Jacksondr5 committed Aug 10, 2020
1 parent 9b5cbe1 commit 1e17414
Show file tree
Hide file tree
Showing 14 changed files with 362 additions and 48 deletions.
13 changes: 13 additions & 0 deletions src/coreclr/src/vm/assembly.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -190,6 +190,19 @@ void Assembly::Init(AllocMemTracker *pamTracker, LoaderAllocator *pLoaderAllocat
// loading it entirely.
//CacheFriendAssemblyInfo();

#ifndef CROSSGEN_COMPILE
if (IsCollectible())
{
COUNT_T size;
BYTE *start = (BYTE*)m_pManifest->GetFile()->GetLoadedImageContents(&size);
if (start != NULL)
{
GCX_COOP();
LoaderAllocator::AssociateMemoryWithLoaderAllocator(start, start + size, m_pLoaderAllocator);
}
}
#endif

{
CANNOTTHROWCOMPLUSEXCEPTION();
FAULT_FORBID();
Expand Down
6 changes: 6 additions & 0 deletions src/coreclr/src/vm/commodule.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -643,6 +643,12 @@ void QCALLTYPE COMModule::SetFieldRVAContent(QCall::ModuleHandle pModule, INT32
if (pContent != NULL)
memcpy(pvBlob, pContent, length);

if (pReflectionModule->IsCollectible())
{
GCX_COOP();
LoaderAllocator::AssociateMemoryWithLoaderAllocator((BYTE*)pvBlob, ((BYTE*)pvBlob) + length, pReflectionModule->GetLoaderAllocator());
}

// set FieldRVA into metadata. Note that this is not final RVA in the image if save to disk. We will do another round of fix up upon save.
IfFailThrow( pRCW->GetEmitter()->SetFieldRVA(tkField, dwRVA) );

Expand Down
43 changes: 43 additions & 0 deletions src/coreclr/src/vm/loaderallocator.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -662,6 +662,11 @@ BOOL QCALLTYPE LoaderAllocator::Destroy(QCall::LoaderAllocatorHandle pLoaderAllo
STRESS_LOG1(LF_CLASSLOADER, LL_INFO100, "Begin LoaderAllocator::Destroy for loader allocator %p\n", reinterpret_cast<void *>(static_cast<PTR_LoaderAllocator>(pLoaderAllocator)));
LoaderAllocatorID *pID = pLoaderAllocator->Id();

{
GCX_COOP();
LoaderAllocator::RemoveMemoryToLoaderAllocatorAssociation(pLoaderAllocator);
}

// This will probably change for shared code unloading
_ASSERTE(pID->GetType() == LAT_Assembly);

Expand Down Expand Up @@ -1984,6 +1989,44 @@ UMEntryThunkCache *LoaderAllocator::GetUMEntryThunkCache()
return m_pUMEntryThunkCache;
}

/* static */
void LoaderAllocator::RemoveMemoryToLoaderAllocatorAssociation(LoaderAllocator* pLoaderAllocator)
{
CONTRACTL {
THROWS;
MODE_COOPERATIVE;
} CONTRACTL_END;

GlobalLoaderAllocator* pGlobalAllocator = (GlobalLoaderAllocator*)SystemDomain::GetGlobalLoaderAllocator();
pGlobalAllocator->m_memoryAssociations.RemoveRanges(pLoaderAllocator);
}

/* static */
void LoaderAllocator::AssociateMemoryWithLoaderAllocator(BYTE *start, const BYTE *end, LoaderAllocator* pLoaderAllocator)
{
CONTRACTL {
THROWS;
MODE_COOPERATIVE;
} CONTRACTL_END;

GlobalLoaderAllocator* pGlobalAllocator = (GlobalLoaderAllocator*)SystemDomain::GetGlobalLoaderAllocator();
pGlobalAllocator->m_memoryAssociations.AddRange(start, end, pLoaderAllocator);
}

/* static */
PTR_LoaderAllocator LoaderAllocator::GetAssociatedLoaderAllocator_Unsafe(TADDR ptr)
{
LIMITED_METHOD_CONTRACT;

GlobalLoaderAllocator* pGlobalAllocator = (GlobalLoaderAllocator*)SystemDomain::GetGlobalLoaderAllocator();
LoaderAllocator* pLoaderAllocator;
if (pGlobalAllocator->m_memoryAssociations.IsInRangeWorker_Unlocked(ptr, reinterpret_cast<TADDR *>(&pLoaderAllocator)))
{
return pLoaderAllocator;
}
return NULL;
}

#endif // !CROSSGEN_COMPILE

#ifdef FEATURE_COMINTEROP
Expand Down
13 changes: 12 additions & 1 deletion src/coreclr/src/vm/loaderallocator.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ class FuncPtrStubs;
#include "methoddescbackpatchinfo.h"
#include "crossloaderallocatorhash.h"
#include "onstackreplacement.h"
#include "lockedrangelist.h"

#define VPTRU_LoaderAllocator 0x3200

Expand Down Expand Up @@ -405,6 +406,13 @@ class LoaderAllocator
virtual LoaderAllocatorID* Id() =0;
BOOL IsCollectible() { WRAPPER_NO_CONTRACT; return m_IsCollectible; }

// This function may only be called while the runtime is suspended
// As it does not lock around access to a RangeList
static PTR_LoaderAllocator GetAssociatedLoaderAllocator_Unsafe(TADDR ptr);

static void AssociateMemoryWithLoaderAllocator(BYTE *start, const BYTE *end, LoaderAllocator* pLoaderAllocator);
static void RemoveMemoryToLoaderAllocatorAssociation(LoaderAllocator* pLoaderAllocator);

#ifdef DACCESS_COMPILE
void EnumMemoryRegions(CLRDataEnumMemoryFlags flags);
#endif
Expand Down Expand Up @@ -627,12 +635,16 @@ typedef VPTR(LoaderAllocator) PTR_LoaderAllocator;

class GlobalLoaderAllocator : public LoaderAllocator
{
friend class LoaderAllocator;
VPTR_VTABLE_CLASS(GlobalLoaderAllocator, LoaderAllocator)
VPTR_UNIQUE(VPTRU_LoaderAllocator+1)

DAC_ALIGNAS(LoaderAllocator) // Align the first member to the alignment of the base class
BYTE m_ExecutableHeapInstance[sizeof(LoaderHeap)];

// Associate memory regions with loader allocator objects
LockedRangeList m_memoryAssociations;

protected:
LoaderAllocatorID m_Id;

Expand Down Expand Up @@ -712,7 +724,6 @@ class AssemblyLoaderAllocator : public LoaderAllocator

typedef VPTR(AssemblyLoaderAllocator) PTR_AssemblyLoaderAllocator;


#include "loaderallocator.inl"

#endif // __LoaderAllocator_h__
Expand Down
61 changes: 61 additions & 0 deletions src/coreclr/src/vm/lockedrangelist.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,61 @@
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.

#ifndef __LockedRangeList_h__
#define __LockedRangeList_h__

// -------------------------------------------------------
// This just wraps the RangeList methods in a read or
// write lock depending on the operation.
// -------------------------------------------------------

class LockedRangeList : public RangeList
{
public:
VPTR_VTABLE_CLASS(LockedRangeList, RangeList)

LockedRangeList() : RangeList(), m_RangeListRWLock(COOPERATIVE_OR_PREEMPTIVE, LOCK_TYPE_DEFAULT)
{
LIMITED_METHOD_CONTRACT;
}

~LockedRangeList()
{
LIMITED_METHOD_CONTRACT;
}

BOOL IsInRangeWorker_Unlocked(TADDR address, TADDR *pID = NULL)
{
WRAPPER_NO_CONTRACT;
SUPPORTS_DAC;
return RangeList::IsInRangeWorker(address, pID);
}

protected:

virtual BOOL AddRangeWorker(const BYTE *start, const BYTE *end, void *id)
{
WRAPPER_NO_CONTRACT;
SimpleWriteLockHolder lh(&m_RangeListRWLock);
return RangeList::AddRangeWorker(start,end,id);
}

virtual void RemoveRangesWorker(void *id, const BYTE *start = NULL, const BYTE *end = NULL)
{
WRAPPER_NO_CONTRACT;
SimpleWriteLockHolder lh(&m_RangeListRWLock);
RangeList::RemoveRangesWorker(id,start,end);
}

virtual BOOL IsInRangeWorker(TADDR address, TADDR *pID = NULL)
{
WRAPPER_NO_CONTRACT;
SUPPORTS_DAC;
SimpleReadLockHolder lh(&m_RangeListRWLock);
return RangeList::IsInRangeWorker(address, pID);
}

SimpleRWLock m_RangeListRWLock;
};

#endif // __LockedRangeList_h__
10 changes: 10 additions & 0 deletions src/coreclr/src/vm/siginfo.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -4900,6 +4900,16 @@ void PromoteCarefully(promote_func fn,
return;
}

#ifndef CROSSGEN_COMPILE
if (sc->promotion)
{
LoaderAllocator*pLoaderAllocator = LoaderAllocator::GetAssociatedLoaderAllocator_Unsafe(PTR_TO_TADDR(*ppObj));
if (pLoaderAllocator != NULL)
{
GcReportLoaderAllocator(fn, sc, pLoaderAllocator);
}
}
#endif // CROSSGEN_COMPILE
#endif // !defined(DACCESS_COMPILE)

(*fn) (ppObj, sc, flags);
Expand Down
48 changes: 1 addition & 47 deletions src/coreclr/src/vm/stubmgr.h
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@
#define __stubmgr_h__

#include "simplerwlock.hpp"
#include "lockedrangelist.h"

// When 'TraceStub' returns, it gives the address of where the 'target' is for a stub'
// TraceType indicates what this 'target' is
Expand Down Expand Up @@ -339,53 +340,6 @@ class StubManager
#endif // !CROSSGEN_COMPILE
};

// -------------------------------------------------------
// This just wraps the RangeList methods in a read or
// write lock depending on the operation.
// -------------------------------------------------------

class LockedRangeList : public RangeList
{
public:
VPTR_VTABLE_CLASS(LockedRangeList, RangeList)

LockedRangeList() : RangeList(), m_RangeListRWLock(COOPERATIVE_OR_PREEMPTIVE, LOCK_TYPE_DEFAULT)
{
LIMITED_METHOD_CONTRACT;
}

~LockedRangeList()
{
LIMITED_METHOD_CONTRACT;
}

protected:

virtual BOOL AddRangeWorker(const BYTE *start, const BYTE *end, void *id)
{
WRAPPER_NO_CONTRACT;
SimpleWriteLockHolder lh(&m_RangeListRWLock);
return RangeList::AddRangeWorker(start,end,id);
}

virtual void RemoveRangesWorker(void *id, const BYTE *start = NULL, const BYTE *end = NULL)
{
WRAPPER_NO_CONTRACT;
SimpleWriteLockHolder lh(&m_RangeListRWLock);
RangeList::RemoveRangesWorker(id,start,end);
}

virtual BOOL IsInRangeWorker(TADDR address, TADDR *pID = NULL)
{
WRAPPER_NO_CONTRACT;
SUPPORTS_DAC;
SimpleReadLockHolder lh(&m_RangeListRWLock);
return RangeList::IsInRangeWorker(address, pID);
}

SimpleRWLock m_RangeListRWLock;
};

#ifndef CROSSGEN_COMPILE

//-----------------------------------------------------------
Expand Down
3 changes: 3 additions & 0 deletions src/coreclr/tests/issues.targets
Original file line number Diff line number Diff line change
Expand Up @@ -961,6 +961,9 @@

<!-- Known failures for mono runtime on *all* architectures/operating systems -->
<ItemGroup Condition="'$(RuntimeFlavor)' == 'mono'" >
<ExcludeList Include="$(XunitTestBinBase)/Loader/CollectibleAssemblies/ByRefLocals/**">
<Issue>https://github.com/dotnet/runtime/issues/40394</Issue>
</ExcludeList>
<ExcludeList Include="$(XunitTestBinBase)/Interop/PInvoke/Miscellaneous/CopyCtor/**">
<Issue>Handling for Copy constructors isn't present in mono interop</Issue>
</ExcludeList>
Expand Down
Loading

0 comments on commit 1e17414

Please sign in to comment.