Commit Graph

129 Commits

Author SHA1 Message Date
David Benjamin
17d553d299 Add a CFI tester to CHECK_ABI.
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>
2019-01-03 22:01:55 +00:00
Adam Langley
a6a049a6fb Add start of infrastructure for checking constant-time properties.
Valgrind's checking of uninitialised memory behaves very much like a
check for constant-time code: branches and memory indexes based on
uninitialised memory trigger warnings. Therefore, if we can tell
Valgrind that some secret is “uninitialised”, it'll give us a warning if
we do something non-constant-time with it.

This was the idea behind https://github.com/agl/ctgrind. But tricks like
that are no longer needed because Valgrind now comes with support for
marking regions of memory as defined or not. Therefore we can use that
API to check constant-time code.

This CL defines |CONSTTIME_SECRET| and |CONSTTIME_DECLASSIFY|, which are
no-ops unless the code is built with
|BORINGSSL_CONSTANT_TIME_VALIDATION| defined, which it isn't by default.
So this CL is a no-op itself so far. But it does show that a couple of
bits of constant-time time are, in fact, constant-time—seemingly even
when compiled with optimisations, which is nice.

The annotations in the RSA code are a) probably not marking all the
secrets as secret, and b) triggers warnings that are a little
interesting:

The anti-glitch check calls |BN_mod_exp_mont| which checks that the
input is less than the modulus. Of course, it is because the input is
the RSA plaintext that we just decrypted, but the plaintext is supposed
to be secret and so branching based on its contents isn't allows by
Valgrind. The answer isn't totally clear, but I've run out of time on
this for now.

Change-Id: I1608ed0b22d201e97595fafe46127159e02d5b1b
Reviewed-on: https://boringssl-review.googlesource.com/c/33504
Reviewed-by: Adam Langley <agl@google.com>
Reviewed-by: David Benjamin <davidben@google.com>
Commit-Queue: Adam Langley <agl@google.com>
2018-12-18 22:43:02 +00:00
David Benjamin
8c23d3a5df Make Windows symbol-prefixing work.
This teaches read_symbols.go to use debug/pe, and fixes miscellaneous
issues with NASM. It also reveals a problem with this strategy of
getting symbols out at the linker level: inline functions.  I'm thinking
a better long-term mechanism may be to parse our header files.

Change-Id: I11b008543a7a97db3db9d4062ee4ddb910d174b7
Reviewed-on: https://boringssl-review.googlesource.com/c/33349
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: Adam Langley <agl@google.com>
2018-11-27 22:13:22 +00:00
Junghoon Jang
e8ba1e3b21 Support assembly building for arm64e architecture.
iPhone XS/XS MAX/XR uses arm64e.

Change-Id: I89bd6b9307176c03fdc1a402ce6b8df080b00653
Reviewed-on: https://boringssl-review.googlesource.com/c/33224
Reviewed-by: Adam Langley <agl@google.com>
Commit-Queue: Adam Langley <agl@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
2018-11-16 01:43:53 +00:00
David Benjamin
6650898e09 Remove -fsanitize-cfi-icall-generalize-pointers.
Bug: chromium:785442
Change-Id: Ia073fcae716541bc9d008e3e2148e9f0ac30e637
Reviewed-on: https://boringssl-review.googlesource.com/c/32121
Commit-Queue: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
Reviewed-by: Adam Langley <agl@google.com>
2018-10-15 23:54:44 +00:00
David Benjamin
5baee45652 Use Go modules with delocate.
This makes running go test, etc., in util/fipstools/delocate work! This
adds a go_executable command to CMake like:

  go_executable(delocate boringssl.googlesource.com/boringssl/util/fipstools/delocate)

which internally gets dependencies and whatnot so it behaves like usual
Go.

