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

Fix embedded coreclr detection in corhost.cpp #80294

Merged
merged 1 commit into from
Jan 6, 2023

Conversation

janvorli
Copy link
Member

@janvorli janvorli commented Jan 6, 2023

The CORECLR_EMBEDDED define that is used in corhost.cpp to detect whether the current host has coreclr and some other native libraries embedded or not doesn't work. The reason is that corhost.cpp is not compiled separately for the cases of embedded and non-embedded coreclr.

The proper way is to check the g_coreclr_embedded global variable that is defined in the ceemain.cpp, which is compiled separately for those two cases.

While we could also make the corhost.cpp build twice, it would be a waste of time.

The `CORECLR_EMBEDDED` define that is used in corhost.cpp to detect
whether the current host has coreclr and some other native libraries
embedded or not doesn't work. The reason is that corhost.cpp is not
compiled separately for the cases of embedded and non-embedded coreclr.

The proper way is to check the `g_coreclr_embedded` global variable
that is defined in the ceemain.cpp, which is compiled separately for
those two cases.

While we could also make the corhost.cpp build twice, it would be
a waste of time.
@janvorli janvorli added this to the 8.0.0 milestone Jan 6, 2023
@janvorli janvorli requested review from jkotas and elinor-fung January 6, 2023 14:38
@janvorli janvorli self-assigned this Jan 6, 2023
@janvorli
Copy link
Member Author

janvorli commented Jan 6, 2023

cc: @am11

Copy link
Member

@am11 am11 left a comment

Choose a reason for hiding this comment

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

LGTM 👍

@janvorli janvorli merged commit bdd12cc into dotnet:main Jan 6, 2023
janvorli added a commit to janvorli/runtime that referenced this pull request Jan 6, 2023
Port of dotnet#78484, dotnet#78790, dotnet#80104 and dotnet#80294
This change adds detecting and logging of failures during coreclr
initialization. For logging, it uses a new host API
`coreclr_set_error_writer` to register a callback to report the errors
to the host. The hosts have support for optional usage of this API so
that they can work with older runtime versions as well.

The logging checks and reports failures with:
* System.Private.CoreLib.dll
* GC initialization
* JIT initialization
* libSystem.Native.so/dylib on Unix

The logging messages should allow customers to self-diagnose the issues
causing the failures.

This change also adds backport of support for standalone GC back compatibility that
is a prerequisite for it.
janvorli added a commit to janvorli/runtime that referenced this pull request Jan 17, 2023
Port of dotnet#78484, dotnet#78790, dotnet#80104 and dotnet#80294
This change adds detecting and logging of failures during coreclr
initialization. For logging, it uses a new host API
`coreclr_set_error_writer` to register a callback to report the errors
to the host. The hosts have support for optional usage of this API so
that they can work with older runtime versions as well.

The logging checks and reports failures with:
* System.Private.CoreLib.dll
* GC initialization
* JIT initialization
* libSystem.Native.so/dylib on Unix

The logging messages should allow customers to self-diagnose the issues
causing the failures.

This change also adds backport of support for standalone GC back compatibility that
is a prerequisite for it.
@ghost ghost locked as resolved and limited conversation to collaborators Feb 5, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants