From d174828a3158a9941ebb5a2776712237f49f9067 Mon Sep 17 00:00:00 2001 From: David Wrighton Date: Wed, 4 Feb 2026 13:16:18 -0800 Subject: [PATCH 1/9] Progress --- src/coreclr/interpreter/CMakeLists.txt | 1 + src/coreclr/interpreter/compiler.cpp | 164 ++++++++++++++++-- src/coreclr/interpreter/compiler.h | 46 ++--- src/coreclr/interpreter/eeinterp.cpp | 23 +-- src/coreclr/interpreter/interpalloc.h | 19 +++ src/coreclr/interpreter/interpmemkind.h | 1 + src/coreclr/interpreter/interpmethoddata.cpp | 166 +++++++++++++++++++ src/coreclr/interpreter/interpmethoddata.h | 146 ++++++++++++++++ 8 files changed, 515 insertions(+), 51 deletions(-) create mode 100644 src/coreclr/interpreter/interpmethoddata.cpp create mode 100644 src/coreclr/interpreter/interpmethoddata.h diff --git a/src/coreclr/interpreter/CMakeLists.txt b/src/coreclr/interpreter/CMakeLists.txt index f6c4884a744f2a..656a6dc2252714 100644 --- a/src/coreclr/interpreter/CMakeLists.txt +++ b/src/coreclr/interpreter/CMakeLists.txt @@ -5,6 +5,7 @@ set(INTERPRETER_SOURCES compileropt.cpp intops.cpp interpconfig.cpp + interpmethoddata.cpp eeinterp.cpp stackmap.cpp naming.cpp diff --git a/src/coreclr/interpreter/compiler.cpp b/src/coreclr/interpreter/compiler.cpp index cbd7d7d6393712..b55cbc63fecc82 100644 --- a/src/coreclr/interpreter/compiler.cpp +++ b/src/coreclr/interpreter/compiler.cpp @@ -277,12 +277,11 @@ void InterpCompiler::CopyToInterpGenericLookup(InterpGenericLookup* dst, const C assert(!src->indirectSecondOffset); } -// Interpreter-FIXME Use specific allocators for their intended purpose -// Allocator for data that is kept alive throughout application execution, -// being freed only if the associated method gets freed. +// Temporary allocator for method data during compilation. +// Data allocated here is copied to the final unified allocation during FinalizeMethodData. void* InterpCompiler::AllocMethodData(size_t numBytes) { - return malloc(numBytes); + return getAllocator(IMK_MethodData).allocate(numBytes); } static int GetDataLen(int opcode) @@ -1850,23 +1849,46 @@ void InterpCompiler::BuildEHInfo() } } -InterpMethod* InterpCompiler::CreateInterpMethod() +void InterpCompiler::PrepareInterpMethod() { - int numDataItems = m_dataItems.GetSize(); - void **pDataItems = (void**)AllocMethodData(numDataItems * sizeof(void*)); + // Store method data for later finalization + m_initLocals = (m_methodInfo->options & CORINFO_OPT_INIT_LOCALS) != 0; + m_unmanagedCallersOnly = m_corJitFlags.IsSet(CORJIT_FLAGS::CORJIT_FLAG_REVERSE_PINVOKE); + m_publishSecretStubParam = m_corJitFlags.IsSet(CORJIT_FLAGS::CORJIT_FLAG_PUBLISH_SECRET_PARAM); - for (int i = 0; i < numDataItems; i++) - pDataItems[i] = m_dataItems.Get(i); + // Reserve space in the builder for each section + // Bytecode section + m_methodDataBuilder.SetBytecodeSize(m_methodCodeSize * sizeof(int32_t)); - bool initLocals = (m_methodInfo->options & CORINFO_OPT_INIT_LOCALS) != 0; + // InterpMethod section + m_methodDataBuilder.AllocateInterpMethod(); - bool unmanagedCallersOnly = m_corJitFlags.IsSet(CORJIT_FLAGS::CORJIT_FLAG_REVERSE_PINVOKE); - bool publishSecretStubParam = m_corJitFlags.IsSet(CORJIT_FLAGS::CORJIT_FLAG_PUBLISH_SECRET_PARAM); + // DataItems section + int numDataItems = m_dataItems.GetSize(); + if (numDataItems > 0) + { + m_methodDataBuilder.AllocateDataItems(numDataItems); + } - void* pMethodData = AllocMethodData(sizeof(InterpMethod)); - InterpMethod *pMethod = new (pMethodData) InterpMethod(m_methodHnd, m_ILLocalsOffset, m_totalVarsStackSize, pDataItems, initLocals, unmanagedCallersOnly, publishSecretStubParam); + // AsyncSuspendData section - reserve space for all async suspend data + for (int32_t i = 0; i < m_asyncSuspendDataItems.GetSize(); i++) + { + m_methodDataBuilder.AllocateAsyncSuspendData(); + } - return pMethod; + // IntervalMaps section - reserve space tracked via m_varIntervalMaps + for (int32_t i = 0; i < m_varIntervalMaps.GetSize(); i++) + { + // Count entries in this interval map (terminated by entry with countBytes == 0) + InterpIntervalMapEntry* pMap = *m_varIntervalMaps.Get(i); + int32_t count = 0; + while (pMap[count].countBytes != 0) + { + count++; + } + count++; // Include the terminator entry + m_methodDataBuilder.AllocateIntervalMap(count); + } } int32_t* InterpCompiler::GetCode(int32_t *pCodeSize) @@ -1875,6 +1897,107 @@ int32_t* InterpCompiler::GetCode(int32_t *pCodeSize) return m_pMethodCode; } +uint32_t InterpCompiler::GetTotalAllocationSize() +{ + return m_methodDataBuilder.GetTotalSize(); +} + +InterpMethod* InterpCompiler::FinalizeMethodData(void* baseAddressRW, void* baseAddressRX) +{ + uint8_t* rwBase = (uint8_t*)baseAddressRW; + uint8_t* rxBase = (uint8_t*)baseAddressRX; + + // Get section offsets + uint32_t headerOffset = m_methodDataBuilder.GetSectionOffset(InterpMethodDataSection::Header); + uint32_t bytecodeOffset = m_methodDataBuilder.GetSectionOffset(InterpMethodDataSection::Bytecode); + uint32_t interpMethodOffset = m_methodDataBuilder.GetSectionOffset(InterpMethodDataSection::InterpMethod); + uint32_t dataItemsOffset = m_methodDataBuilder.GetSectionOffset(InterpMethodDataSection::DataItems); + uint32_t asyncSuspendDataOffset = m_methodDataBuilder.GetSectionOffset(InterpMethodDataSection::AsyncSuspendData); + uint32_t intervalMapsOffset = m_methodDataBuilder.GetSectionOffset(InterpMethodDataSection::IntervalMaps); + + // Copy bytecode + memcpy(rwBase + bytecodeOffset, m_pMethodCode, m_methodCodeSize * sizeof(int32_t)); + + // Calculate data items pointer in final allocation + int numDataItems = m_dataItems.GetSize(); + void** pDataItems = (numDataItems > 0) ? (void**)(rxBase + dataItemsOffset) : nullptr; + + // Copy data items + if (numDataItems > 0) + { + void** pDataItemsRW = (void**)(rwBase + dataItemsOffset); + for (int i = 0; i < numDataItems; i++) + { + pDataItemsRW[i] = m_dataItems.Get(i); + } + } + + // Construct InterpMethod in the final allocation + InterpMethod* pMethodRW = (InterpMethod*)(rwBase + interpMethodOffset); + InterpMethod* pMethodRX = (InterpMethod*)(rxBase + interpMethodOffset); + new (pMethodRW) InterpMethod(m_methodHnd, m_ILLocalsOffset, m_totalVarsStackSize, pDataItems, + m_initLocals, m_unmanagedCallersOnly, m_publishSecretStubParam); + + // Copy async suspend data and fix up pointers + uint32_t currentAsyncOffset = asyncSuspendDataOffset; + uint32_t currentIntervalMapOffset = intervalMapsOffset; + + InterpByteCodeStart* pByteCodeStart = (InterpByteCodeStart*)rxBase; + + for (int32_t i = 0; i < m_asyncSuspendDataItems.GetSize(); i++) + { + InterpAsyncSuspendData* srcData = m_asyncSuspendDataItems.Get(i); + InterpAsyncSuspendData* dstDataRW = (InterpAsyncSuspendData*)(rwBase + currentAsyncOffset); + + // Copy the struct + memcpy(dstDataRW, srcData, sizeof(InterpAsyncSuspendData)); + + // Fix up the methodStartIP to point to the final bytecode start + dstDataRW->methodStartIP = pByteCodeStart; + + // Fix up interval map pointers if they exist + // Note: The interval maps were allocated via AllocMethodData in the old model, + // we need to copy them to the new allocation and fix up the pointers + if (srcData->liveLocalsIntervals != nullptr) + { + // Count entries + int32_t count = 0; + while (srcData->liveLocalsIntervals[count].countBytes != 0) count++; + count++; // Include terminator + + InterpIntervalMapEntry* dstMapRW = (InterpIntervalMapEntry*)(rwBase + currentIntervalMapOffset); + InterpIntervalMapEntry* dstMapRX = (InterpIntervalMapEntry*)(rxBase + currentIntervalMapOffset); + memcpy(dstMapRW, srcData->liveLocalsIntervals, count * sizeof(InterpIntervalMapEntry)); + dstDataRW->liveLocalsIntervals = dstMapRX; + currentIntervalMapOffset += count * sizeof(InterpIntervalMapEntry); + } + + if (srcData->zeroedLocalsIntervals != nullptr) + { + // Count entries + int32_t count = 0; + while (srcData->zeroedLocalsIntervals[count].countBytes != 0) count++; + count++; // Include terminator + + InterpIntervalMapEntry* dstMapRW = (InterpIntervalMapEntry*)(rwBase + currentIntervalMapOffset); + InterpIntervalMapEntry* dstMapRX = (InterpIntervalMapEntry*)(rxBase + currentIntervalMapOffset); + memcpy(dstMapRW, srcData->zeroedLocalsIntervals, count * sizeof(InterpIntervalMapEntry)); + dstDataRW->zeroedLocalsIntervals = dstMapRX; + currentIntervalMapOffset += count * sizeof(InterpIntervalMapEntry); + } + + currentAsyncOffset += sizeof(InterpAsyncSuspendData); + } + + // Write the InterpMethod pointer to the header (InterpByteCodeStart) + *(InterpMethod**)(rwBase + headerOffset) = pMethodRX; + + // Apply any additional relocations tracked by the builder + m_methodDataBuilder.Finalize(baseAddressRW, baseAddressRX); + + return pMethodRX; +} + InterpreterStackMap* InterpCompiler::GetInterpreterStackMap(CORINFO_CLASS_HANDLE classHandle) { InterpreterStackMap* result = nullptr; @@ -1895,6 +2018,7 @@ InterpCompiler::InterpCompiler(COMP_HANDLE compHnd, CORINFO_METHOD_INFO* methodInfo, InterpreterRetryData* pRetryData, InterpArenaAllocator *arenaAllocator) : m_arenaAllocator(arenaAllocator) + , m_methodDataBuilder(GetMemPoolAllocator(IMK_MethodData)) , m_stackmapsByClass(getAllocator(IMK_StackMapHash)) , m_pRetryData(pRetryData) , m_pInitLocalsIns(nullptr) @@ -1904,6 +2028,9 @@ InterpCompiler::InterpCompiler(COMP_HANDLE compHnd, , m_leavesTable(GetMemPoolAllocator(IMK_EHClause)) , m_dataItems(GetMemPoolAllocator(IMK_DataItem)) , m_asyncSuspendDataItems(GetMemPoolAllocator(IMK_DataItem)) + , m_initLocals(false) + , m_unmanagedCallersOnly(false) + , m_publishSecretStubParam(false) , m_globalVarsWithRefsStackTop(0) , m_varIntervalMaps(GetMemPoolAllocator(IMK_IntervalMap)) #ifdef DEBUG @@ -1943,7 +2070,7 @@ InterpCompiler::~InterpCompiler() m_compHnd->freeArray(m_pILToNativeMap); } -InterpMethod* InterpCompiler::CompileMethod() +bool InterpCompiler::CompileMethod() { #ifdef DEBUG if (IsInterpDumpActive() || InterpConfig.InterpList()) @@ -1981,7 +2108,7 @@ InterpMethod* InterpCompiler::CompileMethod() if (m_pRetryData->NeedsRetry()) { INTERP_DUMP("Retrying compilation due to %s\n", m_pRetryData->GetReasonString()); - return nullptr; + return false; } #ifdef DEBUG @@ -2008,7 +2135,8 @@ InterpMethod* InterpCompiler::CompileMethod() } #endif - return CreateInterpMethod(); + PrepareInterpMethod(); + return true; } void InterpCompiler::PatchInitLocals(CORINFO_METHOD_INFO* methodInfo) diff --git a/src/coreclr/interpreter/compiler.h b/src/coreclr/interpreter/compiler.h index ff41961e985256..bcdef2f9f1ea4f 100644 --- a/src/coreclr/interpreter/compiler.h +++ b/src/coreclr/interpreter/compiler.h @@ -12,6 +12,7 @@ #include "simdhash.h" #include "intrinsics.h" #include "interpalloc.h" +#include "interpmethoddata.h" struct InterpException { @@ -27,21 +28,6 @@ struct InterpException class InterpreterStackMap; class InterpCompiler; -// MemPoolAllocator provides an allocator interface for use with TArray and other -// data structures. It wraps the InterpCompiler's arena allocator with a specific -// memory kind for statistics tracking. -class MemPoolAllocator -{ - InterpCompiler* const m_compiler; - InterpMemKind m_memKind; -public: - MemPoolAllocator(InterpCompiler* compiler, InterpMemKind memKind) - : m_compiler(compiler), m_memKind(memKind) {} - void* Alloc(size_t sz) const; - void Free(void* ptr) const; - InterpMemKind getMemKind() const { return m_memKind; } -}; - class InterpDataItemIndexMap { struct VarSizedData @@ -625,6 +611,9 @@ class InterpCompiler // All memory allocated via AllocMemPool is freed when the compiler is destroyed. InterpArenaAllocator *m_arenaAllocator; + // Builder for the unified method data allocation + InterpMethodDataBuilder m_methodDataBuilder; + CORINFO_METHOD_HANDLE m_methodHnd; CORINFO_MODULE_HANDLE m_compScopeHnd; COMP_HANDLE m_compHnd; @@ -707,6 +696,11 @@ class InterpCompiler TArray m_asyncSuspendDataItems; + // Prepared InterpMethod data (stored temporarily until finalization) + bool m_initLocals; + bool m_unmanagedCallersOnly; + bool m_publishSecretStubParam; + InterpDataItemIndexMap m_genericLookupToDataItemIndex; int32_t GetDataItemIndex(void* data) { @@ -811,7 +805,7 @@ class InterpCompiler InterpAllocator getAllocatorInstruction() { return getAllocator(IMK_Instruction); } // Legacy allocation methods - use getAllocator() for new code - MemPoolAllocator GetMemPoolAllocator(InterpMemKind imk) { return MemPoolAllocator(this, imk); } + MemPoolAllocator GetMemPoolAllocator(InterpMemKind imk) { return MemPoolAllocator(getAllocator(imk)); } private: // Instructions @@ -1035,7 +1029,8 @@ class InterpCompiler int32_t* EmitBBCode(int32_t *ip, InterpBasicBlock *bb, TArray *relocs); int32_t* EmitCodeIns(int32_t *ip, InterpInst *pIns, TArray *relocs); void PatchRelocations(TArray *relocs); - InterpMethod* CreateInterpMethod(); + void PrepareInterpMethod(); + void UpdateWithFinalMethodByteCodeAddress(InterpByteCodeStart *pByteCodeStart); void CreateBasicBlocks(CORINFO_METHOD_INFO* methodInfo); void InitializeClauseBuildingBlocks(CORINFO_METHOD_INFO* methodInfo); void CreateLeaveChainIslandBasicBlocks(CORINFO_METHOD_INFO* methodInfo, int32_t leaveOffset, InterpBasicBlock* pLeaveTargetBB); @@ -1081,12 +1076,25 @@ class InterpCompiler InterpCompiler(COMP_HANDLE compHnd, CORINFO_METHOD_INFO* methodInfo, InterpreterRetryData *pretryData, InterpArenaAllocator *arenaAllocator); ~InterpCompiler(); - InterpMethod* CompileMethod(); + // Compile the method. Returns true on success, false if retry is needed. + bool CompileMethod(); + + // Get the total size needed for the unified method data allocation. + // Must be called after CompileMethod() succeeds. + uint32_t GetTotalAllocationSize(); + + // Finalize the method data into the provided allocation. + // baseAddressRW is the writable address, baseAddressRX is the executable address. + // Returns the InterpMethod pointer within the allocation. + InterpMethod* FinalizeMethodData(void* baseAddressRW, void* baseAddressRX); + void BuildGCInfo(InterpMethod *pInterpMethod); void BuildEHInfo(); - void UpdateWithFinalMethodByteCodeAddress(InterpByteCodeStart *pByteCodeStart); void dumpMethodMemStats(); + // Get the method data builder for additional customization + InterpMethodDataBuilder& GetMethodDataBuilder() { return m_methodDataBuilder; } + int32_t* GetCode(int32_t *pCodeSize); #if MEASURE_MEM_ALLOC diff --git a/src/coreclr/interpreter/eeinterp.cpp b/src/coreclr/interpreter/eeinterp.cpp index 09512b2934f9ac..c7569a9e103174 100644 --- a/src/coreclr/interpreter/eeinterp.cpp +++ b/src/coreclr/interpreter/eeinterp.cpp @@ -118,8 +118,8 @@ CorJitResult CILInterp::compileMethod(ICorJitInfo* compHnd, { retryData.StartCompilationAttempt(); InterpCompiler compiler(compHnd, methodInfo, &retryData, &arenaAllocator); - InterpMethod *pMethod = compiler.CompileMethod(); - if (pMethod == NULL) + bool success = compiler.CompileMethod(); + if (!success) { assert(retryData.NeedsRetry()); continue; @@ -128,15 +128,12 @@ CorJitResult CILInterp::compileMethod(ICorJitInfo* compHnd, // Once we reach here we will not attempt to retry again. assert(!retryData.NeedsRetry()); - int32_t IRCodeSize = 0; - int32_t *pIRCode = compiler.GetCode(&IRCodeSize); - - uint32_t sizeOfCode = sizeof(InterpMethod*) + IRCodeSize * sizeof(int32_t); - uint8_t unwindInfo[8] = {0, 0, 0, 0, 0, 0, 0, 0}; + // Get the total size needed for the unified method data allocation + uint32_t totalSize = compiler.GetTotalAllocationSize(); AllocMemChunk codeChunk {}; - codeChunk.alignment = 1; - codeChunk.size = sizeOfCode; + codeChunk.alignment = sizeof(void*); // Align to pointer size for InterpMethod access + codeChunk.size = totalSize; codeChunk.flags = CORJIT_ALLOCMEM_HOT_CODE; AllocMemArgs args {}; @@ -145,13 +142,11 @@ CorJitResult CILInterp::compileMethod(ICorJitInfo* compHnd, args.xcptnsCount = 0; compHnd->allocMem(&args); - // We store first the InterpMethod pointer as the code header, followed by the actual code - *(InterpMethod**)codeChunk.blockRW = pMethod; - memcpy ((uint8_t*)codeChunk.blockRW + sizeof(InterpMethod*), pIRCode, IRCodeSize * sizeof(int32_t)); + // Finalize all method data into the unified allocation + InterpMethod* pMethod = compiler.FinalizeMethodData(codeChunk.blockRW, codeChunk.block); - compiler.UpdateWithFinalMethodByteCodeAddress((InterpByteCodeStart*)codeChunk.block); *entryAddress = (uint8_t*)codeChunk.block; - *nativeSizeOfCode = sizeOfCode; + *nativeSizeOfCode = totalSize; // We can't do this until we've called allocMem compiler.BuildGCInfo(pMethod); diff --git a/src/coreclr/interpreter/interpalloc.h b/src/coreclr/interpreter/interpalloc.h index c487ef27a9ff5a..061b325029e489 100644 --- a/src/coreclr/interpreter/interpalloc.h +++ b/src/coreclr/interpreter/interpalloc.h @@ -54,4 +54,23 @@ using InterpArenaAllocator = ArenaAllocatorT; // It wraps ArenaAllocator and tracks allocations by InterpMemKind. using InterpAllocator = CompAllocatorT; +// MemPoolAllocator provides an allocator interface for use with TArray and other +// data structures. It wraps the InterpCompiler's arena allocator with a specific +// memory kind for statistics tracking. +class MemPoolAllocator +{ + InterpAllocator m_allocator; +public: + MemPoolAllocator(InterpAllocator allocator) + : m_allocator(allocator) {} + void* Alloc(size_t sz) const + { + if (sz == 0) + sz = 1; // TArray expects non-zero allocations + return ((InterpAllocator*)&m_allocator)->allocate(sz); + } + void Free(void* ptr) const + { /* no-op */ } +}; + #endif // _INTERPALLOC_H_ diff --git a/src/coreclr/interpreter/interpmemkind.h b/src/coreclr/interpreter/interpmemkind.h index 0390b327ee9710..b40ef53c75f4db 100644 --- a/src/coreclr/interpreter/interpmemkind.h +++ b/src/coreclr/interpreter/interpmemkind.h @@ -27,6 +27,7 @@ InterpMemKindMacro(ILCode) // IL code buffers InterpMemKindMacro(InterpCode) // Interpreter bytecode InterpMemKindMacro(Instruction) // InterpInst allocations InterpMemKindMacro(IntervalMap) // Variable interval maps +InterpMemKindMacro(MethodData) // Temporary method data during compilation InterpMemKindMacro(NativeToILMapping) // Native to IL offset mappings InterpMemKindMacro(Reloc) // Relocations InterpMemKindMacro(RetryData) // Data for retrying compilation diff --git a/src/coreclr/interpreter/interpmethoddata.cpp b/src/coreclr/interpreter/interpmethoddata.cpp new file mode 100644 index 00000000000000..72425c1dc69827 --- /dev/null +++ b/src/coreclr/interpreter/interpmethoddata.cpp @@ -0,0 +1,166 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +#include "interpreter.h" +#include "interpmethoddata.h" + +uint32_t InterpMethodDataBuilder::AlignUp(uint32_t value, uint32_t alignment) +{ + return (value + alignment - 1) & ~(alignment - 1); +} + +InterpMethodDataBuilder::InterpMethodDataBuilder(MemPoolAllocator allocator) + : m_relocs(allocator) +{ + // Initialize section alignments + m_sections[(int)InterpMethodDataSection::Header].alignment = sizeof(void*); + m_sections[(int)InterpMethodDataSection::Bytecode].alignment = sizeof(int32_t); + m_sections[(int)InterpMethodDataSection::InterpMethod].alignment = sizeof(void*); + m_sections[(int)InterpMethodDataSection::DataItems].alignment = sizeof(void*); + m_sections[(int)InterpMethodDataSection::GenericLookups].alignment = sizeof(void*); + m_sections[(int)InterpMethodDataSection::AsyncSuspendData].alignment = sizeof(void*); + m_sections[(int)InterpMethodDataSection::IntervalMaps].alignment = sizeof(uint32_t); + + // Header is always sizeof(InterpMethod*) for the InterpByteCodeStart + m_sections[(int)InterpMethodDataSection::Header].size = sizeof(InterpMethod*); +} + +InterpMethodDataBuilder::~InterpMethodDataBuilder() +{ +} + +InterpSectionRef InterpMethodDataBuilder::AllocateInSection(InterpMethodDataSection section, uint32_t size, uint32_t alignment) +{ + assert(!m_finalized); + InterpSectionData& sectionData = m_sections[(int)section]; + + if (alignment == 0) + { + alignment = sectionData.alignment; + } + + // Align the current size + uint32_t alignedOffset = AlignUp(sectionData.size, alignment); + sectionData.size = alignedOffset + size; + + return InterpSectionRef(section, alignedOffset); +} + +void InterpMethodDataBuilder::AddReloc(InterpSectionRef sourceRef, uint32_t offsetInSource, InterpSectionRef targetRef) +{ + assert(!m_finalized); + InterpReloc reloc; + reloc.sourceSection = sourceRef.section; + reloc.sourceOffset = sourceRef.offset + offsetInSource; + reloc.targetSection = targetRef.section; + reloc.targetOffset = targetRef.offset; + m_relocs.Add(reloc); +} + +void InterpMethodDataBuilder::SetBytecodeSize(uint32_t sizeInBytes) +{ + assert(!m_finalized); + m_sections[(int)InterpMethodDataSection::Bytecode].size = sizeInBytes; +} + +uint32_t InterpMethodDataBuilder::GetTotalSize() +{ + uint32_t offset = 0; + + // Sections are laid out in order + for (int i = 0; i < (int)InterpMethodDataSection::Count; i++) + { + InterpSectionData& section = m_sections[i]; + if (section.size > 0) + { + offset = AlignUp(offset, section.alignment); + section.finalOffset = offset; + offset += section.size; + } + else + { + section.finalOffset = offset; // Empty section points to next + } + } + + return offset; +} + +uint32_t InterpMethodDataBuilder::GetSectionOffset(InterpMethodDataSection section) const +{ + return m_sections[(int)section].finalOffset; +} + +uint32_t InterpMethodDataBuilder::GetSectionSize(InterpMethodDataSection section) const +{ + return m_sections[(int)section].size; +} + +void* InterpMethodDataBuilder::GetFinalPointer(InterpSectionRef ref) const +{ + assert(m_finalized); + return m_finalBaseAddress + m_sections[(int)ref.section].finalOffset + ref.offset; +} + +void InterpMethodDataBuilder::Finalize(void* baseAddressRW, void* baseAddressRX) +{ + assert(!m_finalized); + m_finalBaseAddress = (uint8_t*)baseAddressRX; + + uint8_t* rwBase = (uint8_t*)baseAddressRW; + + // Apply all relocations + for (int i = 0; i < m_relocs.GetSize(); i++) + { + const InterpReloc& reloc = m_relocs.Get(i); + + // Calculate target address (using RX base for final pointers) + void* targetAddr = m_finalBaseAddress + + m_sections[(int)reloc.targetSection].finalOffset + + reloc.targetOffset; + + // Write to source location (using RW base) + void** sourcePtr = (void**)(rwBase + + m_sections[(int)reloc.sourceSection].finalOffset + + reloc.sourceOffset); + *sourcePtr = targetAddr; + } + + m_finalized = true; +} + +InterpByteCodeStart* InterpMethodDataBuilder::GetByteCodeStart() const +{ + assert(m_finalized); + return (InterpByteCodeStart*)m_finalBaseAddress; +} + +void* InterpMethodDataBuilder::GetWritablePointer(void* baseAddressRW, InterpSectionRef ref) const +{ + return (uint8_t*)baseAddressRW + m_sections[(int)ref.section].finalOffset + ref.offset; +} + +InterpSectionRef InterpMethodDataBuilder::AllocateInterpMethod() +{ + return AllocateInSection(InterpMethodDataSection::InterpMethod, sizeof(InterpMethod)); +} + +InterpSectionRef InterpMethodDataBuilder::AllocateDataItems(int32_t count) +{ + return AllocateInSection(InterpMethodDataSection::DataItems, count * sizeof(void*)); +} + +InterpSectionRef InterpMethodDataBuilder::AllocateGenericLookup() +{ + return AllocateInSection(InterpMethodDataSection::GenericLookups, sizeof(InterpGenericLookup)); +} + +InterpSectionRef InterpMethodDataBuilder::AllocateAsyncSuspendData() +{ + return AllocateInSection(InterpMethodDataSection::AsyncSuspendData, sizeof(InterpAsyncSuspendData)); +} + +InterpSectionRef InterpMethodDataBuilder::AllocateIntervalMap(int32_t count) +{ + return AllocateInSection(InterpMethodDataSection::IntervalMaps, count * sizeof(InterpIntervalMapEntry)); +} diff --git a/src/coreclr/interpreter/interpmethoddata.h b/src/coreclr/interpreter/interpmethoddata.h new file mode 100644 index 00000000000000..55a12ded9e66cc --- /dev/null +++ b/src/coreclr/interpreter/interpmethoddata.h @@ -0,0 +1,146 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +#ifndef _INTERPMETHODDATA_H_ +#define _INTERPMETHODDATA_H_ + +#include "interpalloc.h" +#include "datastructs.h" + +// Forward declarations - actual definitions in interpretershared.h +struct InterpMethod; +struct InterpByteCodeStart; +struct InterpGenericLookup; +struct InterpAsyncSuspendData; +struct InterpIntervalMapEntry; + +// InterpMethodDataBuilder accumulates all persistent data for a compiled method +// during compilation, then finalizes it into a single contiguous allocation. +// +// Memory Layout after finalization: +// ┌────────────────────────────────────────┐ +// │ InterpByteCodeStart (Method ptr) │ <- baseAddress +// ├────────────────────────────────────────┤ +// │ Bytecodes (int32_t[]) │ +// ├────────────────────────────────────────┤ +// │ InterpMethod struct │ +// ├────────────────────────────────────────┤ +// │ DataItems array (void*[]) │ +// ├────────────────────────────────────────┤ +// │ InterpGenericLookup structs │ +// ├────────────────────────────────────────┤ +// │ InterpAsyncSuspendData structs │ +// ├────────────────────────────────────────┤ +// │ InterpIntervalMapEntry arrays │ +// └────────────────────────────────────────┘ + +// Sections within the unified method data allocation +enum class InterpMethodDataSection : uint8_t +{ + Header, // InterpByteCodeStart + Bytecode, // int32_t[] opcodes + InterpMethod, // InterpMethod struct + DataItems, // void*[] array + GenericLookups, // InterpGenericLookup structs + AsyncSuspendData, // InterpAsyncSuspendData structs + IntervalMaps, // InterpIntervalMapEntry arrays + Count +}; + +// Reference to a location within a section, used during compilation +// before final addresses are known +struct InterpSectionRef +{ + InterpMethodDataSection section; + uint32_t offset; // Offset within the section + + InterpSectionRef() : section(InterpMethodDataSection::Header), offset(0) {} + InterpSectionRef(InterpMethodDataSection s, uint32_t o) : section(s), offset(o) {} + + bool IsNull() const { return section == InterpMethodDataSection::Header && offset == 0; } +}; + +// Represents a relocation that needs to be applied at finalization +// A pointer at (sourceSection, sourceOffset) should point to (targetSection, targetOffset) +struct InterpReloc +{ + InterpMethodDataSection sourceSection; + uint32_t sourceOffset; // Offset within source section where the pointer lives + InterpMethodDataSection targetSection; + uint32_t targetOffset; // Offset within target section that the pointer should point to +}; + +// Tracks data for a single section during building +struct InterpSectionData +{ + uint32_t size = 0; + uint32_t alignment = sizeof(void*); // Default to pointer alignment + uint32_t finalOffset = 0; // Offset in final allocation (set during GetTotalSize) +}; + +class InterpMethodDataBuilder +{ +private: + InterpSectionData m_sections[(int)InterpMethodDataSection::Count]; + TArray m_relocs; + + // Cached section base addresses after finalization + uint8_t* m_finalBaseAddress = nullptr; + bool m_finalized = false; + + static uint32_t AlignUp(uint32_t value, uint32_t alignment); + +public: + InterpMethodDataBuilder(MemPoolAllocator allocator); + ~InterpMethodDataBuilder(); + + // Allocate space in a section and return a reference to it + InterpSectionRef AllocateInSection(InterpMethodDataSection section, uint32_t size, uint32_t alignment = 0); + + // Add a relocation: pointer at sourceRef + offsetInSource should point to targetRef after finalization + void AddReloc(InterpSectionRef sourceRef, uint32_t offsetInSource, InterpSectionRef targetRef); + + // Set the bytecode section size (bytecodes are written directly by the compiler) + void SetBytecodeSize(uint32_t sizeInBytes); + + // Calculate total size needed for the unified allocation + uint32_t GetTotalSize(); + + // Get the final offset of a section within the allocation + uint32_t GetSectionOffset(InterpMethodDataSection section) const; + + // Get the size of a section + uint32_t GetSectionSize(InterpMethodDataSection section) const; + + // Convert a section reference to a final pointer (only valid after Finalize) + void* GetFinalPointer(InterpSectionRef ref) const; + + // Finalize: apply all relocations + // baseAddressRW is the writable address, baseAddressRX is the executable address + void Finalize(void* baseAddressRW, void* baseAddressRX); + + // Get the InterpByteCodeStart pointer (only valid after Finalize) + InterpByteCodeStart* GetByteCodeStart() const; + + // Get the writable location for a section reference (for copying data during finalization) + void* GetWritablePointer(void* baseAddressRW, InterpSectionRef ref) const; + + // Helper: Allocate InterpMethod and return its reference + InterpSectionRef AllocateInterpMethod(); + + // Helper: Allocate data items array + InterpSectionRef AllocateDataItems(int32_t count); + + // Helper: Allocate a generic lookup struct + InterpSectionRef AllocateGenericLookup(); + + // Helper: Allocate async suspend data + InterpSectionRef AllocateAsyncSuspendData(); + + // Helper: Allocate interval map entries + InterpSectionRef AllocateIntervalMap(int32_t count); + + bool IsFinalized() const { return m_finalized; } +}; + +#endif // _INTERPMETHODDATA_H_ From c1a30678ec5362e998f4bebdfa451f9febb44b9c Mon Sep 17 00:00:00 2001 From: David Wrighton Date: Thu, 5 Feb 2026 11:24:33 -0800 Subject: [PATCH 2/9] Track async suspend data --- src/coreclr/interpreter/compiler.cpp | 23 +++++++++++++++++++++++ src/coreclr/interpreter/compiler.h | 9 +++++++++ 2 files changed, 32 insertions(+) diff --git a/src/coreclr/interpreter/compiler.cpp b/src/coreclr/interpreter/compiler.cpp index b55cbc63fecc82..94a9550a52fcc1 100644 --- a/src/coreclr/interpreter/compiler.cpp +++ b/src/coreclr/interpreter/compiler.cpp @@ -1989,6 +1989,21 @@ InterpMethod* InterpCompiler::FinalizeMethodData(void* baseAddressRW, void* base currentAsyncOffset += sizeof(InterpAsyncSuspendData); } + // Fix up data item pointers that reference async suspend data + // These pointers were recorded during compilation and now need to point to the final locations + if (numDataItems > 0) + { + void** pDataItemsRW = (void**)(rwBase + dataItemsOffset); + for (int32_t i = 0; i < m_dataItemAsyncSuspendRefs.GetSize(); i++) + { + DataItemAsyncSuspendRef ref = m_dataItemAsyncSuspendRefs.Get(i); + // Calculate the final address of this async suspend data in the RX allocation + InterpAsyncSuspendData* finalAddr = (InterpAsyncSuspendData*)(rxBase + asyncSuspendDataOffset + + ref.asyncSuspendDataIndex * sizeof(InterpAsyncSuspendData)); + pDataItemsRW[ref.dataItemIndex] = finalAddr; + } + } + // Write the InterpMethod pointer to the header (InterpByteCodeStart) *(InterpMethod**)(rwBase + headerOffset) = pMethodRX; @@ -2028,6 +2043,7 @@ InterpCompiler::InterpCompiler(COMP_HANDLE compHnd, , m_leavesTable(GetMemPoolAllocator(IMK_EHClause)) , m_dataItems(GetMemPoolAllocator(IMK_DataItem)) , m_asyncSuspendDataItems(GetMemPoolAllocator(IMK_DataItem)) + , m_dataItemAsyncSuspendRefs(GetMemPoolAllocator(IMK_DataItem)) , m_initLocals(false) , m_unmanagedCallersOnly(false) , m_publishSecretStubParam(false) @@ -6018,6 +6034,13 @@ void InterpCompiler::EmitSuspend(const CORINFO_CALL_INFO &callInfo, Continuation AddIns(handleContinuationOpcode); int32_t suspendDataIndex = GetDataItemIndex(suspendData); + + // Track this data item -> async suspend data reference for fixup during finalization + DataItemAsyncSuspendRef ref; + ref.dataItemIndex = suspendDataIndex; + ref.asyncSuspendDataIndex = m_asyncSuspendDataItems.GetSize() - 1; // suspendData was just added + m_dataItemAsyncSuspendRefs.Add(ref); + m_pLastNewIns->data[0] = suspendDataIndex; m_pLastNewIns->data[1] = GetDataForHelperFtn(helperFuncForAllocatingContinuation); PushInterpType(InterpTypeO, NULL); diff --git a/src/coreclr/interpreter/compiler.h b/src/coreclr/interpreter/compiler.h index bcdef2f9f1ea4f..138b3f7dc1045f 100644 --- a/src/coreclr/interpreter/compiler.h +++ b/src/coreclr/interpreter/compiler.h @@ -696,6 +696,15 @@ class InterpCompiler TArray m_asyncSuspendDataItems; + // Tracks which data items contain pointers to async suspend data + // First = data item index, Second = index into m_asyncSuspendDataItems + struct DataItemAsyncSuspendRef + { + int32_t dataItemIndex; + int32_t asyncSuspendDataIndex; + }; + TArray m_dataItemAsyncSuspendRefs; + // Prepared InterpMethod data (stored temporarily until finalization) bool m_initLocals; bool m_unmanagedCallersOnly; From 9c0fd56bc6eb4f1163e74146e02374bf849b785f Mon Sep 17 00:00:00 2001 From: David Wrighton Date: Mon, 2 Mar 2026 17:01:46 -0800 Subject: [PATCH 3/9] Remove InterpReloc and relocation infrastructure from InterpMethodDataBuilder Remove the unused InterpReloc struct, m_relocs member, and AddReloc method from InterpMethodDataBuilder. The relocation application loop in Finalize is also removed since there are no relocs to apply. The MemPoolAllocator constructor parameter is removed since it was only needed for the TArray member, and the datastructs.h include is dropped as TArray is no longer used in the header. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- src/coreclr/interpreter/compiler.cpp | 4 +-- src/coreclr/interpreter/interpalloc.h | 10 ++---- src/coreclr/interpreter/interpmethoddata.cpp | 33 +------------------- src/coreclr/interpreter/interpmethoddata.h | 19 ++--------- 4 files changed, 7 insertions(+), 59 deletions(-) diff --git a/src/coreclr/interpreter/compiler.cpp b/src/coreclr/interpreter/compiler.cpp index 94a9550a52fcc1..9f8416a7d78bc9 100644 --- a/src/coreclr/interpreter/compiler.cpp +++ b/src/coreclr/interpreter/compiler.cpp @@ -193,7 +193,7 @@ void* MemPoolAllocator::Alloc(size_t sz) const { sz = sizeof(void*); // Arena allocator does not support zero-length allocations, so allocate something of minimum size instead. } - return m_compiler->getAllocator(m_memKind).allocate(sz); + return ((InterpAllocator*)&m_allocator)->allocate(sz); } void MemPoolAllocator::Free(void* ptr) const { /* no-op */ } @@ -2033,7 +2033,7 @@ InterpCompiler::InterpCompiler(COMP_HANDLE compHnd, CORINFO_METHOD_INFO* methodInfo, InterpreterRetryData* pRetryData, InterpArenaAllocator *arenaAllocator) : m_arenaAllocator(arenaAllocator) - , m_methodDataBuilder(GetMemPoolAllocator(IMK_MethodData)) + , m_methodDataBuilder() , m_stackmapsByClass(getAllocator(IMK_StackMapHash)) , m_pRetryData(pRetryData) , m_pInitLocalsIns(nullptr) diff --git a/src/coreclr/interpreter/interpalloc.h b/src/coreclr/interpreter/interpalloc.h index 061b325029e489..157984e0e8f3f8 100644 --- a/src/coreclr/interpreter/interpalloc.h +++ b/src/coreclr/interpreter/interpalloc.h @@ -63,14 +63,8 @@ class MemPoolAllocator public: MemPoolAllocator(InterpAllocator allocator) : m_allocator(allocator) {} - void* Alloc(size_t sz) const - { - if (sz == 0) - sz = 1; // TArray expects non-zero allocations - return ((InterpAllocator*)&m_allocator)->allocate(sz); - } - void Free(void* ptr) const - { /* no-op */ } + void* Alloc(size_t sz) const; + void Free(void* ptr) const; }; #endif // _INTERPALLOC_H_ diff --git a/src/coreclr/interpreter/interpmethoddata.cpp b/src/coreclr/interpreter/interpmethoddata.cpp index 72425c1dc69827..fbb799c8fa86be 100644 --- a/src/coreclr/interpreter/interpmethoddata.cpp +++ b/src/coreclr/interpreter/interpmethoddata.cpp @@ -9,8 +9,7 @@ uint32_t InterpMethodDataBuilder::AlignUp(uint32_t value, uint32_t alignment) return (value + alignment - 1) & ~(alignment - 1); } -InterpMethodDataBuilder::InterpMethodDataBuilder(MemPoolAllocator allocator) - : m_relocs(allocator) +InterpMethodDataBuilder::InterpMethodDataBuilder() { // Initialize section alignments m_sections[(int)InterpMethodDataSection::Header].alignment = sizeof(void*); @@ -46,17 +45,6 @@ InterpSectionRef InterpMethodDataBuilder::AllocateInSection(InterpMethodDataSect return InterpSectionRef(section, alignedOffset); } -void InterpMethodDataBuilder::AddReloc(InterpSectionRef sourceRef, uint32_t offsetInSource, InterpSectionRef targetRef) -{ - assert(!m_finalized); - InterpReloc reloc; - reloc.sourceSection = sourceRef.section; - reloc.sourceOffset = sourceRef.offset + offsetInSource; - reloc.targetSection = targetRef.section; - reloc.targetOffset = targetRef.offset; - m_relocs.Add(reloc); -} - void InterpMethodDataBuilder::SetBytecodeSize(uint32_t sizeInBytes) { assert(!m_finalized); @@ -107,25 +95,6 @@ void InterpMethodDataBuilder::Finalize(void* baseAddressRW, void* baseAddressRX) assert(!m_finalized); m_finalBaseAddress = (uint8_t*)baseAddressRX; - uint8_t* rwBase = (uint8_t*)baseAddressRW; - - // Apply all relocations - for (int i = 0; i < m_relocs.GetSize(); i++) - { - const InterpReloc& reloc = m_relocs.Get(i); - - // Calculate target address (using RX base for final pointers) - void* targetAddr = m_finalBaseAddress + - m_sections[(int)reloc.targetSection].finalOffset + - reloc.targetOffset; - - // Write to source location (using RW base) - void** sourcePtr = (void**)(rwBase + - m_sections[(int)reloc.sourceSection].finalOffset + - reloc.sourceOffset); - *sourcePtr = targetAddr; - } - m_finalized = true; } diff --git a/src/coreclr/interpreter/interpmethoddata.h b/src/coreclr/interpreter/interpmethoddata.h index 55a12ded9e66cc..68bb76a8b32e56 100644 --- a/src/coreclr/interpreter/interpmethoddata.h +++ b/src/coreclr/interpreter/interpmethoddata.h @@ -5,7 +5,6 @@ #define _INTERPMETHODDATA_H_ #include "interpalloc.h" -#include "datastructs.h" // Forward declarations - actual definitions in interpretershared.h struct InterpMethod; @@ -60,16 +59,6 @@ struct InterpSectionRef bool IsNull() const { return section == InterpMethodDataSection::Header && offset == 0; } }; -// Represents a relocation that needs to be applied at finalization -// A pointer at (sourceSection, sourceOffset) should point to (targetSection, targetOffset) -struct InterpReloc -{ - InterpMethodDataSection sourceSection; - uint32_t sourceOffset; // Offset within source section where the pointer lives - InterpMethodDataSection targetSection; - uint32_t targetOffset; // Offset within target section that the pointer should point to -}; - // Tracks data for a single section during building struct InterpSectionData { @@ -82,7 +71,6 @@ class InterpMethodDataBuilder { private: InterpSectionData m_sections[(int)InterpMethodDataSection::Count]; - TArray m_relocs; // Cached section base addresses after finalization uint8_t* m_finalBaseAddress = nullptr; @@ -91,15 +79,12 @@ class InterpMethodDataBuilder static uint32_t AlignUp(uint32_t value, uint32_t alignment); public: - InterpMethodDataBuilder(MemPoolAllocator allocator); + InterpMethodDataBuilder(); ~InterpMethodDataBuilder(); // Allocate space in a section and return a reference to it InterpSectionRef AllocateInSection(InterpMethodDataSection section, uint32_t size, uint32_t alignment = 0); - // Add a relocation: pointer at sourceRef + offsetInSource should point to targetRef after finalization - void AddReloc(InterpSectionRef sourceRef, uint32_t offsetInSource, InterpSectionRef targetRef); - // Set the bytecode section size (bytecodes are written directly by the compiler) void SetBytecodeSize(uint32_t sizeInBytes); @@ -115,7 +100,7 @@ class InterpMethodDataBuilder // Convert a section reference to a final pointer (only valid after Finalize) void* GetFinalPointer(InterpSectionRef ref) const; - // Finalize: apply all relocations + // Finalize the method data // baseAddressRW is the writable address, baseAddressRX is the executable address void Finalize(void* baseAddressRW, void* baseAddressRX); From a73cb8cf60bc8148f54398a4d5602b91061e2c1d Mon Sep 17 00:00:00 2001 From: David Wrighton Date: Tue, 3 Mar 2026 14:28:53 -0800 Subject: [PATCH 4/9] Apply suggestions from code review Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --- src/coreclr/interpreter/compiler.cpp | 4 +++- src/coreclr/interpreter/compiler.h | 2 +- src/coreclr/interpreter/interpmethoddata.cpp | 2 ++ src/coreclr/interpreter/interpmethoddata.h | 4 ++-- 4 files changed, 8 insertions(+), 4 deletions(-) diff --git a/src/coreclr/interpreter/compiler.cpp b/src/coreclr/interpreter/compiler.cpp index 9f8416a7d78bc9..464e21a8791e5d 100644 --- a/src/coreclr/interpreter/compiler.cpp +++ b/src/coreclr/interpreter/compiler.cpp @@ -193,7 +193,9 @@ void* MemPoolAllocator::Alloc(size_t sz) const { sz = sizeof(void*); // Arena allocator does not support zero-length allocations, so allocate something of minimum size instead. } - return ((InterpAllocator*)&m_allocator)->allocate(sz); + + InterpAllocator allocatorCopy = m_allocator; + return allocatorCopy.allocate(sz); } void MemPoolAllocator::Free(void* ptr) const { /* no-op */ } diff --git a/src/coreclr/interpreter/compiler.h b/src/coreclr/interpreter/compiler.h index 138b3f7dc1045f..6f58d0c065cf40 100644 --- a/src/coreclr/interpreter/compiler.h +++ b/src/coreclr/interpreter/compiler.h @@ -1039,7 +1039,7 @@ class InterpCompiler int32_t* EmitCodeIns(int32_t *ip, InterpInst *pIns, TArray *relocs); void PatchRelocations(TArray *relocs); void PrepareInterpMethod(); - void UpdateWithFinalMethodByteCodeAddress(InterpByteCodeStart *pByteCodeStart); + void CreateBasicBlocks(CORINFO_METHOD_INFO* methodInfo); void InitializeClauseBuildingBlocks(CORINFO_METHOD_INFO* methodInfo); void CreateLeaveChainIslandBasicBlocks(CORINFO_METHOD_INFO* methodInfo, int32_t leaveOffset, InterpBasicBlock* pLeaveTargetBB); diff --git a/src/coreclr/interpreter/interpmethoddata.cpp b/src/coreclr/interpreter/interpmethoddata.cpp index fbb799c8fa86be..c58be27b6c4187 100644 --- a/src/coreclr/interpreter/interpmethoddata.cpp +++ b/src/coreclr/interpreter/interpmethoddata.cpp @@ -6,6 +6,8 @@ uint32_t InterpMethodDataBuilder::AlignUp(uint32_t value, uint32_t alignment) { + assert(alignment != 0); + assert((alignment & (alignment - 1)) == 0); return (value + alignment - 1) & ~(alignment - 1); } diff --git a/src/coreclr/interpreter/interpmethoddata.h b/src/coreclr/interpreter/interpmethoddata.h index 68bb76a8b32e56..301639c738720d 100644 --- a/src/coreclr/interpreter/interpmethoddata.h +++ b/src/coreclr/interpreter/interpmethoddata.h @@ -53,10 +53,10 @@ struct InterpSectionRef InterpMethodDataSection section; uint32_t offset; // Offset within the section - InterpSectionRef() : section(InterpMethodDataSection::Header), offset(0) {} + InterpSectionRef() : section(InterpMethodDataSection::Count), offset(0) {} InterpSectionRef(InterpMethodDataSection s, uint32_t o) : section(s), offset(o) {} - bool IsNull() const { return section == InterpMethodDataSection::Header && offset == 0; } + bool IsNull() const { return section == InterpMethodDataSection::Count; } }; // Tracks data for a single section during building From 438cd665e14d8c1b3e45048b3bdb0035f4d02607 Mon Sep 17 00:00:00 2001 From: David Wrighton Date: Tue, 3 Mar 2026 15:31:49 -0800 Subject: [PATCH 5/9] address feedback --- src/coreclr/interpreter/compiler.cpp | 40 ++++++++++++++------ src/coreclr/interpreter/interpmethoddata.cpp | 6 --- src/coreclr/interpreter/interpmethoddata.h | 6 --- 3 files changed, 28 insertions(+), 24 deletions(-) diff --git a/src/coreclr/interpreter/compiler.cpp b/src/coreclr/interpreter/compiler.cpp index 464e21a8791e5d..e58e7665b0a38c 100644 --- a/src/coreclr/interpreter/compiler.cpp +++ b/src/coreclr/interpreter/compiler.cpp @@ -1917,6 +1917,15 @@ InterpMethod* InterpCompiler::FinalizeMethodData(void* baseAddressRW, void* base uint32_t asyncSuspendDataOffset = m_methodDataBuilder.GetSectionOffset(InterpMethodDataSection::AsyncSuspendData); uint32_t intervalMapsOffset = m_methodDataBuilder.GetSectionOffset(InterpMethodDataSection::IntervalMaps); + const uint32_t bytecodeSectionSize = m_methodDataBuilder.GetSectionSize(InterpMethodDataSection::Bytecode); + const uint32_t interpMethodSectionSize = m_methodDataBuilder.GetSectionSize(InterpMethodDataSection::InterpMethod); + const uint32_t dataItemsSectionSize = m_methodDataBuilder.GetSectionSize(InterpMethodDataSection::DataItems); + const uint32_t asyncSuspendDataSectionSize = m_methodDataBuilder.GetSectionSize(InterpMethodDataSection::AsyncSuspendData); + const uint32_t intervalMapsSectionSize = m_methodDataBuilder.GetSectionSize(InterpMethodDataSection::IntervalMaps); + + assert((uint64_t)m_methodCodeSize * sizeof(int32_t) <= bytecodeSectionSize); + assert(sizeof(InterpMethod) <= interpMethodSectionSize); + // Copy bytecode memcpy(rwBase + bytecodeOffset, m_pMethodCode, m_methodCodeSize * sizeof(int32_t)); @@ -1924,6 +1933,8 @@ InterpMethod* InterpCompiler::FinalizeMethodData(void* baseAddressRW, void* base int numDataItems = m_dataItems.GetSize(); void** pDataItems = (numDataItems > 0) ? (void**)(rxBase + dataItemsOffset) : nullptr; + assert((uint64_t)numDataItems * sizeof(void*) <= dataItemsSectionSize); + // Copy data items if (numDataItems > 0) { @@ -1943,11 +1954,15 @@ InterpMethod* InterpCompiler::FinalizeMethodData(void* baseAddressRW, void* base // Copy async suspend data and fix up pointers uint32_t currentAsyncOffset = asyncSuspendDataOffset; uint32_t currentIntervalMapOffset = intervalMapsOffset; + const uint32_t asyncSuspendDataSectionEnd = asyncSuspendDataOffset + asyncSuspendDataSectionSize; + const uint32_t intervalMapsSectionEnd = intervalMapsOffset + intervalMapsSectionSize; InterpByteCodeStart* pByteCodeStart = (InterpByteCodeStart*)rxBase; for (int32_t i = 0; i < m_asyncSuspendDataItems.GetSize(); i++) { + assert(currentAsyncOffset + sizeof(InterpAsyncSuspendData) <= asyncSuspendDataSectionEnd); + InterpAsyncSuspendData* srcData = m_asyncSuspendDataItems.Get(i); InterpAsyncSuspendData* dstDataRW = (InterpAsyncSuspendData*)(rwBase + currentAsyncOffset); @@ -1966,12 +1981,15 @@ InterpMethod* InterpCompiler::FinalizeMethodData(void* baseAddressRW, void* base int32_t count = 0; while (srcData->liveLocalsIntervals[count].countBytes != 0) count++; count++; // Include terminator + + uint32_t mapSize = (uint32_t)count * sizeof(InterpIntervalMapEntry); + assert(currentIntervalMapOffset + mapSize <= intervalMapsSectionEnd); InterpIntervalMapEntry* dstMapRW = (InterpIntervalMapEntry*)(rwBase + currentIntervalMapOffset); InterpIntervalMapEntry* dstMapRX = (InterpIntervalMapEntry*)(rxBase + currentIntervalMapOffset); - memcpy(dstMapRW, srcData->liveLocalsIntervals, count * sizeof(InterpIntervalMapEntry)); + memcpy(dstMapRW, srcData->liveLocalsIntervals, mapSize); dstDataRW->liveLocalsIntervals = dstMapRX; - currentIntervalMapOffset += count * sizeof(InterpIntervalMapEntry); + currentIntervalMapOffset += mapSize; } if (srcData->zeroedLocalsIntervals != nullptr) @@ -1980,17 +1998,23 @@ InterpMethod* InterpCompiler::FinalizeMethodData(void* baseAddressRW, void* base int32_t count = 0; while (srcData->zeroedLocalsIntervals[count].countBytes != 0) count++; count++; // Include terminator + + uint32_t mapSize = (uint32_t)count * sizeof(InterpIntervalMapEntry); + assert(currentIntervalMapOffset + mapSize <= intervalMapsSectionEnd); InterpIntervalMapEntry* dstMapRW = (InterpIntervalMapEntry*)(rwBase + currentIntervalMapOffset); InterpIntervalMapEntry* dstMapRX = (InterpIntervalMapEntry*)(rxBase + currentIntervalMapOffset); - memcpy(dstMapRW, srcData->zeroedLocalsIntervals, count * sizeof(InterpIntervalMapEntry)); + memcpy(dstMapRW, srcData->zeroedLocalsIntervals, mapSize); dstDataRW->zeroedLocalsIntervals = dstMapRX; - currentIntervalMapOffset += count * sizeof(InterpIntervalMapEntry); + currentIntervalMapOffset += mapSize; } currentAsyncOffset += sizeof(InterpAsyncSuspendData); } + assert(currentAsyncOffset <= asyncSuspendDataSectionEnd); + assert(currentIntervalMapOffset <= intervalMapsSectionEnd); + // Fix up data item pointers that reference async suspend data // These pointers were recorded during compilation and now need to point to the final locations if (numDataItems > 0) @@ -11088,14 +11112,6 @@ void InterpCompiler::UnlinkUnreachableBBlocks() } } -void InterpCompiler::UpdateWithFinalMethodByteCodeAddress(InterpByteCodeStart *pByteCodeStart) -{ - for (int32_t i = 0; i < m_asyncSuspendDataItems.GetSize(); i++) - { - m_asyncSuspendDataItems.Get(i)->methodStartIP = pByteCodeStart; - } -} - void InterpreterRetryData::SetOverrideILMergePointStack(int32_t ilOffset, uint32_t stackHeight, StackInfo *pStackInfo) { assert(stackHeight > 0); diff --git a/src/coreclr/interpreter/interpmethoddata.cpp b/src/coreclr/interpreter/interpmethoddata.cpp index c58be27b6c4187..bfa5ea02b15c7e 100644 --- a/src/coreclr/interpreter/interpmethoddata.cpp +++ b/src/coreclr/interpreter/interpmethoddata.cpp @@ -18,7 +18,6 @@ InterpMethodDataBuilder::InterpMethodDataBuilder() m_sections[(int)InterpMethodDataSection::Bytecode].alignment = sizeof(int32_t); m_sections[(int)InterpMethodDataSection::InterpMethod].alignment = sizeof(void*); m_sections[(int)InterpMethodDataSection::DataItems].alignment = sizeof(void*); - m_sections[(int)InterpMethodDataSection::GenericLookups].alignment = sizeof(void*); m_sections[(int)InterpMethodDataSection::AsyncSuspendData].alignment = sizeof(void*); m_sections[(int)InterpMethodDataSection::IntervalMaps].alignment = sizeof(uint32_t); @@ -121,11 +120,6 @@ InterpSectionRef InterpMethodDataBuilder::AllocateDataItems(int32_t count) return AllocateInSection(InterpMethodDataSection::DataItems, count * sizeof(void*)); } -InterpSectionRef InterpMethodDataBuilder::AllocateGenericLookup() -{ - return AllocateInSection(InterpMethodDataSection::GenericLookups, sizeof(InterpGenericLookup)); -} - InterpSectionRef InterpMethodDataBuilder::AllocateAsyncSuspendData() { return AllocateInSection(InterpMethodDataSection::AsyncSuspendData, sizeof(InterpAsyncSuspendData)); diff --git a/src/coreclr/interpreter/interpmethoddata.h b/src/coreclr/interpreter/interpmethoddata.h index 301639c738720d..67c55e09bac1b5 100644 --- a/src/coreclr/interpreter/interpmethoddata.h +++ b/src/coreclr/interpreter/interpmethoddata.h @@ -26,8 +26,6 @@ struct InterpIntervalMapEntry; // ├────────────────────────────────────────┤ // │ DataItems array (void*[]) │ // ├────────────────────────────────────────┤ -// │ InterpGenericLookup structs │ -// ├────────────────────────────────────────┤ // │ InterpAsyncSuspendData structs │ // ├────────────────────────────────────────┤ // │ InterpIntervalMapEntry arrays │ @@ -40,7 +38,6 @@ enum class InterpMethodDataSection : uint8_t Bytecode, // int32_t[] opcodes InterpMethod, // InterpMethod struct DataItems, // void*[] array - GenericLookups, // InterpGenericLookup structs AsyncSuspendData, // InterpAsyncSuspendData structs IntervalMaps, // InterpIntervalMapEntry arrays Count @@ -116,9 +113,6 @@ class InterpMethodDataBuilder // Helper: Allocate data items array InterpSectionRef AllocateDataItems(int32_t count); - // Helper: Allocate a generic lookup struct - InterpSectionRef AllocateGenericLookup(); - // Helper: Allocate async suspend data InterpSectionRef AllocateAsyncSuspendData(); From 4ab8d5bd43649c75476211365dc8975e16445631 Mon Sep 17 00:00:00 2001 From: David Wrighton Date: Tue, 3 Mar 2026 15:32:35 -0800 Subject: [PATCH 6/9] Update src/coreclr/interpreter/compiler.cpp Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --- src/coreclr/interpreter/compiler.cpp | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/coreclr/interpreter/compiler.cpp b/src/coreclr/interpreter/compiler.cpp index 464e21a8791e5d..87e8a17cd2054f 100644 --- a/src/coreclr/interpreter/compiler.cpp +++ b/src/coreclr/interpreter/compiler.cpp @@ -194,8 +194,9 @@ void* MemPoolAllocator::Alloc(size_t sz) const sz = sizeof(void*); // Arena allocator does not support zero-length allocations, so allocate something of minimum size instead. } - InterpAllocator allocatorCopy = m_allocator; - return allocatorCopy.allocate(sz); + // allocate is non-const; cast away constness of m_allocator to avoid an extra copy per allocation. + auto* allocator = const_cast(&m_allocator); + return allocator->allocate(sz); } void MemPoolAllocator::Free(void* ptr) const { /* no-op */ } From 762c88a4c417ee0f4d0d01f488c560758b66b37e Mon Sep 17 00:00:00 2001 From: David Wrighton Date: Tue, 3 Mar 2026 15:32:51 -0800 Subject: [PATCH 7/9] Update src/coreclr/interpreter/compiler.cpp Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --- src/coreclr/interpreter/compiler.cpp | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/coreclr/interpreter/compiler.cpp b/src/coreclr/interpreter/compiler.cpp index 87e8a17cd2054f..63eb518eb6261c 100644 --- a/src/coreclr/interpreter/compiler.cpp +++ b/src/coreclr/interpreter/compiler.cpp @@ -1992,6 +1992,8 @@ InterpMethod* InterpCompiler::FinalizeMethodData(void* baseAddressRW, void* base currentAsyncOffset += sizeof(InterpAsyncSuspendData); } + _ASSERTE(currentAsyncOffset == GetSectionOffset(InterpMethodDataSection::AsyncSuspendData) + GetSectionSize(InterpMethodDataSection::AsyncSuspendData)); + _ASSERTE(currentIntervalMapOffset == GetSectionOffset(InterpMethodDataSection::IntervalMaps) + GetSectionSize(InterpMethodDataSection::IntervalMaps)); // Fix up data item pointers that reference async suspend data // These pointers were recorded during compilation and now need to point to the final locations if (numDataItems > 0) From ff57cfee0a1e9d8be92b6990628fd302f92e9a44 Mon Sep 17 00:00:00 2001 From: David Wrighton Date: Wed, 4 Mar 2026 09:58:58 -0800 Subject: [PATCH 8/9] Fix build issues --- src/coreclr/interpreter/compiler.cpp | 12 ++---------- 1 file changed, 2 insertions(+), 10 deletions(-) diff --git a/src/coreclr/interpreter/compiler.cpp b/src/coreclr/interpreter/compiler.cpp index 63eb518eb6261c..70856362be7e66 100644 --- a/src/coreclr/interpreter/compiler.cpp +++ b/src/coreclr/interpreter/compiler.cpp @@ -1992,8 +1992,8 @@ InterpMethod* InterpCompiler::FinalizeMethodData(void* baseAddressRW, void* base currentAsyncOffset += sizeof(InterpAsyncSuspendData); } - _ASSERTE(currentAsyncOffset == GetSectionOffset(InterpMethodDataSection::AsyncSuspendData) + GetSectionSize(InterpMethodDataSection::AsyncSuspendData)); - _ASSERTE(currentIntervalMapOffset == GetSectionOffset(InterpMethodDataSection::IntervalMaps) + GetSectionSize(InterpMethodDataSection::IntervalMaps)); + assert(currentAsyncOffset == m_methodDataBuilder.GetSectionOffset(InterpMethodDataSection::AsyncSuspendData) + m_methodDataBuilder.GetSectionSize(InterpMethodDataSection::AsyncSuspendData)); + assert(currentIntervalMapOffset == m_methodDataBuilder.GetSectionOffset(InterpMethodDataSection::IntervalMaps) + m_methodDataBuilder.GetSectionSize(InterpMethodDataSection::IntervalMaps)); // Fix up data item pointers that reference async suspend data // These pointers were recorded during compilation and now need to point to the final locations if (numDataItems > 0) @@ -11091,14 +11091,6 @@ void InterpCompiler::UnlinkUnreachableBBlocks() } } -void InterpCompiler::UpdateWithFinalMethodByteCodeAddress(InterpByteCodeStart *pByteCodeStart) -{ - for (int32_t i = 0; i < m_asyncSuspendDataItems.GetSize(); i++) - { - m_asyncSuspendDataItems.Get(i)->methodStartIP = pByteCodeStart; - } -} - void InterpreterRetryData::SetOverrideILMergePointStack(int32_t ilOffset, uint32_t stackHeight, StackInfo *pStackInfo) { assert(stackHeight > 0); From e86c82fa2d387ffd823f80057b0fc6c5974cdd2f Mon Sep 17 00:00:00 2001 From: David Wrighton Date: Wed, 4 Mar 2026 11:15:05 -0800 Subject: [PATCH 9/9] Update src/coreclr/interpreter/compiler.cpp Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --- src/coreclr/interpreter/compiler.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/coreclr/interpreter/compiler.cpp b/src/coreclr/interpreter/compiler.cpp index f2eb3501d5683b..e7986826fa2db7 100644 --- a/src/coreclr/interpreter/compiler.cpp +++ b/src/coreclr/interpreter/compiler.cpp @@ -2034,7 +2034,7 @@ InterpMethod* InterpCompiler::FinalizeMethodData(void* baseAddressRW, void* base // Write the InterpMethod pointer to the header (InterpByteCodeStart) *(InterpMethod**)(rwBase + headerOffset) = pMethodRX; - // Apply any additional relocations tracked by the builder + // Record the finalized base addresses in the method data builder m_methodDataBuilder.Finalize(baseAddressRW, baseAddressRX); return pMethodRX;