Update-Note: delocate has been rearranged a bit.
Change-Id: I244a7317dd8d4f2ab77a0daa624ed3e0b385faef
Reviewed-on: https://boringssl-review.googlesource.com/31885
Commit-Queue: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
Reviewed-by: Adam Langley <agl@google.com>
2018-09-17 22:19:52 +00:00
Adam Langley
e77c27d734 Automatically disable assembly with MSAN.
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>
2018-09-07 21:12:37 +00:00
Joshua Liebow-Feeser
8c7c6356e6 Support symbol prefixes
- 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>
2018-09-06 20:07:52 +00:00
David Benjamin
681ff27720 Flatten most of the crypto target.
The fipsmodule is still separate as that's a lot of build mess. (Though
that too may be worth pulling in eventually. CMake usually has different
opinions on generated files if they're in the same directory. We might
be able to avoid the set_source_properties(GENERATED) thing.)

Change-Id: Ie1f9345009044d4f0e7541ca779e01bdc5ad62f6
Reviewed-on: https://boringssl-review.googlesource.com/31586
Reviewed-by: Adam Langley <agl@google.com>
2018-09-05 23:41:25 +00:00
Adam Langley
9c969bf491 Switch to Clang 6.0's fuzzer support.
With Clang 6.0, libFuzzer is now included and there's a new
-fsanitize=fuzzer command to enable the needed compiler actions.

Change-Id: If8c0d649f494655c5bb1e55ebdbf39450940c75b
Reviewed-on: https://boringssl-review.googlesource.com/31324
Reviewed-by: David Benjamin <davidben@google.com>
2018-08-27 17:18:56 +00:00
David Benjamin
e6fd125d31 Align on a single CMake style.
We currently write a mix of "if (FOO)" and "if(FOO)". While the former looks
more like a usual language, CMake believes everything, even "if" and "else", is
just a really really funny function call (a "command").

We should pick something for consistency. Upstream CMake writes "if(FOO)", so
go with that one.

Change-Id: I67e0eb650a52670110b417312a362c9f161c8721
Reviewed-on: https://boringssl-review.googlesource.com/30807
Reviewed-by: Adam Langley <agl@google.com>
2018-08-10 16:22:31 +00:00
David Benjamin
17dc94e874 Add -handshaker-path to run_test.
Otherwise ninja run_tests doesn't work right build directories named something
other than "build" on Linux.

Change-Id: I4d7ff319aed338870f5ac071e875fda549bbbd06
Reviewed-on: https://boringssl-review.googlesource.com/30824
Commit-Queue: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
Reviewed-by: Steven Valdez <svaldez@google.com>
2018-08-10 14:31:47 +00:00
David Benjamin
678c841cbe Use -flto=thin in the CFI bot.
The CFI bot is currently failing on a mysterious error message, coming from the
recent clang roll.

  Called function is not the same type as the call!
    call void @EVP_MD_CTX_init(%struct.env_md_ctx_st* %8), !dbg !72123
  LLVM ERROR: Broken function found, compilation aborted!

Chromium actually passes -flto=thin, which seems to avoid the error, testing
locally. Why it does, I haven't the slightest clue. The offending calls to
EVP_MD_CTX_init (and EVP_MD_CTX_cleanup) are those buried in
bssl::ScopedEVP_MD_CTX. However, not all calls are problematic, only the one in
test_config.cc. What's more, if I add a call in async_bio.cc, linked into all
the same targets, the copy in test_config.cc is suddenly fine!?

Maybe there's just a bug in the LTO logic that ThinLTO avoids...

Change-Id: I5266eec75edea2a38dee8ad5591db8d65d3bdede
Reviewed-on: https://boringssl-review.googlesource.com/30805
Reviewed-by: Steven Valdez <svaldez@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
2018-08-10 14:18:28 +00:00
David Benjamin
e9ae99bba2 Add an option to statically link a custom libc++.
MSan and TSan both require instrumenting everything. Add some machinery so we
can do this on the bots.

Change-Id: I7d2106bc852ee976455d18787d3a20a35373a9e7
Reviewed-on: https://boringssl-review.googlesource.com/30644
Reviewed-by: Adam Langley <agl@google.com>
2018-08-09 21:37:04 +00:00
Steven Valdez
e5388e097a Add handshaker as run_tests dependency.
Change-Id: I545a61794d6dbb187d973351da6d54e33a608d0d
Reviewed-on: https://boringssl-review.googlesource.com/30244
Reviewed-by: Matt Braithwaite <mab@google.com>
2018-08-01 23:47:11 +00:00
David Benjamin
5852cfccbc Add a basic TSan test for ref-counts.
Confirmed that, if the locks are commented out, TSan catches the threading
error.

