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

[Android] Add callback to host-runtime contract for getting assembly data #112705

Merged
merged 69 commits into from
Feb 25, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
69 commits
Select commit Hold shift + click to select a range
7b144f3
Beginnings of native Android build
grendello Dec 6, 2024
06c1c83
Beginnings of native Android build
grendello Dec 6, 2024
62189fc
Bump min API level to 29 (proper TLS support) and link the right syst…
steveisok Dec 11, 2024
f9f7b90
Fix host/target confusion in R2R tool publish / Add Android target to…
steveisok Dec 13, 2024
2a1535e
Beginnings of native Android build
grendello Dec 6, 2024
10e88b0
Cleanup
ivanpovazan Jan 10, 2025
c2b4eac
Revert changes to R2R that alias Android to linux and put it in a few…
steveisok Jan 10, 2025
aebb13f
Undo host awareness in crossgen_publish as building it is about the t…
steveisok Jan 14, 2025
766af9b
Make sure android runtime pack is generated and crossgen_inbuild uses…
steveisok Jan 15, 2025
4f9e680
Condition min api level for coreclr and not.
steveisok Jan 15, 2025
2509f3f
Add FEATURE_NATIVEAOT_ONLY
steveisok Jan 16, 2025
37d326f
Revert FEATURE_NATIVEAOT_ONLY
steveisok Jan 16, 2025
ee65f01
Set PrimaryRuntimeFlavor on mobile official build and don't try to ge…
steveisok Jan 16, 2025
c17e21e
Override API level in coreclr runtime.proj and flow runtimeFlavor int…
steveisok Jan 16, 2025
83d394b
Disable ThreadStatic optimizations until emulated TLS support is added
steveisok Jan 17, 2025
0f53984
Using API level 21 should be good now
steveisok Jan 17, 2025
55cc33c
Set runtime feature flag in cmake config
kotlarmilos Jan 17, 2025
0b007c3
Don't get in the way of building coreclr+mono runtimes together. Work…
steveisok Jan 18, 2025
686bace
Allow cross build on osx when targeting arm32, fix bad arm32 linker a…
steveisok Jan 22, 2025
6f01f1d
Revert CLR_CMAKE_RUNTIME_CORECLR and enable event pipe TCP stuff
steveisok Jan 22, 2025
6143339
Exclude arm and x86 from generating packages / Always set CrossBuild=…
steveisok Jan 23, 2025
d662757
Initial Android CoreCLR initialization function implementation
grendello Jan 24, 2025
73fd37e
Preparing bundle mode for Android
grendello Jan 28, 2025
6f32ec3
Tracking why System.Private.CoreLib.dll load fails
grendello Jan 31, 2025
439962d
Don't try to mmap assembly image on Android
grendello Feb 3, 2025
0e6fe94
Tracking abort() calls
grendello Feb 4, 2025
d69a550
For now
grendello Feb 5, 2025
5895ad8
Revert "For now"
grendello Feb 6, 2025
d4cdf66
Logging to Android logcat
grendello Feb 6, 2025
75c6e4c
Write to logcat instead of stderr
grendello Feb 6, 2025
91772f4
This should work...
grendello Feb 19, 2025
32ee9a9
Update src/coreclr/vm/peimage.inl
grendello Feb 19, 2025
4026a7b
Update src/coreclr/vm/peimage.cpp
grendello Feb 19, 2025
cc4e734
Post-rebase adjustments
grendello Feb 20, 2025
0650ae7
[WIP] Remove android_coreclr_initialize and modify the existing one
grendello Feb 20, 2025
562c8e9
Cleanup - build failed locally for arm64. Removed unused blocks
steveisok Feb 20, 2025
493a186
Remove host_runtime_contract from coreclr_initialize. Current expecta…
steveisok Feb 20, 2025
3e97a9e
Update src/coreclr/hosts/inc/coreclrhost.h
grendello Feb 20, 2025
f398c6e
Update src/coreclr/inc/bundle.h
grendello Feb 20, 2025
7127b59
Update src/coreclr/hosts/inc/coreclrhost.h
grendello Feb 20, 2025
e1e7e32
Address feedback
grendello Feb 20, 2025
58b2698
Merge remote-tracking branch 'upstream/main' into dev/grendel/android…
steveisok Feb 21, 2025
7d4254a
Remove some logging related things since Elinor's PR landed
steveisok Feb 21, 2025
a16e3ed
Fix HostRuntimeContract tests
elinor-fung Feb 21, 2025
99e2bbf
AndroidGetDataStart -> GetData
elinor-fung Feb 21, 2025
cf72fbe
Don't try to manage external assembly not mapped by the runtime
elinor-fung Feb 21, 2025
f286e15
Remove newline-only changes
elinor-fung Feb 21, 2025
1533d9f
Explicitly check / document checking return value of probes
elinor-fung Feb 21, 2025
1cdb0ce
Check for non-file data instead of assuming that is the case on Android
elinor-fung Feb 21, 2025
5a97aff
Remove unnecessary check
elinor-fung Feb 21, 2025
885d117
Don't set file handle for non-file data
elinor-fung Feb 21, 2025
54ad33c
Check for null data instead of always returns FALSE for PEImage::IsFi…
elinor-fung Feb 21, 2025
2821e75
GetData -> GetExternalData
elinor-fung Feb 21, 2025
ff9021d
Remove AndroidGetAppName
elinor-fung Feb 21, 2025
38c6376
Remove BundleFileLocation::AppName
elinor-fung Feb 21, 2025
8c60c1a
Remove LogToAndroid. Scenario should now use LogToConsole.
AaronRobinsonMSFT Feb 22, 2025
57553d4
Feedback
AaronRobinsonMSFT Feb 22, 2025
dfc64db
Fix the build
AaronRobinsonMSFT Feb 22, 2025
656a6fd
Update src/coreclr/vm/peimage.cpp
AaronRobinsonMSFT Feb 22, 2025
d890acc
Update src/coreclr/vm/util.cpp
elinor-fung Feb 22, 2025
f4fe37a
Remove stripping away path. Dedup script echo
steveisok Feb 24, 2025
cf2aae6
Remove logging - ivan will follow up in a separate PR
steveisok Feb 24, 2025
2222efe
Merge branch 'main' into dev/grendel/android-clr-host-local
jkotas Feb 24, 2025
7c0350b
Remove redundant target_link_libraries
steveisok Feb 24, 2025
331e849
Merge branch 'main' into dev/grendel/android-clr-host-local
jkotas Feb 24, 2025
2dcd170
Merge branch 'main' into dev/grendel/android-clr-host-local
jkotas Feb 24, 2025
1df4b0d
Revert files with whitespace-only changes
elinor-fung Feb 24, 2025
bbf5e47
Merge branch 'main' into dev/grendel/android-clr-host-local
steveisok Feb 24, 2025
792f95e
Merge branch 'main' into dev/grendel/android-clr-host-local
steveisok Feb 25, 2025
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
25 changes: 14 additions & 11 deletions eng/native/build-commons.sh
Copy link
Member

