Redirect DllImport of hostpolicy to the main executable when it's embedded

[vitek-karas]

For singlefilehost the hostpolicy is statically linked into the executable, so we need to apply similar logic to other statically linked libraries by redirecting DllImport loads to the main executable module.

Because this happens also on Windows, the coreclr itself doesn't know statically (unlike on Linux where we link coreclr statically as well, so we can hardcode this knowledge). So the hostpolicy must pass this information to the runtime via a new runtime property.

Tested both on Windows and Linux.

Fixes #39907

[ghost]

Tagging subscribers to this area: @swaroop-sridhar, @agocke
See info in area-owners.md if you want to be subscribed.

[am11]

Would it be a good idea to invert this condition to make this logic future proof; such that all OS except Windows get libhostpolicy?

[VSadov]

Yes, I think everything but Windows should use libhostpolicy

[VSadov]

Is this actually the name used in this context? - not libhostpolicy.so or something with a path?

In the case above we could, in theory, see different file names, but we intentionally only recognize the form used in Interop.Libraries.cs.
Jut wonder whether there is anything to worry about hostpolicy naming.

[vitek-karas]

The story is the same - we don't expect any other code but corelib to call into hostpolicy directly, so these strings need to match what's in Interop.Libraries, but nothing else.

I fixed the condition to be for Windows - should also fix the build failures in CI

[VSadov]

LGTM

[am11]

nit: { on its own line for consistency.

[vitek-karas]

@BruceForstall can you please take a look why the formatting job is failing on this PR? The changes don't touch JIT code. And the generated patch file is empty (0 bytes).

[am11]

Windows formatting job failure is unrelated to PR changes, failing in every PR. See #40034.

[vitek-karas]

Thanks @am11 - I'm trying to figure out the other failures now...

[vitek-karas]

Something weird is happening. One of the tests is failing with:

Launching 'C:\h\w\B8170A18\w\AC3D09A7\e\Interop\COM\NativeClients\Primitives\COMClientPrimitives.exe'...

Expected: 100
Actual: 3
END EXECUTION - FAILED
FAILED

But looking at the code

runtime/src/tests/Interop/COM/NativeClients/Primitives/Client.cpp

Lines 24 to 45 in 7242e14

	int __cdecl main()
	{
	ComMTA init;
	if (FAILED(init.Result))
	return -1;
	try
	{
	Run_NumericTests();
	Run_ArrayTests();
	Run_StringTests();
	Run_ErrorTests();
	Run_ColorTests();
	}
	catch (HRESULT hr)
	{
	::printf("Test Failure: 0x%08x\n", hr);
	return 101;
	}
	return 100;
	}

There's no way it returns exit code 3.

[am11]

The Expected: 3 is indeed very fishy. Going by this investigation: #33733 (comment), it might be a similar intermittent assert is failing in runtime, GUI popping up on the build agent causing exit code 3. Maybe rerunning the CI will fix this error.
git commit -m "Re-run CI" --alow-empty && git push

[vitek-karas]

It is an assert - but seems valid - debugging through it right now.

[vitek-karas]

The test is mocking hostpolicy - so maybe there's something going on there (although I would not expect to affect non-single-file builds at all, given that the embedded variables should be false in that case)

[vitek-karas]

Found it - uninitialized variable (I've been working in C# for way too long now, forgot all of the C++ gotchas)... I can only wish for default compiler settings which would definitely show this as a warning.

[am11]

Heh, it's C++ which first says Values of type bool are either true or false. then later ... uninitialized automatic variable, might cause it to behave as if it is neither true nor false. 🙈

[vitek-karas]

In this case (on my machine anyway) it initialized to magical 204 (literally) - and since conditions treat anything non-zero as true....
Anyway - WARNINGS - but I guess that would be a project for like 3 months to clean up coreclr to be able to turn on at least some C++ warnings.

[vitek-karas]

Networking test failures are being fixed in #40070.

[jkotas]

WARNINGS - but I guess that would be a project for like 3 months to clean up coreclr to be able to turn on at least some C++ warnings.

We have all reasonable C++ warnings turned on. C++ compilers do not seem to provide warning to catch this type of problem. Neither MSVC w/ /W4 nor clang w/ -Wall warn for this code:

extern void g(bool* p);

void f()
{
   bool x;
   g(&x);
}

[vitek-karas]

It's great that we have warnings on... and not so great that the compiler doesn't have a way to warn on this (I guess it is a hard problem in C++). I definitely didn't mean to downplay our efforts on the codebase...