Change-Id: I3e4ef9a7ca85fdbacf8c8b13694a5a54c6d5f99b
Reviewed-on: https://boringssl-review.googlesource.com/29924
Reviewed-by: Adam Langley <agl@google.com>
2018-07-23 18:57:19 +00:00
David Benjamin
e7b2b13fd4 Add link to CMake bugfix.
Change-Id: I6d82cbb33a5ac2acf3f4e0993210e0bddba8f1ad
Reviewed-on: https://boringssl-review.googlesource.com/29644
Reviewed-by: Adam Langley <agl@google.com>
2018-07-06 18:54:12 +00:00
Guillaume Egles
791f2822b2 Fix VS build when assembler is enabled
The whitespace in the _STL_EXTRA_DISABLED_WARNINGS value was creating issues
for the CMake generated assembler build script called by VS.

By narrowing the build scope of this STL (and thus C++ only) variable to only C++
we avoid the problem altogether as it will not be passed to the assembler script.

Change-Id: Id422bdd991492f39acc82d52af2ea6d952deb6c6
Reviewed-on: https://boringssl-review.googlesource.com/29504
Reviewed-by: David Benjamin <davidben@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
2018-06-29 20:38:22 +00:00
David Benjamin
9229b4fb86 Fix typo in build flags.
Change-Id: Ifa2f9f6ae00e6af5ce22254496ed37e2774b79e8
Reviewed-on: https://boringssl-review.googlesource.com/28747
Reviewed-by: Steven Valdez <svaldez@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
2018-05-29 14:33:16 +00:00
Adam Vartanian
189270cd19 Ignore Spectre mitigation warning.
VS2017 has added a new warning that indicates where Spectre mitigation
code would be inserted if /Qspectre were specified.

Change-Id: If80cd6a7d0c5a45313f4c3644b304cadecf465b0
Reviewed-on: https://boringssl-review.googlesource.com/28684
Reviewed-by: Adam Langley <agl@google.com>
Commit-Queue: Adam Langley <agl@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
2018-05-22 15:26:15 +00:00
David Benjamin
6291af4e52 Add -DOPENSSL_SMALL to CMake.
Adding preprocessor flags requires a lot of typing in the CMake
command-line (-DCMAKE_C_FLAGS=-DOPENSSL_SMALL
-DCMAKE_CXX_FLAGS=-DOPENSSL_SMALL).

Change-Id: Ieafc4155d656306c1f22746f780faa5c1d3e27be
Reviewed-on: https://boringssl-review.googlesource.com/26784
Reviewed-by: Adam Langley <agl@google.com>
Commit-Queue: Adam Langley <agl@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
2018-03-23 21:07:48 +00:00
sphawk
3ab1a69545 fix compilation error for non-english windows (like cjk)
add /utf-8 switch for msvc build. source code has several utf-8 characters
fix C2001 error. escape non-printable ascii code generated by embed_test_data.go
fix C4819 warning. add u8 keyword on utf-8 string literal (ripemd_test.cc)

Change-Id: I8c04dc7f0359e6ee27efada066863826d263d5cd
Reviewed-on: https://boringssl-review.googlesource.com/26484
Reviewed-by: David Benjamin <davidben@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
2018-03-15 17:52:23 +00:00
David Benjamin
ec55dc15d3 Update tools.
Also remove the -Wtautological-constant-compare logic. I believe Clang
has since removed that problematic warning from -Wall and that check was
causing problems when we were embedded as a subproject in a project that
didn't set CMP0025.

(In that case, by the time our build file ran, the compiler had already
been detected and the damage done. This unfortunately means the next
Clang version check will hit the same issue, but let's deal with that
when we get there.)

Change-Id: Iea5f262899b74c5b84f707f4cf4ac4b3540c4acb
Reviewed-on: https://boringssl-review.googlesource.com/26375
Reviewed-by: Steven Valdez <svaldez@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
2018-03-05 20:49:21 +00:00
David Benjamin
8d67f6f0cf Update tools.
This is a reland 9d1f96606c, which should
hopefuly be fine after afd1cd959e. Though
I've also gone ahead and gotten the latest versions of things.
(android_tools and clang updated.)

