Skip to content

Multicorejit unification #48326

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

Merged
kouvel merged 7 commits into from
May 14, 2021
Merged

Multicorejit unification #48326

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
ed6862b
Unify simple and generic methods in multicorejit
gbalykov Jan 26, 2021
95de184
Disable NDirect methods in MulticoreJit
gbalykov Feb 3, 2021
26e631a
Add MultiCoreJitMinNumCpus env variable with default value =2 to conf…
gbalykov Feb 4, 2021
0081de5
Review fixes
gbalykov Apr 21, 2021
cd2820f
Do not save overall record length for methods
gbalykov Apr 23, 2021
4a3c5d2
Store non-generic methods using token instead of signatures
gbalykov May 4, 2021
f8af3e6
Additional fixes
gbalykov May 7, 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
Prev Previous commit
Additional fixes
  • Loading branch information
@gbalykov
gbalykov committed May 7, 2021
commit f8af3e6c7d457b9154a70659a8c8414dc35f7a6d
6 changes: 2 additions & 4 deletions src/coreclr/vm/multicorejit.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -387,14 +387,10 @@ HRESULT MulticoreJitRecorder::WriteOutput(IStream * pStream)
HRESULT hr = S_OK;

// Preprocessing Methods
Copy link
Contributor

@kouvel kouvel May 7, 2021

Choose a reason for hiding this comment

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

The additional comments below are no longer valid, could be removed

Copy link
Member Author

@gbalykov gbalykov May 7, 2021

Choose a reason for hiding this comment

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

Fixed

// - Add ModuleDependency to JITInfo
// - Increment MethodCount in Module
LONG skipped = 0;

for (LONG i = 0 ; i < m_JitInfoCount; i++)
{
SigBuilder sigBuilder;

if (m_JitInfoArray[i].IsModuleInfo())
{
// Module records don't need preprocessing
Expand All @@ -405,6 +401,8 @@ HRESULT MulticoreJitRecorder::WriteOutput(IStream * pStream)

if (m_JitInfoArray[i].IsGenericMethodInfo())
{
SigBuilder sigBuilder;

BOOL fSuccess = false;
EX_TRY
{
Expand Down
8 changes: 7 additions & 1 deletion src/coreclr/vm/multicorejitplayer.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1161,7 +1161,7 @@ HRESULT MulticoreJitProfilePlayer::PlayProfile()
unsigned data1 = * (const unsigned *) pBuffer;
unsigned rcdTyp = data1 >> RECORD_TYPE_OFFSET;
unsigned rcdLen = 0;

if (rcdTyp == MULTICOREJIT_MODULE_RECORD_ID)
{
rcdLen = data1 & 0xFFFFFF;
Expand All @@ -1176,6 +1176,12 @@ HRESULT MulticoreJitProfilePlayer::PlayProfile()
}
else if (rcdTyp == MULTICOREJIT_GENERICMETHOD_RECORD_ID)
{
if (nSize < sizeof(unsigned) + sizeof(unsigned short))
{
hr = COR_E_BADIMAGEFORMAT;
break;
}

unsigned signatureLength = * (const unsigned short *) (((const unsigned *) pBuffer) + 1);
Copy link
Contributor

@kouvel kouvel May 7, 2021

Choose a reason for hiding this comment

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

It seems to be the intention in this part to check if there is enough space in the remaining buffer before decoding. From the loop condition we know that there is a DWORD worth of space but this is reading past that, so should check the remaining buffer length before dereferencing.

Copy link
Member Author

@gbalykov gbalykov May 7, 2021

Choose a reason for hiding this comment

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

Fixed

DWORD dataSize = signatureLength + sizeof(DWORD) + sizeof(unsigned short);
dataSize = AlignUp(dataSize, sizeof(DWORD));
Expand Down