Fix Windows/ARM64 assembly build (#1697)

### Issues: * Addresses: aws/aws-lc-rs#453 ### Description of changes: * When building for Windows/ARM64 with the "Visual Studio" generator (instead of Ninja) the "*.S" assembly source files for libraries are ignored. * This change provides a custom command to assemble the require objects files and adds them to the list of sources for the library. ### Call-outs: * See related aws-lc-rs PR: aws/aws-lc-rs#452 ### Testing * I've added CI tests to build for Windows using the "Visual Studio" generator. By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license and the ISC license.
aws · Jul 15, 2024 · 5665ed6 · 5665ed6
1 parent 1fbf584
commit 5665ed6
Show file tree

Hide file tree

Showing 7 changed files with 129 additions and 9 deletions.
diff --git a/.github/workflows/windows-alt.yml b/.github/workflows/windows-alt.yml
@@ -63,13 +63,53 @@ jobs:
           options: |
             CMAKE_SYSTEM_NAME=Windows \
             CMAKE_SYSTEM_PROCESSOR=x86_64 \
-            CMAKE_MAKE_PROGRAM=ninja.exe \
             CMAKE_BUILD_TYPE=Release \
       - name: Build Project
         run: cmake --build ./build --target all
       - name: Run tests
         run: cmake --build ./build --target run_tests
-  clang-cl:
+  clang-cl-msbuild:
+    if: github.repository_owner == 'aws'
+    strategy:
+      fail-fast: false
+      matrix:
+        target:
+          - x64
+          - x64_arm64
+    runs-on: windows-latest
+    env:
+      CMAKE_GENERATOR: "Visual Studio 17 2022"
+      CMAKE_GENERATOR_TOOLSET: "ClangCL,host=x64"
+    steps:
+      - if: ${{ matrix.target  == 'x64' }}
+        name: Install NASM
+        uses: ilammy/setup-nasm@v1.5.1
+      - name: Checkout
+        uses: actions/checkout@v4
+      - uses: TheMrMilchmann/setup-msvc-dev@v3
+        with:
+          arch: ${{ matrix.target }}
+      - if: ${{ matrix.target  == 'x64' }}
+        name: Setup CMake
+        uses: threeal/cmake-action@v1.3.0
+        with:
+          options: |
+            CMAKE_BUILD_TYPE=Release \
+      - if: ${{ matrix.target  == 'x64_arm64' }}
+        name: Setup CMake
+        uses: threeal/cmake-action@v1.3.0
+        with:
+          options: |
+            CMAKE_GENERATOR_PLATFORM=ARM64 \
+            CMAKE_SYSTEM_NAME=Windows \
+            CMAKE_SYSTEM_PROCESSOR=ARM64 \
+            CMAKE_BUILD_TYPE=Release \
+      - name: Build Project
+        run: cmake --build ./build --target all_tests
+      - if: ${{ matrix.target  == 'x64' }}
+        name: Run tests
+        run: cmake --build ./build --target run_tests
+  clang-cl-ninja:
     if: github.repository_owner == 'aws'
     strategy:
       fail-fast: false
@@ -97,9 +137,6 @@ jobs:
           c-compiler: clang-cl
           cxx-compiler: clang-cl
           options: |
-            CMAKE_CROSSCOMPILING=${{ ((matrix.target  == 'x64') && '0') || '1' }} \
-            CMAKE_SYSTEM_NAME=Windows \
-            CMAKE_SYSTEM_PROCESSOR=x86_64 \
             CMAKE_BUILD_TYPE=Release \
       - if: ${{ matrix.target  == 'x64_arm64' }}
         name: Setup CMake
@@ -109,7 +146,6 @@ jobs:
           c-compiler: clang-cl
           cxx-compiler: clang-cl
           options: |
-            CMAKE_CROSSCOMPILING=1 \
             CMAKE_SYSTEM_NAME=Windows \
             CMAKE_SYSTEM_PROCESSOR=ARM64 \
             CMAKE_C_COMPILER_TARGET=arm64-pc-windows-msvc \

diff --git a/CMakeLists.txt b/CMakeLists.txt
@@ -9,6 +9,10 @@ if(MSVC)
   set(CMAKE_GENERATOR_CC cl)
 endif()
 
+if(ARCH STREQUAL "aarch64" AND CMAKE_GENERATOR MATCHES "Visual Studio" AND NOT "${CMAKE_VS_PLATFORM_TOOLSET}" MATCHES "ClangCL")
+  message(FATAL_ERROR "AWS-LC Windows/ARM64 assembly code requires ClangCL. Current toolset: ${CMAKE_VS_PLATFORM_TOOLSET}")
+endif()
+
 include(sources.cmake)
 include(TestBigEndian)
 
@@ -366,10 +370,10 @@ if(GCC OR CLANG)
     else()
       set(CMAKE_C_FLAGS "${CMAKE_C_FLAGS} -Wextra")
       set(C_CXX_FLAGS "${C_CXX_FLAGS} -Wall -fvisibility=hidden -fno-common")
+      set(CMAKE_ASM_FLAGS "${CMAKE_ASM_FLAGS} -Wno-newline-eof")
     endif()
     set(CMAKE_C_FLAGS "${CMAKE_C_FLAGS} -Wunused -Wcomment -Wchar-subscripts -Wuninitialized -Wshadow")
     set(CMAKE_C_FLAGS "${CMAKE_C_FLAGS} -Wwrite-strings -Wformat-security -Wunused-result -Wno-overlength-strings")
-    set(CMAKE_ASM_FLAGS "${CMAKE_ASM_FLAGS} -Wno-newline-eof")
     set(C_CXX_FLAGS "${C_CXX_FLAGS} -Wno-c11-extensions -Wvla -Wtype-limits -Wno-unused-parameter")
   endif()
   set(C_CXX_FLAGS "${C_CXX_FLAGS} -Werror -Wformat=2 -Wsign-compare -Wmissing-field-initializers -Wwrite-strings")
@@ -945,6 +949,20 @@ if(BUILD_TESTING)
   endif()
 
   add_subdirectory(util/fipstools/acvp/modulewrapper)
+
+  macro(set_test_location executable_name)
+    if(CMAKE_GENERATOR MATCHES "Visual Studio")
+      message(NOTICE "Location  for ${executable_name} is: ${CMAKE_CURRENT_BINARY_DIR}")
+      # Set the output directory for the executable
+      set_target_properties(${executable_name} PROPERTIES
+        RUNTIME_OUTPUT_DIRECTORY "${CMAKE_CURRENT_BINARY_DIR}"
+        RUNTIME_OUTPUT_DIRECTORY_DEBUG "${CMAKE_CURRENT_BINARY_DIR}"
+        RUNTIME_OUTPUT_DIRECTORY_RELEASE "${CMAKE_CURRENT_BINARY_DIR}"
+        RUNTIME_OUTPUT_DIRECTORY_RELWITHDEBINFO "${CMAKE_CURRENT_BINARY_DIR}"
+        RUNTIME_OUTPUT_DIRECTORY_MINSIZEREL "${CMAKE_CURRENT_BINARY_DIR}"
+      )
+    endif()
+  endmacro()
 endif()
 
 add_subdirectory(crypto)

diff --git a/crypto/CMakeLists.txt b/crypto/CMakeLists.txt
@@ -1,3 +1,48 @@
+
+# Function to handle assembly files for ARM64 targets when using MSBuild (the "Visual Studio" generator)
+# This function is necessary because MSBuild ignores ARM64 assembly file dependencies
+include(CMakeParseArguments)
+function(msbuild_aarch64_asm)
+  set(options "")
+  set(oneValueArgs TARGET OUTPUT_OBJECTS)
+  set(multiValueArgs ASM_FILES)
+
+  cmake_parse_arguments(MSBUILD_AARCH64_ASM "${options}" "${oneValueArgs}" "${multiValueArgs}" ${ARGV})
+
+  # Set the output directory for object files
+  set(OBJ_DIR "${CMAKE_CURRENT_BINARY_DIR}/${MSBUILD_AARCH64_ASM_TARGET}.dir/$<CONFIG>")
+
+  # Initialize list to store assembled object files
+  set(ASSEMBLED_OBJECTS "")
+
+  foreach(ASM_FILE ${MSBUILD_AARCH64_ASM_ASM_FILES})
+    # Get the filename without extension
+    get_filename_component(ASM_NAME ${ASM_FILE} NAME_WE)
+    set(OBJ_FILE "${OBJ_DIR}/${ASM_NAME}.obj")
+
+    add_custom_command(
+      OUTPUT ${OBJ_FILE}
+      COMMAND "${CMAKE_ASM_COMPILER}" 
+              --target=arm64-pc-windows-msvc 
+              /c 
+              /I "${PROJECT_BINARY_DIR}/symbol_prefix_include/"
+              /I "${PROJECT_SOURCE_DIR}/include"
+              /o "${OBJ_FILE}"
+              "${ASM_FILE}"
+      DEPENDS ${ASM_FILE}
+      COMMENT "Assembling ${ASM_FILE}"
+    )
+
+    # Mark the generated object file as an external object
+    set_source_files_properties(${OBJ_FILE} PROPERTIES EXTERNAL_OBJECT TRUE)
+
+    list(APPEND ASSEMBLED_OBJECTS ${OBJ_FILE})
+  endforeach()
+
+  # Set the output variable in the parent scope
+  set(${MSBUILD_AARCH64_ASM_OUTPUT_OBJECTS} ${ASSEMBLED_OBJECTS} PARENT_SCOPE)
+endfunction()
+
 if(NOT OPENSSL_NO_ASM)
   if(UNIX)
     if(ARCH STREQUAL "aarch64")
@@ -281,6 +326,11 @@ if(ENABLE_DILITHIUM)
   )
 endif()
 
