Force-disable BMI2 instruction set when building snappy.lib (#47)

Firebase-cpp-sdk on Windows pulls in firebase-ios-sdk (for firestore code), which pulls in leveldb (for key/value persistence) and Snappy (for efficient data compression). For faster compression, Snappy uses one specific CPU instruction for bit manipulation, bzhi, from the BMI2 instruction set. However, this instruction crashes firestore on a CPU that does not support the BMI2 instructions, which is the case of older CPUs, for example Intel pre-Haswell and AMD pre-Excavator CPUs.

By default, Snappy detects if it should use the BMI2 optimized instruction by performing a compilation test on the cmake configure phase , which obviously returns if the build machine supports BMI2, not the target machines.
Snappy build setup does not support a compiler directive that overrides that specific compilation test, so it can only build for the host machine BMI2 setup.

Unfortunately, due to the very involved dependency chain, there is no good way to patch Snappy's code and the easiest solution in terms of maintenance is to patch the config.h created during the configure step, which is what this PR does. It only changes the automated build process (Github Actions workflow).

Author: mangini

.github/workflows/bcny-firebase.yml

@@ -119,6 +119,25 @@ jobs:
                 -D CMAKE_MSVC_DEBUG_INFORMATION_FORMAT=Embedded `
                 -D FIREBASE_PYTHON_HOST_EXECUTABLE:FILEPATH=${{ steps.python.outputs.python-path }} `
                 -D FLATBUFFERS_FLATC_EXECUTABLE=${{ github.workspace }}/BinaryCache/flatbuffers/Release/flatc.exe
+
+      # The step below is necessary since Snappy detects if it should use BMI2 instructions on x64 based only on its ability to
+      # compile BMI2 code on the host machine, but we need to disable BMI2 instructions so that the resulting nuget package
+      # can be used on target machines without BMI2 support. Unfortunately Snappy is pulled indirectly from
+      # another dependency (firebase-ios-sdk) in a very involved dependency chain and there is no good way to
+      # set SNAPPY_HAVE_BMI2 without patching its code since compiler directives have less precendence than
+      # the result of the host machine's check_cxx_source_compiles (which ends up in the config.h file).
+      # So, the less messy solution seems to be to patch the config.h file after the configure step, which is what we do below.
+      - name: Change SNAPPY_HAVE_BMI2 in snappy config.h
+        # BMI2 instructions are only relevant in amd64
+        if: matrix.arch == 'amd64'
+        run: |
+          $snappy_config = "${{ github.workspace }}\BinaryCache\firebase\external\src\firestore-build\external\src\snappy-build\config.h"
+          if (-not (Select-String -Path $snappy_config -Pattern "#define SNAPPY_HAVE_BMI2 1")) {
+            Write-Error "String '#define SNAPPY_HAVE_BMI2 1' expected but not found in $snappy_config"
+            exit 1
+          }
+          (Get-Content $snappy_config) -replace '#define SNAPPY_HAVE_BMI2 1', '#define SNAPPY_HAVE_BMI2 0' | Set-Content $snappy_config
+
       - name: Build firebase
         run: cmake --build ${{ github.workspace }}/BinaryCache/firebase --config RelWithDebInfo
       - name: Install firebase
@@ -137,6 +156,30 @@ jobs:
             Copy-Item -Path $library.FullName -Destination $destination -Force
             Write-Host "... copied ${destination}"
           }
+
+      # We need this library to be used on CPUs without BMI2 support, so we check that snappy.lib was built correctly,
+      # and fail if it contains BMI2 instructions.
+      #
+      # Note: We explicitly check for bzhi because that is the BMI2 instruction used in snappy source code if BMI2 is enabled:
+      # https://github.com/google/snappy/blob/2c94e11145f0b7b184b831577c93e5a41c4c0346/snappy.cc#L1197
+      # It may be possible that the compiler decides to add other BMI2 instructions automatically, but by checking
+      # for the absense of 'bzhi' we are at least ensuring that the explicit SNAPPY_HAVE_BMI2 change in config.h is
+      # being respected.
+      - name: Check for Snappy BMI2 instructions
+        # BMI2 instructions are only relevant in amd64
+        if: matrix.arch == 'amd64'
+        run: |
+          Write-Host "Checking for BMI2 instructions on Snappy.lib..."
+          $snappy_lib = "${{ github.workspace }}/BuildRoot/Library/firebase/usr/libs/windows/snappy.lib"
+          $output = dumpbin /DISASM "$snappy_lib" | Select-String -Pattern "bzhi"
+          if ($output) {
+            Write-Host $output
+            Write-Error "ERROR: A BMI2 instruction ('bzhi') was not supposed to be found in $snappy_lib."
+            exit 1
+          } else {
+            Write-Output "Success! No BMI2 instructions were found in snappy.lib."
+          }
+
       - uses: actions/upload-artifact@v3
         with:
           name: firebase-windows-${{ matrix.arch }}