Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Generate the DAC table on Windows with MSVC linker directives instead of resource injection #68065

Merged
merged 18 commits into from
May 12, 2022
Merged
Show file tree
Hide file tree
Changes from 13 commits
Commits
Show all changes
18 commits
Select commit Hold shift + click to select a range
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 5 additions & 2 deletions docs/design/coreclr/botr/dac-notes.md
Original file line number Diff line number Diff line change
Expand Up @@ -137,12 +137,15 @@ Even better, in a DAC build, the DAC will automatically marshal variables, data

`PTR` types defined with `DPTR` are the most common in the runtime, but we also have `PTR` types for global and static pointers, restricted-use arrays, pointers to variable-sized objects, and pointers to classes with virtual functions that we may need to call from mscordacwks.dll (msdaccore.dll). Most of these are rare and you can refer to [daccess.h][daccess.h] to learn more about them if you need them.

The `GPTR` and `VPTR` macros are common enough to warrant special mention here. Both the way we use these and their external behavior is quite similar to `DPTR`s. Again, marshaling is automatic and transparent. The `VPTR` macro declares a marshaled pointer type for a class with virtual functions. This special macro is necessary because the virtual function table is essentially an implicit extra field. The DAC has to marshal this separately, since the function addresses are all target addresses that the DAC must convert to host addresses. Treating these classes in this way means that the DAC automatically instantiates the correct implementation class, making casts between base and derived types unnecessary. When you declare a `VPTR` type, you must also list it in [vptr_list.h][vptr_list.h]. `__GlobalPtr` types provide base functionality to marshal both global variables and static data members through the `GPTR`, `GVAL`, `SPTR` and `SVAL` macros. The implementation of global variables is almost identical to that of static fields (both use the `__GlobalPtr` class) and require the addition of an entry in [dacvars.h][dacvars.h]. The comments in [daccess.h][daccess.h] and [dacvars.h][dacvars.h] provide more details about declaring these types.
The `GPTR` and `VPTR` macros are common enough to warrant special mention here. Both the way we use these and their external behavior is quite similar to `DPTR`s. Again, marshaling is automatic and transparent. The `VPTR` macro declares a marshaled pointer type for a class with virtual functions. This special macro is necessary because the virtual function table is essentially an implicit extra field. The DAC has to marshal this separately, since the function addresses are all target addresses that the DAC must convert to host addresses. Treating these classes in this way means that the DAC automatically instantiates the correct implementation class, making casts between base and derived types unnecessary. When you declare a `VPTR` type, you must also list it in [vptr_list.h][vptr_list.h]. `__GlobalPtr` types provide base functionality to marshal both global variables and static data members through the `GPTR`, `GVAL`, `SPTR` and `SVAL` macros. The implementation of global variables is almost identical to that of static fields (both use the `__GlobalPtr` class) and require the addition of an entry in [dacvars.h][dacvars.h]. Global functions used in the DAC do not require macros at the implementation site, but they must be declared in the [gfunc_list.h][gfunc_list.h] header to have their addresses be automatically available to the DAC. The comments in [daccess.h][daccess.h] and [dacvars.h][dacvars.h] provide more details about declaring these types.

[dacvars.h]: https://github.com/dotnet/runtime/blob/main/src/coreclr/inc/dacvars.h
[vptr_list.h]: https://github.com/dotnet/runtime/blob/main/src/coreclr/inc/vptr_list.h
[gfunc_list.h]: https://github.com/dotnet/runtime/blob/main/src/coreclr/inc/gfunc_list.h

Global and static values and pointers are interesting because they form the entry points to the target address space (all other uses of the DAC require you to have a target address already). Many of the globals in the runtime are already DACized. It occasionally becomes necessary to make a previously un-DACized (or a newly introduced) global available to the DAC. By using the appropriate macros and [dacvars.h][dacvars.h] entry, you enable a post-build step (DacTableGen.exe run by the build in ndp\clr\src\dacupdatedll) to save the address of the global (from clr.pdb) into a table that is embedded into mscordacwks.dll. The DAC uses this table at run-time to determine where to look in the target address space when the code accesses a global.
Global and static values and pointers are interesting because they form the entry points to the target address space (all other uses of the DAC require you to have a target address already). Many of the globals in the runtime are already DACized. It occasionally becomes necessary to make a previously un-DACized (or a newly introduced) global available to the DAC. By using the appropriate macros and [dacvars.h][dacvars.h] entry, you enable the dac table machinery (in [dactable.cpp]) to save the address of the global into a table that is exported from coreclr.dll. The DAC uses this table at run-time to determine where to look in the target address space when the code accesses a global.