In particular, get the new NDK. Unfortunately, the new clang picks up
an unfortunate change for clang-cl that we now must work around.

http://llvm.org/viewvc/llvm-project?view=revision&revision=319116

Bug: 109
Change-Id: If19b09c585957fefaffa8c3197a50189402a555a
Reviewed-on: https://boringssl-review.googlesource.com/25025
Reviewed-by: Steven Valdez <svaldez@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
2018-01-22 18:30:18 +00:00
David Benjamin
3c92e80d7a Revert "Update tools."
This reverts commit 9d1f96606c.

Reason for revert: aarch64 bots are breaking for some reason.

Original change's description:
> Update tools.
> 
> In particular, get the new NDK. Unfortunately, the new clang picks up
> an unfortunate change for clang-cl that we now must work around.
> 
> http://llvm.org/viewvc/llvm-project?view=revision&revision=319116
> 
> Bug: 109
> Change-Id: I091ca7160683e70cd79b5c2b7a4267fea258ec17
> Reviewed-on: https://boringssl-review.googlesource.com/24644
> Reviewed-by: Steven Valdez <svaldez@google.com>
> Commit-Queue: David Benjamin <davidben@google.com>
> CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>

TBR=davidben@google.com,svaldez@google.com

Change-Id: I98960f295987857c4e42c312059b6d5934bb5e43
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: 109
Reviewed-on: https://boringssl-review.googlesource.com/24747
Reviewed-by: David Benjamin <davidben@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
2018-01-09 21:45:11 +00:00
David Benjamin
9d1f96606c Update tools.
In particular, get the new NDK. Unfortunately, the new clang picks up
an unfortunate change for clang-cl that we now must work around.

http://llvm.org/viewvc/llvm-project?view=revision&revision=319116

Bug: 109
Change-Id: I091ca7160683e70cd79b5c2b7a4267fea258ec17
Reviewed-on: https://boringssl-review.googlesource.com/24644
Reviewed-by: Steven Valdez <svaldez@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
2018-01-09 21:28:00 +00:00
Marek Gilbert
11850d5f61 Rename all googletest CMake targets
CMake targets are visible globally but gtest_main has boringssl-specific
behavior that isn't appropriate for general use.

This change makes it possible to use boringssl and abseil-cpp in the
same project (since abseil-cpp expects gtest_main to exist and be useful
for its own tests).

Change-Id: Icc81c11b8bb4b1e21cea7c9fa725b6c082bd5369
Reviewed-on: https://boringssl-review.googlesource.com/24604
Reviewed-by: Adam Langley <agl@google.com>
Commit-Queue: Adam Langley <agl@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
2018-01-04 16:30:54 +00:00
David Benjamin
9894ee9de2 Scope CMAKE_ASM_FLAGS workaround to the old NDK toolchain.
The one in the NDK works just fine. In particular, this means one can
pass -DCMAKE_ASM_FLAGS="-march=armv8-a" and test the ARMv8 assembler
warnings.

Additionally, make the workaround put the flags in the other order, so
-march is user-overridable.

Change-Id: I278ddd17ab688f83ee01f2aca4ff32307f5b0a2d
Reviewed-on: https://boringssl-review.googlesource.com/24164
Reviewed-by: Adam Langley <agl@google.com>
2017-12-14 01:55:26 +00:00
David Benjamin
6979c7e8eb Disable Clang -Wtautological-constant-compare.
This is a recent Clang warning, but it's far too aggressive. The earlier
unsigned long silliness was worth fixing, but it otherwise complains on
32-bit platforms with:

  if (some_size_t > 0xffffffff) {
    ...
  }

which is unreasonable as, on 64-bit platforms, this check is meaningful
and requiring the programmer add ifdefs is error-prone. This matches
Chromium in https://crbug.com/767059.

Bug: chromium:767059
Change-Id: I0bb0f3a4b60f222e9d1b3c569471fbcf5518caed
Reviewed-on: https://boringssl-review.googlesource.com/23845
Commit-Queue: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
Reviewed-by: Adam Langley <agl@google.com>
2017-12-05 19:34:47 +00:00
David Benjamin
c367ee5439 Add a CFI build flag.
This uses Clang's CFI feature.