Choose a reason for hiding this comment

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

I think these changes may break source-build, as it effectively regresses #35727. cc: @omajid can you take a look?

@grendello What was the motivation behind this change?

Copy link
Member

Choose a reason for hiding this comment

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

This change is expanding the coverage of #35727 to cmake configure step. How is it regressing, do you have an example?

Copy link
Member

Choose a reason for hiding this comment

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

From #35727:

The normal way to do that is by exporting environment variables like CFLAGS and CXXFLAGS. Fedora provides a bunch of values that we can use . But this doesn't play very nice with the liberal use of -Werror in corefx. Worse, the flags leak into various configure tests . For example HAVE_IN_PKTINFO would fail due to an unrelated warning

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@jkoritzinsky *FLAGS must be exported before cmake configuration run, otherwise they will be ignored. Where they were previously exported, they were essentially ignored. My motivation, tbh, was to get color in errors and warnings working, but it just seemed to be a mistake to have the exports without any effect. I think it may also be a requirement for packagers in some Linux distributions. For instance, Debian can specify standard system-wide package compilation flags via various dpkg scripts and it's done by pre-setting the *FLAGS environment variables.

Copy link
Member

Choose a reason for hiding this comment

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

it just seemed to be a mistake to have the exports without any effect.

Before this PR extra flags did had effect but only over cmake --build step, not the config step.

