common: Move backend includes into new backends.h shim#1692
common: Move backend includes into new backends.h shim#1692hanno-becker wants to merge 1 commit into
Conversation
451a82a to
f2bf5e8
Compare
CBMC Results (ML-KEM-768)Full Results (191 proofs)
|
CBMC Results (ML-KEM-1024)Full Results (191 proofs)
|
CBMC Results (ML-KEM-512)Full Results (191 proofs)
|
| # Inject the backends.h header into the BUILD file's hdrs list. | ||
| # Upstream expo's BUILD.mlkem_native.bazel enumerates every header | ||
| # explicitly, and mlkem-native split the backend includes out of | ||
| # common.h into a new backends.h shim that the expo pin doesn't | ||
| # know about. Drop this patch once expo lists backends.h itself. | ||
| BUILD=third_party/mlkem_native/BUILD.mlkem_native.bazel | ||
| if ! grep -q '"mlkem/src/backends.h"' "$BUILD"; then | ||
| sed -i 's|"mlkem/src/common.h",|"mlkem/src/common.h",\n "mlkem/src/backends.h",|' "$BUILD" | ||
| fi | ||
|
|
There was a problem hiding this comment.
Do you mind doing this as a patch? That will make it easier to actually update alter.
| + # Drop the backends.h include. In mlkem-native it surfaces the | ||
| + # MLK_ARITH_BACKEND_* gate macro to the asm sources, but unifdef above | ||
| + # has already statically resolved that gate, so the include has nothing | ||
| + # left to provide here. Leaving it in would also break the build, since | ||
| + # the relative path (../../../backends.h) does not resolve in the | ||
| + # AWS-LC vendored layout. | ||
| + sed "${SED_I[@]}" '/^#include "\.\.\/\.\.\/\.\.\/backends\.h"$/d' "$file" |
There was a problem hiding this comment.
Would it be cleaner for AWS-LC to gate the include via the MLK_CONFIG_USE_NATIVE_BACKEND_ARITH macro? Probably I am missing soemthing here.
There was a problem hiding this comment.
The importer patches the common.h include to an s2n-bignum include and manually removes the backend guards so the code is unconditionally included. We hence don't need backends.h anymore here.
There was a problem hiding this comment.
An alternative would be to import just backends.h (because that also includes common.h) in the ASM files, and then just patch that. That's a smaller change but deviates from the convention that all files should first include common.h.
common.h previously pulled in MLK_CONFIG_ARITH_BACKEND_FILE and MLK_CONFIG_FIPS202_BACKEND_FILE for every translation unit, which caused gcc-4.8 / -O0 link failures when a TU was compiled in isolation without the backend (issue #1182). Introduce mlkem/src/backends.h as the single shim that materialises backend metadata and the MLK_CHECK_APIS sanity includes. Consumer C files (compress.c, indcpa.c, poly.c, poly_k.c, sampling.c, fips202/keccakf1600.c) and backend-internal asm/C sources include backends.h instead of either the old common.h block or a direct ../meta.h / ../auto.h reference. Update the AWS-LC importer patch to strip the backends.h include from asm sources post-import: unifdef has already statically resolved the MLK_ARITH_BACKEND_* gate the include was meant to surface, and the relative path does not resolve in the AWS-LC vendored layout. Signed-off-by: Hanno Becker <beckphan@amazon.co.uk>
|
Perhaps we can wait merging this until it is confirmed that the ABI checker is unblocked by it. |
common.h previously pulled in MLK_CONFIG_ARITH_BACKEND_FILE and MLK_CONFIG_FIPS202_BACKEND_FILE for every translation unit, which caused gcc-4.8 / -O0 link failures when a TU was compiled in isolation without the backend (issue #1182).
Introduce mlkem/src/backends.h as the single shim that materialises backend metadata and the MLK_CHECK_APIS sanity includes. Consumer C files (compress.c, indcpa.c, poly.c, poly_k.c, sampling.c, fips202/keccakf1600.c) and backend-internal asm/C sources include backends.h instead of either the old common.h block or a direct ../meta.h / ../auto.h reference.