From cdccbe121fa5dc8c6df7102eaedb62c99eec8273 Mon Sep 17 00:00:00 2001 From: David Benjamin Date: Sat, 19 Nov 2022 10:15:55 -0500 Subject: [PATCH] Fully condition all assembly files. For the C files, rather than force the caller to juggle crypto_linux_sources, etc., we just wrap the whole file in ifdefs and ask the callers to link everything together. Assembly is typically built by a different tool, so we have less room here. However, there are really only two families of tools we care about: gas (which runs the C preprocessor) and nasm (which has its own preprocessor). Callers should be able to limit themselves to special-casing Windows x86(_64) for NASM and then pass all the remaining assembly files to their gas-like tool. File-wide ifdefs can take care of the rest. We're almost set up to allow this, except the files condition on architecture, but not OS. Add __ELF__, __APPLE__, and _WIN32 conditions as appropriate. One subtlety: the semantics of .note.GNU-stack are that *any* unmarked object file makes the stack executable. (In current GNU ld. lld doesn't have this issue, and GNU ld claims they'll remove it in a later release.) Empirically, this doesn't seem to apply to empty object files but, to be safe, we should ensure all object files have the marking. That leads to a second subtlety: on targets where @ is a comment, @progbits is spelled %progbits, per [0]. If we want all .S files to work in all targets, that includes these markers. Fortunately, %progbits appears to work universally (see [1], [2], [3], [4]), so I've just switched us to that spelling. I've also tightened up the __arm__ and __aarch64__ checks to __ARMEL__ and __AARCH64EL__. We don't support big-endian Arm (or any other platform) and, even if we did, the conditions in the assembly files should match the conditions in the C files that pull them in. This CL doesn't change our build to take advantage of this (though I'll give it a go later), just makes it possible for builds to do it. [0] https://sourceware.org/binutils/docs/as/Section.html [1] https://patchwork.kernel.org/project/linux-crypto/patch/20170119212805.18049-1-dvlasenk@redhat.com/#20050285 [2] https://gcc.gnu.org/bugzilla/show_bug.cgi?id=92820#c11 [3] https://sourceware.org/legacy-ml/gdb-patches/2016-01/msg00319.html [4] https://github.com/llvm/llvm-project/commit/de990b270d73632a834cb37e6ea50db093321aad Bug: 542 Change-Id: I0a8ded24423087c0da13bd0335cbd757d4eee65a Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/55626 Reviewed-by: Adam Langley Commit-Queue: David Benjamin --- crypto/curve25519/asm/x25519-asm-arm.S | 4 +-- crypto/hrss/asm/poly_rq_mul.S | 2 +- crypto/perlasm/arm-xlate.pl | 39 +++++++++++++++----- crypto/perlasm/ppc-xlate.pl | 13 ++++--- crypto/perlasm/x86_64-xlate.pl | 50 +++++++++++++++++++------- crypto/perlasm/x86asm.pl | 38 +++++++++++++++----- crypto/poly1305/poly1305_arm_asm.S | 4 +-- 7 files changed, 110 insertions(+), 40 deletions(-) diff --git a/crypto/curve25519/asm/x25519-asm-arm.S b/crypto/curve25519/asm/x25519-asm-arm.S index 41bc0c6e8..ab84c104b 100644 --- a/crypto/curve25519/asm/x25519-asm-arm.S +++ b/crypto/curve25519/asm/x25519-asm-arm.S @@ -23,7 +23,7 @@ #endif #endif -#if !defined(OPENSSL_NO_ASM) && defined(__arm__) && !defined(__APPLE__) +#if !defined(OPENSSL_NO_ASM) && defined(__ARMEL__) && defined(__ELF__) #if defined(BORINGSSL_PREFIX) #include @@ -2129,7 +2129,7 @@ mov sp,r12 vpop {q4,q5,q6,q7} bx lr -#endif /* !OPENSSL_NO_ASM && __arm__ && !__APPLE__ */ +#endif /* !OPENSSL_NO_ASM && __ARMEL__ && __ELF__ */ #if defined(__ELF__) .section .note.GNU-stack,"",%progbits diff --git a/crypto/hrss/asm/poly_rq_mul.S b/crypto/hrss/asm/poly_rq_mul.S index c37d7d0b4..2b410fe5f 100644 --- a/crypto/hrss/asm/poly_rq_mul.S +++ b/crypto/hrss/asm/poly_rq_mul.S @@ -8489,5 +8489,5 @@ ret #endif #if defined(__ELF__) -.section .note.GNU-stack,"",@progbits +.section .note.GNU-stack,"",%progbits #endif diff --git a/crypto/perlasm/arm-xlate.pl b/crypto/perlasm/arm-xlate.pl index c000e020a..e876c8b27 100755 --- a/crypto/perlasm/arm-xlate.pl +++ b/crypto/perlasm/arm-xlate.pl @@ -151,6 +151,28 @@ sub expand_line { return $line; } +my ($arch_defines, $target_defines); +if ($flavour =~ /32/) { + $arch_defines = "defined(__ARMEL__)"; +} elsif ($flavour =~ /64/) { + $arch_defines = "defined(__AARCH64EL__)"; +} else { + die "unknown architecture: $flavour"; +} +if ($flavour =~ /linux/) { + # Although the flavour is specified as "linux", it is really used by all + # ELF platforms. + $target_defines = "defined(__ELF__)"; +} elsif ($flavour =~ /ios/) { + # Although the flavour is specified as "ios", it is really used by all Apple + # platforms. + $target_defines = "defined(__APPLE__)"; +} elsif ($flavour =~ /win/) { + $target_defines = "defined(_WIN32)"; +} else { + die "unknown target: $flavour"; +} + print <<___; // This file is generated from a similarly-named Perl script in the BoringSSL // source tree. Do not edit by hand. @@ -162,12 +184,9 @@ sub expand_line { #define OPENSSL_NO_ASM #endif -#if !defined(OPENSSL_NO_ASM) +#if !defined(OPENSSL_NO_ASM) && $arch_defines && $target_defines ___ -print "#if defined(__arm__)\n" if ($flavour eq "linux32"); -print "#if defined(__aarch64__)\n" if ($flavour eq "linux64" || $flavour eq "win64"); - print "#if defined(BORINGSSL_PREFIX)\n"; print "#include \n"; print "#endif\n"; @@ -239,10 +258,12 @@ sub expand_line { print "\n"; } -print "#endif\n" if ($flavour eq "linux32" || $flavour eq "linux64" || $flavour eq "win64"); -print "#endif // !OPENSSL_NO_ASM\n"; - -# See https://www.airs.com/blog/archives/518. -print ".section\t.note.GNU-stack,\"\",\%progbits\n" if ($flavour =~ /linux/); +print <<___; +#endif // !OPENSSL_NO_ASM && $arch_defines && $target_defines +#if defined(__ELF__) +// See https://www.airs.com/blog/archives/518. +.section .note.GNU-stack,"",\%progbits +#endif +___ close STDOUT or die "error closing STDOUT: $!"; diff --git a/crypto/perlasm/ppc-xlate.pl b/crypto/perlasm/ppc-xlate.pl index fff9c004f..1c51577ed 100644 --- a/crypto/perlasm/ppc-xlate.pl +++ b/crypto/perlasm/ppc-xlate.pl @@ -265,7 +265,7 @@ sub vcrypto_op { #endif #endif -#if !defined(OPENSSL_NO_ASM) && defined(__powerpc64__) +#if !defined(OPENSSL_NO_ASM) && defined(__powerpc64__) && defined(__ELF__) ___ while($line=<>) { @@ -309,9 +309,12 @@ sub vcrypto_op { print "\n"; } -print "#endif // !OPENSSL_NO_ASM && __powerpc64__\n"; - -# See https://www.airs.com/blog/archives/518. -print ".section\t.note.GNU-stack,\"\",\@progbits\n" if ($flavour =~ /linux/); +print <<___; +#endif // !OPENSSL_NO_ASM && __powerpc64__ && __ELF__ +#if defined(__ELF__) +// See https://www.airs.com/blog/archives/518. +.section .note.GNU-stack,"",\%progbits +#endif +___ close STDOUT or die "error closing STDOUT: $!"; diff --git a/crypto/perlasm/x86_64-xlate.pl b/crypto/perlasm/x86_64-xlate.pl index e3745f898..1b1e793f3 100755 --- a/crypto/perlasm/x86_64-xlate.pl +++ b/crypto/perlasm/x86_64-xlate.pl @@ -72,6 +72,7 @@ my $gas=1; $gas=0 if ($output =~ /\.asm$/); my $elf=1; $elf=0 if (!$gas); +my $apple=0; my $win64=0; my $prefix=""; my $decor=".L"; @@ -91,7 +92,7 @@ $prefix=`echo __USER_LABEL_PREFIX__ | $ENV{CC} -E -P -`; $prefix =~ s|\R$||; # Better chomp } -elsif ($flavour eq "macosx") { $gas=1; $elf=0; $prefix="_"; $decor="L\$"; } +elsif ($flavour eq "macosx") { $gas=1; $elf=0; $apple=1; $prefix="_"; $decor="L\$"; } elsif ($flavour eq "masm") { $gas=0; $elf=0; $masm=$masmref; $win64=1; $decor="\$L\$"; } elsif ($flavour eq "nasm") { $gas=0; $elf=0; $nasm=$nasmref; $win64=1; $decor="\$L\$"; $PTR=""; } elsif (!$gas) { die "unknown flavour $flavour"; } @@ -1146,15 +1147,17 @@ sub rxb { } if ($nasm) { + die "unknown target" unless ($win64); print <<___; +\%ifidn __OUTPUT_FORMAT__, win64 default rel -%define XMMWORD -%define YMMWORD -%define ZMMWORD +\%define XMMWORD +\%define YMMWORD +\%define ZMMWORD -%ifdef BORINGSSL_PREFIX -%include "boringssl_prefix_symbols_nasm.inc" -%endif +\%ifdef BORINGSSL_PREFIX +\%include "boringssl_prefix_symbols_nasm.inc" +\%endif ___ } elsif ($masm) { print <<___; @@ -1163,14 +1166,24 @@ sub rxb { } if ($gas) { - print <<___; + my $target; + if ($elf) { + # The "elf" target is really ELF with SysV ABI, but every ELF platform + # uses the SysV ABI. + $target = "defined(__ELF__)"; + } elsif ($apple) { + $target = "defined(__APPLE__)"; + } else { + die "unknown target: $flavour"; + } + print <<___; #if defined(__has_feature) #if __has_feature(memory_sanitizer) && !defined(OPENSSL_NO_ASM) #define OPENSSL_NO_ASM #endif #endif -#if defined(__x86_64__) && !defined(OPENSSL_NO_ASM) +#if defined(__x86_64__) && !defined(OPENSSL_NO_ASM) && $target #if defined(BORINGSSL_PREFIX) #include #endif @@ -1259,10 +1272,21 @@ sub rxb { } print "\n$current_segment\tENDS\n" if ($current_segment && $masm); -print "END\n" if ($masm); -print "#endif\n" if ($gas); -# See https://www.airs.com/blog/archives/518. -print ".section\t.note.GNU-stack,\"\",\@progbits\n" if ($elf); +if ($masm) { + print "END\n"; +} elsif ($gas) { + print <<___; +#endif +#if defined(__ELF__) +// See https://www.airs.com/blog/archives/518. +.section .note.GNU-stack,"",\%progbits +#endif +___ +} elsif ($nasm) { + print "\%endif\n"; +} else { + die "unknown assembler"; +} close STDOUT or die "error closing STDOUT: $!"; diff --git a/crypto/perlasm/x86asm.pl b/crypto/perlasm/x86asm.pl index c22421f9a..743c79288 100644 --- a/crypto/perlasm/x86asm.pl +++ b/crypto/perlasm/x86asm.pl @@ -284,22 +284,44 @@ sub ::asm_finish ___ if ($win32) { print <<___ unless $masm; -%ifdef BORINGSSL_PREFIX -%include "boringssl_prefix_symbols_nasm.inc" -%endif +\%ifdef BORINGSSL_PREFIX +\%include "boringssl_prefix_symbols_nasm.inc" +\%endif +\%ifidn __OUTPUT_FORMAT__, win32 ___ + print @out; + print "\%endif\n"; } else { + my $target; + if ($elf) { + $target = "defined(__ELF__)"; + } elsif ($macosx) { + $target = "defined(__APPLE__)"; + } else { + die "unknown target"; + } + print <<___; -#if defined(__i386__) +#if defined(__has_feature) +#if __has_feature(memory_sanitizer) && !defined(OPENSSL_NO_ASM) +#define OPENSSL_NO_ASM +#endif +#endif + +#if !defined(OPENSSL_NO_ASM) && defined(__i386__) && $target #if defined(BORINGSSL_PREFIX) #include #endif +___ + print @out; + print <<___; +#endif // !defined(OPENSSL_NO_ASM) && defined(__i386__) && $target +#if defined(__ELF__) +// See https://www.airs.com/blog/archives/518. +.section .note.GNU-stack,"",\%progbits +#endif ___ } - print @out; - print "#endif\n" unless ($win32); - # See https://www.airs.com/blog/archives/518. - print ".section\t.note.GNU-stack,\"\",\@progbits\n" if ($elf); } sub ::asm_init diff --git a/crypto/poly1305/poly1305_arm_asm.S b/crypto/poly1305/poly1305_arm_asm.S index 80a4b31fa..7895ab49d 100644 --- a/crypto/poly1305/poly1305_arm_asm.S +++ b/crypto/poly1305/poly1305_arm_asm.S @@ -4,7 +4,7 @@ #endif #endif -#if defined(__arm__) && !defined(OPENSSL_NO_ASM) && !defined(__APPLE__) +#if defined(__ARMEL__) && !defined(OPENSSL_NO_ASM) && defined(__ELF__) #if defined(BORINGSSL_PREFIX) #include @@ -2022,7 +2022,7 @@ vst1.8 d4,[r0,: 64] add sp,sp,#0 bx lr -#endif /* __arm__ && !OPENSSL_NO_ASM && !__APPLE__ */ +#endif /* __ARMEL__ && !OPENSSL_NO_ASM && __ELF__ */ #if defined(__ELF__) .section .note.GNU-stack,"",%progbits