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

Fix conflicting TADDR definition #98925

Merged
merged 8 commits into from
Feb 29, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
4 changes: 2 additions & 2 deletions src/coreclr/debug/daccess/dacdbiimpl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -7455,13 +7455,13 @@ HRESULT DacDbiInterfaceImpl::GetILCodeVersionNodeData(VMPTR_ILCodeVersionNode vm
#ifdef FEATURE_REJIT
ILCodeVersion ilCode(vmILCodeVersionNode.GetDacPtr());
pData->m_state = ilCode.GetRejitState();
pData->m_pbIL = PTR_TO_CORDB_ADDRESS(dac_cast<ULONG_PTR>(ilCode.GetIL()));
pData->m_pbIL = PTR_TO_CORDB_ADDRESS(dac_cast<TADDR>(ilCode.GetIL()));
pData->m_dwCodegenFlags = ilCode.GetJitFlags();
const InstrumentedILOffsetMapping* pMapping = ilCode.GetInstrumentedILMap();
if (pMapping)
{
pData->m_cInstrumentedMapEntries = (ULONG)pMapping->GetCount();
pData->m_rgInstrumentedMapEntries = PTR_TO_CORDB_ADDRESS(dac_cast<ULONG_PTR>(pMapping->GetOffsets()));
pData->m_rgInstrumentedMapEntries = PTR_TO_CORDB_ADDRESS(dac_cast<TADDR>(pMapping->GetOffsets()));
}
else
{
Expand Down
6 changes: 5 additions & 1 deletion src/coreclr/debug/daccess/request.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -3810,9 +3810,13 @@ ClrDataAccess::GetJumpThunkTarget(T_CONTEXT *ctx, CLRDATA_ADDRESS *targetIP, CLR
#ifdef TARGET_AMD64
SOSDacEnter();

if (!GetAnyThunkTarget(ctx, targetIP, targetMD))
TADDR tempTargetIP, tempTargetMD;
if (!GetAnyThunkTarget(ctx, &tempTargetIP, &tempTargetMD))
hr = E_FAIL;

*targetIP = TO_CDADDR(tempTargetIP);
*targetMD = TO_CDADDR(tempTargetMD);

jkotas marked this conversation as resolved.
Show resolved Hide resolved
SOSDacLeave();
return hr;
#else
Expand Down
2 changes: 1 addition & 1 deletion src/coreclr/debug/ee/controller.h
Original file line number Diff line number Diff line change
Expand Up @@ -827,7 +827,7 @@ class DebuggerPatchTable : private CHashTableAndData<CNewZeroData>
DebuggerControllerPatch * GetPatch(PTR_CORDB_ADDRESS_TYPE address)
{
SUPPORTS_DAC;
ARM_ONLY(_ASSERTE(dac_cast<DWORD>(address) & THUMB_CODE));
ARM_ONLY(_ASSERTE(dac_cast<TADDR>(address) & THUMB_CODE));

DebuggerControllerPatch * pPatch =
dac_cast<PTR_DebuggerControllerPatch>(Find(HashAddress(address), (SIZE_T)(dac_cast<TADDR>(address))));
Expand Down
6 changes: 3 additions & 3 deletions src/coreclr/debug/inc/arm64/primitives.h
Original file line number Diff line number Diff line change
Expand Up @@ -153,9 +153,9 @@ inline void CORDbgSetInstruction(CORDB_ADDRESS_TYPE* address,
#if !defined(DBI_COMPILE) && !defined(DACCESS_COMPILE) && defined(HOST_OSX)
ExecutableWriterHolder<void> instructionWriterHolder((LPVOID)address, sizeof(PRD_TYPE));

ULONGLONG ptraddr = dac_cast<ULONGLONG>(instructionWriterHolder.GetRW());
TADDR ptraddr = dac_cast<TADDR>(instructionWriterHolder.GetRW());
#else // !DBI_COMPILE && !DACCESS_COMPILE && HOST_OSX
ULONGLONG ptraddr = dac_cast<ULONGLONG>(address);
TADDR ptraddr = dac_cast<TADDR>(address);
#endif // !DBI_COMPILE && !DACCESS_COMPILE && HOST_OSX
*(PRD_TYPE *)ptraddr = instruction;
FlushInstructionCache(GetCurrentProcess(),
Expand All @@ -167,7 +167,7 @@ inline PRD_TYPE CORDbgGetInstruction(UNALIGNED CORDB_ADDRESS_TYPE* address)
{
LIMITED_METHOD_CONTRACT;

ULONGLONG ptraddr = dac_cast<ULONGLONG>(address);
TADDR ptraddr = dac_cast<TADDR>(address);
return *(PRD_TYPE *)ptraddr;
}

Expand Down
4 changes: 2 additions & 2 deletions src/coreclr/debug/inc/loongarch64/primitives.h
Original file line number Diff line number Diff line change
Expand Up @@ -135,7 +135,7 @@ inline void CORDbgSetInstruction(CORDB_ADDRESS_TYPE* address,
// In a DAC build, this function assumes the input is an host address.
LIMITED_METHOD_DAC_CONTRACT;

ULONGLONG ptraddr = dac_cast<ULONGLONG>(address);
TADDR ptraddr = dac_cast<TADDR>(address);
*(PRD_TYPE *)ptraddr = instruction;
FlushInstructionCache(GetCurrentProcess(),
address,
Expand All @@ -146,7 +146,7 @@ inline PRD_TYPE CORDbgGetInstruction(UNALIGNED CORDB_ADDRESS_TYPE* address)
{
LIMITED_METHOD_CONTRACT;

ULONGLONG ptraddr = dac_cast<ULONGLONG>(address);
TADDR ptraddr = dac_cast<TADDR>(address);
return *(PRD_TYPE *)ptraddr;
}

Expand Down
4 changes: 2 additions & 2 deletions src/coreclr/debug/inc/riscv64/primitives.h
Original file line number Diff line number Diff line change
Expand Up @@ -137,7 +137,7 @@ inline void CORDbgSetInstruction(CORDB_ADDRESS_TYPE* address,
// In a DAC build, this function assumes the input is an host address.
LIMITED_METHOD_DAC_CONTRACT;

ULONGLONG ptraddr = dac_cast<ULONGLONG>(address);
TADDR ptraddr = dac_cast<TADDR>(address);
*(PRD_TYPE *)ptraddr = instruction;
FlushInstructionCache(GetCurrentProcess(),
address,
Expand All @@ -148,7 +148,7 @@ inline PRD_TYPE CORDbgGetInstruction(UNALIGNED CORDB_ADDRESS_TYPE* address)
{
LIMITED_METHOD_CONTRACT;

ULONGLONG ptraddr = dac_cast<ULONGLONG>(address);
TADDR ptraddr = dac_cast<TADDR>(address);
return *(PRD_TYPE *)ptraddr;
}

Expand Down
12 changes: 12 additions & 0 deletions src/coreclr/inc/check.h
Original file line number Diff line number Diff line change
Expand Up @@ -684,13 +684,19 @@ CHECK CheckAligned(UINT value, UINT alignment);
CHECK CheckAligned(ULONG value, UINT alignment);
#endif
CHECK CheckAligned(UINT64 value, UINT alignment);
#ifdef __APPLE__
CHECK CheckAligned(SIZE_T value, UINT alignment);
#endif
jkotas marked this conversation as resolved.
Show resolved Hide resolved
CHECK CheckAligned(const void *address, UINT alignment);

CHECK CheckOverflow(UINT value1, UINT value2);
#if defined(_MSC_VER)
CHECK CheckOverflow(ULONG value1, ULONG value2);
#endif
CHECK CheckOverflow(UINT64 value1, UINT64 value2);
#ifdef __APPLE__
CHECK CheckOverflow(SIZE_T value1, SIZE_T value2);
#endif
CHECK CheckOverflow(PTR_CVOID address, UINT offset);
#if defined(_MSC_VER)
CHECK CheckOverflow(const void *address, ULONG offset);
Expand All @@ -702,11 +708,17 @@ CHECK CheckUnderflow(UINT value1, UINT value2);
CHECK CheckUnderflow(ULONG value1, ULONG value2);
#endif
CHECK CheckUnderflow(UINT64 value1, UINT64 value2);
#ifdef __APPLE__
CHECK CheckUnderflow(SIZE_T value1, SIZE_T value2);
#endif
CHECK CheckUnderflow(const void *address, UINT offset);
#if defined(_MSC_VER)
CHECK CheckUnderflow(const void *address, ULONG offset);
#endif
CHECK CheckUnderflow(const void *address, UINT64 offset);
#ifdef __APPLE__
CHECK CheckUnderflow(const void *address, SIZE_T offset);
#endif
CHECK CheckUnderflow(const void *address, void *address2);

CHECK CheckZeroedMemory(const void *memory, SIZE_T size);
Expand Down
40 changes: 40 additions & 0 deletions src/coreclr/inc/check.inl
Original file line number Diff line number Diff line change
Expand Up @@ -156,6 +156,15 @@ inline CHECK CheckAligned(UINT64 value, UINT alignment)
CHECK_OK;
}

#ifdef __APPLE__
inline CHECK CheckAligned(SIZE_T value, UINT alignment)
{
STATIC_CONTRACT_WRAPPER;
CHECK(AlignmentTrim(value, alignment) == 0);
CHECK_OK;
}
#endif

inline CHECK CheckAligned(const void *address, UINT alignment)
{
STATIC_CONTRACT_WRAPPER;
Expand Down Expand Up @@ -183,6 +192,14 @@ inline CHECK CheckOverflow(UINT64 value1, UINT64 value2)
CHECK_OK;
}

#ifdef __APPLE__
inline CHECK CheckOverflow(SIZE_T value1, SIZE_T value2)
{
CHECK(value1 + value2 >= value1);
CHECK_OK;
}
#endif

inline CHECK CheckOverflow(PTR_CVOID address, UINT offset)
{
TADDR targetAddr = dac_cast<TADDR>(address);
Expand Down Expand Up @@ -254,6 +271,15 @@ inline CHECK CheckUnderflow(UINT64 value1, UINT64 value2)
CHECK_OK;
}

#ifdef __APPLE__
inline CHECK CheckUnderflow(SIZE_T value1, SIZE_T value2)
{
CHECK(value1 - value2 <= value1);

CHECK_OK;
}
#endif

inline CHECK CheckUnderflow(const void *address, UINT offset)
{
#if POINTER_BITS == 32
Expand Down Expand Up @@ -290,6 +316,20 @@ inline CHECK CheckUnderflow(const void *address, UINT64 offset)
CHECK_OK;
}

#ifdef __APPLE__
inline CHECK CheckUnderflow(const void *address, SIZE_T offset)
{
#if POINTER_BITS == 32
CHECK(offset >> 32 == 0);
CHECK((UINT) (SIZE_T) address - (UINT) offset <= (UINT) (SIZE_T) address);
#else
CHECK((UINT64) address - offset <= (UINT64) address);
#endif

CHECK_OK;
}
#endif

inline CHECK CheckUnderflow(const void *address, void *address2)
{
#if POINTER_BITS == 32
Expand Down
4 changes: 2 additions & 2 deletions src/coreclr/inc/clr_std/type_traits
Original file line number Diff line number Diff line change
Expand Up @@ -358,7 +358,7 @@ namespace std

// On Unix 'long' is a 64-bit type (same as __int64) and the following two definitions
// conflict with _Is_integral<unsigned __int64> and _Is_integral<signed __int64>.
#ifndef HOST_UNIX
#if !defined(HOST_UNIX) || defined(__APPLE__)
template<>
struct _Is_integral<unsigned long>
: true_type
Expand All @@ -370,7 +370,7 @@ namespace std
: true_type
{ // determine whether _Ty is integral
};
#endif /* HOST_UNIX */
#endif /* !HOST_UNIX || __APPLE__ */

#if _HAS_CHAR16_T_LANGUAGE_SUPPORT
template<>
Expand Down
9 changes: 9 additions & 0 deletions src/coreclr/inc/clrtypes.h
Original file line number Diff line number Diff line change
Expand Up @@ -370,6 +370,15 @@ inline UINT64 AlignDown(UINT64 value, UINT alignment)
return (value&~(UINT64)(alignment-1));
}

#ifdef __APPLE__
inline SIZE_T AlignDown(SIZE_T value, UINT alignment)
{
STATIC_CONTRACT_LEAF;
STATIC_CONTRACT_SUPPORTS_DAC;
return (value&~(SIZE_T)(alignment-1));
}
#endif // __APPLE__

inline UINT AlignmentPad(UINT value, UINT alignment)
{
STATIC_CONTRACT_WRAPPER;
Expand Down
5 changes: 2 additions & 3 deletions src/coreclr/inc/daccess.h
Original file line number Diff line number Diff line change
Expand Up @@ -614,8 +614,7 @@ struct DacTableHeader
// Define TADDR as a non-pointer value so use of it as a pointer
// will not work properly. Define it as unsigned so
// pointer comparisons aren't affected by sign.
// This requires special casting to ULONG64 to sign-extend if necessary.
typedef ULONG_PTR TADDR;
typedef uintptr_t TADDR;

// TSIZE_T used for counts or ranges that need to span the size of a
// target pointer. For cross-plat, this may be different than SIZE_T
Expand Down Expand Up @@ -2128,7 +2127,7 @@ inline void DACCOP_IGNORE(DacCopWarningCode code, const char * szReasonString)
// Declare TADDR as a non-pointer type so that arithmetic
// can be done on it directly, as with the DACCESS_COMPILE definition.
// This also helps expose pointer usage that may need to be changed.
typedef ULONG_PTR TADDR;
typedef uintptr_t TADDR;

typedef void* PTR_VOID;
typedef LPVOID* PTR_PTR_VOID;
Expand Down
2 changes: 1 addition & 1 deletion src/coreclr/inc/regdisp.h
Original file line number Diff line number Diff line change
Expand Up @@ -318,7 +318,7 @@ struct REGDISPLAY : public REGDISPLAY_BASE {
memset(this, 0, sizeof(REGDISPLAY));

// Setup the pointer to ControlPC field
pPC = &ControlPC;
pPC = (DWORD *)&ControlPC;
}
};

Expand Down
2 changes: 1 addition & 1 deletion src/coreclr/vm/amd64/cgenamd64.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -206,7 +206,7 @@ void HelperMethodFrame::UpdateRegDisplay(const PREGDISPLAY pRD, bool updateFloat

#endif // TARGET_UNIX

#define CALLEE_SAVED_REGISTER(regname) pRD->pCurrentContextPointers->regname = m_MachState.m_Ptrs.p##regname;
#define CALLEE_SAVED_REGISTER(regname) pRD->pCurrentContextPointers->regname = (DWORD64 *)(TADDR *)m_MachState.m_Ptrs.p##regname;
Copy link
Member Author

Choose a reason for hiding this comment

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

This feels pretty ugly, but uint64_t (& DWORD64) is defined as unsigned long long and uintptr_t (& TADDR) is defined as unsigned long, at least on osx-x64.

Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
#define CALLEE_SAVED_REGISTER(regname) pRD->pCurrentContextPointers->regname = (DWORD64 *)(TADDR *)m_MachState.m_Ptrs.p##regname;
#define CALLEE_SAVED_REGISTER(regname) pRD->pCurrentContextPointers->regname = (DWORD64 *)m_MachState.m_Ptrs.p##regname;

Is this enough?

Copy link
Member Author

Choose a reason for hiding this comment

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

Nope, doesn’t work. It’s typed as the __DPtr class and it doesn’t have the cast operator to make that work.

Copy link
Member Author

Choose a reason for hiding this comment

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

osx-x64 with single cast:

  /Users/filipnavara/Projects/runtime/src/coreclr/vm/amd64/cgenamd64.cpp:210:5: error: cannot cast from type 'PTR_TADDR' (aka '__DPtr<unsigned long>') to pointer type 'DWORD64 *' (aka 'unsigned long long *')
      ENUM_CALLEE_SAVED_REGISTERS();
      ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~
  /Users/filipnavara/Projects/runtime/src/coreclr/vm/amd64/cgencpu.h:182:5: note: expanded from macro 'ENUM_CALLEE_SAVED_REGISTERS'
      CALLEE_SAVED_REGISTER(R12) \
      ^~~~~~~~~~~~~~~~~~~~~~~~~~
  /Users/filipnavara/Projects/runtime/src/coreclr/vm/amd64/cgenamd64.cpp:209:80: note: expanded from macro 'CALLEE_SAVED_REGISTER'

ENUM_CALLEE_SAVED_REGISTERS();
#undef CALLEE_SAVED_REGISTER

Expand Down
2 changes: 1 addition & 1 deletion src/coreclr/vm/amd64/gmsamd64.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -136,7 +136,7 @@ void LazyMachState::unwindLazyState(LazyMachState* baseState,

#else // !DACCESS_COMPILE

#define CALLEE_SAVED_REGISTER(regname) unwoundState->m_Ptrs.p##regname = PTR_ULONG64(nonVolRegPtrs.regname);
#define CALLEE_SAVED_REGISTER(regname) unwoundState->m_Ptrs.p##regname = PTR_TADDR(nonVolRegPtrs.regname);
ENUM_CALLEE_SAVED_REGISTERS();
#undef CALLEE_SAVED_REGISTER

Expand Down
2 changes: 1 addition & 1 deletion src/coreclr/vm/arm/stubs.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1619,7 +1619,7 @@ void InlinedCallFrame::UpdateRegDisplay(const PREGDISPLAY pRD, bool updateFloats

// Update the frame pointer in the current context.
pRD->pCurrentContext->R11 = m_pCalleeSavedFP;
pRD->pCurrentContextPointers->R11 = &m_pCalleeSavedFP;
pRD->pCurrentContextPointers->R11 = (DWORD *)&m_pCalleeSavedFP;

// This is necessary to unwind methods with alloca. This needs to stay
// in sync with definition of REG_SAVED_LOCALLOC_SP in the JIT.
Expand Down
2 changes: 1 addition & 1 deletion src/coreclr/vm/arm64/stubs.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -731,7 +731,7 @@ void InlinedCallFrame::UpdateRegDisplay(const PREGDISPLAY pRD, bool updateFloats


// Update the frame pointer in the current context.
pRD->pCurrentContextPointers->Fp = &m_pCalleeSavedFP;
pRD->pCurrentContextPointers->Fp = (DWORD64 *)&m_pCalleeSavedFP;

LOG((LF_GCROOTS, LL_INFO100000, "STACKWALK InlinedCallFrame::UpdateRegDisplay(pc:%p, sp:%p)\n", pRD->ControlPC, pRD->SP));

Expand Down
2 changes: 1 addition & 1 deletion src/coreclr/vm/ceeload.inl
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ inline
void LookupMap<SIZE_T>::SetValueAt(PTR_TADDR pValue, SIZE_T value, TADDR flags)
{
WRAPPER_NO_CONTRACT;
VolatileStore(pValue, value | flags);
VolatileStore(pValue, dac_cast<TADDR>(value | flags));
}
#endif // DACCESS_COMPILE

Expand Down