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

Enable loading composite r2r images from a singlefile bundle #53739

Merged
merged 8 commits into from
Jun 18, 2021
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
21 changes: 18 additions & 3 deletions src/coreclr/utilcode/pedecoder.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1034,14 +1034,29 @@ CHECK PEDecoder::CheckCorHeader() const

IMAGE_COR20_HEADER *pCor = GetCorHeader();

// Currently composite r2r images miss some information, for example the version is 0.0.
// We may want to change that to something more conforming and explicit.
// For now, for compatibility purposes, we will accept that as a valid format.
bool possiblyCompositeR2R =
pCor->MinorRuntimeVersion == 0 &&
pCor->MajorRuntimeVersion == 0;

//CHECK(((ULONGLONG)pCor & 0x3)==0);

// If the file is COM+ 1.0, which by definition has nothing the runtime can
// use, or if the file requires a newer version of this engine than us,
// it cannot be run by this engine.
CHECK(VAL16(pCor->MajorRuntimeVersion) > 1 && VAL16(pCor->MajorRuntimeVersion) <= COR_VERSION_MAJOR);
if (!possiblyCompositeR2R)
CHECK(VAL16(pCor->MajorRuntimeVersion) > 1 && VAL16(pCor->MajorRuntimeVersion) <= COR_VERSION_MAJOR);

#ifdef HOST_WINDOWS
CHECK(CheckDirectory(&pCor->MetaData, IMAGE_SCN_MEM_WRITE, HasNativeHeader() ? NULL_OK : NULL_NOT_OK));
#else
CHECK(CheckDirectory(
&pCor->MetaData,
possiblyCompositeR2R ? 0 : IMAGE_SCN_MEM_WRITE,
HasNativeHeader() ? NULL_OK : NULL_NOT_OK));
#endif
CHECK(CheckDirectory(&pCor->Resources, IMAGE_SCN_MEM_WRITE, NULL_OK));
CHECK(CheckDirectory(&pCor->StrongNameSignature, IMAGE_SCN_MEM_WRITE, NULL_OK));
CHECK(CheckDirectory(&pCor->CodeManagerTable, IMAGE_SCN_MEM_WRITE, NULL_OK));
Expand Down Expand Up @@ -1083,7 +1098,7 @@ CHECK PEDecoder::CheckCorHeader() const

// IL library files (really a misnomer - these are native images or ReadyToRun images)
// only they can have a native image header
if ((pCor->Flags&VAL32(COMIMAGE_FLAGS_IL_LIBRARY)) == 0)
if ((pCor->Flags&VAL32(COMIMAGE_FLAGS_IL_LIBRARY)) == 0 && !possiblyCompositeR2R)
{
CHECK(VAL32(pCor->ManagedNativeHeader.Size) == 0);
}
Expand Down Expand Up @@ -1769,7 +1784,7 @@ void PEDecoder::LayoutILOnly(void *base, bool enableExecution) const
PAGE_READONLY, &oldProtection))
ThrowLastError();