Bug: 201
Change-Id: I7a42ec73dc8bfb3893ec69f2d2f4d7e3a2fd2cc4
Reviewed-on: https://boringssl-review.googlesource.com/23225
Commit-Queue: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
Reviewed-by: Steven Valdez <svaldez@google.com>
2017-11-21 17:40:40 +00:00
Andres Erbsen
5b280a80df Move curve25519 code to third_party/fiat.
This change doesn't actually introduce any Fiat code yet. It sets up the
directory structure to make the diffs in the next change clearer.

Change-Id: I38a21fb36b18a08b0907f9d37b7ef5d7d3137ede
Reviewed-on: https://boringssl-review.googlesource.com/22624
Reviewed-by: David Benjamin <davidben@google.com>
2017-11-03 22:23:59 +00:00
David Benjamin
f5beb883c2 Disable macOS architecture hack on CMake 3.0.
Per the comment, it's no longer necessary. macOS i386 does not exist,
but apparently iOS i386 does! We can probably just remove it altogether,
but our cmake_minimum_required is nominally 2.8, so I just put the
version check in.

Bug: 210
Change-Id: I6e0617a3f292a218b2465eee85bd4814bd0e55c7
Reviewed-on: https://boringssl-review.googlesource.com/22304
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>
2017-10-27 18:58:08 +00:00
Daniel Wagner-Hall
3b358b25b0 Specify -stdlib=libc++ if APPLE
If you specify any --target which refers to a x86_64-apple-darwin
triple, or a more specific variant derived from it, specifying
-stdlib=libc++ is required, otherwise clang falls back to libstdc++
which didn't include c++11, and fails to compile in very obscure ways
(simply failing to find any c++11 symbols).

Change-Id: I58025cea91eaa0c16d9b5831f9965889b75bbc31
Reviewed-on: https://boringssl-review.googlesource.com/21984
Reviewed-by: David Benjamin <davidben@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
2017-10-23 18:16:27 +00:00
Daniel Wagner-Hall
10154320fd Set -Wno-array-bounds on gcc<4.8
It spuriously complains about pointer math on function arguments which
are arrays.

Change-Id: I23b3494740196d5d46ce525a32dd43782e77f0ce
Reviewed-on: https://boringssl-review.googlesource.com/21404
Reviewed-by: Adam Langley <agl@google.com>
Reviewed-by: David Benjamin <davidben@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
2017-10-09 20:43:39 +00:00
David Benjamin
4519a5a063 Pass -fmsc-version=1900 to clang-cl.
This matches the Chromium build. There are some build errors when using
the newer toolchain's headers. This might resolve it? clang-cl
apparently claims VS2013 by default and Microsoft's headers are
sensitive to this.

Change-Id: Ib849e33d8a28649d981ea73972f568fd81e534a1
Reviewed-on: https://boringssl-review.googlesource.com/21244
Reviewed-by: Steven Valdez <svaldez@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
2017-10-06 15:41:27 +00:00
David Benjamin
02afbd338e Build with clang-cl standalone.
Our build logic needed to revised and and clang implements more warnings
than MSVC, so GTest needed more fixes.

Bug: 200
Change-Id: I84c5dd0c51079dd9c990e08dbea7f9022a7d6842
Reviewed-on: https://boringssl-review.googlesource.com/21204
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>
2017-10-05 20:42:49 +00:00
David Benjamin
12fdd08a40 Remove C4245 suppression.
Chromium builds with this warning on. This lets us notice problems (of
which there were only one) sooner. I'll try to align the other warnings
in a follow-up.

Change-Id: Id0960b782733b799e1c3e82f89c2aaba0bdd6833
Reviewed-on: https://boringssl-review.googlesource.com/21164
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>
2017-10-05 17:34:58 +00:00
Vincent Batts
60931e2d8a Explicit fallthrough on switch
Fixes failed compile with [-Werror=implicit-fallthrough=], which is
default on gcc-7.x on distributions like fedora.

Enabling no implicit fallthrough for more than just clang as well to
catch this going forward.

