Skip to content

Commit

Permalink
Move computation of DebuggerAssemblyControlFlags from DomainAssembly …
Browse files Browse the repository at this point in the history
…to Assembly (dotnet#107809)

This moves the initial computation into `Assembly` and removes the flags from `DomainAssembly`. Actual consumers were already getting the flags off of `Assembly` or `Module`
  • Loading branch information
elinor-fung authored and sirntar committed Oct 3, 2024
1 parent deceea4 commit 825dada
Show file tree
Hide file tree
Showing 5 changed files with 68 additions and 119 deletions.
10 changes: 6 additions & 4 deletions src/coreclr/vm/assembly.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -117,7 +117,7 @@ void Assembly::Initialize()
// It cannot do any allocations or operations that might fail. Those operations should be done
// in Assembly::Init()
//----------------------------------------------------------------------------------------------
Assembly::Assembly(PEAssembly* pPEAssembly, DebuggerAssemblyControlFlags debuggerFlags, LoaderAllocator *pLoaderAllocator)
Assembly::Assembly(PEAssembly* pPEAssembly, LoaderAllocator *pLoaderAllocator)
: m_pClassLoader(NULL)
, m_pEntryPoint(NULL)
, m_pModule(NULL)
Expand All @@ -140,7 +140,7 @@ Assembly::Assembly(PEAssembly* pPEAssembly, DebuggerAssemblyControlFlags debugge
#ifdef _DEBUG
, m_bDisableActivationCheck{false}
#endif
, m_debuggerFlags(debuggerFlags)
, m_debuggerFlags{DACF_NONE}
, m_hExposedObject{}
{
CONTRACTL
Expand All @@ -167,6 +167,9 @@ void Assembly::Init(AllocMemTracker *pamTracker)
{
STANDARD_VM_CONTRACT;

m_debuggerFlags = ComputeDebuggingConfig();
LOG((LF_CORDB, LL_INFO10, "Assembly %s: bits=0x%x\n", GetDebugName(), GetDebuggerInfoBits()));

m_pClassLoader = new ClassLoader(this);
m_pClassLoader->Init(pamTracker);

Expand Down Expand Up @@ -319,7 +322,6 @@ void Assembly::Terminate( BOOL signalProfiler )

Assembly * Assembly::Create(
PEAssembly * pPEAssembly,
DebuggerAssemblyControlFlags debuggerFlags,
AllocMemTracker * pamTracker,
LoaderAllocator * pLoaderAllocator)
{
Expand All @@ -332,7 +334,7 @@ Assembly * Assembly::Create(
}
CONTRACTL_END

NewHolder<Assembly> pAssembly (new Assembly(pPEAssembly, debuggerFlags, pLoaderAllocator));
NewHolder<Assembly> pAssembly (new Assembly(pPEAssembly, pLoaderAllocator));

#ifdef PROFILING_SUPPORTED
{
Expand Down
9 changes: 7 additions & 2 deletions src/coreclr/vm/assembly.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ class Assembly
friend class ClrDataAccess;

private:
Assembly(PEAssembly *pPEAssembly, DebuggerAssemblyControlFlags debuggerFlags, LoaderAllocator* pLoaderAllocator);
Assembly(PEAssembly *pPEAssembly, LoaderAllocator* pLoaderAllocator);
void Init(AllocMemTracker *pamTracker);

// Load state tracking
Expand Down Expand Up @@ -163,7 +163,7 @@ class Assembly
void StartUnload();
void Terminate( BOOL signalProfiler = TRUE );

static Assembly *Create(PEAssembly *pPEAssembly, DebuggerAssemblyControlFlags debuggerFlags, AllocMemTracker *pamTracker, LoaderAllocator *pLoaderAllocator);
static Assembly *Create(PEAssembly *pPEAssembly, AllocMemTracker *pamTracker, LoaderAllocator *pLoaderAllocator);
static void Initialize();

BOOL IsSystem() { WRAPPER_NO_CONTRACT; return m_pPEAssembly->IsSystem(); }
Expand Down Expand Up @@ -324,6 +324,11 @@ class Assembly
m_debuggerFlags = flags;
}

private:
DebuggerAssemblyControlFlags ComputeDebuggingConfig(void);
HRESULT GetDebuggingCustomAttributes(DWORD* pdwFlags);

public:
// On failure:
// if loadFlag == Loader::Load => throw
// if loadFlag != Loader::Load => return NULL
Expand Down
3 changes: 0 additions & 3 deletions src/coreclr/vm/comdynamic.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -894,9 +894,6 @@ void UpdateRuntimeStateForAssemblyCustomAttribute(Module* pModule, mdToken tkCus
DomainAssembly* pDomainAssembly = pAssembly->GetDomainAssembly();

DWORD actualFlags;
actualFlags = ((DWORD)pDomainAssembly->GetDebuggerInfoBits() & mask) | flags;
pDomainAssembly->SetDebuggerInfoBits((DebuggerAssemblyControlFlags)actualFlags);

actualFlags = ((DWORD)pAssembly->GetDebuggerInfoBits() & mask) | flags;
pAssembly->SetDebuggerInfoBits((DebuggerAssemblyControlFlags)actualFlags);

Expand Down
141 changes: 55 additions & 86 deletions src/coreclr/vm/domainassembly.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,6 @@ DomainAssembly::DomainAssembly(PEAssembly* pPEAssembly, LoaderAllocator* pLoader
, m_fCollectible(pLoaderAllocator->IsCollectible())
, m_NextDomainAssemblyInSameALC(NULL)
, m_pLoaderAllocator(pLoaderAllocator)
, m_debuggerFlags(DACF_NONE)
{
CONTRACTL
{
Expand All @@ -45,10 +44,8 @@ DomainAssembly::DomainAssembly(PEAssembly* pPEAssembly, LoaderAllocator* pLoader
pPEAssembly->AddRef();
pPEAssembly->ValidateForExecution();

SetupDebuggingConfig();

// Create the Assembly
NewHolder<Assembly> assembly = Assembly::Create(pPEAssembly, GetDebuggerInfoBits(), memTracker, pLoaderAllocator);
NewHolder<Assembly> assembly = Assembly::Create(pPEAssembly, memTracker, pLoaderAllocator);

m_pAssembly = assembly.Extract();
m_pModule = m_pAssembly->GetModule();
Expand Down Expand Up @@ -519,7 +516,7 @@ void Assembly::DeliverSyncEvents()
#endif // DEBUGGING_SUPPORTED
}

DWORD DomainAssembly::ComputeDebuggingConfig()
DebuggerAssemblyControlFlags Assembly::ComputeDebuggingConfig()
{
CONTRACTL
{
Expand All @@ -534,36 +531,15 @@ DWORD DomainAssembly::ComputeDebuggingConfig()
#ifdef DEBUGGING_SUPPORTED
DWORD dacfFlags = DACF_ALLOW_JIT_OPTS;
IfFailThrow(GetDebuggingCustomAttributes(&dacfFlags));
return dacfFlags;
return (DebuggerAssemblyControlFlags)dacfFlags;
#else // !DEBUGGING_SUPPORTED
return 0;
#endif // DEBUGGING_SUPPORTED
}

void DomainAssembly::SetupDebuggingConfig(void)
{
CONTRACTL
{
INSTANCE_CHECK;
THROWS;
WRAPPER(GC_TRIGGERS);
MODE_ANY;
INJECT_FAULT(COMPlusThrowOM(););
}
CONTRACTL_END;

#ifdef DEBUGGING_SUPPORTED
DWORD dacfFlags = ComputeDebuggingConfig();

SetDebuggerInfoBits((DebuggerAssemblyControlFlags)dacfFlags);

LOG((LF_CORDB, LL_INFO10, "Assembly %s: bits=0x%x\n", GetDebugName(), GetDebuggerInfoBits()));
#endif // DEBUGGING_SUPPORTED
}

// For right now, we only check to see if the DebuggableAttribute is present - later may add fields/properties to the
// attributes.
HRESULT DomainAssembly::GetDebuggingCustomAttributes(DWORD *pdwFlags)
HRESULT Assembly::GetDebuggingCustomAttributes(DWORD *pdwFlags)
{
CONTRACTL
{
Expand All @@ -576,70 +552,63 @@ HRESULT DomainAssembly::GetDebuggingCustomAttributes(DWORD *pdwFlags)
}
CONTRACTL_END;

HRESULT hr = S_OK;
ULONG size;
BYTE *blob;
IMDInternalImport* mdImport = GetPEAssembly()->GetMDImport();
mdAssembly asTK = TokenFromRid(mdtAssembly, 1);

HRESULT hr = mdImport->GetCustomAttributeByName(asTK,
DEBUGGABLE_ATTRIBUTE_TYPE,
(const void**)&blob,
&size);

// If there is no custom value, then there is no entrypoint defined.
if (FAILED(hr) || hr == S_FALSE)
return hr;

// We're expecting a 6 or 8 byte blob:
//
// 1, 0, enable tracking, disable opts, 0, 0
if ((size == 6) || (size == 8))
{
ULONG size;
BYTE *blob;
mdModule mdMod;
IMDInternalImport* mdImport = GetPEAssembly()->GetMDImport();
mdMod = mdImport->GetModuleFromScope();
mdAssembly asTK = TokenFromRid(mdtAssembly, 1);
if (!((blob[0] == 1) && (blob[1] == 0)))
{
BAD_FORMAT_NOTHROW_ASSERT(!"Invalid blob format for custom attribute");
return COR_E_BADIMAGEFORMAT;
}

hr = mdImport->GetCustomAttributeByName(asTK,
DEBUGGABLE_ATTRIBUTE_TYPE,
(const void**)&blob,
&size);
if (blob[2] & 0x1)
{
*pdwFlags |= DACF_OBSOLETE_TRACK_JIT_INFO;
}
else
{
*pdwFlags &= (~DACF_OBSOLETE_TRACK_JIT_INFO);
}

// If there is no custom value, then there is no entrypoint defined.
if (!(FAILED(hr) || hr == S_FALSE))
if (blob[2] & 0x2)
{
// We're expecting a 6 or 8 byte blob:
//
// 1, 0, enable tracking, disable opts, 0, 0
if ((size == 6) || (size == 8))
{
if (!((blob[0] == 1) && (blob[1] == 0)))
{
BAD_FORMAT_NOTHROW_ASSERT(!"Invalid blob format for custom attribute");
return COR_E_BADIMAGEFORMAT;
}

if (blob[2] & 0x1)
{
*pdwFlags |= DACF_OBSOLETE_TRACK_JIT_INFO;
}
else
{
*pdwFlags &= (~DACF_OBSOLETE_TRACK_JIT_INFO);
}

if (blob[2] & 0x2)
{
*pdwFlags |= DACF_IGNORE_PDBS;
}
else
{
*pdwFlags &= (~DACF_IGNORE_PDBS);
}


// For compatibility, we enable optimizations if the tracking byte is zero,
// even if disable opts is nonzero
if (((blob[2] & 0x1) == 0) || (blob[3] == 0))
{
*pdwFlags |= DACF_ALLOW_JIT_OPTS;
}
else
{
*pdwFlags &= (~DACF_ALLOW_JIT_OPTS);
}

LOG((LF_CORDB, LL_INFO10, "Assembly %s: has %s=%d,%d bits = 0x%x\n", GetDebugName(),
DEBUGGABLE_ATTRIBUTE_TYPE_NAME,
blob[2], blob[3], *pdwFlags));
}
*pdwFlags |= DACF_IGNORE_PDBS;
}
else
{
*pdwFlags &= (~DACF_IGNORE_PDBS);
}

// For compatibility, we enable optimizations if the tracking byte is zero,
// even if disable opts is nonzero
if (((blob[2] & 0x1) == 0) || (blob[3] == 0))
{
*pdwFlags |= DACF_ALLOW_JIT_OPTS;
}
else
{
*pdwFlags &= (~DACF_ALLOW_JIT_OPTS);
}

LOG((LF_CORDB, LL_INFO10, "Assembly %s: has %s=%d,%d bits = 0x%x\n", GetDebugName(),
DEBUGGABLE_ATTRIBUTE_TYPE_NAME,
blob[2], blob[3], *pdwFlags));
}

return hr;
Expand Down
24 changes: 0 additions & 24 deletions src/coreclr/vm/domainassembly.h
Original file line number Diff line number Diff line change
Expand Up @@ -149,28 +149,6 @@ class DomainAssembly final

DomainAssembly(PEAssembly* pPEAssembly, LoaderAllocator* pLoaderAllocator, AllocMemTracker* memTracker);

public:
// ------------------------------------------------------------
// Debugger control API
// ------------------------------------------------------------

DebuggerAssemblyControlFlags GetDebuggerInfoBits(void)
{
LIMITED_METHOD_CONTRACT;
return m_debuggerFlags;
}

void SetDebuggerInfoBits(DebuggerAssemblyControlFlags newBits)
{
LIMITED_METHOD_CONTRACT;
m_debuggerFlags = newBits;
}

void SetupDebuggingConfig(void);
DWORD ComputeDebuggingConfig(void);

HRESULT GetDebuggingCustomAttributes(DWORD* pdwFlags);

private:
// ------------------------------------------------------------
// Instance data
Expand All @@ -183,8 +161,6 @@ class DomainAssembly final
BOOL m_fCollectible;
DomainAssembly* m_NextDomainAssemblyInSameALC;
PTR_LoaderAllocator m_pLoaderAllocator;

DebuggerAssemblyControlFlags m_debuggerFlags;
};

#endif // _DOMAINASSEMBLY_H_

0 comments on commit 825dada

Please sign in to comment.