gcc -flto=auto causes mh_sha256/mh_sha256_test to fail #165

Closed
opened 2025-11-25 08:47:22 -06:00 by pobrn · 7 comments
pobrn commented 2025-11-25 08:47:22 -06:00 (Migrated from github.com)

Setting -flto=auto when compiling with gcc 15 causes mh_sha256/mh_sha256_test to fail:

test-suite.log
=============================================
   libisal_crypto 2.25.0: ./test-suite.log
=============================================

# TOTAL: 37
# PASS:  36
# SKIP:  0
# XFAIL: 0
# FAIL:  1
# XPASS: 0
# ERROR: 0

System information (uname -a): Linux 6.17.9-1-MANJARO #1 SMP PREEMPT_DYNAMIC Mon, 24 Nov 2025 13:47:26 +0000 x86_64
Distribution information (/etc/os-release):
NAME="Manjaro Linux"
PRETTY_NAME="Manjaro Linux"
ID=manjaro
ID_LIKE=arch
BUILD_ID=rolling
ANSI_COLOR="32;1;24;144;200"
HOME_URL="https://manjaro.org/"
DOCUMENTATION_URL="https://wiki.manjaro.org/"

.. contents:: :depth: 2

FAIL: mh_sha256/mh_sha256_test
==============================

isal_mh_sha256_update_test:
mh_sha256 fail test
ref:  88 f5 d1 50 4c 95 2e f8 ba  e 68 fa c7 86 fd 71 d5 8b 83 d9 a4 4c 8a 10 b9 c9 44  8 cd e0 c3 c4
test:   e  2 f2 30 df 10 2a 99 24  5 5d 80 b7 a5 35 77 8e 60  5 51 9d 9b b8 94 50 79 ec  f 86  1 18 2d
fail rand1 test
isal_mh_sha256_update_test: Fail
FAIL mh_sha256/mh_sha256_test (exit status: 8)

Steps to reproduce:

  1. clone master branch (5fd4cdae15)
  2. ./autogen.sh
  3. CFLAGS="-flto=auto -O2 -g" ./configure
  4. make check
output
[...]
$ CFLAGS="-flto=auto -O2 -g" ./configure
[...]

        isa-l_crypto 2.25.0
        =====

        prefix:                 /usr
        sysconfdir:             ${prefix}/etc
        libdir:                 ${exec_prefix}/lib
        includedir:             ${prefix}/include

        compiler:               gcc
        cflags:                 -flto=auto -O2 -g
        ldflags:                -Wl,-z,noexecstack -Wl,-z,relro -Wl,-z,now 

        debug:                  no
        safe_data:              yes
        safe_param:             yes
        FIPS mode:              no

[...]
$ make check
[...]
make --no-print-directory check-TESTS
PASS: sha1_mb/sha1_mb_test
PASS: sha1_mb/sha1_mb_rand_test
PASS: sha1_mb/sha1_mb_rand_update_test
PASS: sha1_mb/sha1_mb_flush_test
PASS: sha1_mb/sha1_mb_param_test
PASS: mh_sha1/mh_sha1_test
PASS: mh_sha1/mh_sha1_param_test
PASS: md5_mb/md5_mb_test
PASS: md5_mb/md5_mb_rand_test
PASS: md5_mb/md5_mb_rand_update_test
PASS: md5_mb/md5_mb_param_test
PASS: sha256_mb/sha256_mb_test
PASS: sha256_mb/sha256_mb_rand_test
PASS: sha256_mb/sha256_mb_rand_update_test
PASS: sha256_mb/sha256_mb_flush_test
PASS: sha256_mb/sha256_mb_param_test
PASS: sha512_mb/sha512_mb_test
PASS: sha512_mb/sha512_mb_rand_test
PASS: sha512_mb/sha512_mb_rand_update_test
PASS: sha512_mb/sha512_mb_param_test
PASS: mh_sha1_murmur3_x64_128/mh_sha1_murmur3_x64_128_param_test
FAIL: mh_sha256/mh_sha256_test
PASS: mh_sha256/mh_sha256_param_test
PASS: rolling_hash/rolling_hash2_test
PASS: rolling_hash/rolling_hash2_param_test
PASS: sm3_mb/sm3_ref_test
PASS: sm3_mb/sm3_mb_param_test
PASS: misc/version_test
PASS: aes/cbc_std_vectors_test
PASS: aes/gcm_std_vectors_test
PASS: aes/gcm_nt_std_vectors_test
PASS: aes/xts_128_test
PASS: aes/xts_256_test
PASS: aes/xts_128_expanded_key_test
PASS: aes/xts_256_expanded_key_test
PASS: aes/aes_param_test
PASS: fips/self_tests_test
============================================================================
Testsuite summary for libisal_crypto 2.25.0
============================================================================
# TOTAL: 37
# PASS:  36
# SKIP:  0
# XFAIL: 0
# FAIL:  1
# XPASS: 0
# ERROR: 0
============================================================================
See ./test-suite.log for debugging.
Some test(s) failed.  Please report this to https://github.com/intel/isa-l_crypto/issues,
together with the test-suite.log file (gzipped) and your system
information.  Thanks.
============================================================================
lscpu
$ lscpu
Architecture:                x86_64
  CPU op-mode(s):            32-bit, 64-bit
  Address sizes:             39 bits physical, 48 bits virtual
  Byte Order:                Little Endian