+set(CRYPTO_ARCH_OBJECTS "")
+if (ARCH STREQUAL "aarch64" AND CMAKE_GENERATOR MATCHES "Visual Studio")
+  msbuild_aarch64_asm(TARGET crypto_objects ASM_FILES ${CRYPTO_ARCH_SOURCES} OUTPUT_OBJECTS CRYPTO_ARCH_OBJECTS)
+endif()
+
 add_library(
   crypto_objects
   OBJECT
@@ -532,6 +582,7 @@ add_library(
   decrepit/x509/x509_decrepit.c
 
   ${CRYPTO_ARCH_SOURCES}
+  ${CRYPTO_ARCH_OBJECTS}
 )
 
 target_compile_definitions(crypto_objects PRIVATE BORINGSSL_IMPLEMENTATION)
@@ -674,6 +725,7 @@ if(BUILD_TESTING)
     add_dependencies(${executable_name} boringssl_prefix_symbols)
     target_include_directories(${executable_name} BEFORE PRIVATE ${PROJECT_BINARY_DIR}/symbol_prefix_include)
     add_dependencies(all_tests ${executable_name})
+    set_test_location(${executable_name})
   endmacro()
 
   # Below tests are added as new executables to be executed in single process.
@@ -689,6 +741,7 @@ if(BUILD_TESTING)
     ${RANDOM_TEST_EXEC}
     fipsmodule/rand/urandom_test.cc
   )
