awslabs / s2n-bignum

Other
126 stars 22 forks source link

Run assembler in CI to catch issues #96

Open torben-hansen opened 12 months ago

torben-hansen commented 12 months ago

This typo https://github.com/awslabs/s2n-bignum/pull/95 should be caught by some CI test. Run an assembler, or similar(?). Or just verify that all files ensure that it's not used in an executable stack.

aqjune-aws commented 12 months ago

Hi, which assembler are you using? It is because as in DevDesktop doesn't seem to raise any warning or error about the typo.

aarch64 (23-11-21 21:12:21) <0> [~/s2n-bignum-ci/arm]
dev-dsk-lebjuney-1d-d3fc5ff7 % cat curve25519/bignum_madd_n25519.S | cc -E -I../include  -xassembler-with-cpp - | tr ';' '\n' >a.S

aarch64 (23-11-21 21:12:36) <0> [~/s2n-bignum-ci/arm]
dev-dsk-lebjuney-1d-d3fc5ff7 % tail -5 a.S
        ldp x19, x20, [sp], 16
        ret

.section .note.GNU-stacz,"",%progbits

aarch64 (23-11-21 21:12:40) <0> [~/s2n-bignum-ci/arm]
dev-dsk-lebjuney-1d-d3fc5ff7 % as -o a.o a.S

aarch64 (23-11-21 21:12:48) <0> [~/s2n-bignum-ci/arm]
dev-dsk-lebjuney-1d-d3fc5ff7 % as --version
GNU assembler version 2.29.1-31.amzn2
Copyright (C) 2017 Free Software Foundation, Inc.
This program is free software; you may redistribute it under the terms of
the GNU General Public License version 3 or later.
This program has absolutely no warranty.
This assembler was configured for a target of `aarch64-redhat-linux'.
aqjune-aws commented 12 months ago

The first command strips away C-style comments and processes include directives. The second command shows that stacz is still there, and then the third command (as) is compiling the assembly without warnings.

aqjune-aws commented 12 months ago

To record, the error message appeared when building the FIPS version of AWS-LC.

[393/584] Building ASM object crypto/fipsmodule/CMakeFiles/bcm_hashunset.dir/bcm-delocated.S.o                                                  
/home/ec2-user/humble/aws-lc/third_party/s2n-bignum/arm/curve25519/bignum_madd_n25519_alt.S: Assembler messages:                                
/home/ec2-user/humble/aws-lc/third_party/s2n-bignum/arm/curve25519/bignum_madd_n25519_alt.S:318: Warning: ignoring incorrect section type for .n
ote.GNU-stacz                                                                                                                                   
/home/ec2-user/humble/aws-lc/third_party/s2n-bignum/arm/curve25519/bignum_madd_n25519_alt.S:318: Warning: ignoring changed section type for .not
e.GNU-stacz  
$ as --version
GNU assembler version 2.39-6.amzn2023.0.10

The error message could not be reproduced on my side, but this check seems helpful indeed.

aqjune-aws commented 1 month ago

We should add Windows CI too.