CPU(s):                      12
  On-line CPU(s) list:       0-11
Vendor ID:                   GenuineIntel
  Model name:                Intel(R) Core(TM) i7-9750H CPU @ 2.60GHz
    CPU family:              6
    Model:                   158
    Thread(s) per core:      2
    Core(s) per socket:      6
    Socket(s):               1
    Stepping:                10
    CPU(s) scaling MHz:      29%
    CPU max MHz:             4500,0000
    CPU min MHz:             800,0000
    BogoMIPS:                5199,98
    Flags:                   fpu vme de pse tsc msr pae mce cx8 apic sep mtrr pg
                             e mca cmov pat pse36 clflush dts acpi mmx fxsr sse 
                             sse2 ss ht tm pbe syscall nx pdpe1gb rdtscp lm cons
                             tant_tsc art arch_perfmon pebs bts rep_good nopl xt
                             opology nonstop_tsc cpuid aperfmperf pni pclmulqdq 
                             dtes64 monitor ds_cpl vmx est tm2 ssse3 sdbg fma cx
                             16 xtpr pdcm pcid sse4_1 sse4_2 x2apic movbe popcnt
                              tsc_deadline_timer aes xsave avx f16c rdrand lahf_
                             lm abm 3dnowprefetch cpuid_fault epb pti ssbd ibrs 
                             ibpb stibp tpr_shadow flexpriority ept vpid ept_ad 
                             fsgsbase tsc_adjust bmi1 avx2 smep bmi2 erms invpci
                             d mpx rdseed adx smap clflushopt intel_pt xsaveopt 
                             xsavec xgetbv1 xsaves dtherm ida arat pln pts hwp h
                             wp_notify hwp_act_window hwp_epp vnmi md_clear flus
                             h_l1d arch_capabilities
Virtualization features:     
  Virtualization:            VT-x
Caches (sum of all):         
  L1d:                       192 KiB (6 instances)
  L1i:                       192 KiB (6 instances)
  L2:                        1,5 MiB (6 instances)
  L3:                        12 MiB (1 instance)
NUMA:                        
  NUMA node(s):              1
  NUMA node0 CPU(s):         0-11
Vulnerabilities:             
  Gather data sampling:      Mitigation; Microcode
  Ghostwrite:                Not affected
  Indirect target selection: Not affected
  Itlb multihit:             KVM: Mitigation: VMX disabled
  L1tf:                      Mitigation; PTE Inversion; VMX conditional cache fl
                             ushes, SMT vulnerable
  Mds:                       Mitigation; Clear CPU buffers; SMT vulnerable
  Meltdown:                  Mitigation; PTI
  Mmio stale data:           Mitigation; Clear CPU buffers; SMT vulnerable
  Old microcode:             Not affected
  Reg file data sampling:    Not affected
  Retbleed:                  Mitigation; IBRS
  Spec rstack overflow:      Not affected
  Spec store bypass:         Mitigation; Speculative Store Bypass disabled via p
                             rctl
  Spectre v1:                Mitigation; usercopy/swapgs barriers and __user poi
                             nter sanitization
  Spectre v2:                Mitigation; IBRS; IBPB conditional; STIBP condition
                             al; RSB filling; PBRSB-eIBRS Not affected; BHI Not 
                             affected
  Srbds:                     Mitigation; Microcode
  Tsa:                       Not affected
  Tsx async abort:           Not affected
  Vmscape:                   Mitigation; IBPB before exit to userspace
