asm: missing function alignment

[npajkovsky]

clang-22 reports missing alignment on MacOS.

ld: warning: arm64 function not 4-byte aligned: _asm_aescbc_sha1_hmac from libcrypto.a(libcrypto-lib-aes-sha1-armv8.o)
ld: warning: arm64 function not 4-byte aligned: _asm_aescbc_sha256_hmac from libcrypto.a(libcrypto-lib-aes-sha256-armv8.o)
ld: warning: arm64 function not 4-byte aligned: _asm_sha1_hmac_aescbc_dec from libcrypto.a(libcrypto-lib-aes-sha1-armv8.o)
ld: warning: arm64 function not 4-byte aligned: _asm_sha256_hmac_aescbc_dec from libcrypto.a(libcrypto-lib-aes-sha256-armv8.o)

[Sashan]

I think I'm fine with it. Still have a question for arm folks (@tom-cosgrove-arm)
why some files are using .align 4 for functions for example here in
crypto/aes/asm/aes-sha512-armv8.pl

  2071  
  2072  .global asm_sha512_hmac_aescbc_dec
  2073  .type   asm_sha512_hmac_aescbc_dec,%function
  2074  
  2075  .align  4
  2076  asm_sha512_hmac_aescbc_dec:

and other places OpenSSL uses .align 5 like here for montgomery multiplication in ./bn/asm/armv8-mont.pl

    76  
    77  .globl  bn_mul_mont
    78  .type   bn_mul_mont,%function
    79  .align  5
    80  bn_mul_mont:
    81          AARCH64_SIGN_LINK_REGISTER
    82  .Lbn_mul_mont:
    83          tst     $num,#3

Is .align 5 meaningful? in other words: when .align 4 should be used and when .align 5 should be used? thanks.

[esyr]

.align 4 aligns to 16 bytes and .align 5 aligns to 32 bytes on arm: "For other systems, including ppc, i386 using a.out format, arm and strongarm, it is the number of low-order zero bits the location counter must have after advancement. For example .align 3 advances the location counter until it is a multiple of 8. If the location counter is already a multiple of 8, no change is needed. "[1].

[1] https://sourceware.org/binutils/docs/as.html#Align

[tom-cosgrove-arm]

LGTM, thanks

[npajkovsky]

@tom-cosgrove-arm is there any reason to use .align 5 in ./bn/asm/armv8-mont.pl or is it a bug?

[tom-cosgrove-arm]

@tom-cosgrove-arm is there any reason to use .align 5 in ./bn/asm/armv8-mont.pl or is it a bug?

Given the use of both .align 4 and .align 5 it's almost certainly intentional, and on earlier AArch64 cores (the code dates back at least as far as 2015) might have made a small performance improvement. Probably not worth changing - the risk of small perf regression on earlier cores vs slight size benefit would suggest making no change.

[esyr]

Not sure what the actual alignment should be.

[esyr]

Hmmm, a quick git grep -C 10 '%function' **/*arm*| awk '/^--$/{ if (found == 0) { print p; }; found = 0; p = ""; } /.*/{ p = p "\n" $0; } /\.align/{ found = 1; }' yielded also the following:

crypto/armv4cpuid.pl-.global	OPENSSL_cleanse
crypto/modes/asm/ghash-armv4.pl-.global	gcm_gmult_4bit
crypto/sha/asm/sha512-armv4.pl-.global	sha512_block_data_order

@tom-cosgrove-arm, may I ask to evaluate whether those functions need .align directives as well?

[tom-cosgrove-arm]

@tom-cosgrove-arm, may I ask to evaluate whether those functions need .align directives as well?

Thanks for checking. Yes, looks like OPENSSL_cleanse and gcm_gmult_4bit are definitely missing an align. sha512_block_data_order is almost certainly okay, because the data before it starts with .align 5 then has a load of WORD64s, then either .word OPENSSL_armcap_P followed by skip 32-4 or just skip 32. However, adding an align 5 there too wouldn't hurt, and would definitely be more future-proof if anyone goes to add more data.

[openssl-machine]

24 hours has passed since 'approval: done' was set, but as this PR has been updated in that time the label 'approval: ready to merge' is not being automatically set. Please review the updates and set the label manually.

[esyr]

sha512_block_data_order is almost certainly okay, because the data before it starts with .align 5

And that array should be in .rodata, strictly speaking.

[tom-cosgrove-arm]

sha512_block_data_order is almost certainly okay, because the data before it starts with .align 5

And that array should be in .rodata, strictly speaking.

Yes, but that's not a change for a "missing function alignment" PR

[esyr]

The commit that has introduced the affected files, openssl-3.6.0-alpha1~907 "Implement interleaving aes-cbc-hmac-sha on aarch64", is not present in openssl-3.5, removing the branch label.

[esyr]

Applied to master, openssl-4.0, and openssl-3.6, thank you for your contribution.