[dactable.cpp]: https://github.com/dotnet/runtime/blob/main/src/coreclr/debug/ee/dactable.cpp

### VAL Types

Expand Down
8 changes: 0 additions & 8 deletions src/coreclr/EmptyProps.props

This file was deleted.

3 changes: 0 additions & 3 deletions src/coreclr/build-runtime.cmd
Original file line number Diff line number Diff line change
Expand Up @@ -225,9 +225,6 @@ if not exist "%__IntermediatesDir%" md "%__IntermediatesDir%"
if not exist "%__LogsDir%" md "%__LogsDir%"
if not exist "%__MsbuildDebugLogsDir%" md "%__MsbuildDebugLogsDir%"

if not exist "%__RootBinDir%\Directory.Build.props" copy "%__ProjectDir%\EmptyProps.props" "%__RootBinDir%\Directory.Build.props"
if not exist "%__RootBinDir%\Directory.Build.targets" copy "%__ProjectDir%\EmptyProps.props" "%__RootBinDir%\Directory.Build.targets"

REM Set up the directory for MSBuild debug logs.
set MSBUILDDEBUGPATH=%__MsbuildDebugLogsDir%

Expand Down
4 changes: 2 additions & 2 deletions src/coreclr/debug/createdump/crashinfomac.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -252,9 +252,9 @@ void CrashInfo::VisitModule(MachOModule& module)
m_coreclrPath = module.Name().substr(0, last + 1);

