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.

Sign up for GitHub

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

Jump to bottom

Fix alloc-dealloc mismatches #54701

Merged
merged 3 commits into from
Jun 28, 2021
Merged

Fix alloc-dealloc mismatches #54701

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
2d51acd
Use specialized new operators to correctly allocate space with the no…
jkoritzinsky Jun 23, 2021
1e41179
Fix alloc-dealloc mismatch in ILStubResolver
jkoritzinsky Jun 23, 2021
b5313e2
Update src/coreclr/vm/ilstubresolver.cpp
jkoritzinsky Jun 24, 2021
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
3 changes: 1 addition & 2 deletions src/coreclr/vm/ilstubresolver.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -344,8 +344,7 @@ ILStubResolver::AllocGeneratedIL(
if (!UseLoaderHeap())
{
NewArrayHolder<BYTE> pNewILCodeBuffer = new BYTE[cbCode];
NewArrayHolder<CompileTimeState> pNewCompileTimeState = (CompileTimeState*)new BYTE[sizeof(CompileTimeState)];
memset(pNewCompileTimeState, 0, sizeof(CompileTimeState));
NewHolder<CompileTimeState> pNewCompileTimeState = new CompileTimeState{};
Copy link
Member

Choose a reason for hiding this comment

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

For my education - does new CompileTimeState{} allocate the space without calling any constructor?

Copy link
Member Author

@jkoritzinsky jkoritzinsky Jun 24, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

Good catch. It actually zero-inits, so I can remove the memset on the next line.

NewArrayHolder<BYTE> pNewLocalSig = NULL;

if (0 != cbLocalSig)
Expand Down
9 changes: 2 additions & 7 deletions src/coreclr/vm/methodtable.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8504,10 +8504,7 @@ MethodTable::GetMethodDataHelper(
MethodDataWrapper hDecl(GetMethodData(pMTDecl, FALSE));
MethodDataWrapper hImpl(GetMethodData(pMTImpl, FALSE));

UINT32 cb = MethodDataInterfaceImpl::GetObjectSize(pMTDecl);
NewArrayHolder<BYTE> pb(new BYTE[cb]);
MethodDataInterfaceImpl * pData = new (pb.GetValue()) MethodDataInterfaceImpl(rgDeclTypeIDs, cDeclTypeIDs, hDecl, hImpl);
pb.SuppressRelease();
MethodDataInterfaceImpl * pData = new ({ pMTDecl }) MethodDataInterfaceImpl(rgDeclTypeIDs, cDeclTypeIDs, hDecl, hImpl);

return pData;
} // MethodTable::GetMethodDataHelper
Expand Down Expand Up @@ -8548,10 +8545,8 @@ MethodTable::MethodData *MethodTable::GetMethodDataHelper(MethodTable *pMTDecl,
}
else {
UINT32 cb = MethodDataObject::GetObjectSize(pMTDecl);
NewArrayHolder<BYTE> pb(new BYTE[cb]);
MethodDataHolder h(FindParentMethodDataHelper(pMTDecl));
pData = new (pb.GetValue()) MethodDataObject(pMTDecl, h.GetValue());
pb.SuppressRelease();
pData = new ({ pMTDecl }) MethodDataObject(pMTDecl, h.GetValue());
}
}
else {
Expand Down
44 changes: 35 additions & 9 deletions src/coreclr/vm/methodtable.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3153,7 +3153,7 @@ public :

protected:
//--------------------------------------------------------------------------------------
class MethodDataObject : public MethodData
class MethodDataObject final : public MethodData
{
public:
// Static method that returns the amount of memory to allocate for a particular type.
Expand Down Expand Up @@ -3233,19 +3233,32 @@ public :
{ LIMITED_METHOD_CONTRACT; return m_pMDImpl; }
};

//
// At the end of this object is an array, so you cannot derive from this class.
//

inline MethodDataObjectEntry *GetEntryData()
{ LIMITED_METHOD_CONTRACT; return (MethodDataObjectEntry *)(this + 1); }
{ LIMITED_METHOD_CONTRACT; return &m_rgEntries[0]; }

inline MethodDataObjectEntry *GetEntry(UINT32 i)
{ LIMITED_METHOD_CONTRACT; CONSISTENCY_CHECK(i < GetNumMethods()); return GetEntryData() + i; }

void FillEntryDataForAncestor(MethodTable *pMT);

// MethodDataObjectEntry m_rgEntries[...];
//
// At the end of this object is an array
//
MethodDataObjectEntry m_rgEntries[0];

public:
struct TargetMethodTable
{
MethodTable* pMT;
};

static void* operator new(size_t size, TargetMethodTable targetMT)
{
_ASSERTE(size <= GetObjectSize(targetMT.pMT));
return ::operator new(GetObjectSize(targetMT.pMT));
}
static void* operator new(size_t size) = delete;
}; // class MethodDataObject

//--------------------------------------------------------------------------------------
Expand Down Expand Up @@ -3299,7 +3312,7 @@ public :
}; // class MethodDataInterface

//--------------------------------------------------------------------------------------
class MethodDataInterfaceImpl : public MethodData
class MethodDataInterfaceImpl final : public MethodData
{
public:
// Object construction-related methods
Expand Down Expand Up @@ -3373,12 +3386,25 @@ public :
//

inline MethodDataEntry *GetEntryData()
{ LIMITED_METHOD_CONTRACT; return (MethodDataEntry *)(this + 1); }
{ LIMITED_METHOD_CONTRACT; return &m_rgEntries[0]; }

inline MethodDataEntry *GetEntry(UINT32 i)
{ LIMITED_METHOD_CONTRACT; CONSISTENCY_CHECK(i < GetNumMethods()); return GetEntryData() + i; }

// MethodDataEntry m_rgEntries[...];
MethodDataEntry m_rgEntries[0];

public:
struct TargetMethodTable
{
MethodTable* pMT;
};

static void* operator new(size_t size, TargetMethodTable targetMT)
{
_ASSERTE(size <= GetObjectSize(targetMT.pMT));
return ::operator new(GetObjectSize(targetMT.pMT));
}
static void* operator new(size_t size) = delete;
}; // class MethodDataInterfaceImpl

//--------------------------------------------------------------------------------------
Expand Down