Trying to migrate Chromium to the "link all the asm files together"
strategy broke the aarch64 Android build because some of the ifdef'd out
assembly files were missing the .note.gnu.property section for BTI. If
we add support for IBT, that'll be another one.
To fix this, introduce <openssl/asm_base.h>, which must be included at
the start of every assembly file (before the target ifdefs). This does a
couple things:
- It emits BTI and noexecstack markers into every assembly file, even
those that ifdef themselves out.
- It resolves the MSan -> OPENSSL_NO_ASM logic, so we only need to do it
once.
- It defines the same OPENSSL_X86_64, etc., defines we set elsewhere, so
we can ensure they're consistent.
This required carving files up a bit. <openssl/base.h> has a lot of
things, such that trying to guard everything in it on __ASSEMBLER__
would be tedious. Instead, I moved the target defines to a new
<openssl/target.h>. Then <openssl/asm_base.h> is the new header that
pulls in all those things.
Bug: 542
Change-Id: I1682b4d929adea72908655fa1bb15765a6b3473b
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/60765
Reviewed-by: Bob Beck <bbe@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
I did not have "find a bug in the assembler" on my bingo card today, but
here we are.
NASM 2.15, prior to 2.15.04, has a bug where, if a section that already
exists is referenced again with alignment qualifiers, it incorrect adds
padding and mangles the output. See
https://bugzilla.nasm.us/show_bug.cgi?id=3392701.
Work around this by suppressing the perlasm-emitted qualifiers the
second time a section is emitted. We likely don't need these qualifiers
because, for all sections we care about, NASM's defaults are fine, but
perlasm tries to align .text more aggressively than the default, so let
it do that.
Bug: chromium:1422018
Change-Id: Iade5702c139b70772d4957a83c8f9be86c8af97c
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/57825
Reviewed-by: Adam Langley <agl@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
Various constants and strings identifying the authors are currently
misplaced in .text. This change allows using execute-only .text on
platforms that enforce it by default, such as OpenBSD.
Modify x86_64-xlate.pl to replace .rodata with __DATA,__const for macs.
Adapt the nasm/masm path to emit an .rdata segment with alignment of 8.
This last change is not strictly needed but makes things explicit.
Change-Id: If716b892c1faabd85c6c70bdd75e145304841f83
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/57445
Reviewed-by: David Benjamin <davidben@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
If you pass an empty assembly file into nasm, it crashes. Add a dummy
instruction which the static linker will hopefully dropped. (This is a
no-op unless you try to link all the assembly files together for a
simpler build.)
Bug: 542
Change-Id: Idd2b96c129a3a39d5f21e3905762cc34c720f6b2
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/56326
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: Adam Langley <agl@google.com>
This implements similar directives as MASM, so we do not need to build
all the structures by hand. It does not provide any help to abstract
between Win64 and SysV differences, however.
This is pulled together from some old draft CLs I had, one of which
actually synthesized CFI directives from SEH, so it should be possible.
I've intentionally omitted that however, as it also brings in questions
about how to handle the calling convention differences (the existing
machinery won't *quite* work). I've uploaded just this for now, so
review can focus on the basic mechanism.
I've also preserved perlasm's weird mixed tabs and spaces indentation
convention for now, though it is a bit tedious.
Bug: 259
Change-Id: Ib3f46a27751a5319b758d12c462c660cf9f3e632
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/56126
Auto-Submit: David Benjamin <davidben@google.com>
Reviewed-by: Adam Langley <agl@google.com>
Commit-Queue: Adam Langley <agl@google.com>
This is purely cosmetic, but makes it consistent with the DW, etc.,
lines in the block above. The SEH unwind code support will emit a mix of
DW and DB directives and this makes them look more consistent.
Bug: 259
Change-Id: Ia16166ab2495aa813d6076d55af5b62511933c28
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/56125
Reviewed-by: Adam Langley <agl@google.com>
Auto-Submit: David Benjamin <davidben@google.com>
Commit-Queue: Adam Langley <agl@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
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] de990b270d
Bug: 542
Change-Id: I0a8ded24423087c0da13bd0335cbd757d4eee65a
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/55626
Reviewed-by: Adam Langley <agl@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
We generate .S files for assembly, which means they run through the C
preprocessor first. In gas targets where # is the comment marker, there
is a conflict with cpp directives.
The comments actually rely on #This and #source not being directives. If
I begin a line with "if", the build fails. Since the C preprocessor is
responsible for removing C preprocessor comments, we should be able to
safely use // everywhere with less ambiguity.
(In fact, we were already relying on this for 32-bit ARM. The 32-bit ARM
gas line comment marker is @. 64-bit ARM uses //, and x86/x86_64/ppc64
use #.)
This reportedly causes issues for goma. See
https://bugs.chromium.org/p/boringssl/issues/detail?id=448#c3
Bug: 448
Change-Id: Ib58f3152691c1dbcccfc045f21f486b56824283d
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/49965
Reviewed-by: Adam Langley <agl@google.com>
Revert the names used in the BoringSSL C/asm code to the names used in
BoringSSL. This substantially reduces the diff between *ring* and
BoringSSL for these files.
Use a variant of BoringSSL's symbol prefixing machinery to semi-
automatically prefix FFI symbols with the `GFp_` prefix. The names aren't
all exactly the same as before, because previously we *replaced* a
symbol's original prefix with the `GFp_` prefix; now we're prepending
`GFp_`. In the future we'll use a different prefix entirely.
This paves the way for using different prefixes for each version so that
multiple versions of *ring* can be linked into an executable at once.
When building with |OPENSSL_NO_ASM|, the section that marks assembly
files as no-exec-stack will currently be omitted. That results in an
empty assembly file but that's still enough to trigger warnings:
warning: crypto_tests/trampoline-x86_64.o: missing .note.GNU-stack section implies executable stack
This change makes it so that the section marker will always be emitted,
even if the file is otherwise empty.
Change-Id: I2d08d34ed9dbe9e9592c88dcd42d3ba4fa3d7652
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/38084
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: David Benjamin <davidben@google.com>
GNU-based toolchains on ELF platforms default the stack to executable
and rely on a .note.GNU-stack section in *each* object file to flip it
off. The compiler knows to do this for its object files, but assembly
does everything by hand. See this link for details:
https://www.airs.com/blog/archives/518
We do this in the cmake build by passing -Wa,--noexecstack to the
assembler. However, since we have to deal with many buildsystems, it
would be more robust to put it in the source.
It's unclear whether this should be gated on ELF or Linux. The Gentoo
and Ubuntu documents recommend checking for Linux with gas, but only ELF
with NASM.
https://wiki.gentoo.org/wiki/Hardened/GNU_stack_quickstarthttps://wiki.ubuntu.com/SecurityTeam/Roadmap/ExecutableStacks
At the same time, these links suggest it is an ELF-wide issue and not
just Linux:
https://github.com/golang/go/issues/5392https://reviews.freebsd.org/D11033https://github.com/openssl/openssl/issues/4575 also discusses this but
the rationale lists both ELF and non-ELF platforms, so it's unclear.
Treat it as ELF-wide for now. We can revisit this if necessary.
Update-Note: If there is a build failure due to .note.GNU-stack, holler.
Change-Id: Ic59096aa1fc2bf5380a412c9991de22cb46c0faf
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/37984
Reviewed-by: Adam Langley <agl@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
The .type foo, @abi-omnipotent lines weren't being parsed correctly.
This doesn't change the generated files, but some internal state (used
in-progress work on perlasm SEH directives) wasn't quite right.
Change-Id: Id6aec79281a59f45b2eb2aea9f1fb8806b4c483e
Reviewed-on: https://boringssl-review.googlesource.com/c/34786
Reviewed-by: Adam Langley <agl@google.com>
Hexadecimals were erroneously recognized as symbols in .xdata.
(Imported from upstream's b068a9b914887af5cc99895754412582fbb0e10b)
Change-Id: I5d8e8e1969669a8961733802d9f034cf26c45552
Reviewed-on: https://boringssl-review.googlesource.com/c/34704
Reviewed-by: Adam Langley <agl@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
The first attempt involved using Linux's support for hardware
breakpoints to detect when assembly code was run. However, this doesn't
work with SDE, which is a problem.
This version has the assembly code update a global flags variable when
it's run, but only in non-FIPS and non-debug builds.
Update-Note: Assembly files now pay attention to the NDEBUG preprocessor
symbol. Ensure the build passes the symbol in. (If release builds fail
to link due to missing BORINGSSL_function_hit, this is the cause.)
Change-Id: I6b7ced442b7a77d0b4ae148b00c351f68af89a6e
Reviewed-on: https://boringssl-review.googlesource.com/c/33384
Commit-Queue: Adam Langley <agl@google.com>
Reviewed-by: David Benjamin <davidben@google.com>
This involves fixing some bugs in aes_nohw_cbc_encrypt's annotations,
and working around a libunwind bug. In doing so, support .cfi_remember_state
and .cfi_restore_state in perlasm.
Change-Id: Iaedfe691356b0468327a6be0958d034dafa760e5
Reviewed-on: https://boringssl-review.googlesource.com/c/34189
Reviewed-by: Adam Langley <agl@google.com>
Commit-Queue: Adam Langley <agl@google.com>
This uses the x86 trap flag and libunwind to test CFI works at each
instruction. For now, it just uses the system one out of pkg-config and
disables unwind tests if unavailable. We'll probably want to stick a
copy into //third_party and perhaps try the LLVM one later.
This tester caught two bugs in P-256 CFI annotations already:
I47b5f9798b3bcee1748e537b21c173d312a14b42 and
I9f576d868850312d6c14d1386f8fbfa85021b347
An earlier design used PTRACE_SINGLESTEP with libunwind's remote
unwinding features. ptrace is a mess around stop signals (see group-stop
discussion in ptrace(2)) and this is 10x faster, so I went with it. The
question of which is more future-proof is complex:
- There are two libunwinds with the same API,
https://www.nongnu.org/libunwind/ and LLVM's. This currently uses the
system nongnu.org for convenience. In future, LLVM's should be easier
to bundle (less complex build) and appears to even support Windows,
but I haven't tested this. Moreover, setting the trap flag keeps the
test single-process, which is less complex on Windows. That suggests
the trap flag design and switching to LLVM later. However...
- Not all architectures have a trap flag settable by userspace. As far
as I can tell, ARMv8's PSTATE.SS can only be set from the kernel. If
we stick with nongnu.org libunwind, we can use PTRACE_SINGLESTEP and
remote unwinding. Or we implement it for LLVM. Another thought is for
the ptracer to bounce SIGTRAP back into the process, to share the
local unwinding code.
- ARMv7 has no trap flag at all and PTRACE_SINGLESTEP fails. Debuggers
single-step by injecting breakpoints instead. However, ARMv8's trap
flag seems to work in both AArch32 and AArch64 modes, so we may be
able to condition it on a 64-bit kernel.
Sadly, neither strategy works with Intel SDE. Adding flags to cpucap
vectors as we do with ARM would help, but it would not emulate CPUs
newer than the host CPU. For now, I've just had SDE tests disable these.
Annoyingly, CMake does not allow object libraries to have dependencies,
so make test_support a proper static library. Rename the target to
test_support_lib to avoid
https://gitlab.kitware.com/cmake/cmake/issues/17785
Update-Note: This adds a new optional test dependency, but it's disabled
by default (define BORINGSSL_HAVE_LIBUNWIND), so consumers do not need
to do anything. We'll probably want to adjust this in the future.
Bug: 181
Change-Id: I817263d7907aff0904a9cee83f8b26747262cc0c
Reviewed-on: https://boringssl-review.googlesource.com/c/33966
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: Adam Langley <agl@google.com>
Win64 unwind tables place distances from the start of a function in
byte-wide values.
Change-Id: Ie2aad7f6f5b702a60933bd52d872a83cba4e73a9
Reviewed-on: https://boringssl-review.googlesource.com/c/33784
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: Adam Langley <alangley@gmail.com>
Folks keep assuming checked-in assembly files are the source. Between
the preprocessor, delocate, NASM not using the C preprocessor, and GAS's
arch-specific comment syntax, comment markers are kind of a disaster.
This set appears to work for now.
Change-Id: I48e26dafb444dfa310df80dcce87ac291fde8037
Reviewed-on: https://boringssl-review.googlesource.com/c/33304
Commit-Queue: Adam Langley <agl@google.com>
Reviewed-by: Adam Langley <agl@google.com>
MSAN is incompatible with hand-written assembly code. Previously we
required that OPENSSL_NO_ASM be set when building with MSAN, and the
CMake build would take care of this. However, with other build systems
it wasn't always so easy.
This change automatically disables assembly when the compiler is
configured for MSAN.
Change-Id: I6c219120f62d16b99bafc2efb02948ecbecaf87f
Reviewed-on: https://boringssl-review.googlesource.com/31724
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
- In base.h, if BORINGSSL_PREFIX is defined, include
boringssl_prefix_symbols.h
- In all .S files, if BORINGSSL_PREFIX is defined, include
boringssl_prefix_symbols_asm.h
- In base.h, BSSL_NAMESPACE_BEGIN and BSSL_NAMESPACE_END are
defined with appropriate values depending on whether
BORINGSSL_PREFIX is defined; these macros are used in place
of 'namespace bssl {' and '}'
- Add util/make_prefix_headers.go, which takes a list of symbols
and auto-generates the header files mentioned above
- In CMakeLists.txt, if BORINGSSL_PREFIX and BORINGSSL_PREFIX_SYMBOLS
are defined, run util/make_prefix_headers.go to generate header
files
- In various CMakeLists.txt files, add "global_target" that all
targets depend on to give us a place to hook logic that must run
before all other targets (in particular, the header file generation
logic)
- Document this in BUILDING.md, including the fact that it is
the caller's responsibility to provide the symbol list and keep it
up to date
- Note that this scheme has not been tested on Windows, and likely
does not work on it; Windows support will need to be added in a
future commit
Change-Id: If66a7157f46b5b66230ef91e15826b910cf979a2
Reviewed-on: https://boringssl-review.googlesource.com/31364
Commit-Queue: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
Reviewed-by: David Benjamin <davidben@google.com>
These files are otherwise up-to-date with OpenSSL master as of
50ea9d2b3521467a11559be41dcf05ee05feabd6, modulo a couple of spelling
fixes which I've imported.
I've also reverted the same-line label and instruction patch to
x86_64-mont*.pl. The new delocate parser handles that fine.
Change-Id: Ife35c671a8104c3cc2fb6c5a03127376fccc4402
Reviewed-on: https://boringssl-review.googlesource.com/25644
Reviewed-by: Adam Langley <agl@google.com>
This partially reverts commit 38636aba74400272afbb7155bbdfef86be6ed715.
Some build on Android seems to break now. I'm not really sure what the
situation is, but if the weird common symbols are still there (can we
remove them?), they probably ought to have the right flags.
Change-Id: Ief589d763d16b995ac6be536505acf7596a87b30
Reviewed-on: https://boringssl-review.googlesource.com/22404
Commit-Queue: David Benjamin <davidben@google.com>
Commit-Queue: Steven Valdez <svaldez@google.com>
Reviewed-by: Steven Valdez <svaldez@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
Our assembly does not use the GOT to reference symbols, which means
references to visible symbols will often require a TEXTREL. This is
undesirable, so all assembly-referenced symbols should be hidden. CPU
capabilities are the only such symbols defined in C.
These symbols may be hidden by doing at least one of:
1. Build with -fvisibility=hidden
2. __attribute__((visibility("hidden"))) in C.
3. .extern + .hidden in some assembly file referencing the symbol.
We have lots of consumers and can't always rely on (1) happening. We
were doing (3) by way of d216b71f909fe56255813dab0a8d052534bdcb91 and
16e38b2b8f50a3d048f61d2979d5ceddacd70fc3, but missed 32-bit x86 because
it doesn't cause a linker error.
Those two patches are not in upstream. Upstream instead does (3) by way
of x86cpuid.pl and friends, but we have none of these files.
Standardize on doing (2). This avoids accidentally getting TEXTRELs on
some 32-bit x86 build configurations. This also undoes
d216b71f909fe56255813dab0a8d052534bdcb91 and
16e38b2b8f50a3d048f61d2979d5ceddacd70fc3. They are no now longer needed
and reduce the upstream diff.
Change-Id: Ib51c43fce6a7d8292533635e5d85d3c197a93644
Reviewed-on: https://boringssl-review.googlesource.com/22064
Commit-Queue: Matt Braithwaite <mab@google.com>
Reviewed-by: Matt Braithwaite <mab@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
Perl, multiple versions, for some reason occasionally takes issue with
letter b[?] in ox([0-9a-f]+) regex. As result some constants, such as
0xb1 came out wrong when generating code for MASM. Fixes upstream
GH#3241.
(Imported from upstream's c47aea8af1e28e46e1ad5e2e7468b49fec3f4f29.)
This does not affect of the configurations we generate and is imported
to avoid a diff against upstream.
Change-Id: Iacde0ca5220c3607681fad081fbe72d8d613518f
Reviewed-on: https://boringssl-review.googlesource.com/15985
Reviewed-by: Adam Langley <agl@google.com>
This dates to ded93581f1674f81faa0dba4b15a842756066ab2, but we have
since switched to building with nasm, to match upstream's supported
assemblers. Since this doesn't affect anything we generate, remove the
workaround to reduce the diff against upstream.
Change-Id: I549ae97ad6d6f28836f6c9d54dcf51c518de7521
Reviewed-on: https://boringssl-review.googlesource.com/15986
Reviewed-by: Adam Langley <agl@google.com>
Change-Id: I32b37306265e89afca568f20bfba2e04559c4f0b
Reviewed-on: https://boringssl-review.googlesource.com/14527
Commit-Queue: Adam Langley <agl@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