Change-Id: I6cd880dac70ec126bd7812e2d9e5ff804d32cadd
Signed-off-by: Vincent Batts <vbatts@redhat.com>
Reviewed-on: https://boringssl-review.googlesource.com/20564
Reviewed-by: David Benjamin <davidben@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
2017-09-20 19:58:25 +00:00
David Benjamin
e2568c41cb Tidy up some Windows compiler assumptions.
Someone tried to build us with Ubuntu's MinGW. This is too old to be
supported (the tests rather badly fail to build), but some of the fixes
will likely be useful for eventually building Clang for Windows
standalone too.

Change-Id: I6d279a0da1346b4e0813de51df3373b7412de33a
Reviewed-on: https://boringssl-review.googlesource.com/19364
Commit-Queue: Adam Langley <agl@google.com>
Reviewed-by: Adam Langley <agl@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
2017-08-16 19:57:06 +00:00
David Benjamin
9fb6feaa84 Turn on clang -fcolor-diagnostics.
I'm not sure why these aren't on by default, but Chromium does this too.
Colors are nice.

Change-Id: I7d7bf006014e9f40ec2f48290ad8fe7a70c1cfce
Reviewed-on: https://boringssl-review.googlesource.com/18704
Reviewed-by: Steven Valdez <svaldez@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
2017-07-31 19:03:11 +00:00
David Benjamin
2507d9e709 Add a CMake toggle to allow the C++ runtime.
In particular, this removes -fno-rtti, which allows the OSS-Fuzz folks
to run with -fsanitize=vptr. See
https://github.com/google/oss-fuzz/issues/741.

(-fsanitize=vptr isn't especially useful right now as we're just
starting with C++ support, but perhaps it'll be more useful in the
future.)

Change-Id: Ie8944a3e637ebc8dc28c03d331923a7528d7d328
Reviewed-on: https://boringssl-review.googlesource.com/18484
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>
2017-07-26 15:51:43 +00:00
David Benjamin
c937699735 Avoid a C++ runtime dependency.
Short-term, we will need to use these macros and build without RTTI when
defining any virtual base class. Long-term, it would be good to remove
these constraints, but it will require some downstream work.

Bug: 132
Change-Id: I3bc65bb12d7653978612b7d1bf06f772a2f3b1cd
Reviewed-on: https://boringssl-review.googlesource.com/18344
Commit-Queue: David Benjamin <davidben@google.com>
Commit-Queue: Adam Langley <agl@google.com>
Reviewed-by: Adam Langley <agl@google.com>
2017-07-24 19:58:14 +00:00
David Benjamin
6f2cd5d5c2 Build with -fno-exceptions standalone.
This serves two purposes:

1. Make sure we don't accidentally write "throw" or "catch" in our code.

2. If a C project statically links to our libssl.a, they'll use gcc
   rather than g++ as the linker which does not pull in the C++ runtime.
   -fno-exceptions cuts out the C++ runtime dependency. (Though we will
   need to give up on this should we attempt to allow a runtime
   dependency in the future.)

Bug: 132
Change-Id: I9fc81f034833ec9ed38b98d98df7b45c32cfa30c
Reviewed-on: https://boringssl-review.googlesource.com/18084
Reviewed-by: Steven Valdez <svaldez@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
2017-07-18 18:23:24 +00:00
David Benjamin
818031ec17 Build with -Wimplicit-fallthrough in Clang.
This will catch failures standalone before they reach consumers.

Change-Id: I1957df92e02ef7b4c609cd85420535c94ae3a04c
Reviewed-on: https://boringssl-review.googlesource.com/18005
Commit-Queue: Adam Langley <agl@google.com>
Reviewed-by: Adam Langley <agl@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
2017-07-17 19:26:32 +00:00
David Benjamin
6fb16cc93d Fix linux_fuzz bot.
Change-Id: Iefe4790094e2b2c6f9ef0833c00ab37707b7b19a
Reviewed-on: https://boringssl-review.googlesource.com/17805
Commit-Queue: David Benjamin <davidben@google.com>
Commit-Queue: Steven Valdez <svaldez@google.com>
Reviewed-by: Steven Valdez <svaldez@google.com>
2017-07-14 15:39:31 +00:00
David Benjamin
4a8d1f3519 Make missing prototypes warning work in clang/C++.
in GCC, -Wmissing-prototypes is only in C and the C++ spelling is
-Wmissing-declarations.
https://gcc.gnu.org/onlinedocs/gcc-7.1.0/gcc/Warning-Options.html#Warning-Options