Setting `-flto=auto` when compiling with gcc 15 causes `mh_sha256/mh_sha256_test` to fail: <details> <summary>test-suite.log</summary> ``` ============================================= libisal_crypto 2.25.0: ./test-suite.log ============================================= # TOTAL: 37 # PASS: 36 # SKIP: 0 # XFAIL: 0 # FAIL: 1 # XPASS: 0 # ERROR: 0 System information (uname -a): Linux 6.17.9-1-MANJARO #1 SMP PREEMPT_DYNAMIC Mon, 24 Nov 2025 13:47:26 +0000 x86_64 Distribution information (/etc/os-release): NAME="Manjaro Linux" PRETTY_NAME="Manjaro Linux" ID=manjaro ID_LIKE=arch BUILD_ID=rolling ANSI_COLOR="32;1;24;144;200" HOME_URL="https://manjaro.org/" DOCUMENTATION_URL="https://wiki.manjaro.org/" .. contents:: :depth: 2 FAIL: mh_sha256/mh_sha256_test ============================== isal_mh_sha256_update_test: mh_sha256 fail test ref: 88 f5 d1 50 4c 95 2e f8 ba e 68 fa c7 86 fd 71 d5 8b 83 d9 a4 4c 8a 10 b9 c9 44 8 cd e0 c3 c4 test: e 2 f2 30 df 10 2a 99 24 5 5d 80 b7 a5 35 77 8e 60 5 51 9d 9b b8 94 50 79 ec f 86 1 18 2d fail rand1 test isal_mh_sha256_update_test: Fail FAIL mh_sha256/mh_sha256_test (exit status: 8) ``` </details> Steps to reproduce: 1. clone master branch (5fd4cdae154755f3d93ada75ab3a8eddff5ce76e) 2. `./autogen.sh` 3. `CFLAGS="-flto=auto -O2 -g" ./configure` 4. `make check` <details> <summary>output</summary> ``` [...] $ CFLAGS="-flto=auto -O2 -g" ./configure [...] isa-l_crypto 2.25.0 ===== prefix: /usr sysconfdir: ${prefix}/etc libdir: ${exec_prefix}/lib includedir: ${prefix}/include compiler: gcc cflags: -flto=auto -O2 -g ldflags: -Wl,-z,noexecstack -Wl,-z,relro -Wl,-z,now debug: no safe_data: yes safe_param: yes FIPS mode: no [...] $ make check [...] make --no-print-directory check-TESTS PASS: sha1_mb/sha1_mb_test PASS: sha1_mb/sha1_mb_rand_test PASS: sha1_mb/sha1_mb_rand_update_test PASS: sha1_mb/sha1_mb_flush_test PASS: sha1_mb/sha1_mb_param_test PASS: mh_sha1/mh_sha1_test PASS: mh_sha1/mh_sha1_param_test PASS: md5_mb/md5_mb_test PASS: md5_mb/md5_mb_rand_test PASS: md5_mb/md5_mb_rand_update_test PASS: md5_mb/md5_mb_param_test PASS: sha256_mb/sha256_mb_test PASS: sha256_mb/sha256_mb_rand_test PASS: sha256_mb/sha256_mb_rand_update_test PASS: sha256_mb/sha256_mb_flush_test PASS: sha256_mb/sha256_mb_param_test PASS: sha512_mb/sha512_mb_test PASS: sha512_mb/sha512_mb_rand_test PASS: sha512_mb/sha512_mb_rand_update_test PASS: sha512_mb/sha512_mb_param_test PASS: mh_sha1_murmur3_x64_128/mh_sha1_murmur3_x64_128_param_test FAIL: mh_sha256/mh_sha256_test PASS: mh_sha256/mh_sha256_param_test PASS: rolling_hash/rolling_hash2_test PASS: rolling_hash/rolling_hash2_param_test PASS: sm3_mb/sm3_ref_test PASS: sm3_mb/sm3_mb_param_test PASS: misc/version_test PASS: aes/cbc_std_vectors_test PASS: aes/gcm_std_vectors_test PASS: aes/gcm_nt_std_vectors_test PASS: aes/xts_128_test PASS: aes/xts_256_test PASS: aes/xts_128_expanded_key_test PASS: aes/xts_256_expanded_key_test PASS: aes/aes_param_test PASS: fips/self_tests_test ============================================================================ Testsuite summary for libisal_crypto 2.25.0 ============================================================================ # TOTAL: 37 # PASS: 36 # SKIP: 0 # XFAIL: 0 # FAIL: 1 # XPASS: 0 # ERROR: 0 ============================================================================ See ./test-suite.log for debugging. Some test(s) failed. Please report this to https://github.com/intel/isa-l_crypto/issues, together with the test-suite.log file (gzipped) and your system information. Thanks. ============================================================================ ``` </details> <details> <summary>lscpu</summary> ``` $ lscpu Architecture: x86_64 CPU op-mode(s): 32-bit, 64-bit Address sizes: 39 bits physical, 48 bits virtual Byte Order: Little Endian CPU(s): 12 On-line CPU(s) list: 0-11 Vendor ID: GenuineIntel Model name: Intel(R) Core(TM) i7-9750H CPU @ 2.60GHz CPU family: 6 Model: 158 Thread(s) per core: 2 Core(s) per socket: 6 Socket(s): 1 Stepping: 10 CPU(s) scaling MHz: 29% CPU max MHz: 4500,0000 CPU min MHz: 800,0000 BogoMIPS: 5199,98 Flags: fpu vme de pse tsc msr pae mce cx8 apic sep mtrr pg e mca cmov pat pse36 clflush dts acpi mmx fxsr sse sse2 ss ht tm pbe syscall nx pdpe1gb rdtscp lm cons tant_tsc art arch_perfmon pebs bts rep_good nopl xt opology nonstop_tsc cpuid aperfmperf pni pclmulqdq dtes64 monitor ds_cpl vmx est tm2 ssse3 sdbg fma cx 16 xtpr pdcm pcid sse4_1 sse4_2 x2apic movbe popcnt tsc_deadline_timer aes xsave avx f16c rdrand lahf_ lm abm 3dnowprefetch cpuid_fault epb pti ssbd ibrs ibpb stibp tpr_shadow flexpriority ept vpid ept_ad fsgsbase tsc_adjust bmi1 avx2 smep bmi2 erms invpci d mpx rdseed adx smap clflushopt intel_pt xsaveopt xsavec xgetbv1 xsaves dtherm ida arat pln pts hwp h wp_notify hwp_act_window hwp_epp vnmi md_clear flus h_l1d arch_capabilities Virtualization features: Virtualization: VT-x Caches (sum of all): L1d: 192 KiB (6 instances) L1i: 192 KiB (6 instances) L2: 1,5 MiB (6 instances) L3: 12 MiB (1 instance) NUMA: NUMA node(s): 1 NUMA node0 CPU(s): 0-11 Vulnerabilities: Gather data sampling: Mitigation; Microcode Ghostwrite: Not affected Indirect target selection: Not affected Itlb multihit: KVM: Mitigation: VMX disabled L1tf: Mitigation; PTE Inversion; VMX conditional cache fl ushes, SMT vulnerable Mds: Mitigation; Clear CPU buffers; SMT vulnerable Meltdown: Mitigation; PTI Mmio stale data: Mitigation; Clear CPU buffers; SMT vulnerable Old microcode: Not affected Reg file data sampling: Not affected Retbleed: Mitigation; IBRS Spec rstack overflow: Not affected Spec store bypass: Mitigation; Speculative Store Bypass disabled via p rctl Spectre v1: Mitigation; usercopy/swapgs barriers and __user poi nter sanitization Spectre v2: Mitigation; IBRS; IBPB conditional; STIBP condition al; RSB filling; PBRSB-eIBRS Not affected; BHI Not affected Srbds: Mitigation; Microcode Tsa: Not affected Tsx async abort: Not affected Vmscape: Mitigation; IBPB before exit to userspace ``` </detail>
pablodelara commented 2025-11-28 04:05:21 -06:00 (Migrated from github.com)

