Skip to content

Commit

Permalink
Make composite MVID checks resilient to failures (#51018)
Browse files Browse the repository at this point in the history
I noticed that several ALC-related CoreCLR tests fail in Crossgen2
composite mode. I tracked this down to a missing null check.
In addition to that, we were erroneously putting the composite
image to the validation list even when it had been loaded previously.

Thanks

Tomas
  • Loading branch information
trylek authored Apr 10, 2021
1 parent b05a892 commit 0af5228
Show file tree
Hide file tree
Showing 3 changed files with 16 additions and 6 deletions.
13 changes: 9 additions & 4 deletions src/coreclr/vm/assemblyloadcontext.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -24,12 +24,17 @@ NativeImage *AssemblyLoadContext::LoadNativeImage(Module *componentModule, LPCUT
AssemblyLoadContext *loadContext = componentModule->GetFile()->GetAssemblyLoadContext();
PTR_LoaderAllocator moduleLoaderAllocator = componentModule->GetLoaderAllocator();

NativeImage *nativeImage = NativeImage::Open(componentModule, nativeImageName, loadContext, moduleLoaderAllocator);
m_nativeImages.Append(nativeImage);
bool isNewNativeImage;
NativeImage *nativeImage = NativeImage::Open(componentModule, nativeImageName, loadContext, moduleLoaderAllocator, &isNewNativeImage);

for (COUNT_T assemblyIndex = 0; assemblyIndex < m_loadedAssemblies.GetCount(); assemblyIndex++)
if (isNewNativeImage && nativeImage != nullptr)
{
nativeImage->CheckAssemblyMvid(m_loadedAssemblies[assemblyIndex]);
m_nativeImages.Append(nativeImage);

for (COUNT_T assemblyIndex = 0; assemblyIndex < m_loadedAssemblies.GetCount(); assemblyIndex++)
{
nativeImage->CheckAssemblyMvid(m_loadedAssemblies[assemblyIndex]);
}
}

return nativeImage;
Expand Down
6 changes: 5 additions & 1 deletion src/coreclr/vm/nativeimage.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -114,13 +114,15 @@ NativeImage *NativeImage::Open(
Module *componentModule,
LPCUTF8 nativeImageFileName,
AssemblyLoadContext *pAssemblyLoadContext,
LoaderAllocator *pLoaderAllocator)
LoaderAllocator *pLoaderAllocator,
/* out */ bool *isNewNativeImage)
{
STANDARD_VM_CONTRACT;

NativeImage *pExistingImage = AppDomain::GetCurrentDomain()->GetNativeImage(nativeImageFileName);
if (pExistingImage != nullptr)
{
*isNewNativeImage = false;
return pExistingImage->GetAssemblyLoadContext() == pAssemblyLoadContext ? pExistingImage : nullptr;
}

Expand Down Expand Up @@ -216,10 +218,12 @@ NativeImage *NativeImage::Open(
if (pExistingImage == nullptr)
{
// No pre-existing image, new image has been stored in the map
*isNewNativeImage = true;
amTracker.SuppressRelease();
return image.Extract();
}
// Return pre-existing image if it was loaded into the same ALC, null otherwise
*isNewNativeImage = false;
return (pExistingImage->GetAssemblyLoadContext() == pAssemblyLoadContext ? pExistingImage : nullptr);
}
#endif
Expand Down
3 changes: 2 additions & 1 deletion src/coreclr/vm/nativeimage.h
Original file line number Diff line number Diff line change
Expand Up @@ -105,7 +105,8 @@ class NativeImage
Module *componentModule,
LPCUTF8 nativeImageFileName,
AssemblyLoadContext *pAssemblyLoadContext,
LoaderAllocator *pLoaderAllocator);
LoaderAllocator *pLoaderAllocator,
/* out */ bool *isNewNativeImage);

Crst *EagerFixupsLock() { return &m_eagerFixupsLock; }
bool EagerFixupsHaveRun() const { return m_eagerFixupsHaveRun; }
Expand Down

0 comments on commit 0af5228

Please sign in to comment.