In clang, -Wmissing-prototypes is in both C/C++ and
-Wmissing-declarations means you forgot to name a typedef or something?
https://clang.llvm.org/docs/DiagnosticsReference.html#wmissing-prototypes
https://clang.llvm.org/docs/DiagnosticsReference.html#wmissing-declarations

Pass clang slightly different flags to account for this mess.

Change-Id: I574ff7ecf32ba5908355639de83e8a102dab5f04
Reviewed-on: https://boringssl-review.googlesource.com/17784
Reviewed-by: Steven Valdez <svaldez@google.com>
2017-07-14 13:45:26 +00:00
David Benjamin
9b6ff440ef Use _STL_EXTRA_DISABLED_WARNINGS in VS2017.
Although Microsoft does not support building with /Wall, starting
VS2017, they provide a way to suppress warnings only in STL code. This
lets us keep some warnings active on our code while disabling them in
the STL.
https://blogs.msdn.microsoft.com/vcblog/2017/02/06/stl-fixes-in-vs-2017-rtm/

We currently still support VS2015, so we can't switch most of our
suppressions to this, but anything which applies only to VS2017 and up
will work.

Change-Id: I5f6d621dd1dbc060e09bded776d1714785a63147
Reviewed-on: https://boringssl-review.googlesource.com/17245
Commit-Queue: David Benjamin <davidben@google.com>
Commit-Queue: Adam Langley <agl@google.com>
Reviewed-by: Adam Langley <agl@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
2017-06-16 19:37:40 +00:00
David Benjamin
8ba6a1496b Fix build with VS 2017.
Lots more warnings to disable...

Change-Id: Ic240dd74d9abab8fe6d696c15267138b857d0dc1
Reviewed-on: https://boringssl-review.googlesource.com/16745
Reviewed-by: Adam Langley <agl@google.com>
Commit-Queue: Adam Langley <agl@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
2017-06-07 18:56:06 +00:00
Adam Langley
f64a6eeaf0 Switch to new delocate tool.
Most importantly, this version of delocate works for ppc64le. It should
also work for x86-64, but will need significant testing to make sure
that it covers all the cases that the previous delocate.go covered.

It's less stringtastic than the old code, however the parser isn't as
nice as I would have liked. I thought that the reason we put up with
AT&T syntax with Intel is so that assembly syntax could be somewhat
consistent across platforms. At least for ppc64le, that does not appear
to be the case.

Change-Id: Ic7e3c6acc3803d19f2c3ff5620c5e39703d74212
Reviewed-on: https://boringssl-review.googlesource.com/16464
Reviewed-by: Adam Langley <agl@google.com>
Commit-Queue: Adam Langley <agl@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
2017-05-30 18:00:16 +00:00
David Benjamin
3ecd0a5fca Convert aes_test to GTest.
This introduces machinery to start embedding the test data files into
the crypto_test binary. Figuring out every CI's test data story is more
trouble than is worth it. The GTest FileTest runner is considerably
different from the old one:

- It returns void and expects failures to use the GTest EXPECT_* and
  ASSERT_* macros, rather than ExpectBytesEqual. This is more monkey
  work to convert, but ultimately less work to add new tests. I think
  it's also valuable for our FileTest and normal test patterns to align
  as much as possible. The line number is emitted via SCOPED_TRACE.

- I've intentionally omitted the Error attribute handling, since that
  doesn't work very well with the new callback. This means evp_test.cc
  will take a little more work to convert, but this is again to keep our
  two test patterns aligned.

- The callback takes a std::function rather than a C-style void pointer.
  This means we can go nuts with lambdas. It also places the path first
  so clang-format doesn't go nuts.

BUG=129

Change-Id: I0d1920a342b00e64043e3ea05f5f5af57bfe77b3
Reviewed-on: https://boringssl-review.googlesource.com/16507
Reviewed-by: Adam Langley <agl@google.com>
2017-05-23 22:33:25 +00:00