Thanks for reporting this. I can reproduce it also with gcc 13. Looks like the issue comes from mh_sha256_ref.c

Thanks for reporting this. I can reproduce it also with gcc 13. Looks like the issue comes from mh_sha256_ref.c
pobrn commented 2025-12-06 15:55:56 -06:00 (Migrated from github.com)

It seems to me that this might be a case of undefined behaviour. It can also be triggered by the following (without lto):

diff --git a/mh_sha256/mh_sha256_ref.c b/mh_sha256/mh_sha256_ref.c
index 94be975..9d5f8d9 100644
--- a/mh_sha256/mh_sha256_ref.c
+++ b/mh_sha256/mh_sha256_ref.c
@@ -53,6 +53,7 @@
         d += t1;                                                                                   \
         h = t1 + t2;
 
+static inline
 void
 sha256_single_for_mh_sha256_ref(const uint8_t *data, uint32_t digest[])
 {

and even with lto, it can avoided by:

diff --git a/mh_sha256/mh_sha256_ref.c b/mh_sha256/mh_sha256_ref.c
index 94be975..a2ec1b1 100644
--- a/mh_sha256/mh_sha256_ref.c
+++ b/mh_sha256/mh_sha256_ref.c
@@ -53,6 +53,7 @@
         d += t1;                                                                                   \
         h = t1 + t2;
 
+__attribute__((noipa))
 void
 sha256_single_for_mh_sha256_ref(const uint8_t *data, uint32_t digest[])
 {

But this also prevents the error:

diff --git a/mh_sha256/mh_sha256_ref.c b/mh_sha256/mh_sha256_ref.c
index 94be975..927cb36 100644
--- a/mh_sha256/mh_sha256_ref.c
+++ b/mh_sha256/mh_sha256_ref.c
@@ -176,7 +176,7 @@ sha256_for_mh_sha256_ref(const uint8_t *input_data, uint32_t *digest, const uint
         else
                 i = ISAL_SHA256_BLOCK_SIZE;
 
-        *(uint64_t *) (buf + i - 8) = to_be64((uint64_t) len * 8);
+        memcpy(buf + i - 8, &(uint64_t) { to_be64((uint64_t) len * 8) }, sizeof(uint64_t));
 
         sha256_single_for_mh_sha256_ref(buf, digest);
         if (i == (2 * ISAL_SHA256_BLOCK_SIZE))
@@ -346,7 +346,7 @@ mh_sha256_tail_ref(uint8_t *partial_buffer, uint32_t total_len,
         }
         // Padding the block
         len_in_bit = to_be64((uint64_t) total_len * 8);
-        *(uint64_t *) (partial_buffer + ISAL_MH_SHA256_BLOCK_SIZE - 8) = len_in_bit;
+        memcpy(partial_buffer + ISAL_MH_SHA256_BLOCK_SIZE - 8, &len_in_bit, sizeof(len_in_bit));
         mh_sha256_block_ref(partial_buffer, mh_sha256_segs_digests, frame_buffer, 1);
 
         // Calculate multi-hash SHA256 digests (segment digests as input message)

Which leads me to believe that gcc's inter-procedural analysis might detect some kind of aliasing issue or such, and removes or modifies these stores.

And it appears so, since using

CFLAGS="-flto=auto -O2 -g -fno-strict-aliasing" ./configure --prefix=/usr --libdir=/usr/lib

seems to also prevent the issue.

Since the code base does not seem to be "aliasing aware", I think it would be best to add -fno-strict-aliasing by default.


I'm also attaching a single c source file that can be used to trigger this issue (at least on gcc 15.2.1 20251112): repro.c

$ gcc -Wall -std=c11 -O2 -g repro.c && ./a.out
00 00 00 00 00 00 00 00 6b fa 4c 48 98 2f 8a 42 00 00 00 00 00 00 00 00 28 f9 2b 8d 98 2f 8a 42
$ gcc -Wall -std=c11 -O2 -g -DMAKE_INLINE repro.c && ./a.out
00 00 00 00 00 00 00 00 6b fa 4c 48 98 2f 8a 42 00 00 00 00 00 00 00 00 28 f9 2b 8d 98 2f 8a 42
$ gcc -Wall -std=c11 -O2 -g -fPIC repro.c && ./a.out
00 00 00 00 00 00 00 00 eb f9 6c 44 99 2f 8a 42 00 00 00 00 00 00 00 00 a8 f4 43 49 99 2f 8a 42
$ gcc -Wall -std=c11 -O2 -g -fPIC -DMAKE_INLINE repro.c && ./a.out
00 00 00 00 00 00 00 00 6b fa 4c 48 98 2f 8a 42 00 00 00 00 00 00 00 00 28 f9 2b 8d 98 2f 8a 42

$ clang -Wall -std=c11 -O2 -g repro.c && ./a.out
00 00 00 00 00 00 00 00 eb f9 6c 44 99 2f 8a 42 00 00 00 00 00 00 00 00 a8 f4 43 49 99 2f 8a 42
$ clang -Wall -std=c11 -O2 -g -DMAKE_INLINE repro.c && ./a.out
00 00 00 00 00 00 00 00 eb f9 6c 44 99 2f 8a 42 00 00 00 00 00 00 00 00 a8 f4 43 49 99 2f 8a 42
$ clang -Wall -std=c11 -O2 -g -fPIC repro.c && ./a.out
00 00 00 00 00 00 00 00 eb f9 6c 44 99 2f 8a 42 00 00 00 00 00 00 00 00 a8 f4 43 49 99 2f 8a 42
$ clang -Wall -std=c11 -O2 -g -fPIC -DMAKE_INLINE repro.c && ./a.out
00 00 00 00 00 00 00 00 eb f9 6c 44 99 2f 8a 42 00 00 00 00 00 00 00 00 a8 f4 43 49 99 2f 8a 42

Notice that depending on whether sha256_single_for_mh_sha256_ref() is not static inline and the -fPIC is present, then the output is different in the case of gcc, and that different out matches the result with clang in all cases.

In fact, the disassembly of (3) and (1) shows a stark difference:

--- /tmp/good	2025-12-06 22:53:59.561741460 +0100
+++ /tmp/bad	2025-12-06 22:54:26.106319124 +0100
@@ -9,8 +9,6 @@
     XXXX:       48 89 e7                mov    rdi,rsp
     XXXX:       0f 29 04 24             movaps XMMWORD PTR [rsp],xmm0
     XXXX:       0f 29 44 24 10          movaps XMMWORD PTR [rsp+0x10],xmm0
-    XXXX:       48 c7 04 24 01 00 00    mov    QWORD PTR [rsp],0x1
-    XXXX:       00 
     XXXX:       0f 29 44 24 20          movaps XMMWORD PTR [rsp+0x20],xmm0
     XXXX:       0f 29 44 24 30          movaps XMMWORD PTR [rsp+0x30],xmm0
     XXXX:       0f 29 44 24 40          movaps XMMWORD PTR [rsp+0x40],xmm0

The store of 1 in sha256_for_mh_sha256_ref() disappears.

It seems to me that this might be a case of undefined behaviour. It can also be triggered by the following (without lto): ```diff diff --git a/mh_sha256/mh_sha256_ref.c b/mh_sha256/mh_sha256_ref.c index 94be975..9d5f8d9 100644 --- a/mh_sha256/mh_sha256_ref.c +++ b/mh_sha256/mh_sha256_ref.c @@ -53,6 +53,7 @@ d += t1; \ h = t1 + t2; +static inline void sha256_single_for_mh_sha256_ref(const uint8_t *data, uint32_t digest[]) { ``` and even with lto, it can avoided by: ```diff diff --git a/mh_sha256/mh_sha256_ref.c b/mh_sha256/mh_sha256_ref.c index 94be975..a2ec1b1 100644 --- a/mh_sha256/mh_sha256_ref.c +++ b/mh_sha256/mh_sha256_ref.c @@ -53,6 +53,7 @@ d += t1; \ h = t1 + t2; +__attribute__((noipa)) void sha256_single_for_mh_sha256_ref(const uint8_t *data, uint32_t digest[]) { ``` But this also prevents the error: ```diff diff --git a/mh_sha256/mh_sha256_ref.c b/mh_sha256/mh_sha256_ref.c index 94be975..927cb36 100644 --- a/mh_sha256/mh_sha256_ref.c +++ b/mh_sha256/mh_sha256_ref.c @@ -176,7 +176,7 @@ sha256_for_mh_sha256_ref(const uint8_t *input_data, uint32_t *digest, const uint else i = ISAL_SHA256_BLOCK_SIZE; - *(uint64_t *) (buf + i - 8) = to_be64((uint64_t) len * 8); + memcpy(buf + i - 8, &(uint64_t) { to_be64((uint64_t) len * 8) }, sizeof(uint64_t)); sha256_single_for_mh_sha256_ref(buf, digest); if (i == (2 * ISAL_SHA256_BLOCK_SIZE)) @@ -346,7 +346,7 @@ mh_sha256_tail_ref(uint8_t *partial_buffer, uint32_t total_len, } // Padding the block len_in_bit = to_be64((uint64_t) total_len * 8); - *(uint64_t *) (partial_buffer + ISAL_MH_SHA256_BLOCK_SIZE - 8) = len_in_bit; + memcpy(partial_buffer + ISAL_MH_SHA256_BLOCK_SIZE - 8, &len_in_bit, sizeof(len_in_bit)); mh_sha256_block_ref(partial_buffer, mh_sha256_segs_digests, frame_buffer, 1); // Calculate multi-hash SHA256 digests (segment digests as input message) ``` Which leads me to believe that gcc's inter-procedural analysis might detect some kind of aliasing issue or such, and removes or modifies these stores. And it appears so, since using ``` CFLAGS="-flto=auto -O2 -g -fno-strict-aliasing" ./configure --prefix=/usr --libdir=/usr/lib ``` seems to also prevent the issue. Since the code base does not seem to be "aliasing aware", I think it would be best to add `-fno-strict-aliasing` by default. --- I'm also attaching a single c source file that can be used to trigger this issue (at least on gcc 15.2.1 20251112): [repro.c](https://github.com/user-attachments/files/24001669/repro.c) ``` $ gcc -Wall -std=c11 -O2 -g repro.c && ./a.out 00 00 00 00 00 00 00 00 6b fa 4c 48 98 2f 8a 42 00 00 00 00 00 00 00 00 28 f9 2b 8d 98 2f 8a 42 $ gcc -Wall -std=c11 -O2 -g -DMAKE_INLINE repro.c && ./a.out 00 00 00 00 00 00 00 00 6b fa 4c 48 98 2f 8a 42 00 00 00 00 00 00 00 00 28 f9 2b 8d 98 2f 8a 42 $ gcc -Wall -std=c11 -O2 -g -fPIC repro.c && ./a.out 00 00 00 00 00 00 00 00 eb f9 6c 44 99 2f 8a 42 00 00 00 00 00 00 00 00 a8 f4 43 49 99 2f 8a 42 $ gcc -Wall -std=c11 -O2 -g -fPIC -DMAKE_INLINE repro.c && ./a.out 00 00 00 00 00 00 00 00 6b fa 4c 48 98 2f 8a 42 00 00 00 00 00 00 00 00 28 f9 2b 8d 98 2f 8a 42 $ clang -Wall -std=c11 -O2 -g repro.c && ./a.out 00 00 00 00 00 00 00 00 eb f9 6c 44 99 2f 8a 42 00 00 00 00 00 00 00 00 a8 f4 43 49 99 2f 8a 42 $ clang -Wall -std=c11 -O2 -g -DMAKE_INLINE repro.c && ./a.out 00 00 00 00 00 00 00 00 eb f9 6c 44 99 2f 8a 42 00 00 00 00 00 00 00 00 a8 f4 43 49 99 2f 8a 42 $ clang -Wall -std=c11 -O2 -g -fPIC repro.c && ./a.out 00 00 00 00 00 00 00 00 eb f9 6c 44 99 2f 8a 42 00 00 00 00 00 00 00 00 a8 f4 43 49 99 2f 8a 42 $ clang -Wall -std=c11 -O2 -g -fPIC -DMAKE_INLINE repro.c && ./a.out 00 00 00 00 00 00 00 00 eb f9 6c 44 99 2f 8a 42 00 00 00 00 00 00 00 00 a8 f4 43 49 99 2f 8a 42 ``` Notice that depending on whether `sha256_single_for_mh_sha256_ref()` is not `static inline` and the `-fPIC` is present, then the output is different in the case of gcc, and that different out matches the result with clang in all cases. In fact, the disassembly of (3) and (1) shows a stark difference: ```diff --- /tmp/good 2025-12-06 22:53:59.561741460 +0100 +++ /tmp/bad 2025-12-06 22:54:26.106319124 +0100 @@ -9,8 +9,6 @@ XXXX: 48 89 e7 mov rdi,rsp XXXX: 0f 29 04 24 movaps XMMWORD PTR [rsp],xmm0 XXXX: 0f 29 44 24 10 movaps XMMWORD PTR [rsp+0x10],xmm0 - XXXX: 48 c7 04 24 01 00 00 mov QWORD PTR [rsp],0x1 - XXXX: 00 XXXX: 0f 29 44 24 20 movaps XMMWORD PTR [rsp+0x20],xmm0 XXXX: 0f 29 44 24 30 movaps XMMWORD PTR [rsp+0x30],xmm0 XXXX: 0f 29 44 24 40 movaps XMMWORD PTR [rsp+0x40],xmm0 ``` The store of `1` in `sha256_for_mh_sha256_ref()` disappears.
pablodelara commented 2025-12-09 07:58:00 -06:00 (Migrated from github.com)

Adding "no-strict-aliasing" seems to be working for all scenarios I'm testing, so thanks for this. I'm pushing a commit with this soon, so I would like to get your name and email address to add you as a contributor, if you are OK with it.

Adding "no-strict-aliasing" seems to be working for all scenarios I'm testing, so thanks for this. I'm pushing a commit with this soon, so I would like to get your name and email address to add you as a contributor, if you are OK with it.
pobrn commented 2025-12-09 17:06:16 -06:00 (Migrated from github.com)

It's fine, feel free to just go ahead. But if you want, name is on my github profile, email is my username at protonmail.com.

It's fine, feel free to just go ahead. But if you want, name is on my github profile, email is my username at protonmail.com.
pablodelara commented 2025-12-15 11:44:53 -06:00 (Migrated from github.com)

Given this only happens when -flto=auto is passed, I decided to document the workaround of adding -fno-strict-aliasing:
github.com/intel/isa-l_crypto@6351fa0914

Given this only happens when -flto=auto is passed, I decided to document the workaround of adding -fno-strict-aliasing: https://github.com/intel/isa-l_crypto/commit/6351fa09143b1137121e88f0b74bb5dd216c4fe9
pobrn commented 2025-12-15 13:08:56 -06:00 (Migrated from github.com)

The code base, at least the tests, is not strict aliasing compatible. I see no reason not to enable it globally. Any reason not to do it?

The code base, at least the tests, is not strict aliasing compatible. I see no reason not to enable it globally. Any reason not to do it?
pablodelara commented 2026-01-08 08:25:32 -06:00 (Migrated from github.com)

The reason is that we need to check if this affects performance in the library somehow. Since this has minor impact and there is a workaround documented, we will release the code as is and we will look further into it after the release.

The reason is that we need to check if this affects performance in the library somehow. Since this has minor impact and there is a workaround documented, we will release the code as is and we will look further into it after the release.
Sign in to join this conversation.
No milestone
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set

Reference
gh-intel/isa-l_crypto#165
No description provided.