// Finally, apply proper protection to copied sections
// Finally, apply proper protection to copied sections
for (section = sectionStart; section < sectionEnd; section++)
{
// Add appropriate page protection.
Expand Down
100 changes: 55 additions & 45 deletions src/coreclr/vm/nativeimage.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -143,59 +143,69 @@ NativeImage *NativeImage::Open(

NewHolder<PEImageLayout> peLoadedImage;

EX_TRY
BundleFileLocation bundleFileLocation = Bundle::ProbeAppBundle(fullPath, /*pathIsBundleRelative */ true);
if (bundleFileLocation.IsValid())
Copy link
Member

Choose a reason for hiding this comment

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

It looks like the bundler also records the file type of everything in the bundle. Is there any reason why we would want to add a new "R2R composite" file type?

It doesn't really look like we'll need it, as long as we assume that the bundle-relative path is constructed properly at the beginning.

Copy link
Member Author

Choose a reason for hiding this comment

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

I do not think we have concrete reasons to distinguish and propagate the distinction for composite r2r files vs. regular r2r assemblies. The main difference is that composite r2r file may contain native code for more than one assembly, which is opaque to the host/bundle. Also it does not seem to contain any IL (I am not sure if it actually can, but it would make no difference either).

As things are now, a separate classification for composite r2r at bundle level would not add much benefit. I think we can wait until/if we have a use case for that.

{
peLoadedImage = PEImageLayout::LoadNative(fullPath);
PEImageHolder pImage = PEImage::OpenImage(fullPath, MDInternalImport_NoCache, bundleFileLocation);
peLoadedImage = pImage->GetLayout(PEImageLayout::LAYOUT_MAPPED, PEImage::LAYOUT_CREATEIFNEEDED);
}
EX_CATCH

if (peLoadedImage.IsNull())
{
SString searchPaths(searchPathsConfig);
SString::CIterator start = searchPaths.Begin();
while (start != searchPaths.End())
EX_TRY
{
SString::CIterator end = start;
if (!searchPaths.Find(end, PATH_SEPARATOR_CHAR_W))
{
end = searchPaths.End();
}
fullPath.Set(searchPaths, start, (COUNT_T)(end - start));

if (end != searchPaths.End())
{
// Skip path separator character
++end;
}
start = end;

if (fullPath.GetCount() == 0)
{
continue;
}

fullPath.Append(DIRECTORY_SEPARATOR_CHAR_W);
fullPath += compositeImageFileName;

EX_TRY
{
peLoadedImage = PEImageLayout::LoadNative(fullPath);
break;
}
EX_CATCH
peLoadedImage = PEImageLayout::LoadNative(fullPath);
}
EX_CATCH
{
SString searchPaths(searchPathsConfig);
SString::CIterator start = searchPaths.Begin();
while (start != searchPaths.End())
{
SString::CIterator end = start;
if (!searchPaths.Find(end, PATH_SEPARATOR_CHAR_W))
{
end = searchPaths.End();
}
fullPath.Set(searchPaths, start, (COUNT_T)(end - start));

if (end != searchPaths.End())
{
// Skip path separator character
++end;
}
start = end;

if (fullPath.GetCount() == 0)
{
continue;
}

fullPath.Append(DIRECTORY_SEPARATOR_CHAR_W);
fullPath += compositeImageFileName;

EX_TRY
{
peLoadedImage = PEImageLayout::LoadNative(fullPath);
break;
}
EX_CATCH
{
}
EX_END_CATCH(SwallowAllExceptions)
}
EX_END_CATCH(SwallowAllExceptions)
}
}
EX_END_CATCH(SwallowAllExceptions)
EX_END_CATCH(SwallowAllExceptions)

if (peLoadedImage.IsNull())
{
// Failed to locate the native composite R2R image
LOG((LF_LOADER, LL_ALWAYS, "LOADER: failed to load native image '%s' for component assembly '%S' using search paths: '%S'\n",
nativeImageFileName,
path.GetUnicode(),
searchPathsConfig != nullptr ? searchPathsConfig : W("<use COMPlus_NativeImageSearchPaths to set>")));
RaiseFailFastException(nullptr, nullptr, 0);
if (peLoadedImage.IsNull())
{
// Failed to locate the native composite R2R image
LOG((LF_LOADER, LL_ALWAYS, "LOADER: failed to load native image '%s' for component assembly '%S' using search paths: '%S'\n",
nativeImageFileName,
path.GetUnicode(),
searchPathsConfig != nullptr ? searchPathsConfig : W("<use COMPlus_NativeImageSearchPaths to set>")));
RaiseFailFastException(nullptr, nullptr, 0);
}
}

READYTORUN_HEADER *pHeader = (READYTORUN_HEADER *)peLoadedImage->GetExport("RTR_HEADER");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -126,6 +126,18 @@ public void Bundled_Self_Contained_Targeting50_WithCompression_Throws()
Assert.Throws<ArgumentException>(()=>BundleHelper.BundleApp(fixture, options, new Version(5, 0)));
}

[Fact]
[ActiveIssue("https://github.com/dotnet/runtime/issues/54234")]
// NOTE: when enabling this test take a look at commented code maked by "ACTIVE ISSUE:" in SharedTestState
public void Bundled_Self_Contained_Composite_App_Run_Succeeds()
{
var fixture = sharedTestState.TestSelfContainedFixtureComposite.Copy();
var singleFile = BundleSelfContainedApp(fixture, BundleOptions.None, disableCompression: true);

// Run the app
RunTheApp(singleFile, fixture);
}

[InlineData(BundleOptions.None)]
[InlineData(BundleOptions.BundleNativeBinaries)]
[InlineData(BundleOptions.BundleAllContent)]
Expand All @@ -144,6 +156,7 @@ public class SharedTestState : SharedTestStateBase, IDisposable
public TestProjectFixture TestFrameworkDependentFixture { get; set; }
public TestProjectFixture TestSelfContainedFixture { get; set; }
public TestProjectFixture TestAppWithEmptyFileFixture { get; set; }
public TestProjectFixture TestSelfContainedFixtureComposite { get; set; }

public SharedTestState()
{
Expand All @@ -169,13 +182,26 @@ public SharedTestState()
.EnsureRestoredForRid(TestAppWithEmptyFileFixture.CurrentRid)
.PublishProject(runtime: TestAppWithEmptyFileFixture.CurrentRid,
outputDirectory: BundleHelper.GetPublishPath(TestAppWithEmptyFileFixture));

TestSelfContainedFixtureComposite = new TestProjectFixture("AppWithSubDirs", RepoDirectories);
BundleHelper.AddLongNameContentToAppWithSubDirs(TestSelfContainedFixtureComposite);
TestSelfContainedFixtureComposite
.EnsureRestoredForRid(TestSelfContainedFixtureComposite.CurrentRid)
.PublishProject(runtime: TestSelfContainedFixtureComposite.CurrentRid,
// ACTIVE ISSUE: https://github.com/dotnet/runtime/issues/54234
// uncomment extraArgs when fixed.
outputDirectory: BundleHelper.GetPublishPath(TestSelfContainedFixtureComposite) /*,
extraArgs: new string[] {
"/p:PublishReadyToRun=true",
"/p:PublishReadyToRunComposite=true" } */);
}

public void Dispose()
{
TestFrameworkDependentFixture.Dispose();
TestSelfContainedFixture.Dispose();
TestAppWithEmptyFileFixture.Dispose();
TestSelfContainedFixtureComposite.Dispose();
}
}
}
Expand Down