If something is breaking because of this, it is most likely the problem which needs to be fixed in that config.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@omajid The policies talk about build, but they don't mean cmake build stage. cmake is a build system generator, so it takes the *FLAGS into account only when generating the make/ninja build files, during cmake configuration phase. Here's dotnet/runtime's build.ninja:

artifacts/obj/coreclr/android.arm64.Release $ grep -e '\(CXX\|LD\|CC\)FLAGS' build.ninja | wc -l
0

artifacts/obj/coreclr/android.arm64.Release $ grep -e 'FLAGS' build.ninja | wc -l
2833

artifacts/obj/coreclr/android.arm64.Release $ grep -e 'FLAGS' build.ninja | head -1
  FLAGS = -g -DANDROID -fdata-sections -ffunction-sections -funwind-tables -fstack-protector-strong -no-canonical-prefixes -D_FORTIFY_SOURCE=2 -Wformat -Werror=format-security -fno-rtti  -O3 -DNDEBUG  -std=gnu++11 -fPIE -O2 -Wall -Wno-null-conversion -glldb -fno-omit-frame-pointer -fwrapv -fstack-protector-strong -Werror -Wno-unused-variable -Wno-unused-value -Wno-unused-function -Wno-tautological-compare -Wno-unknown-pragmas -Wimplicit-fallthrough -Wvla -Wno-invalid-offsetof -Wno-unused-but-set-variable -ffp-contract=off -fno-rtti -Wno-unknown-warning-option -ferror-limit=4096 -Wno-unused-private-field -Wno-constant-logical-operand -Wno-pragma-pack -Wno-incompatible-ms-struct -Wno-reserved-identifier -Wno-unsafe-buffer-usage -Wno-single-bit-bitfield-constant-conversion -Wno-cast-function-type-strict -Wno-switch-default -Wno-nontrivial-memaccess -fsigned-char -fvisibility=hidden -ffunction-sections

As you can see, there's no mention of any of the *FLAGS - all of them were expanded and placed by cmake configuration stage in the build file.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@omajid if setting some flags at configure time breaks .NET tests, then the problem is in those tests and it should be fixed there. If any component is incompatible with some flags, it can (and should) check for and remove the offending flags/defines. This post gives a potential solution (not tested by me):

function(remove_compiler_flag target flag)
  foreach(property IN COMPILE_DEFINITIONS COMPILE_FEATURES COMPILE_FLAGS COMPILE_OPTIONS etc)
    # Get the target property
    # If the specified flag is in that list, remove it and re-set the target property
  endforeach()
endfunction()

Copy link
Member

@am11 am11 Feb 26, 2025

Choose a reason for hiding this comment

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

@omajid, somehow it stopped working. The cmake generated makefiles (or ninja files) do not explicitly account for $CFLAGS etc. from environment variable anymore (assuming it was working when the original issue was fixed):

FROM alpine:latest

RUN apk add --no-cache cmake make ninja gcc musl-dev

WORKDIR /app

RUN cat <<'EOF' > CMakeLists.txt
cmake_minimum_required(VERSION 3.10)
project(HelloWorld C)

add_executable(hello main.c)
EOF

RUN cat <<'EOF' > main.c
#include <stdio.h>

int main() {
    #ifdef PRE_CONFIG_FLAG
        #error pre config CFLAGS were respected!
    #else
        #error pre config CFLAGS were ignored!
    #endif

    #ifdef POST_CONFIG_FLAG
        #error post config CFLAGS were respected!
    #else
        #error post config CFLAGS were ignored!
    #endif

    return 0;
}
EOF

ENV CFLAGS="-Wall -Wextra -DPRE_CONFIG_FLAG"

RUN cmake -G Ninja -B build_ninja
RUN cmake -G "Unix Makefiles" -B build_make