+  set_test_location(${RANDOM_TEST_EXEC})
 
   add_dependencies(${RANDOM_TEST_EXEC} boringssl_prefix_symbols)
   target_link_libraries(${RANDOM_TEST_EXEC} test_support_lib boringssl_gtest crypto)
@@ -784,6 +837,7 @@ if(BUILD_TESTING)
 
     $<TARGET_OBJECTS:crypto_test_data>
   )
+  set_test_location(${CRYPTO_TEST_EXEC})
 
   add_dependencies(${CRYPTO_TEST_EXEC} boringssl_prefix_symbols)
   target_link_libraries(${CRYPTO_TEST_EXEC} boringssl_gtest_main)
@@ -799,6 +853,7 @@ if(BUILD_TESTING)
 
   add_executable(${DYNAMIC_LOADING_TEST_EXEC} dynamic_loading_test.c)
   add_dependencies(${DYNAMIC_LOADING_TEST_EXEC} crypto)
+  set_test_location(${DYNAMIC_LOADING_TEST_EXEC})
 
   add_dependencies(${DYNAMIC_LOADING_TEST_EXEC} boringssl_prefix_symbols)
   target_include_directories(${DYNAMIC_LOADING_TEST_EXEC} BEFORE PRIVATE ${PROJECT_BINARY_DIR}/symbol_prefix_include)
@@ -823,6 +878,7 @@ if(BUILD_TESTING)
   message(STATUS "Generating test executable ${RWLOCK_STATIC_INIT_TEST_EXEC}.")
   add_executable(${RWLOCK_STATIC_INIT_TEST_EXEC} rwlock_static_init.cc)
   add_dependencies(${RWLOCK_STATIC_INIT_TEST_EXEC} crypto)
