From 07f27b1d445a27433f2c871935da2cefcfbdb458 Mon Sep 17 00:00:00 2001 From: David Benjamin Date: Sun, 12 May 2024 10:34:17 -0400 Subject: [PATCH 1/2] Fix alignment of generated UNWIND_INFO structures I missed a few parts of the Windows documentation: > The UNWIND_INFO structure must be DWORD aligned in memory. > For alignment purposes, this array [unwind codes] always has an even > number of entries, and the final entry is potentially unused. In that > case, the array is one longer than indicated by the count of unwind > codes field. https://learn.microsoft.com/en-us/cpp/build/exception-handling-x64?view=msvc-170 This didn't seem to have any practical effect (unwinding tests worked as-is), but I noticed this while rewriting some handwritten codes. Bug: 259 Change-Id: I655f3a7f3a907797e7665a276f4926a31a1e1639 Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/68407 Reviewed-by: Adam Langley Auto-Submit: David Benjamin Commit-Queue: Adam Langley --- crypto/perlasm/x86_64-xlate.pl | 10 ++++++++++ gen/bcm/aesni-gcm-x86_64-win.asm | 3 +++ gen/bcm/ghash-ssse3-x86_64-win.asm | 3 +++ gen/bcm/ghash-x86_64-win.asm | 2 ++ gen/test_support/trampoline-x86_64-win.asm | 4 ++++ 5 files changed, 22 insertions(+) diff --git a/crypto/perlasm/x86_64-xlate.pl b/crypto/perlasm/x86_64-xlate.pl index 5b7705d4e..3011c9153 100755 --- a/crypto/perlasm/x86_64-xlate.pl +++ b/crypto/perlasm/x86_64-xlate.pl @@ -903,6 +903,16 @@ $info{info_label}: $info{unwind_codes} ____ + # UNWIND_INFOs must be 4-byte aligned. If needed, we must add an extra + # unwind code. This does not change the unwind code count. Windows + # documentation says "For alignment purposes, this array always has an + # even number of entries, and the final entry is potentially unused. In + # that case, the array is one longer than indicated by the count of + # unwind codes field." + if ($info{num_codes} & 1) { + $xdata .= "\t.value\t0\n"; + } + %info = (); return $end_label; } diff --git a/gen/bcm/aesni-gcm-x86_64-win.asm b/gen/bcm/aesni-gcm-x86_64-win.asm index 7564a1cf0..e8324d283 100644 --- a/gen/bcm/aesni-gcm-x86_64-win.asm +++ b/gen/bcm/aesni-gcm-x86_64-win.asm @@ -1039,6 +1039,7 @@ $L$SEH_info_aesni_gcm_decrypt_0: DB $L$SEH_prologue_aesni_gcm_decrypt_2-$L$SEH_begin_aesni_gcm_decrypt_1 DB 80 + DW 0 $L$SEH_info_aesni_gcm_encrypt_0: DB 1 DB $L$SEH_endprologue_aesni_gcm_encrypt_22-$L$SEH_begin_aesni_gcm_encrypt_1 @@ -1097,6 +1098,8 @@ $L$SEH_info_aesni_gcm_encrypt_0: DB 48 DB $L$SEH_prologue_aesni_gcm_encrypt_2-$L$SEH_begin_aesni_gcm_encrypt_1 DB 80 + + DW 0 %else ; Work around https://bugzilla.nasm.us/show_bug.cgi?id=3392738 ret diff --git a/gen/bcm/ghash-ssse3-x86_64-win.asm b/gen/bcm/ghash-ssse3-x86_64-win.asm index a8be60ed8..e0de96243 100644 --- a/gen/bcm/ghash-ssse3-x86_64-win.asm +++ b/gen/bcm/ghash-ssse3-x86_64-win.asm @@ -477,6 +477,7 @@ $L$SEH_info_gcm_gmult_ssse3_0: DB $L$SEH_prologue_gcm_gmult_ssse3_2-$L$SEH_begin_gcm_gmult_ssse3_1 DB 66 + DW 0 $L$SEH_info_gcm_ghash_ssse3_0: DB 1 DB $L$SEH_endprologue_gcm_ghash_ssse3_6-$L$SEH_begin_gcm_ghash_ssse3_1 @@ -493,6 +494,8 @@ $L$SEH_info_gcm_ghash_ssse3_0: DW 0 DB $L$SEH_prologue_gcm_ghash_ssse3_2-$L$SEH_begin_gcm_ghash_ssse3_1 DB 98 + + DW 0 %else ; Work around https://bugzilla.nasm.us/show_bug.cgi?id=3392738 ret diff --git a/gen/bcm/ghash-x86_64-win.asm b/gen/bcm/ghash-x86_64-win.asm index bd4d691bd..b5416b323 100644 --- a/gen/bcm/ghash-x86_64-win.asm +++ b/gen/bcm/ghash-x86_64-win.asm @@ -1246,6 +1246,7 @@ $L$SEH_info_gcm_init_clmul_0: DB $L$SEH_prologue_gcm_init_clmul_2-$L$SEH_begin_gcm_init_clmul_1 DB 34 + DW 0 $L$SEH_info_gcm_ghash_clmul_0: DB 1 DB $L$SEH_endprologue_gcm_ghash_clmul_13-$L$SEH_begin_gcm_ghash_clmul_1 @@ -1296,6 +1297,7 @@ $L$SEH_info_gcm_init_avx_0: DB $L$SEH_prologue_gcm_init_avx_2-$L$SEH_begin_gcm_init_avx_1 DB 34 + DW 0 $L$SEH_info_gcm_ghash_avx_0: DB 1 DB $L$SEH_endprologue_gcm_ghash_avx_13-$L$SEH_begin_gcm_ghash_avx_1 diff --git a/gen/test_support/trampoline-x86_64-win.asm b/gen/test_support/trampoline-x86_64-win.asm index dca395782..7c7d3c322 100644 --- a/gen/test_support/trampoline-x86_64-win.asm +++ b/gen/test_support/trampoline-x86_64-win.asm @@ -698,6 +698,7 @@ $L$SEH_info_abi_test_bad_unwind_wrong_register_0: DB $L$SEH_prologue_abi_test_bad_unwind_wrong_register_2-$L$SEH_begin_abi_test_bad_unwind_wrong_register_1 DB 208 + DW 0 $L$SEH_info_abi_test_bad_unwind_temporary_0: DB 1 DB $L$SEH_endprologue_abi_test_bad_unwind_temporary_3-$L$SEH_begin_abi_test_bad_unwind_temporary_1 @@ -706,6 +707,7 @@ $L$SEH_info_abi_test_bad_unwind_temporary_0: DB $L$SEH_prologue_abi_test_bad_unwind_temporary_2-$L$SEH_begin_abi_test_bad_unwind_temporary_1 DB 192 + DW 0 $L$SEH_info_abi_test_bad_unwind_epilog_0: DB 1 DB $L$SEH_endprologue_abi_test_bad_unwind_epilog_3-$L$SEH_begin_abi_test_bad_unwind_epilog_1 @@ -713,6 +715,8 @@ $L$SEH_info_abi_test_bad_unwind_epilog_0: DB 0 DB $L$SEH_prologue_abi_test_bad_unwind_epilog_2-$L$SEH_begin_abi_test_bad_unwind_epilog_1 DB 192 + + DW 0 %else ; Work around https://bugzilla.nasm.us/show_bug.cgi?id=3392738 ret From 3a01cba9a5a133799dbb58b5fbf15d0ddfe23cee Mon Sep 17 00:00:00 2001 From: David Benjamin Date: Wed, 15 May 2024 17:00:42 -0400 Subject: [PATCH 2/2] Fix the Bazel build The good news is we now have a way to test mistakes in the header lists standalone. The bad news is we don't run it on CI. Change-Id: Ie9e6efeb7922374efa38f8e4e8aab85174424f26 Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/68469 Commit-Queue: David Benjamin Reviewed-by: Adam Langley --- MODULE.bazel.lock | 6 +++--- build.json | 2 +- gen/sources.bzl | 2 +- gen/sources.cmake | 2 +- gen/sources.json | 2 +- 5 files changed, 7 insertions(+), 7 deletions(-) diff --git a/MODULE.bazel.lock b/MODULE.bazel.lock index 7ac13bb5f..93be30301 100644 --- a/MODULE.bazel.lock +++ b/MODULE.bazel.lock @@ -1375,7 +1375,7 @@ }, "@@rules_foreign_cc~//foreign_cc:extensions.bzl%ext": { "general": { - "bzlTransitiveDigest": "QbxK92//k6c63fpMer2Lkk6224s9gwYoVFFS6mdkucI=", + "bzlTransitiveDigest": "0/GTFp9D0gb6hOu9jXXnaWa5hPbzPpIzLVxITdJTwvo=", "recordedFileInputs": {}, "recordedDirentsInputs": {}, "envVariables": {}, @@ -1646,7 +1646,7 @@ }, "@@rules_java~//java:extensions.bzl%toolchains": { "general": { - "bzlTransitiveDigest": "tJHbmWnq7m+9eUBnUdv7jZziQ26FmcGL9C5/hU3Q9UQ=", + "bzlTransitiveDigest": "0N5b5J9fUzo0sgvH4F3kIEaeXunz4Wy2/UtSFV/eXUY=", "recordedFileInputs": {}, "recordedDirentsInputs": {}, "envVariables": {}, @@ -2151,7 +2151,7 @@ }, "@@rules_python~//python/extensions:python.bzl%python": { "general": { - "bzlTransitiveDigest": "o0WIKfdQRSZd/9+sY+LDTrUuYozMBFuYsL85uwJYKk8=", + "bzlTransitiveDigest": "hcJ2K4XvZ3hi0G4g5MkUEs8xowZlCfrgq3JX4dyipWY=", "recordedFileInputs": {}, "recordedDirentsInputs": {}, "envVariables": {}, diff --git a/build.json b/build.json index f2c1ac9b1..f8948fba5 100644 --- a/build.json +++ b/build.json @@ -478,7 +478,6 @@ "crypto/bytestring/internal.h", "crypto/chacha/internal.h", "crypto/cipher_extra/internal.h", - "crypto/conf/conf_def.h", "crypto/conf/internal.h", "crypto/cpu_arm_linux.h", "crypto/curve25519/curve25519_tables.h", @@ -610,6 +609,7 @@ "hdrs": [ "include/openssl/pki/certificate.h", "include/openssl/pki/signature_verify_cache.h", + "include/openssl/pki/verify.h", "include/openssl/pki/verify_error.h" ], "internal_hdrs": [ diff --git a/gen/sources.bzl b/gen/sources.bzl index 86f2cbe41..690342753 100644 --- a/gen/sources.bzl +++ b/gen/sources.bzl @@ -582,7 +582,6 @@ crypto_internal_headers = [ "crypto/bytestring/internal.h", "crypto/chacha/internal.h", "crypto/cipher_extra/internal.h", - "crypto/conf/conf_def.h", "crypto/conf/internal.h", "crypto/cpu_arm_linux.h", "crypto/curve25519/curve25519_tables.h", @@ -1070,6 +1069,7 @@ pki_sources = [ pki_headers = [ "include/openssl/pki/certificate.h", "include/openssl/pki/signature_verify_cache.h", + "include/openssl/pki/verify.h", "include/openssl/pki/verify_error.h", ] diff --git a/gen/sources.cmake b/gen/sources.cmake index 9d15ef0d4..d7e1e747b 100644 --- a/gen/sources.cmake +++ b/gen/sources.cmake @@ -600,7 +600,6 @@ set( crypto/bytestring/internal.h crypto/chacha/internal.h crypto/cipher_extra/internal.h - crypto/conf/conf_def.h crypto/conf/internal.h crypto/cpu_arm_linux.h crypto/curve25519/curve25519_tables.h @@ -1106,6 +1105,7 @@ set( include/openssl/pki/certificate.h include/openssl/pki/signature_verify_cache.h + include/openssl/pki/verify.h include/openssl/pki/verify_error.h ) diff --git a/gen/sources.json b/gen/sources.json index 86da01d09..5d96a60f0 100644 --- a/gen/sources.json +++ b/gen/sources.json @@ -564,7 +564,6 @@ "crypto/bytestring/internal.h", "crypto/chacha/internal.h", "crypto/cipher_extra/internal.h", - "crypto/conf/conf_def.h", "crypto/conf/internal.h", "crypto/cpu_arm_linux.h", "crypto/curve25519/curve25519_tables.h", @@ -1051,6 +1050,7 @@ "hdrs": [ "include/openssl/pki/certificate.h", "include/openssl/pki/signature_verify_cache.h", + "include/openssl/pki/verify.h", "include/openssl/pki/verify_error.h" ], "internal_hdrs": [