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>
diff --git a/CMakeLists.txt b/CMakeLists.txt
index 43277f9..0a86f39 100644
--- a/CMakeLists.txt
+++ b/CMakeLists.txt
@@ -36,10 +36,26 @@
add_definitions(-DBORINGSSL_ALLOW_CXX_RUNTIME)
endif()
-if(CMAKE_COMPILER_IS_GNUCXX OR CMAKE_CXX_COMPILER_ID MATCHES "Clang")
- set(C_CXX_FLAGS "-Wall -Werror -Wformat=2 -Wsign-compare -Wmissing-field-initializers -Wwrite-strings -ggdb -fvisibility=hidden -fno-common")
+if(CMAKE_CXX_COMPILER_ID MATCHES "Clang")
+ set(CLANG 1)
+endif()
- if(CMAKE_CXX_COMPILER_ID MATCHES "Clang")
+if(CMAKE_COMPILER_IS_GNUCXX OR CLANG)
+ # Note clang-cl is odd and sets both CLANG and MSVC. We base our configuration
+ # primarily on our normal Clang one because the MSVC one is mostly
+ # suppressions for an overaggressive -Wall.
+ set(C_CXX_FLAGS "-Wall -Werror -Wformat=2 -Wsign-compare -Wmissing-field-initializers -Wwrite-strings")
+ if(MSVC)
+ # clang-cl sets different default warnings than clang.
+ set(C_CXX_FLAGS "${C_CXX_FLAGS} -Wno-unused-parameter")
+ # googletest suppresses warning C4996 via a pragma, but clang-cl does not
+ # honor it. Suppress it here to compensate. See https://crbug.com/772117.
+ set(C_CXX_FLAGS "${C_CXX_FLAGS} -Wno-deprecated-declarations")
+ else()
+ set(C_CXX_FLAGS "${C_CXX_FLAGS} -ggdb -fvisibility=hidden -fno-common")
+ endif()
+
+ if(CLANG)
set(C_CXX_FLAGS "${C_CXX_FLAGS} -Wnewline-eof -fcolor-diagnostics")
else()
# GCC (at least 4.8.4) has a bug where it'll find unreachable free() calls
@@ -47,16 +63,18 @@
set(C_CXX_FLAGS "${C_CXX_FLAGS} -Wno-free-nonheap-object")
endif()
- if(CMAKE_CXX_COMPILER_ID MATCHES "Clang" OR
- NOT "7.0.0" VERSION_GREATER CMAKE_C_COMPILER_VERSION)
+ if(CLANG OR NOT "7.0.0" VERSION_GREATER CMAKE_C_COMPILER_VERSION)
set(C_CXX_FLAGS "${C_CXX_FLAGS} -Wimplicit-fallthrough")
endif()
set(CMAKE_C_FLAGS "${CMAKE_C_FLAGS} ${C_CXX_FLAGS} -Wmissing-prototypes -Wold-style-definition -Wstrict-prototypes")
- set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -std=c++11 ${C_CXX_FLAGS} -Wmissing-declarations")
+ set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} ${C_CXX_FLAGS} -Wmissing-declarations")
- if(NOT BORINGSSL_ALLOW_CXX_RUNTIME)
- set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -fno-exceptions -fno-rtti")
+ if(NOT MSVC)
+ set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -std=c++11")
+ if(NOT BORINGSSL_ALLOW_CXX_RUNTIME)
+ set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -fno-exceptions -fno-rtti")
+ endif()
endif()
# In GCC, -Wmissing-declarations is the C++ spelling of -Wmissing-prototypes
@@ -66,7 +84,7 @@
# https://gcc.gnu.org/onlinedocs/gcc-7.1.0/gcc/Warning-Options.html#Warning-Options
# https://clang.llvm.org/docs/DiagnosticsReference.html#wmissing-prototypes
# https://clang.llvm.org/docs/DiagnosticsReference.html#wmissing-declarations
- if(CMAKE_CXX_COMPILER_ID MATCHES "Clang")
+ if(CLANG)
set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -Wmissing-prototypes")
endif()
elseif(MSVC)
@@ -129,7 +147,6 @@
${MSVC_LEVEL4_WARNINGS_LIST})
set(CMAKE_C_FLAGS "-Wall -WX ${MSVC_DISABLED_WARNINGS_STR} ${MSVC_LEVEL4_WARNINGS_STR}")
set(CMAKE_CXX_FLAGS "-Wall -WX ${MSVC_DISABLED_WARNINGS_STR} ${MSVC_LEVEL4_WARNINGS_STR}")
- set(CMAKE_ASM_NASM_FLAGS "-g cv8")
endif()
if(WIN32)
@@ -143,14 +160,13 @@
endif()
if((CMAKE_COMPILER_IS_GNUCXX AND CMAKE_C_COMPILER_VERSION VERSION_GREATER "4.7.99") OR
- CMAKE_CXX_COMPILER_ID MATCHES "Clang")
+ CLANG)
set(CMAKE_C_FLAGS "${CMAKE_C_FLAGS} -Wshadow")
set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -Wshadow")
endif()
if(CMAKE_COMPILER_IS_GNUCXX)
- if ((CMAKE_C_COMPILER_VERSION VERSION_GREATER "4.8.99") OR
- CMAKE_CXX_COMPILER_ID MATCHES "Clang")
+ if ((CMAKE_C_COMPILER_VERSION VERSION_GREATER "4.8.99") OR CLANG)
set(CMAKE_C_FLAGS "${CMAKE_C_FLAGS} -std=c11")
else()
set(CMAKE_C_FLAGS "${CMAKE_C_FLAGS} -std=c99")
@@ -163,7 +179,7 @@
endif()
if(FUZZ)
- if(NOT CMAKE_CXX_COMPILER_ID MATCHES "Clang")
+ if(NOT CLANG)
message(FATAL_ERROR "You need to build with Clang for fuzzing to work")
endif()
@@ -191,7 +207,7 @@
endif()
if (MSAN)
- if(NOT CMAKE_CXX_COMPILER_ID MATCHES "Clang")
+ if(NOT CLANG)
message(FATAL_ERROR "Cannot enable MSAN unless using Clang")
endif()
@@ -205,7 +221,7 @@
endif()
if (ASAN)
- if(NOT CMAKE_CXX_COMPILER_ID MATCHES "Clang")
+ if(NOT CLANG)
message(FATAL_ERROR "Cannot enable ASAN unless using Clang")
endif()
diff --git a/crypto/CMakeLists.txt b/crypto/CMakeLists.txt
index 1cd8458..3266ea2 100644
--- a/crypto/CMakeLists.txt
+++ b/crypto/CMakeLists.txt
@@ -53,6 +53,7 @@
set(PERLASM_STYLE win32n)
set(PERLASM_FLAGS "-DOPENSSL_IA32_SSE2")
endif()
+ set(CMAKE_ASM_NASM_FLAGS "-g cv8")
# On Windows, we use the NASM output, specifically built with Yasm.
set(ASM_EXT asm)
diff --git a/include/openssl/base.h b/include/openssl/base.h
index aa65b4a..adb5047 100644
--- a/include/openssl/base.h
+++ b/include/openssl/base.h
@@ -180,7 +180,7 @@
#endif // defined(BORINGSSL_SHARED_LIBRARY)
-#if defined(__GNUC__)
+#if defined(__GNUC__) || defined(__clang__)
// MinGW has two different printf implementations. Ensure the format macro
// matches the selected implementation. See
// https://sourceforge.net/p/mingw-w64/wiki2/gnu%20printf/.
diff --git a/third_party/googletest/METADATA b/third_party/googletest/METADATA
index 88139d1..51ee7a2 100644
--- a/third_party/googletest/METADATA
+++ b/third_party/googletest/METADATA
@@ -16,4 +16,5 @@
"https://github.com/google/googletest/pull/991 was applied."
"https://github.com/google/googletest/pull/1007 was applied."
"https://github.com/google/googletest/pull/1109 was applied."
+ "clang-cl fixes in https://boringssl-review.googlesource.com/c/boringssl/+/21204"
}
diff --git a/third_party/googletest/include/gtest/internal/gtest-port.h b/third_party/googletest/include/gtest/internal/gtest-port.h
index 8f12823..3a491ab 100644
--- a/third_party/googletest/include/gtest/internal/gtest-port.h
+++ b/third_party/googletest/include/gtest/internal/gtest-port.h
@@ -876,7 +876,7 @@
#endif
// Use this annotation before a function that takes a printf format string.
-#if defined(__GNUC__) && !defined(COMPILER_ICC)
+#if (defined(__GNUC__) || defined(__clang__)) && !defined(COMPILER_ICC)
# if defined(__MINGW_PRINTF_FORMAT)
// MinGW has two different printf implementations. Ensure the format macro
// matches the selected implementation. See
diff --git a/third_party/googletest/src/gtest-death-test.cc b/third_party/googletest/src/gtest-death-test.cc
index e92dbaa..1a270f2 100644
--- a/third_party/googletest/src/gtest-death-test.cc
+++ b/third_party/googletest/src/gtest-death-test.cc
@@ -1227,12 +1227,12 @@
// Recreates the pipe and event handles from the provided parameters,
// signals the event, and returns a file descriptor wrapped around the pipe
// handle. This function is called in the child process only.
-int GetStatusFileDescriptor(unsigned int parent_process_id,
- size_t write_handle_as_size_t,
- size_t event_handle_as_size_t) {
+static int GetStatusFileDescriptor(unsigned int parent_process_id,
+ size_t write_handle_as_size_t,
+ size_t event_handle_as_size_t) {
AutoHandle parent_process_handle(::OpenProcess(PROCESS_DUP_HANDLE,
- FALSE, // Non-inheritable.
- parent_process_id));
+ FALSE, // Non-inheritable.
+ parent_process_id));
if (parent_process_handle.Get() == INVALID_HANDLE_VALUE) {
DeathTestAbort("Unable to open parent process " +
StreamableToString(parent_process_id));
diff --git a/third_party/googletest/src/gtest-port.cc b/third_party/googletest/src/gtest-port.cc
index af5049e..aded80c 100644
--- a/third_party/googletest/src/gtest-port.cc
+++ b/third_party/googletest/src/gtest-port.cc
@@ -671,7 +671,7 @@
}
// Helper function used by ValidateRegex() to format error messages.
-std::string FormatRegexSyntaxError(const char* regex, int index) {
+static std::string FormatRegexSyntaxError(const char* regex, int index) {
return (Message() << "Syntax error at index " << index
<< " in simple regular expression \"" << regex << "\": ").GetString();
}
diff --git a/third_party/googletest/src/gtest.cc b/third_party/googletest/src/gtest.cc
index e996a86..0fbfea9 100644
--- a/third_party/googletest/src/gtest.cc
+++ b/third_party/googletest/src/gtest.cc
@@ -2889,7 +2889,7 @@
!GTEST_OS_WINDOWS_PHONE && !GTEST_OS_WINDOWS_RT
// Returns the character attribute for the given color.
-WORD GetColorAttribute(GTestColor color) {
+static WORD GetColorAttribute(GTestColor color) {
switch (color) {
case COLOR_RED: return FOREGROUND_RED;
case COLOR_GREEN: return FOREGROUND_GREEN;
diff --git a/tool/client.cc b/tool/client.cc
index d439860..2ec381f 100644
--- a/tool/client.cc
+++ b/tool/client.cc
@@ -196,7 +196,15 @@
}
while (!resume_session) {
+#if defined(OPENSSL_WINDOWS)
+ // Windows sockets are really of type SOCKET, not int, but everything here
+ // casts them to ints. Clang gets unhappy about signed values as a result.
+ //
+ // TODO(davidben): Keep everything as the appropriate platform type.
+ FD_SET(static_cast<SOCKET>(sock), &read_fds);
+#else
FD_SET(sock, &read_fds);
+#endif
int ret = select(sock + 1, &read_fds, NULL, NULL, NULL);
if (ret <= 0) {
perror("select");
diff --git a/tool/transport_common.cc b/tool/transport_common.cc
index 73f9e2d..a7c2b36 100644
--- a/tool/transport_common.cc
+++ b/tool/transport_common.cc
@@ -170,7 +170,14 @@
OPENSSL_memset(&addr, 0, sizeof(addr));
addr.sin6_family = AF_INET6;
+ // Windows' IN6ADDR_ANY_INIT does not have enough curly braces for clang-cl
+ // (https://crbug.com/772108), while other platforms like NaCl are missing
+ // in6addr_any, so use a mix of both.
+#if defined(OPENSSL_WINDOWS)
+ addr.sin6_addr = in6addr_any;
+#else
addr.sin6_addr = IN6ADDR_ANY_INIT;
+#endif
addr.sin6_port = htons(atoi(port.c_str()));
#if defined(OPENSSL_WINDOWS)