Why can't we pass two separate parameters?

I am concerned about framework-dependent scenarios where we may be calling the 5.0 version of the probe. Having the same signature allows the old clients to be forwards-compatible with new runtimes.
I.E. 5.0 probe will use only the first element of the array, which contains the file size, which is compatible with the new {size, uncompressedSize}.

Passing an extra parameter "may work", depending on calling conventions. Passing an array does not need to rely on that.

BTW - if we may need another piece of information from the probe in the future, we can continue adding optional array elements.

But framework dependent app should use hostpolicy.dll which is next to the runtime - we should not be statically linking hostpolicy anywhere but superhost. So far we've made the assumption that hostpolicy/runtime interface is not "public" and can be change at any time without versioning considerations, because we always ship hostpolicy and runtime together.

Hmm. I think I saw adding an extra parameter causing failures in framework-dependent 5.0 tests on OSX.
Although we also had uninitialized data problem as well and it could be just that. Let me think about this.

I thought we could have a case where 6.0 clr calls the probe and there is 5.0 hostpolicy on the other end. Perhaps this can't happen.

I think you are correct. The probe is always in sync with runtime. hostpolicy is either bundled together with runtime in superhost or picked up next to the runtime. Either way it will match the version/build.

That allows just using another parameter, so I have reverted the change that packs sizes in an array.

I have reverted the change that packs sizes into an array. The failures in framework-dependent 5.0 tests on OSX were indeed caused by uninitialized data (although they looked like a probe mismatch)

It just occurred to me - technically there's no reason to store the uncompressed size in the bundle headers. We would really need just a boolean. I assume it makes the code more efficient to know how much to allocate upfront to uncompress.

But what if the size in the header doesn't match the actual uncompressed size - we should have a test for that at the very least.

Passing uncompressed allows to set up the storage for uncompressed data precisely, so it is slightly more convenient than a bool.

If we have corrupted data we will not overrun the buffer, since we provide the size limit to the zlib.
It does make sense to add a check though and failfast if uncompressed size is less than expected, or compressed data was not all consumed. Good point!

Nit: Personally I would prefer to have a full == 0 in this case. I hate the C++ tricks of treating numericals as booleans - it confuses me when reading the code.

Will this actually fail correctly - ThrowLastError presumably throws the last Win32 error code, but in this case there was no OS call which failed before this... so the last error code is effectively random value.

This is a failfast for "impossible" situation. Something is badly corrupted and we cannot recover, if we hit this.
I will check what we use in such cases.

Does Windows not allow unicode characters in the path? Or is this really a byte stream?

If I remember correctly - all runtime/host interactions use UTF8 strings.

You use IntPtr for offset here, but the function pointer at the top is uint64_t. Won't that be the wrong size on x86?

Never mind, forgot the pointer part.

Why couldn't it stay in the common list? I have originally structured the lists so that no file needed to be duplicated in multiple lists.

The common list is compiled by the build only once. That includes almost every file. And we reuse objects for singlefile and regular coreclr.

Very few files need to know about singlefile and have parts conditionally compiled for CORECLR_EMBEDDED. peimagelayout.cpp cannot and does not need to support decompression in regular coreclr mode, so it needs to use #ifdef CORECLR_EMBEDDED

Are these additions really needed? Isn't the parameter-less constructor used by default?

Only if you do not have a constructor, compiler may generate one. Once you have a constructor, compiler just calls it.

new PEImage() would leave these fields uninitialized, but typically we were lucky except for OSX with optimizations (i.e. does not fail in Debug, but started failing in Checked/Release)

I don't like long relative paths in general, they are a burden when structure of the source tree changes. It seems it would be nicer to add the ../../../libraries/Native/AnyOS/zlib to the include paths in cmake scripts and have just

#include "pal_zlib.h"

here. It would be also nice to define the libraries path at one place in the cmake files and use it as a basis for this and the path in src/coreclr/vm/CMakeLists.txt.

There was another long pall_zlib.h include in apphost/static. I have changed that too.

Similar as the other place - I would prefer pOwner->GetUncompressedSize() != 0 - using numbers as booleans is confusing.