+  set_test_location(${RWLOCK_STATIC_INIT_TEST_EXEC})
 
   target_link_libraries(${RWLOCK_STATIC_INIT_TEST_EXEC} crypto)
   target_include_directories(${RWLOCK_STATIC_INIT_TEST_EXEC} BEFORE PRIVATE ${PROJECT_BINARY_DIR}/symbol_prefix_include)

diff --git a/crypto/fipsmodule/CMakeLists.txt b/crypto/fipsmodule/CMakeLists.txt
@@ -553,6 +553,11 @@ elseif(FIPS_SHARED)
   set(BCM_NAME ${BCM_NAME} PARENT_SCOPE)
 
 else()
+  set(BCM_ASM_OBJECTS "")
+  if (ARCH STREQUAL "aarch64" AND CMAKE_GENERATOR MATCHES "Visual Studio")
+    msbuild_aarch64_asm(TARGET fipsmodule ASM_FILES ${BCM_ASM_SOURCES} OUTPUT_OBJECTS BCM_ASM_OBJECTS)
+  endif()
+
   add_library(
     fipsmodule
 
@@ -563,10 +568,12 @@ else()
     cpucap/cpucap.c
 
     ${BCM_ASM_SOURCES}
+    ${BCM_ASM_OBJECTS}
   )
   target_compile_definitions(fipsmodule PRIVATE BORINGSSL_IMPLEMENTATION)
 
   add_dependencies(fipsmodule boringssl_prefix_symbols)
   target_include_directories(fipsmodule BEFORE PRIVATE ${PROJECT_BINARY_DIR}/symbol_prefix_include)
   target_include_directories(fipsmodule PRIVATE ${PROJECT_SOURCE_DIR}/include)
+
 endif()
diff --git a/ssl/CMakeLists.txt b/ssl/CMakeLists.txt
@@ -73,6 +73,7 @@ if(BUILD_TESTING)
           boringssl_gtest_main ssl )
   target_include_directories(${INTEGRATION_TEST_EXEC} BEFORE PRIVATE ${PROJECT_BINARY_DIR}/symbol_prefix_include)
   add_dependencies(all_tests ${INTEGRATION_TEST_EXEC})
+  set_test_location(${INTEGRATION_TEST_EXEC})
 
   add_executable(
     ${SSL_TEST_EXEC}
@@ -83,6 +84,7 @@ if(BUILD_TESTING)
   )
 
   target_link_libraries(${SSL_TEST_EXEC} boringssl_gtest_main ssl)
+  set_test_location(${SSL_TEST_EXEC})
 
   target_include_directories(${SSL_TEST_EXEC} BEFORE PRIVATE ${PROJECT_BINARY_DIR}/symbol_prefix_include)
 

diff --git a/tool-openssl/CMakeLists.txt b/tool-openssl/CMakeLists.txt
@@ -52,4 +52,5 @@ if(BUILD_TESTING)
     target_link_libraries(tool_openssl_test boringssl_gtest_main ssl crypto)
     target_include_directories(tool_openssl_test BEFORE PRIVATE ${PROJECT_BINARY_DIR}/symbol_prefix_include)
     add_dependencies(all_tests tool_openssl_test)
+    set_test_location(tool_openssl_test)
 endif()
diff --git a/util/build_compilation_database.sh b/util/build_compilation_database.sh
@@ -12,8 +12,8 @@ MY_CMAKE_FLAGS=("-GNinja" "-DCMAKE_BUILD_TYPE=Debug" "-DCMAKE_EXPORT_COMPILE_COM
 
 mkdir -p "${AWS_LC_BUILD}"
 
-cmake "${BASE_DIR}" -B "${AWS_LC_BUILD}" ${MY_CMAKE_FLAGS[@]} "${@}"
+cmake "${BASE_DIR}" -B "${AWS_LC_BUILD}" "${MY_CMAKE_FLAGS[@]}" "${@}"
 
-cmake --build "${AWS_LC_BUILD}" --target all
+cmake --build "${AWS_LC_BUILD}" -j 4 --target all_tests
 
 cp "${AWS_LC_BUILD}"/compile_commands.json "${BASE_DIR}"/