uint64_t symbolOffset;
if (!module.TryLookupSymbol("g_dacTable", &symbolOffset))
if (!module.TryLookupSymbol(DACCESS_TABLE_SYMBOL, &symbolOffset))
{
TRACE("TryLookupSymbol(g_dacTable) FAILED\n");
TRACE("TryLookupSymbol(" DACCESS_TABLE_SYMBOL ") FAILED\n");
}
}
}
Expand Down
4 changes: 2 additions & 2 deletions src/coreclr/debug/createdump/crashinfounix.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -275,9 +275,9 @@ CrashInfo::VisitModule(uint64_t baseAddress, std::string& moduleName)
// necessary is in the core dump.
if (PopulateForSymbolLookup(baseAddress)) {
uint64_t symbolOffset;
if (!TryLookupSymbol("g_dacTable", &symbolOffset))
if (!TryLookupSymbol(DACCESS_TABLE_SYMBOL, &symbolOffset))
{
TRACE("TryLookupSymbol(g_dacTable) FAILED\n");
TRACE("TryLookupSymbol(" DACCESS_TABLE_SYMBOL ") FAILED\n");
}
}
}
Expand Down
3 changes: 3 additions & 0 deletions src/coreclr/debug/createdump/createdump.h
Original file line number Diff line number Diff line change
Expand Up @@ -109,3 +109,6 @@ extern bool CreateDump(const char* dumpPathTemplate, int pid, const char* dumpTy

extern void printf_status(const char* format, ...);
extern void printf_error(const char* format, ...);

// Keep in sync with the definitions in dbgutil.cpp and daccess.h
#define DACCESS_TABLE_SYMBOL "g_dacTable"
247 changes: 2 additions & 245 deletions src/coreclr/debug/daccess/daccess.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -24,13 +24,11 @@
#include "primitives.h"
#include "dbgutil.h"

#ifdef TARGET_UNIX
#ifdef USE_DAC_TABLE_RVA
#include <dactablerva.h>
#else
extern "C" bool TryGetSymbol(ICorDebugDataTarget* dataTarget, uint64_t baseAddress, const char* symbolName, uint64_t* symbolAddress);
#endif
#endif

#include "dwbucketmanager.hpp"
#include "gcinterface.dac.h"
Expand Down Expand Up @@ -6885,129 +6883,6 @@ bool ClrDataAccess::TargetConsistencyAssertsEnabled()
//
HRESULT ClrDataAccess::VerifyDlls()
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So we are always going to say CoreCLR and DAC match? The DAC table used to have the bits to verify the CLR matched, granted - now you don't embed anything, but it's

  • If we still care about making sure bits match, we need a new sort of signature. Maybe a global embedded on both that's derived from the build version that the DAC can verify against?
  • If not, we should probably remove this function entirely. It doesn't make sense to just return S_OK.

Thoughts @dotnet/dotnet-diag ?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We're already assuming that they match on non-Windows, so if we are introducing a new signature mechanism, we should make sure it works for all platforms we support.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yup - I am happy to have it be a wide check

{
#ifndef TARGET_UNIX
// Provide a knob for disabling this check if we really want to try and proceed anyway with a
// DAC mismatch. DAC behavior may be arbitrarily bad - globals probably won't be at the same
// address, data structures may be laid out differently, etc.
if (CLRConfig::GetConfigValue(CLRConfig::INTERNAL_DbgDACSkipVerifyDlls))
{
return S_OK;
}

// Read the debug directory timestamp from the target mscorwks image using DAC
// Note that we don't use the PE timestamp because the PE pPEAssembly might be changed in ways
// that don't effect the PDB (and therefore don't effect DAC). Specifically, we rebase
// our DLLs at the end of a build, that changes the PE pPEAssembly, but not the PDB.
// Note that if we wanted to be extra careful, we could read the CV contents (which includes
// the GUID signature) and verify it matches. Using the timestamp is useful for helpful error
// messages, and should be sufficient in any real scenario.
DWORD timestamp = 0;
HRESULT hr = S_OK;
DAC_ENTER();
EX_TRY
{
// Note that we don't need to worry about ensuring the image memory read by this code
// is saved in a minidump. Managed minidump debugging already requires that you have
// the full mscorwks.dll available at debug time (eg. windbg won't even load DAC without it).
PEDecoder pedecoder(dac_cast<PTR_VOID>(m_globalBase));

// We use the first codeview debug directory entry since this should always refer to the single
// PDB for mscorwks.dll.
const UINT k_maxDebugEntries = 32; // a reasonable upper limit in case of corruption
for( UINT i = 0; i < k_maxDebugEntries; i++)
{
PTR_IMAGE_DEBUG_DIRECTORY pDebugEntry = pedecoder.GetDebugDirectoryEntry(i);

// If there are no more entries, then stop
if (pDebugEntry == NULL)
break;

// Ignore non-codeview entries. Some scenarios (eg. optimized builds), there may be extra
// debug directory entries at the end of some other type.
if (pDebugEntry->Type == IMAGE_DEBUG_TYPE_CODEVIEW)
{
// Found a codeview entry - use it's timestamp for comparison
timestamp = pDebugEntry->TimeDateStamp;
break;
}
}
char szMsgBuf[1024];
_snprintf_s(szMsgBuf, sizeof(szMsgBuf), _TRUNCATE,
"Failed to find any valid codeview debug directory entry in %s image",
MAIN_CLR_MODULE_NAME_A);
_ASSERTE_MSG(timestamp != 0, szMsgBuf);
}
EX_CATCH
{
if (!DacExceptionFilter(GET_EXCEPTION(), this, &hr))
{
EX_RETHROW;
}
}
EX_END_CATCH(SwallowAllExceptions)
DAC_LEAVE();
if (FAILED(hr))
{
return hr;
}

// Validate that we got a timestamp and it matches what the DAC table told us to expect
if (timestamp == 0 || timestamp != g_dacTableInfo.dwID0)
{
// Timestamp mismatch. This means mscordacwks is being used with a version of
// mscorwks other than the one it was built for. This will not work reliably.

#ifdef _DEBUG
// Check if verbose asserts are enabled. The default is up to the specific instantiation of
// ClrDataAccess, but can be overridden (in either direction) by a COMPlus_ knob.
// Note that we check this knob every time because it may be handy to turn it on in
// the environment mid-flight.
DWORD dwAssertDefault = m_fEnableDllVerificationAsserts ? 1 : 0;
if (CLRConfig::GetConfigValue(CLRConfig::INTERNAL_DbgDACAssertOnMismatch, dwAssertDefault))
{
// Output a nice error message that contains the timestamps in string format.
time_t actualTime = timestamp;
char szActualTime[30];
ctime_s(szActualTime, sizeof(szActualTime), &actualTime);

time_t expectedTime = g_dacTableInfo.dwID0;
char szExpectedTime[30];
ctime_s(szExpectedTime, sizeof(szExpectedTime), &expectedTime);

// Create a nice detailed message for the assert dialog.
// Note that the strings returned by ctime_s have terminating newline characters.
// This is technically a TARGET_CONSISTENCY_CHECK because a corrupt target could,
// in-theory, have a corrupt mscrowks PE header and cause this check to fail
// unnecessarily. However, this check occurs during startup, before we know
// whether target consistency checks should be enabled, so it's always enabled
// at the moment.

char szMsgBuf[1024];
_snprintf_s(szMsgBuf, sizeof(szMsgBuf), _TRUNCATE,
"DAC fatal error: %s/mscordacwks.dll version mismatch\n\n"\
"The debug directory timestamp of the loaded %s does not match the\n"\
"version mscordacwks.dll was built for.\n"\
"Expected %s timestamp: %s"\
"Actual %s timestamp: %s\n"\
"DAC will now fail to initialize with a CORDBG_E_MISMATCHED_CORWKS_AND_DACWKS_DLLS\n"\
"error. If you really want to try and use the mimatched DLLs, you can disable this\n"\
"check by setting COMPlus_DbgDACSkipVerifyDlls=1. However, using a mismatched DAC\n"\
"DLL will usually result in arbitrary debugger failures.\n",
TARGET_MAIN_CLR_DLL_NAME_A,
TARGET_MAIN_CLR_DLL_NAME_A,
TARGET_MAIN_CLR_DLL_NAME_A,
szExpectedTime,
TARGET_MAIN_CLR_DLL_NAME_A,
szActualTime);
_ASSERTE_MSG(false, szMsgBuf);
}
#endif

// Return a specific hresult indicating this problem
return CORDBG_E_MISMATCHED_CORWKS_AND_DACWKS_DLLS;
}
#endif // TARGET_UNIX

return S_OK;
}

Expand Down Expand Up @@ -7096,17 +6971,9 @@ bool ClrDataAccess::MdCacheGetEEName(TADDR taEEStruct, SString & eeName)

#endif // FEATURE_MINIMETADATA_IN_TRIAGEDUMPS

// Needed for RT_RCDATA.
#define MAKEINTRESOURCE(v) MAKEINTRESOURCEW(v)

// this funny looking double macro forces x to be macro expanded before L is prepended
#define _WIDE(x) _WIDE2(x)
#define _WIDE2(x) W(x)

HRESULT
GetDacTableAddress(ICorDebugDataTarget* dataTarget, ULONG64 baseAddress, PULONG64 dacTableAddress)
{
#ifdef TARGET_UNIX
#ifdef USE_DAC_TABLE_RVA
#ifdef DAC_TABLE_SIZE
if (DAC_TABLE_SIZE != sizeof(g_dacGlobals))
Expand All @@ -7117,20 +6984,18 @@ GetDacTableAddress(ICorDebugDataTarget* dataTarget, ULONG64 baseAddress, PULONG6
// On MacOS, FreeBSD or NetBSD use the RVA include file
*dacTableAddress = baseAddress + DAC_TABLE_RVA;
#else
// On Linux/MacOS try to get the dac table address via the export symbol
if (!TryGetSymbol(dataTarget, baseAddress, "g_dacTable", dacTableAddress))
// Otherwise, try to get the dac table address via the export symbol
if (!TryGetSymbol(dataTarget, baseAddress, DACCESS_TABLE_SYMBOL, dacTableAddress))
{
return CORDBG_E_MISSING_DEBUGGER_EXPORTS;
}
#endif
#endif
return S_OK;
}

HRESULT
ClrDataAccess::GetDacGlobals()
{
#ifdef TARGET_UNIX
ULONG64 dacTableAddress;
HRESULT hr = GetDacTableAddress(m_pTarget, m_globalBase, &dacTableAddress);
if (FAILED(hr))
Expand All @@ -7146,116 +7011,8 @@ ClrDataAccess::GetDacGlobals()
return CORDBG_E_UNSUPPORTED;
}
return S_OK;
#else
HRESULT status = E_FAIL;
DWORD rsrcRVA = 0;
LPVOID rsrcData = NULL;
DWORD rsrcSize = 0;

DWORD resourceSectionRVA = 0;

if (FAILED(status = GetMachineAndResourceSectionRVA(m_pTarget, m_globalBase, NULL, &resourceSectionRVA)))
{
_ASSERTE_MSG(false, "DAC fatal error: can't locate resource section in " TARGET_MAIN_CLR_DLL_NAME_A);
return CORDBG_E_MISSING_DEBUGGER_EXPORTS;
}

if (FAILED(status = GetResourceRvaFromResourceSectionRvaByName(m_pTarget, m_globalBase,
resourceSectionRVA, (DWORD)(size_t)RT_RCDATA, _WIDE(DACCESS_TABLE_RESOURCE), 0,
&rsrcRVA, &rsrcSize)))
{
_ASSERTE_MSG(false, "DAC fatal error: can't locate DAC table resource in " TARGET_MAIN_CLR_DLL_NAME_A);
return CORDBG_E_MISSING_DEBUGGER_EXPORTS;
}

rsrcData = new (nothrow) BYTE[rsrcSize];
if (rsrcData == NULL)
return E_OUTOFMEMORY;

if (FAILED(status = ReadFromDataTarget(m_pTarget, m_globalBase + rsrcRVA, (BYTE*)rsrcData, rsrcSize)))
{
_ASSERTE_MSG(false, "DAC fatal error: can't load DAC table resource from " TARGET_MAIN_CLR_DLL_NAME_A);
return CORDBG_E_MISSING_DEBUGGER_EXPORTS;
}


PBYTE rawData = (PBYTE)rsrcData;
DWORD bytesLeft = rsrcSize;

// Read the header
struct DacTableHeader header;

// We currently expect the header to be 2 32-bit values and 1 16-byte value,
// make sure there is no packing going on or anything.
static_assert_no_msg(sizeof(DacTableHeader) == 2 * 4 + 16);

if (bytesLeft < sizeof(DacTableHeader))
{
_ASSERTE_MSG(false, "DAC fatal error: DAC table too small for header.");
goto Exit;
}
memcpy(&header, rawData, sizeof(DacTableHeader));
rawData += sizeof(DacTableHeader);
bytesLeft -= sizeof(DacTableHeader);

// Save the table info for later use
g_dacTableInfo = header.info;

// Sanity check that the DAC table is the size we expect.
// This could fail if a different version of dacvars.h or vptr_list.h was used when building
// mscordacwks.dll than when running DacTableGen.

if (offsetof(DacGlobals, EEJitManager__vtAddr) != header.numGlobals * sizeof(ULONG))
{
#ifdef _DEBUG
char szMsgBuf[1024];
_snprintf_s(szMsgBuf, sizeof(szMsgBuf), _TRUNCATE,
"DAC fatal error: mismatch in number of globals in DAC table. Read from file: %d, expected: %zd.",
header.numGlobals,
(size_t)offsetof(DacGlobals, EEJitManager__vtAddr) / sizeof(ULONG));
_ASSERTE_MSG(false, szMsgBuf);
#endif // _DEBUG

status = E_INVALIDARG;
goto Exit;
}

if (sizeof(DacGlobals) != (header.numGlobals + header.numVptrs) * sizeof(ULONG))
{
#ifdef _DEBUG
char szMsgBuf[1024];
_snprintf_s(szMsgBuf, sizeof(szMsgBuf), _TRUNCATE,
"DAC fatal error: mismatch in number of vptrs in DAC table. Read from file: %d, expected: %zd.",
header.numVptrs,
(size_t)(sizeof(DacGlobals) - offsetof(DacGlobals, EEJitManager__vtAddr)) / sizeof(ULONG));
_ASSERTE_MSG(false, szMsgBuf);
#endif // _DEBUG

status = E_INVALIDARG;
goto Exit;
}

// Copy the DAC table into g_dacGlobals
if (bytesLeft < sizeof(DacGlobals))
{
_ASSERTE_MSG(false, "DAC fatal error: DAC table resource too small for DacGlobals.");
status = E_UNEXPECTED;
goto Exit;
}
memcpy(&g_dacGlobals, rawData, sizeof(DacGlobals));
rawData += sizeof(DacGlobals);
bytesLeft -= sizeof(DacGlobals);

status = S_OK;

Exit:

return status;
#endif
}

#undef MAKEINTRESOURCE

//----------------------------------------------------------------------------
//
// IsExceptionFromManagedCode - report if pExceptionRecord points to an exception belonging to the current runtime
Expand Down
Loading