ENV CFLAGS="-Wall -Wextra -DPOST_CONFIG_FLAG"

RUN ninja -C build_ninja -v || true

RUN make -C build_make VERBOSE=1

with docker build . -t foo 2>&1 | tee
both ninja and make (penultimate and last lines) show us:

#12 0.125 /app/main.c:5:10: error: #error pre config CFLAGS were respected!
#12 0.125     5 |         #error pre config CFLAGS were respected!
#12 0.125       |          ^~~~~
#12 0.125 /app/main.c:13:10: error: #error post config CFLAGS were ignored!
#12 0.125    13 |         #error post config CFLAGS were ignored!
#12 0.125       |          ^~~~~

Copy link
Member

Choose a reason for hiding this comment

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

Should we revert the part of this change that impacts source build? Or are we ok with fixing outside of runtime?

Copy link
Member

@omajid omajid Feb 26, 2025

Choose a reason for hiding this comment

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

Thanks, @am11 and @grendello !

@steveisok said:

Or are we ok with fixing outside of runtime?

Based on the comments and feedback, I think we should fix it as a separate PR. We don't know what that exactly looks like, right now.

Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,20 @@ build_native()
# All set to commence the build
echo "Commencing build of \"$target\" target in \"$message\" for $__TargetOS.$__TargetArch.$__BuildType in $intermediatesDir"

SAVED_CFLAGS="${CFLAGS}"
SAVED_CXXFLAGS="${CXXFLAGS}"
SAVED_LDFLAGS="${LDFLAGS}"

# Let users provide additional compiler/linker flags via EXTRA_CFLAGS/EXTRA_CXXFLAGS/EXTRA_LDFLAGS.
# If users directly override CFLAG/CXXFLAGS/LDFLAGS, that may lead to some configure tests working incorrectly.
# See https://github.com/dotnet/runtime/issues/35727 for more information.
#
# These flags MUST be exported before gen-buildsys.sh runs or cmake will ignore them
#
export CFLAGS="${CFLAGS} ${EXTRA_CFLAGS}"
export CXXFLAGS="${CXXFLAGS} ${EXTRA_CXXFLAGS}"
export LDFLAGS="${LDFLAGS} ${EXTRA_LDFLAGS}"

if [[ "$targetOS" == osx || "$targetOS" == maccatalyst ]]; then
if [[ "$hostArch" == x64 ]]; then
cmakeArgs="-DCMAKE_OSX_ARCHITECTURES=\"x86_64\" $cmakeArgs"
Expand Down Expand Up @@ -194,17 +208,6 @@ build_native()
return
fi

SAVED_CFLAGS="${CFLAGS}"
SAVED_CXXFLAGS="${CXXFLAGS}"
SAVED_LDFLAGS="${LDFLAGS}"

# Let users provide additional compiler/linker flags via EXTRA_CFLAGS/EXTRA_CXXFLAGS/EXTRA_LDFLAGS.
# If users directly override CFLAG/CXXFLAGS/LDFLAGS, that may lead to some configure tests working incorrectly.
# See https://github.com/dotnet/runtime/issues/35727 for more information.
export CFLAGS="${CFLAGS} ${EXTRA_CFLAGS}"
export CXXFLAGS="${CXXFLAGS} ${EXTRA_CXXFLAGS}"
export LDFLAGS="${LDFLAGS} ${EXTRA_LDFLAGS}"

local exit_code
if [[ "$__StaticAnalyzer" == 1 ]]; then
pushd "$intermediatesDir"
Expand Down
14 changes: 4 additions & 10 deletions eng/native/gen-buildsys.sh
Original file line number Diff line number Diff line change
Expand Up @@ -106,15 +106,9 @@ if [[ "$host_arch" == "wasm" ]]; then
fi
fi

