From cd35bc5989305c191299e576e23b59db10403451 Mon Sep 17 00:00:00 2001
From: "Dustin L. Howett" <duhowett@microsoft.com>
Date: Thu, 9 Jun 2022 14:48:28 -0500
Subject: [PATCH] Restore OpenConsoleProxyStub's dependency on the CRT to fix
 QI (#13254)

When #13160 introduced a new interface to the IConsoleHandoff idl, it
changed midl's RPC proxy stub lookup algorithm from a direct GUID
comparison to an unrolled binary search. Now, that would ordinarily not
be a problem...

However, in #11610, we took a shortcut and replaced `memcmp` -- used
only by RPC for GUID comparison -- with a direct GUID-only equality
comparator. This worked totally fine, and ordinarily would not be a
problem...

The unrolled binary search unfortunately _relies on memcmp's contract_:
it uses memcmp to match against a fully sorted set. Our memcmp only
returned 0 or 1 (equal or not), and it knew nothing about ordering.

When a package that contains a PackagedCOM proxy stub is installed, it
is selected as the primary proxy stub for any interfaces it can proxy.
After all, interfaces are immutable, so it doesn't matter whose proxy
you're using. Now, given that we installed a *broken* proxy... *all*
IIDs that got whacked by our memcmp issue broke for every consumer.

To fix it: instead of implementing memcmp ourselves, we're just going to
take a page out of WinAppSDK's book and link this binary using the
"Hybrid CRT" model. It will statically link any parts of the STL it uses
(none) and dynamically link the ucrt (which is guaranteed to be present
on Windows.)

Sure, the binary size goes up from 8k to 24k, but... the cost is never
having to worry about this again.

Closes #13251
---
 .github/actions/spelling/expect/expect.txt |  2 -
 src/host/proxy/Host.Proxy.vcxproj          | 56 ++++++++++++++++------
 src/host/proxy/Host.Proxy.vcxproj.filters  |  3 --
 src/host/proxy/nodefaultlib_shim.h         |  8 ----
 4 files changed, 41 insertions(+), 28 deletions(-)
 delete mode 100644 src/host/proxy/nodefaultlib_shim.h

diff --git a/.github/actions/spelling/expect/expect.txt b/.github/actions/spelling/expect/expect.txt
index dd4342d9d25..6e4f259dcea 100644
--- a/.github/actions/spelling/expect/expect.txt
+++ b/.github/actions/spelling/expect/expect.txt
@@ -959,7 +959,6 @@ guardxfg
 guc
 gui
 guidatom
-guiddef
 GValue
 GWL
 GWLP
@@ -1567,7 +1566,6 @@ NOCOLOR
 NOCOMM
 NOCONTEXTHELP
 NOCOPYBITS
-nodefaultlib
 nodiscard
 NODUP
 noexcept
diff --git a/src/host/proxy/Host.Proxy.vcxproj b/src/host/proxy/Host.Proxy.vcxproj
index dd0551e104d..fc8ff483073 100644
--- a/src/host/proxy/Host.Proxy.vcxproj
+++ b/src/host/proxy/Host.Proxy.vcxproj
@@ -21,7 +21,7 @@
   <Import Project="$(SolutionDir)src\common.build.pre.props" />
   <ItemGroup>
     <Midl Include="IConsoleHandoff.idl">
-      <!--
+      <!-- 
         In Razzle, IDL files generate %FileName%.h
         In Visual Studio, IDL files generate %FileName%_h.h
         Visual Studio is easier to override than Razzle.
@@ -41,7 +41,6 @@
   <ItemGroup>
     <ClInclude Include="$(IntDir)\IConsoleHandoff.h" />
     <ClInclude Include="$(IntDir)\ITerminalHandoff.h" />
-    <ClInclude Include="nodefaultlib_shim.h" />
   </ItemGroup>
   <ItemGroup>
     <ClCompile Include="$(IntDir)\dlldata.c" />
@@ -67,27 +66,54 @@
   </ItemDefinitionGroup>
   <ItemDefinitionGroup>
     <ClCompile>
-      <AdditionalIncludeDirectories>.;..;%(AdditionalIncludeDirectories)</AdditionalIncludeDirectories>
-      <CallingConvention Condition="'$(Platform)'!='ARM64'">StdCall</CallingConvention>
+      <AdditionalIncludeDirectories>..;%(AdditionalIncludeDirectories)</AdditionalIncludeDirectories>
+      <CallingConvention>StdCall</CallingConvention>
       <!-- Must be Stdcall on all platforms to resolve _ObjectStublessClient3 -->
       <PreprocessorDefinitions>REGISTER_PROXY_DLL;WIN32;%(PreprocessorDefinitions)</PreprocessorDefinitions>
       <PrecompiledHeader>NotUsing</PrecompiledHeader>
-      <BasicRuntimeChecks>Default</BasicRuntimeChecks>
-      <BufferSecurityCheck>false</BufferSecurityCheck>
-      <SDLCheck>false</SDLCheck>
-      <ForcedIncludeFiles>nodefaultlib_shim.h;%(ForcedIncludeFiles)</ForcedIncludeFiles>
     </ClCompile>
     <Link>
       <ModuleDefinitionFile>OpenConsoleProxy.def</ModuleDefinitionFile>
-      <!--
-        Not depending on the CRT cuts binary size by half and prevents issues if this DLL
-        is copied elsewhere and executed outside of our app package without our bundled CRT present.
-      -->
-      <AdditionalDependencies />
-      <IgnoreAllDefaultLibraries>true</IgnoreAllDefaultLibraries>
-      <EntryPointSymbol>DllMain</EntryPointSymbol>
     </Link>
   </ItemDefinitionGroup>
+
+  <!--
+    OpenConsoleProxy gets copied out of our app package and into a shared system store. As such, it can't take a
+    dependency on any libraries inside our package **or** inside any of our dependency packages. It has to stand
+    on its own.
+    Therefore, we're going to use the Hybrid CRT model from WinAppSDK for only OpenConsoleProxy. It statically
+    links the runtime and STL and dynamically links the UCRT instead of the VC++ CRT. The UCRT ships with Windows.
+    WinAppSDK asserts that this is "supported according to the CRT maintainer."
+  -->
+  <ItemDefinitionGroup Condition="'$(Configuration)'=='Debug'">
+    <ClCompile>
+      <!-- We use MultiThreadedDebug, rather than MultiThreadedDebugDLL, to avoid DLL dependencies on VCRUNTIME140d.dll and MSVCP140d.dll. -->
+      <RuntimeLibrary>MultiThreadedDebug</RuntimeLibrary>
+    </ClCompile>
+    <Link>
+      <!-- Link statically against the runtime and STL, but link dynamically against the CRT by ignoring the static CRT
+           lib and instead linking against the Universal CRT DLL import library. This "hybrid" linking mechanism is
+           supported according to the CRT maintainer. Dynamic linking against the CRT makes the binaries a bit smaller
+           than they would otherwise be if the CRT, runtime, and STL were all statically linked in. -->
+      <IgnoreSpecificDefaultLibraries>%(IgnoreSpecificDefaultLibraries);libucrtd.lib</IgnoreSpecificDefaultLibraries>
+      <AdditionalOptions>%(AdditionalOptions) /defaultlib:ucrtd.lib</AdditionalOptions>
+    </Link>
+  </ItemDefinitionGroup>
+  <ItemDefinitionGroup Condition="'$(Configuration)'=='Release'">
+    <ClCompile>
+      <!-- We use MultiThreaded, rather than MultiThreadedDLL, to avoid DLL dependencies on VCRUNTIME140.dll and MSVCP140.dll. -->
+      <RuntimeLibrary>MultiThreaded</RuntimeLibrary>
+    </ClCompile>
+    <Link>
+      <!-- Link statically against the runtime and STL, but link dynamically against the CRT by ignoring the static CRT
+           lib and instead linking against the Universal CRT DLL import library. This "hybrid" linking mechanism is
+           supported according to the CRT maintainer. Dynamic linking against the CRT makes the binaries a bit smaller
+           than they would otherwise be if the CRT, runtime, and STL were all statically linked in. -->
+      <IgnoreSpecificDefaultLibraries>%(IgnoreSpecificDefaultLibraries);libucrt.lib</IgnoreSpecificDefaultLibraries>
+      <AdditionalOptions>%(AdditionalOptions) /defaultlib:ucrt.lib</AdditionalOptions>
+    </Link>
+  </ItemDefinitionGroup>
+
   <!-- Careful reordering these. Some default props (contained in these files) are order sensitive. -->
   <Import Project="$(SolutionDir)src\common.build.post.props" />
 </Project>
diff --git a/src/host/proxy/Host.Proxy.vcxproj.filters b/src/host/proxy/Host.Proxy.vcxproj.filters
index 1cdf7ca2512..df6bcbd10c6 100644
--- a/src/host/proxy/Host.Proxy.vcxproj.filters
+++ b/src/host/proxy/Host.Proxy.vcxproj.filters
@@ -41,9 +41,6 @@
     <ClInclude Include="$(IntDir)\ITerminalHandoff.h">
       <Filter>Header Files</Filter>
     </ClInclude>
-    <ClInclude Include="nodefaultlib_shim.h">
-      <Filter>Header Files</Filter>
-    </ClInclude>
   </ItemGroup>
   <ItemGroup>
     <Midl Include="IConsoleHandoff.idl">
diff --git a/src/host/proxy/nodefaultlib_shim.h b/src/host/proxy/nodefaultlib_shim.h
deleted file mode 100644
index 4face4e49f2..00000000000
--- a/src/host/proxy/nodefaultlib_shim.h
+++ /dev/null
@@ -1,8 +0,0 @@
-// Copyright (c) Microsoft Corporation.
-// Licensed under the MIT license.
-
-#pragma once
-
-#include <guiddef.h>
-
-#define memcmp(a, b, c) (!InlineIsEqualGUID(a, b))