$cmake_command \
--no-warn-unused-cli \
-G "$generator" \
"-DCMAKE_BUILD_TYPE=$buildtype" \
"-DCMAKE_INSTALL_PREFIX=$__CMakeBinDir" \
$cmake_extra_defines \
$__UnprocessedCMakeArgs \
"${cmake_extra_defines_wasm[@]}" \
-S "$1" \
-B "$2"
buildsys_command="$cmake_command --no-warn-unused-cli -G \"$generator\" \"-DCMAKE_BUILD_TYPE=$buildtype\" \"-DCMAKE_INSTALL_PREFIX=$__CMakeBinDir\" $cmake_extra_defines $__UnprocessedCMakeArgs \"${cmake_extra_defines_wasm[@]}\" -S \"$1\" -B \"$2\""
buildsys_command=$(echo $buildsys_command | sed 's/""//g')
echo $buildsys_command
eval $buildsys_command

# don't add anything after this line so the cmake exit code gets propagated correctly
5 changes: 3 additions & 2 deletions src/coreclr/dlls/mscoree/exports.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -292,9 +292,10 @@ int coreclr_initialize(

ConstWStringHolder appDomainFriendlyNameW = StringToUnicode(appDomainFriendlyName);

if (bundleProbe != nullptr)
ExternalAssemblyProbeFn* externalAssemblyProbe = hostContract != nullptr ? hostContract->external_assembly_probe : nullptr;
if (bundleProbe != nullptr || externalAssemblyProbe != nullptr)
{
static Bundle bundle(exePath, bundleProbe);
static Bundle bundle(exePath, bundleProbe, externalAssemblyProbe);
Bundle::AppBundle = &bundle;
}

Expand Down
1 change: 1 addition & 0 deletions src/coreclr/hosts/inc/coreclrhost.h
Copy link
Member

@ivanpovazan ivanpovazan Feb 24, 2025

Choose a reason for hiding this comment

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

As we have changes in hosting header files (this one and src/native/corehost/host_runtime_contract.h below), I think we could consider shipping hosting headers as part of the Android CoreCLR runtime pack?

@elinor-fung @jkotas what do you think?

Copy link
Member

Choose a reason for hiding this comment

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

We have been treating the coreclr.dll hosting interface as an internal detail that is not officially documented and that is not guaranteed to be stable. Are you suggesting that we change that?

Copy link
Member

Choose a reason for hiding this comment

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

Can we ship headers and still consider them not guaranteed to be stable? That's what mono does with certain parts of its hosting API.

Copy link
Member

Choose a reason for hiding this comment

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

In addition to what Steve said:

Currently, on dotnet/macios side the build downloads the coreclrhost.h header file to build the CoreCLR host:
https://github.com/dotnet/macios/blob/30cce24d4c18d149879fe8afa64331491535ea0b/runtime/Makefile#L38
so with every change to the file, the download URL needs to be updated to have consistent behavior. This is difficult to maintain as the version of the file needs to be aligned with the runtime pack version including the actual implementation changes.
As we are now bringing CoreCLR support for more platforms and as we are in the development stage syncing changes between the runtime and embedders/hosts, I was curious if we would want to ship the required header files as part of the runtime pack (like mono does), to avoid manual downloads or potential mismatches.

Copy link
Member

Choose a reason for hiding this comment

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

There's always the option of sharing a transport package if we don't want to ship it.

Copy link
Member

@elinor-fung elinor-fung Feb 24, 2025

Choose a reason for hiding this comment

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

I don't think we want the internal coreclr interface as part of the shipped runtime pack. We do include public hosting headers (coreclr_delegates.h, hostfxr.h, nethost.h) in the host pack.

Transport package seems reasonable if we need a better way for other repos to get at internals.

Original file line number Diff line number Diff line change
Expand Up @@ -150,6 +150,7 @@ CORECLR_HOSTING_API(coreclr_execute_assembly,
//
// Callback types used by the hosts
//
typedef bool(CORECLR_CALLING_CONVENTION ExternalAssemblyProbeFn)(const char* path, void** data_start, int64_t* size);
typedef bool(CORECLR_CALLING_CONVENTION BundleProbeFn)(const char* path, int64_t* offset, int64_t* size, int64_t* compressedSize);
typedef const void* (CORECLR_CALLING_CONVENTION PInvokeOverrideFn)(const char* libraryName, const char* entrypointName);

Expand Down
14 changes: 8 additions & 6 deletions src/coreclr/inc/bundle.h
Original file line number Diff line number Diff line change
Expand Up @@ -18,29 +18,31 @@ class Bundle;
struct BundleFileLocation
{
INT64 Size;
void* DataStart;
INT64 Offset;
INT64 UncompresedSize;

BundleFileLocation()
{
{
LIMITED_METHOD_CONTRACT;

Size = 0;
Offset = 0;
DataStart = nullptr;
Offset = 0;
UncompresedSize = 0;
}

static BundleFileLocation Invalid() { LIMITED_METHOD_CONTRACT; return BundleFileLocation(); }

const SString &Path() const;

bool IsValid() const { LIMITED_METHOD_CONTRACT; return Offset != 0; }
bool IsValid() const { LIMITED_METHOD_CONTRACT; return DataStart != nullptr || Offset != 0; }
};

class Bundle
{
public:
Bundle(LPCSTR bundlePath, BundleProbeFn *probe);
Bundle(LPCSTR bundlePath, BundleProbeFn *probe, ExternalAssemblyProbeFn* externalAssemblyProbe = nullptr);
BundleFileLocation Probe(const SString& path, bool pathIsBundleRelative = false) const;

const SString &Path() const { LIMITED_METHOD_CONTRACT; return m_path; }
Expand All @@ -51,9 +53,9 @@ class Bundle
static BundleFileLocation ProbeAppBundle(const SString& path, bool pathIsBundleRelative = false);

private:

SString m_path; // The path to single-file executable
SString m_path; // The path to single-file executable or package name/id on Android
BundleProbeFn *m_probe;
ExternalAssemblyProbeFn *m_externalAssemblyProbe;

SString m_basePath; // The prefix to denote a path within the bundle
COUNT_T m_basePathLength;
Expand Down
3 changes: 1 addition & 2 deletions src/coreclr/pal/src/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -300,8 +300,7 @@ if(CLR_CMAKE_TARGET_LINUX)
else(NOT CLR_CMAKE_TARGET_ANDROID)
target_link_libraries(coreclrpal
PUBLIC
${ANDROID_GLOB}
${LZMA})
${ANDROID_GLOB})
endif(NOT CLR_CMAKE_TARGET_ANDROID)

target_link_libraries(coreclrpal
Expand Down
57 changes: 38 additions & 19 deletions src/coreclr/vm/bundle.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -30,15 +30,18 @@ const SString &BundleFileLocation::Path() const
return Bundle::AppBundle->Path();
}

Bundle::Bundle(LPCSTR bundlePath, BundleProbeFn *probe)
Bundle::Bundle(LPCSTR bundlePath, BundleProbeFn *probe, ExternalAssemblyProbeFn* externalAssemblyProbe)
: m_probe(probe)
, m_externalAssemblyProbe(externalAssemblyProbe)
, m_basePathLength(0)
{
STANDARD_VM_CONTRACT;

_ASSERTE(probe != nullptr);
_ASSERTE(m_probe != nullptr || m_externalAssemblyProbe != nullptr);

// On Android this is not a real path, but rather the application's package name
m_path.SetUTF8(bundlePath);
m_probe = probe;

#if !defined(TARGET_ANDROID)
// The bundle-base path is the directory containing the single-file bundle.
// When the Probe() function searches within the bundle, it masks out the basePath from the assembly-path (if found).

Expand All @@ -47,14 +50,13 @@ Bundle::Bundle(LPCSTR bundlePath, BundleProbeFn *probe)
size_t baseLen = pos - bundlePath + 1; // Include DIRECTORY_SEPARATOR_CHAR_A in m_basePath
m_basePath.SetUTF8(bundlePath, (COUNT_T)baseLen);
m_basePathLength = (COUNT_T)baseLen;
#endif // !TARGET_ANDROID
}

BundleFileLocation Bundle::Probe(const SString& path, bool pathIsBundleRelative) const
{
STANDARD_VM_CONTRACT;

BundleFileLocation loc;

// Skip over m_base_path, if any. For example:
// Bundle.Probe("lib.dll") => m_probe("lib.dll")
// Bundle.Probe("path/to/exe/lib.dll") => m_probe("lib.dll")
Expand All @@ -77,27 +79,44 @@ BundleFileLocation Bundle::Probe(const SString& path, bool pathIsBundleRelative)
else
{
// This is not a file within the bundle
return loc;
return BundleFileLocation::Invalid();
}
}

INT64 fileSize = 0;
INT64 compressedSize = 0;

m_probe(utf8Path, &loc.Offset, &fileSize, &compressedSize);

if (compressedSize)
if (m_probe != nullptr)
{
loc.Size = compressedSize;
loc.UncompresedSize = fileSize;
BundleFileLocation loc;
INT64 fileSize = 0;
INT64 compressedSize = 0;
if (m_probe(utf8Path, &loc.Offset, &fileSize, &compressedSize))
{
// Found assembly in bundle
if (compressedSize)
{
loc.Size = compressedSize;
loc.UncompresedSize = fileSize;
}
else
{
loc.Size = fileSize;
loc.UncompresedSize = 0;
}

return loc;
}
}
else

if (m_externalAssemblyProbe != nullptr)
{
loc.Size = fileSize;
loc.UncompresedSize = 0;
BundleFileLocation loc;
if (m_externalAssemblyProbe(utf8Path, &loc.DataStart, &loc.Size))
{
// Found via external assembly probe
return loc;
}
}

return loc;
return BundleFileLocation::Invalid();
}

BundleFileLocation Bundle::ProbeAppBundle(const SString& path, bool pathIsBundleRelative)
Expand Down
9 changes: 6 additions & 3 deletions src/coreclr/vm/coreassemblyspec.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -89,10 +89,13 @@ STDAPI BinderAcquirePEImage(LPCWSTR wszAssemblyPath,
PEImageHolder pImage = PEImage::OpenImage(wszAssemblyPath, MDInternalImport_Default, bundleFileLocation);

// Make sure that the IL image can be opened.
hr=pImage->TryOpenFile();
if (FAILED(hr))
if (pImage->IsFile())
{
goto Exit;
hr = pImage->TryOpenFile();
if (FAILED(hr))
{
goto Exit;
}
}

if (pImage)
Expand Down
7 changes: 4 additions & 3 deletions src/coreclr/vm/peimage.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -863,13 +863,15 @@ HRESULT PEImage::TryOpenFile(bool takeLock)
{
STANDARD_VM_CONTRACT;

_ASSERTE(IsFile());

SimpleWriteLockHolder lock(m_pLayoutLock, takeLock);

if (m_hFile!=INVALID_HANDLE_VALUE)
if (m_hFile != INVALID_HANDLE_VALUE)
return S_OK;

ErrorModeHolder mode{};
m_hFile=WszCreateFile((LPCWSTR)GetPathToLoad(),
m_hFile = WszCreateFile((LPCWSTR)GetPathToLoad(),
GENERIC_READ
#if TARGET_WINDOWS
// the file may have native code sections, make sure we are allowed to execute the file
Expand All @@ -881,7 +883,6 @@ HRESULT PEImage::TryOpenFile(bool takeLock)
OPEN_EXISTING,
FILE_ATTRIBUTE_NORMAL,
NULL);

if (m_hFile != INVALID_HANDLE_VALUE)
return S_OK;

Expand Down
1 change: 1 addition & 0 deletions src/coreclr/vm/peimage.h
Original file line number Diff line number Diff line change
Expand Up @@ -138,6 +138,7 @@ class PEImage final

BOOL IsFile();
BOOL IsInBundle() const;
void* GetExternalData(INT64* size);
INT64 GetOffset() const;
INT64 GetSize() const;
INT64 GetUncompressedSize() const;
Expand Down
13 changes: 11 additions & 2 deletions src/coreclr/vm/peimage.inl
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,16 @@ inline const SString& PEImage::GetPathToLoad()
return IsInBundle() ? m_bundleFileLocation.Path() : m_path;
}

inline void* PEImage::GetExternalData(INT64* size)
{
LIMITED_METHOD_CONTRACT;

_ASSERTE(size != nullptr);

*size = m_bundleFileLocation.Size;
return m_bundleFileLocation.DataStart;
}

inline INT64 PEImage::GetOffset() const
{
LIMITED_METHOD_CONTRACT;
Expand Down Expand Up @@ -98,8 +108,7 @@ inline const SString &PEImage::GetModuleFileNameHintForDAC()
inline BOOL PEImage::IsFile()
{
WRAPPER_NO_CONTRACT;

return !GetPathToLoad().IsEmpty();
return m_bundleFileLocation.DataStart == nullptr && !GetPathToLoad().IsEmpty();
}

//
Expand Down
21 changes: 16 additions & 5 deletions src/coreclr/vm/peimagelayout.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -616,17 +616,28 @@ FlatImageLayout::FlatImageLayout(PEImage* pOwner)
PRECONDITION(CheckPointer(pOwner));
}
CONTRACTL_END;
m_pOwner=pOwner;

HANDLE hFile = pOwner->GetFileHandle();
INT64 offset = pOwner->GetOffset();
INT64 size = pOwner->GetSize();
m_pOwner = pOwner;

#ifdef LOGGING
SString ownerPath{ pOwner->GetPath() };
LOG((LF_LOADER, LL_INFO100, "PEImage: Opening flat %s\n", ownerPath.GetUTF8()));
#endif // LOGGING

INT64 dataSize;
void* data = pOwner->GetExternalData(&dataSize);
if (data != nullptr)
{
// Image was provided as flat data via external assembly probing.
// We do not manage the data - just initialize with it directly.
_ASSERTE(dataSize != 0);
Init(data, (COUNT_T)dataSize);
return;
}

HANDLE hFile = pOwner->GetFileHandle();
INT64 offset = pOwner->GetOffset();
INT64 size = pOwner->GetSize();

// If a size is not specified, load the whole file
if (size == 0)
{
Expand Down
10 changes: 6 additions & 4 deletions src/coreclr/vm/peimagelayout.h
Original file line number Diff line number Diff line change
Expand Up @@ -81,11 +81,7 @@ class FlatImageLayout : public PEImageLayout
{
VPTR_VTABLE_CLASS(FlatImageLayout, PEImageLayout)
VPTR_UNIQUE(0x59)
protected:
CLRMapViewHolder m_FileView;
public:
HandleHolder m_FileMap;

#ifndef DACCESS_COMPILE
FlatImageLayout(PEImage* pOwner);
FlatImageLayout(PEImage* pOwner, const BYTE* array, COUNT_T size);
Expand All @@ -94,6 +90,12 @@ class FlatImageLayout : public PEImageLayout
void* LoadImageByMappingParts(SIZE_T* m_imageParts) const;
#endif
#endif

private:
// Handles for the mapped image.
// These will be null if the image data is not mapped by the runtime (for example, provided via an external assembly probe).
CLRMapViewHolder m_FileView;
HandleHolder m_FileMap;
};

// ConvertedImageView is for the case when we construct a loaded
Expand Down
10 changes: 8 additions & 2 deletions src/coreclr/vm/util.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,9 @@

#ifndef DACCESS_COMPILE

#if defined(TARGET_ANDROID)
#include <android/log.h>
#endif // defined(TARGET_ANDROID)

thread_local size_t t_ThreadType;

Expand Down Expand Up @@ -177,10 +180,13 @@ void PrintToStdErrA(const char *pszString) {
}
CONTRACTL_END

HANDLE Handle = GetStdHandle(STD_ERROR_HANDLE);

#if defined(TARGET_ANDROID)
__android_log_write(ANDROID_LOG_FATAL, MAIN_CLR_MODULE_NAME_A, pszString);
#else
HANDLE Handle = GetStdHandle(STD_ERROR_HANDLE);
size_t len = strlen(pszString);
NPrintToHandleA(Handle, pszString, len);
#endif // defined(TARGET_ANDROID)
}

void PrintToStdErrW(const WCHAR *pwzString)
Expand Down
Loading
Loading