Bug 1128413, Part 3: Enable more compiler warnings, r=mmc

2024-09-13 09:24:08 -07:00 · 2015-02-07 14:38:40 -08:00 · 2015-02-07 14:38:40 -08:00 · 5c3f56a357
commit 5c3f56a357
parent 1aa8a904ff
23 changed files with 262 additions and 128 deletions
--- a/security/pkix/include/pkix/Input.h
+++ b/security/pkix/include/pkix/Input.h
@ -80,6 +80,9 @@ public:
  {
  }

+  // This is intentionally not explicit in order to allow value semantics.
+  Input(const Input&) = default;
+
  // Initialize the input. data must be non-null and len must be less than
  // 65536. Init may not be called more than once.
  Result Init(const uint8_t* data, size_t len)
@ -271,7 +274,7 @@ public:

  Result SkipToEnd(/*out*/ Input& skipped)
  {
-    return Skip(static_cast<size_t>(end - input), skipped);
+    return Skip(static_cast<Input::size_type>(end - input), skipped);
  }

  Result EnsureLength(Input::size_type len)
@ -286,6 +289,8 @@ public:

  class Mark final
  {
+  public:
+    Mark(const Mark&) = default; // Intentionally not explicit.
  private:
    friend class Reader;
    Mark(const Reader& input, const uint8_t* mark) : input(input), mark(mark) { }
--- a/security/pkix/lib/pkixder.h
+++ b/security/pkix/lib/pkixder.h
@ -55,7 +55,7 @@ enum Constructed
  CONSTRUCTED = 1 << 5
 };

-enum Tag
+enum Tag : uint8_t
 {
  BOOLEAN = UNIVERSAL | 0x01,
  INTEGER = UNIVERSAL | 0x02,
--- a/security/pkix/lib/pkixnames.cpp
+++ b/security/pkix/lib/pkixnames.cpp
@ -1642,13 +1642,14 @@ FinishIPv6Address(/*in/out*/ uint8_t (&address)[16], int numComponents,
  }

  // Shift components that occur after the contraction over.
-  int componentsToMove = numComponents - contractionIndex;
-  memmove(address + (2u * (8 - componentsToMove)),
-          address + (2u * contractionIndex),
+  size_t componentsToMove = static_cast<size_t>(numComponents -
+                                                contractionIndex);
+  memmove(address + (2u * static_cast<size_t>(8 - componentsToMove)),
+          address + (2u * static_cast<size_t>(contractionIndex)),
          componentsToMove * 2u);
  // Fill in the contracted area with zeros.
-  memset(address + (2u * contractionIndex), 0u,
-         (8u - numComponents) * 2u);
+  memset(address + (2u * static_cast<size_t>(contractionIndex)), 0u,
+         (8u - static_cast<size_t>(numComponents)) * 2u);

  return true;
 }
@ -1785,7 +1786,6 @@ ParseIPv6Address(Input hostname, /*out*/ uint8_t (&out)[16])
      if (contractionIndex != -1) {
        return false; // multiple contractions are not allowed.
      }
-      uint8_t b;
      if (input.Read(b) != Success || b != ':') {
        assert(false);
        return false;
--- a/security/pkix/lib/pkixtime.cpp
+++ b/security/pkix/lib/pkixtime.cpp
@ -24,8 +24,15 @@

 #include "pkix/Time.h"
 #include "pkixutil.h"
+
 #ifdef WIN32
+#ifdef _MSC_VER
+#pragma warning(push, 3)
+#endif
 #include "windows.h"
+#ifdef _MSC_VER
+#pragma warning(pop)
+#endif
 #else
 #include "sys/time.h"
 #endif
@ -53,7 +60,8 @@ Now()
  //   - http://pubs.opengroup.org/onlinepubs/009695399/functions/gettimeofday.html
  timeval tv;
  (void) gettimeofday(&tv, nullptr);
-  seconds = (DaysBeforeYear(1970) * Time::ONE_DAY_IN_SECONDS) + tv.tv_sec;
+  seconds = (DaysBeforeYear(1970) * Time::ONE_DAY_IN_SECONDS) +
+            static_cast<uint64_t>(tv.tv_sec);
 #endif

  return TimeFromElapsedSecondsAD(seconds);
--- a/security/pkix/lib/pkixutil.h
+++ b/security/pkix/lib/pkixutil.h
@ -53,7 +53,7 @@ public:
  Result Init();

  const Input GetDER() const { return der; }
-  const der::Version GetVersion() const { return version; }
+  der::Version GetVersion() const { return version; }
  const SignedDataWithSignature& GetSignedData() const { return signedData; }
  const Input GetIssuer() const { return issuer; }
  // XXX: "validity" is a horrible name for the structure that holds
@ -232,24 +232,25 @@ WrappedVerifySignedData(TrustDomain& trustDomain,
 //         MOZILLA_PKIX_UNREACHABLE_DEFAULT_ENUM
 //       }
 //     }
-#if defined(__clang__) && (__clang_major__ == 3 && __clang_minor__ < 5)
-  // Earlier versions of Clang will warn if not all cases are covered
-  // (-Wswitch-enum) AND they always, inappropriately, assume the default case
-  // is unreachable. This was fixed in
-  // http://llvm.org/klaus/clang/commit/28cd22d7c2d2458575ce9cc19dfe63c6321010ce/
-# define MOZILLA_PKIX_UNREACHABLE_DEFAULT_ENUM // empty
-#elif defined(__GNUC__) || defined(__clang__)
-  // GCC and recent versions of clang will warn if not all cases are covered
-  // (-Wswitch-enum). They do not assume that the default case is unreachable.
-# define MOZILLA_PKIX_UNREACHABLE_DEFAULT_ENUM \
-         default: assert(false); __builtin_unreachable();
+#if defined(__clang__)
+// Clang will warn if not all cases are covered (-Wswitch-enum) AND it will
+// warn if a switch statement that covers every enum label has a default case
+// (-W-covered-switch-default). Versions prior to 3.5 warned about unreachable
+// code in such default cases (-Wunreachable-code) even when
+// -W-covered-switch-default was disabled, but that changed in Clang 3.5.
+#define MOZILLA_PKIX_UNREACHABLE_DEFAULT_ENUM // empty
+#elif defined(__GNUC__)
+// GCC will warn if not all cases are covered (-Wswitch-enum). It does not
+// assume that the default case is unreachable.
+#define MOZILLA_PKIX_UNREACHABLE_DEFAULT_ENUM \
+        default: assert(false); __builtin_unreachable();
 #elif defined(_MSC_VER)
-  // MSVC will warn if not all cases are covered (C4061, level 4). It does not
-  // assume that the default case is unreachable.
-# define MOZILLA_PKIX_UNREACHABLE_DEFAULT_ENUM \
-         default: assert(false); __assume(0);
+// MSVC will warn if not all cases are covered (C4061, level 4). It does not
+// assume that the default case is unreachable.
+#define MOZILLA_PKIX_UNREACHABLE_DEFAULT_ENUM \
+        default: assert(false); __assume(0);
 #else
-# error Unsupported compiler for MOZILLA_PKIX_UNREACHABLE_DEFAULT.
+#error Unsupported compiler for MOZILLA_PKIX_UNREACHABLE_DEFAULT.
 #endif

 } } // namespace mozilla::pkix
--- a/security/pkix/moz.build
+++ b/security/pkix/moz.build
@ -25,18 +25,7 @@ TEST_DIRS += [
    'test/lib',
 ]

-CXXFLAGS += ['-Wall']
-# -Wall with Visual C++ enables too many problematic warnings
-if CONFIG['_MSC_VER']:
-  CXXFLAGS += [
-    '-wd4514', # 'function': unreferenced inline function has been removed
-    '-wd4668', # 'symbol' is not defined as a preprocessor macro...
-    '-wd4710', # 'function': function not inlined
-    '-wd4711', # function 'function' selected for inline expansion
-    '-wd4820', # 'bytes' bytes padding added after construct 'member_name'
-  ]
-
-FAIL_ON_WARNINGS = True
+include('warnings.mozbuild')

 Library('mozillapkix')

--- a/security/pkix/test/gtest/moz.build
+++ b/security/pkix/test/gtest/moz.build
@ -30,11 +30,24 @@ LOCAL_INCLUDES += [

 FINAL_LIBRARY = 'xul-gtest'

-include('/ipc/chromium/chromium-config.mozbuild')
+include('../../warnings.mozbuild')

-if CONFIG['_MSC_VER']:
+# These warnings are disabled in order to minimize the amount of boilerplate
+# required to implement tests, and/or because they originate in the GTest
+# framework in a way we cannot otherwise work around.
+if CONFIG['CLANG_CXX']:
  CXXFLAGS += [
-    '-wd4275', # non dll-interface class used as base for dll-interface class
+    '-Wno-exit-time-destructors',
+    '-Wno-global-constructors',
+    '-Wno-old-style-cast',
+    '-Wno-used-but-marked-unused',
+  ]
+elif CONFIG['_MSC_VER']:
+  CXXFLAGS += [
+    '-wd4350', # behavior change: 'std::_Wrap_alloc<std::allocator<_Ty>>::...
+    '-wd4275', # non dll-interface class used as base for dll-interface class
+    '-wd4548', # Expression before comma has no effect
+    '-wd4625', # copy constructor could not be generated.
+    '-wd4626', # assugment operator could not be generated.
+    '-wd4640', # construction of local static object is not thread safe.
  ]
-
-FAIL_ON_WARNINGS = True
--- a/security/pkix/test/gtest/pkixbuild_tests.cpp
+++ b/security/pkix/test/gtest/pkixbuild_tests.cpp
@ -22,8 +22,20 @@
 * limitations under the License.
 */

+#if defined(_MSC_VER) && _MSC_VER < 1900
+// When building with -D_HAS_EXCEPTIONS=0, MSVC's <xtree> header triggers
+// warning C4702: unreachable code.
+// https://connect.microsoft.com/VisualStudio/feedback/details/809962
+#pragma warning(push)
+#pragma warning(disable: 4702)
+#endif
+
 #include <map>
-#include "cert.h"
+
+#if defined(_MSC_VER) && _MSC_VER < 1900
+#pragma warning(pop)
+#endif
+
 #include "pkix/pkix.h"
 #include "pkixgtest.h"
 #include "pkixtestutil.h"
@ -110,7 +122,7 @@ private:
    return Success;
  }

-  Result FindIssuer(Input encodedIssuerName, IssuerChecker& checker, Time time)
+  Result FindIssuer(Input encodedIssuerName, IssuerChecker& checker, Time)
                    override
  {
    ByteString subjectDER(InputToByteString(encodedIssuerName));
@ -147,8 +159,7 @@ private:
    return TestVerifySignedData(signedData, subjectPublicKeyInfo);
  }

-  Result DigestBuf(Input item, /*out*/ uint8_t *digestBuf, size_t digestBufLen)
-                   override
+  Result DigestBuf(Input, /*out*/ uint8_t*, size_t) override
  {
    ADD_FAILURE();
    return Result::FATAL_ERROR_LIBRARY_FAILURE;
@ -261,9 +272,8 @@ public:
  }

  // The CertPolicyId argument is unused because we don't care about EV.
-  Result GetCertTrust(EndEntityOrCA endEntityOrCA, const CertPolicyId&,
-                      Input candidateCert, /*out*/ TrustLevel& trustLevel)
-                      override
+  Result GetCertTrust(EndEntityOrCA, const CertPolicyId&, Input candidateCert,
+                      /*out*/ TrustLevel& trustLevel) override
  {
    Input rootCert;
    Result rv = rootCert.Init(rootDER.data(), rootDER.length());
@ -278,8 +288,7 @@ public:
    return Success;
  }

-  Result FindIssuer(Input encodedIssuerName, IssuerChecker& checker, Time time)
-                    override
+  Result FindIssuer(Input, IssuerChecker& checker, Time) override
  {
    // keepGoing is an out parameter from IssuerChecker.Check. It would tell us
    // whether or not to continue attempting other potential issuers. We only
@ -343,7 +352,7 @@ TEST_F(pkixbuild, NoRevocationCheckingForExpiredCert)
  ByteString certDER(CreateEncodedCertificate(
                       v3, sha256WithRSAEncryption,
                       serialNumber, issuerDER,
-                       oneDayBeforeNow - Time::ONE_DAY_IN_SECONDS,
+                       oneDayBeforeNow - ONE_DAY_IN_SECONDS_AS_TIME_T,
                       oneDayBeforeNow,
                       subjectDER, *reusedKey, nullptr, *reusedKey,
                       sha256WithRSAEncryption));
@ -389,8 +398,7 @@ public:
    return Success;
  }

-  Result VerifySignedData(const SignedDataWithSignature& signedData,
-                          Input subjectPublicKeyInfo) override
+  Result VerifySignedData(const SignedDataWithSignature&, Input) override
  {
    ADD_FAILURE();
    return Result::FATAL_ERROR_LIBRARY_FAILURE;
@ -463,7 +471,7 @@ public:
    return Success;
  }

-  Result FindIssuer(Input subjectCert, IssuerChecker& checker, Time) override
+  Result FindIssuer(Input, IssuerChecker& checker, Time) override
  {
    Input issuerInput;
    EXPECT_EQ(Success, issuerInput.Init(issuer.data(), issuer.length()));
--- a/security/pkix/test/gtest/pkixcert_extension_tests.cpp
+++ b/security/pkix/test/gtest/pkixcert_extension_tests.cpp
@ -63,7 +63,7 @@ CreateCertWithOneExtension(const char* subjectStr, const ByteString& extension)
 class TrustEverythingTrustDomain final : public TrustDomain
 {
 private:
-  Result GetCertTrust(EndEntityOrCA, const CertPolicyId&, Input candidateCert,
+  Result GetCertTrust(EndEntityOrCA, const CertPolicyId&, Input,
                      /*out*/ TrustLevel& trustLevel) override
  {
    trustLevel = TrustLevel::TrustAnchor;
--- a/security/pkix/test/gtest/pkixcheck_CheckKeyUsage_tests.cpp
+++ b/security/pkix/test/gtest/pkixcheck_CheckKeyUsage_tests.cpp
@ -22,7 +22,7 @@
 * limitations under the License.
 */

-#include "gtest/gtest.h"
+#include "pkixgtest.h"
 #include "pkix/pkixtypes.h"
 #include "pkixtestutil.h"

--- a/security/pkix/test/gtest/pkixcheck_CheckValidity_tests.cpp
+++ b/security/pkix/test/gtest/pkixcheck_CheckValidity_tests.cpp
@ -22,7 +22,7 @@
 * limitations under the License.
 */

-#include "gtest/gtest.h"
+#include "pkixgtest.h"
 #include "pkix/pkixtypes.h"
 #include "pkixtestutil.h"

--- a/security/pkix/test/gtest/pkixder_input_tests.cpp
+++ b/security/pkix/test/gtest/pkixder_input_tests.cpp
@ -24,7 +24,7 @@

 #include <functional>
 #include <vector>
-#include <gtest/gtest.h>
+#include "pkixgtest.h"

 #include "pkixder.h"

@ -467,7 +467,7 @@ TEST_F(pkixder_input_tests, MarkAndGetInput)
 }

 // Cannot run this test on debug builds because of the NotReached
-#ifndef DEBUG
+#ifdef NDEBUG
 TEST_F(pkixder_input_tests, MarkAndGetInputDifferentInput)
 {
  const uint8_t der[] = { 0x11, 0x22, 0x33, 0x44 };
@ -847,7 +847,7 @@ TEST_F(pkixder_input_tests, NestedOf)
                     [&readValues](Reader& r) {
                       return NestedOfHelper(r, readValues);
                     }));
-  ASSERT_EQ((size_t) 3, readValues.size());
+  ASSERT_EQ(3u, readValues.size());
  ASSERT_EQ(0x01, readValues[0]);
  ASSERT_EQ(0x02, readValues[1]);
  ASSERT_EQ(0x03, readValues[2]);
@ -865,7 +865,7 @@ TEST_F(pkixder_input_tests, NestedOfWithTruncatedData)
                     [&readValues](Reader& r) {
                       return NestedOfHelper(r, readValues);
                     }));
-  ASSERT_EQ((size_t) 0, readValues.size());
+  ASSERT_EQ(0u, readValues.size());
 }

 TEST_F(pkixder_input_tests, MatchRestAtEnd)
--- a/security/pkix/test/gtest/pkixder_pki_types_tests.cpp
+++ b/security/pkix/test/gtest/pkixder_pki_types_tests.cpp
@ -25,7 +25,7 @@
 #include <functional>
 #include <vector>

-#include "gtest/gtest.h"
+#include "pkixgtest.h"
 #include "pkix/pkixtypes.h"
 #include "pkixder.h"

--- a/security/pkix/test/gtest/pkixder_universal_types_tests.cpp
+++ b/security/pkix/test/gtest/pkixder_universal_types_tests.cpp
@ -24,7 +24,7 @@

 #include <limits>
 #include <vector>
-#include <gtest/gtest.h>
+#include "pkixgtest.h"

 #include "pkixder.h"
 #include "pkixtestutil.h"
@ -287,8 +287,9 @@ TEST_F(pkixder_universal_types_tests, EnumeratedInvalidZeroLength)
 // other encodings is actually encouraged.

 // e.g. TWO_CHARS(53) => '5', '3'
-#define TWO_CHARS(t) static_cast<uint8_t>('0' + ((t) / 10u)), \
-                     static_cast<uint8_t>('0' + ((t) % 10u))
+#define TWO_CHARS(t) \
+  static_cast<uint8_t>('0' + (static_cast<uint8_t>(t) / 10u)), \
+  static_cast<uint8_t>('0' + (static_cast<uint8_t>(t) % 10u))

 // Calls TimeChoice on the UTCTime variant of the given generalized time.
 template <uint16_t LENGTH>
@ -555,7 +556,7 @@ static const uint8_t DAYS_IN_MONTH[] = {

 TEST_F(pkixder_universal_types_tests, TimeMonthDaysValidRange)
 {
-  for (uint8_t month = 1; month <= 12; ++month) {
+  for (uint16_t month = 1; month <= 12; ++month) {
    for (uint8_t day = 1; day <= DAYS_IN_MONTH[month]; ++day) {
      const uint8_t DER[] = {
        0x18,                           // Generalized Time
@ -604,7 +605,7 @@ TEST_F(pkixder_universal_types_tests, TimeDayInvalid0)

 TEST_F(pkixder_universal_types_tests, TimeMonthDayInvalidPastEndOfMonth)
 {
-  for (uint8_t month = 1; month <= 12; ++month) {
+  for (int16_t month = 1; month <= 12; ++month) {
    const uint8_t DER[] = {
      0x18,                           // Generalized Time
      15,                             // Length = 15
@ -905,7 +906,7 @@ TEST_F(pkixder_universal_types_tests, Integer_0_127)
    Input input(DER);
    Reader reader(input);

-    uint8_t value = i + 1; // initialize with a value that is NOT i.
+    uint8_t value = i + 1u; // initialize with a value that is NOT i.
    ASSERT_EQ(Success, Integer(reader, value));
    ASSERT_EQ(i, value);
  }
--- a/security/pkix/test/gtest/pkixgtest.cpp
+++ b/security/pkix/test/gtest/pkixgtest.cpp
@ -30,10 +30,15 @@

 namespace mozilla { namespace pkix { namespace test {

+const std::time_t ONE_DAY_IN_SECONDS_AS_TIME_T =
+  static_cast<std::time_t>(Time::ONE_DAY_IN_SECONDS);
+
 // This assumes that time/time_t are POSIX-compliant in that time() returns
 // the number of seconds since the Unix epoch.
 const std::time_t now(time(nullptr));
-const std::time_t oneDayBeforeNow(time(nullptr) - Time::ONE_DAY_IN_SECONDS);
-const std::time_t oneDayAfterNow(time(nullptr) + Time::ONE_DAY_IN_SECONDS);
+const std::time_t oneDayBeforeNow(time(nullptr) -
+                                  ONE_DAY_IN_SECONDS_AS_TIME_T);
+const std::time_t oneDayAfterNow(time(nullptr) +
+                                 ONE_DAY_IN_SECONDS_AS_TIME_T);

 } } } // namespace mozilla::pkix::test
--- a/security/pkix/test/gtest/pkixgtest.h
+++ b/security/pkix/test/gtest/pkixgtest.h
@ -26,7 +26,36 @@

 #include <ostream>

+#if defined(__clang__)
+#pragma clang diagnostic push
+#pragma clang diagnostic ignored "-Wdeprecated"
+#pragma clang diagnostic ignored "-Wmissing-noreturn"
+#pragma clang diagnostic ignored "-Wshift-sign-overflow"
+#pragma clang diagnostic ignored "-Wsign-conversion"
+#pragma clang diagnostic ignored "-Wundef"
+#elif defined(__GNUC__)
+#pragma GCC diagnostic push
+#pragma GCC diagnostic ignored "-Wextra"
+#elif defined(_MSC_VER)
+#pragma warning(push, 3)
+// C4224: Nonstandard extension used: formal parameter 'X' was previously
+//        defined as a type.
+#pragma warning(disable: 4224)
+// C4826: Conversion from 'type1 ' to 'type_2' is sign - extended. This may
+//        cause unexpected runtime behavior.
+#pragma warning(disable: 4826)
+#endif
+
 #include "gtest/gtest.h"
+
+#if defined(__clang__)
+#pragma clang diagnostic pop
+#elif defined(__GNUC__)
+#pragma GCC diagnostic pop
+#elif defined(_MSC_VER)
+#pragma warning(pop)
+#endif
+
 #include "pkix/Result.h"

 // PrintTo must be in the same namespace as the type we're overloading it for.
@ -47,6 +76,8 @@ PrintTo(const Result& result, ::std::ostream* os)

 namespace mozilla { namespace pkix { namespace test {

+extern const std::time_t ONE_DAY_IN_SECONDS_AS_TIME_T;
+
 extern const std::time_t now;
 extern const std::time_t oneDayBeforeNow;
 extern const std::time_t oneDayAfterNow;
--- a/security/pkix/test/gtest/pkixnames_tests.cpp
+++ b/security/pkix/test/gtest/pkixnames_tests.cpp
@ -72,7 +72,8 @@ struct PresentedMatchesReference
  { \
    ByteString(reinterpret_cast<const uint8_t*>(a), sizeof(a) - 1), \
    ByteString(reinterpret_cast<const uint8_t*>(b), sizeof(b) - 1), \
-    Result::ERROR_BAD_DER \
+    Result::ERROR_BAD_DER, \
+    false \
  }

 static const PresentedMatchesReference DNSID_MATCH_PARAMS[] =
--- a/security/pkix/test/gtest/pkixocsp_CreateEncodedOCSPRequest_tests.cpp
+++ b/security/pkix/test/gtest/pkixocsp_CreateEncodedOCSPRequest_tests.cpp
@ -22,7 +22,7 @@
 * limitations under the License.
 */

-#include "gtest/gtest.h"
+#include "pkixgtest.h"
 #include "pkix/pkix.h"
 #include "pkixder.h"
 #include "pkixtestutil.h"
@ -71,7 +71,7 @@ private:
    return TestDigestBuf(item, digestBuf, digestBufLen);
  }

-  Result CheckPublicKey(Input subjectPublicKeyInfo) override
+  Result CheckPublicKey(Input) override
  {
    ADD_FAILURE();
    return Result::FATAL_ERROR_LIBRARY_FAILURE;
--- a/security/pkix/test/gtest/pkixocsp_VerifyEncodedOCSPResponse.cpp
+++ b/security/pkix/test/gtest/pkixocsp_VerifyEncodedOCSPResponse.cpp
@ -53,7 +53,7 @@ public:
    return Result::FATAL_ERROR_LIBRARY_FAILURE;
  }

-  Result CheckRevocation(EndEntityOrCA endEntityOrCA, const CertID&, Time time,
+  Result CheckRevocation(EndEntityOrCA, const CertID&, Time,
                         /*optional*/ const Input*, /*optional*/ const Input*)
                         final override
  {
@ -119,7 +119,8 @@ public:
      abort();
    }

-    serialNumberDER = CreateEncodedSerialNumber(++rootIssuedCount);
+    serialNumberDER =
+      CreateEncodedSerialNumber(static_cast<long>(++rootIssuedCount));
    if (ENCODING_FAILED(serialNumberDER)) {
      abort();
    }
@ -143,7 +144,7 @@ public:
  }

  static ScopedTestKeyPair rootKeyPair;
-  static long rootIssuedCount;
+  static uint32_t rootIssuedCount;
  OCSPTestTrustDomain trustDomain;

  // endEntityCertID references rootKeyPair, rootNameDER, and serialNumberDER.
@ -154,7 +155,7 @@ public:
 };

 /*static*/ ScopedTestKeyPair pkixocsp_VerifyEncodedResponse::rootKeyPair;
-/*static*/ long pkixocsp_VerifyEncodedResponse::rootIssuedCount = 0;
+/*static*/ uint32_t pkixocsp_VerifyEncodedResponse::rootIssuedCount = 0;

 ///////////////////////////////////////////////////////////////////////////////
 // responseStatus
@ -258,7 +259,7 @@ public:
    context.signatureAlgorithm = signatureAlgorithm;
    context.certs = certs;

-    context.certStatus = certStatus;
+    context.certStatus = static_cast<uint8_t>(certStatus);
    context.thisUpdate = thisUpdate;
    context.nextUpdate = nextUpdate ? *nextUpdate : 0;
    context.includeNextUpdate = nextUpdate != nullptr;
@ -408,7 +409,8 @@ TEST_F(pkixocsp_VerifyEncodedResponse_successful, check_validThrough)
    ASSERT_FALSE(expired);
    // The response was created to be valid until one day after now, so the
    // value we got for validThrough should be after that.
-    Time oneDayAfterNowAsPKIXTime(TimeFromEpochInSeconds(oneDayAfterNow));
+    Time oneDayAfterNowAsPKIXTime(
+          TimeFromEpochInSeconds(static_cast<uint64_t>(oneDayAfterNow)));
    ASSERT_TRUE(validThrough > oneDayAfterNowAsPKIXTime);
  }
  {
@ -518,7 +520,8 @@ protected:
                                /*optional*/ const ByteString* extensions,
                                             const TestKeyPair& signerKeyPair)
  {
-    ByteString serialNumberDER(CreateEncodedSerialNumber(serialNumber));
+    ByteString serialNumberDER(CreateEncodedSerialNumber(
+                                 static_cast<long>(serialNumber)));
    if (ENCODING_FAILED(serialNumberDER)) {
      return ByteString();
    }
@ -634,8 +637,8 @@ TEST_F(pkixocsp_VerifyEncodedResponse_DelegatedResponder, good_expired)
  ScopedTestKeyPair signerKeyPair(GenerateKeyPair());
  ByteString signerDER(CreateEncodedCertificate(
                          ++rootIssuedCount, sha256WithRSAEncryption, rootName,
-                          now - (10 * Time::ONE_DAY_IN_SECONDS),
-                          now - (2 * Time::ONE_DAY_IN_SECONDS),
+                          now - (10 * ONE_DAY_IN_SECONDS_AS_TIME_T),
+                          now - (2 * ONE_DAY_IN_SECONDS_AS_TIME_T),
                          signerName, *signerKeyPair, extensions,
                          *rootKeyPair));
  ASSERT_FALSE(ENCODING_FAILED(signerDER));
@ -670,8 +673,8 @@ TEST_F(pkixocsp_VerifyEncodedResponse_DelegatedResponder, good_future)
  ByteString signerDER(CreateEncodedCertificate(
                         ++rootIssuedCount, sha256WithRSAEncryption,
                         rootName,
-                         now + (2 * Time::ONE_DAY_IN_SECONDS),
-                         now + (10 * Time::ONE_DAY_IN_SECONDS),
+                         now + (2 * ONE_DAY_IN_SECONDS_AS_TIME_T),
+                         now + (10 * ONE_DAY_IN_SECONDS_AS_TIME_T),
                         signerName, *signerKeyPair, extensions,
                         *rootKeyPair));
  ASSERT_FALSE(ENCODING_FAILED(signerDER));
@ -1017,13 +1020,14 @@ TEST_F(pkixocsp_VerifyEncodedResponse_GetCertTrust, ActivelyDistrusted)
 {
  ASSERT_TRUE(trustDomain.SetCertTrust(signerCertDER,
                                       TrustLevel::ActivelyDistrusted));
-  Input response;
+  Input responseInput;
  ASSERT_EQ(Success,
-            response.Init(responseString.data(), responseString.length()));
+            responseInput.Init(responseString.data(),
+                               responseString.length()));
  bool expired;
  ASSERT_EQ(Result::ERROR_OCSP_INVALID_SIGNING_CERT,
            VerifyEncodedOCSPResponse(trustDomain, *endEntityCertID, Now(),
                                      END_ENTITY_MAX_LIFETIME_IN_DAYS,
-                                      response, expired));
+                                      responseInput, expired));
  ASSERT_FALSE(expired);
 }
--- a/security/pkix/test/lib/pkixtestnss.cpp
+++ b/security/pkix/test/lib/pkixtestnss.cpp
@ -115,7 +115,8 @@ public:
    }

    SECItem signatureItem;
-    if (SEC_SignData(&signatureItem, tbs.data(), tbs.length(),
+    if (SEC_SignData(&signatureItem, tbs.data(),
+                     static_cast<int>(tbs.length()),
                     privateKey.get(), signatureAlgorithmOidTag)
          != SECSuccess) {
      return MapPRErrorCodeToResult(PR_GetError());
--- a/security/pkix/test/lib/pkixtestutil.cpp
+++ b/security/pkix/test/lib/pkixtestutil.cpp
@ -127,10 +127,10 @@ TLV(uint8_t tag, const ByteString& value)
  result.push_back(tag);

  if (value.length() < 128) {
-    result.push_back(value.length());
+    result.push_back(static_cast<uint8_t>(value.length()));
  } else if (value.length() < 256) {
    result.push_back(0x81u);
-    result.push_back(value.length());
+    result.push_back(static_cast<uint8_t>(value.length()));
  } else if (value.length() < 65536) {
    result.push_back(0x82u);
    result.push_back(static_cast<uint8_t>(value.length() / 256));
@ -158,7 +158,7 @@ OCSPResponseContext::OCSPResponseContext(const CertID& certID, time_t time)
  , certStatus(good)
  , revocationTime(0)
  , thisUpdate(time)
-  , nextUpdate(time + Time::ONE_DAY_IN_SECONDS)
+  , nextUpdate(time + static_cast<time_t>(Time::ONE_DAY_IN_SECONDS))
  , includeNextUpdate(true)
 {
 }
@ -202,7 +202,7 @@ ByteString
 Boolean(bool value)
 {
  ByteString encodedValue;
-  encodedValue.push_back(value ? 0xff : 0x00);
+  encodedValue.push_back(value ? 0xffu : 0x00u);
  return TLV(der::BOOLEAN, encodedValue);
 }

@ -266,22 +266,22 @@ TimeToEncodedTime(time_t time, TimeEncoding encoding)
  ByteString value;

  if (encoding == GeneralizedTime) {
-    value.push_back('0' + (year / 1000));
-    value.push_back('0' + ((year % 1000) / 100));
+    value.push_back(static_cast<uint8_t>('0' + (year / 1000)));
+    value.push_back(static_cast<uint8_t>('0' + ((year % 1000) / 100)));
  }

-  value.push_back('0' + ((year % 100) / 10));
-  value.push_back('0' + (year % 10));
-  value.push_back('0' + ((exploded.tm_mon + 1) / 10));
-  value.push_back('0' + ((exploded.tm_mon + 1) % 10));
-  value.push_back('0' + (exploded.tm_mday / 10));
-  value.push_back('0' + (exploded.tm_mday % 10));
-  value.push_back('0' + (exploded.tm_hour / 10));
-  value.push_back('0' + (exploded.tm_hour % 10));
-  value.push_back('0' + (exploded.tm_min / 10));
-  value.push_back('0' + (exploded.tm_min % 10));
-  value.push_back('0' + (exploded.tm_sec / 10));
-  value.push_back('0' + (exploded.tm_sec % 10));
+  value.push_back(static_cast<uint8_t>('0' + ((year % 100) / 10)));
+  value.push_back(static_cast<uint8_t>('0' + (year % 10)));
+  value.push_back(static_cast<uint8_t>('0' + ((exploded.tm_mon + 1) / 10)));
+  value.push_back(static_cast<uint8_t>('0' + ((exploded.tm_mon + 1) % 10)));
+  value.push_back(static_cast<uint8_t>('0' + (exploded.tm_mday / 10)));
+  value.push_back(static_cast<uint8_t>('0' + (exploded.tm_mday % 10)));
+  value.push_back(static_cast<uint8_t>('0' + (exploded.tm_hour / 10)));
+  value.push_back(static_cast<uint8_t>('0' + (exploded.tm_hour % 10)));
+  value.push_back(static_cast<uint8_t>('0' + (exploded.tm_min / 10)));
+  value.push_back(static_cast<uint8_t>('0' + (exploded.tm_min % 10)));
+  value.push_back(static_cast<uint8_t>('0' + (exploded.tm_sec / 10)));
+  value.push_back(static_cast<uint8_t>('0' + (exploded.tm_sec % 10)));
  value.push_back('Z');

  return TLV(encoding == GeneralizedTime ? der::GENERALIZED_TIME : der::UTCTime,
@ -315,18 +315,15 @@ TimeToTimeChoice(time_t time)
 }

 Time
-YMDHMS(int16_t year, int16_t month, int16_t day,
-       int16_t hour, int16_t minutes, int16_t seconds)
+YMDHMS(uint16_t year, uint16_t month, uint16_t day,
+       uint16_t hour, uint16_t minutes, uint16_t seconds)
 {
  assert(year <= 9999);
  assert(month >= 1);
  assert(month <= 12);
  assert(day >= 1);
-  assert(hour >= 0);
  assert(hour < 24);
-  assert(minutes >= 0);
  assert(minutes < 60);
-  assert(seconds >= 0);
  assert(seconds < 60);

  uint64_t days = DaysBeforeYear(year);
@ -439,6 +436,31 @@ EmptyExtension(Input extnID, Critical critical)
  return TLV(der::SEQUENCE, encoded);
 }

+std::string
+GetEnv(const char* name)
+{
+  std::string result;
+
+#ifndef _MSC_VER
+  // XXX: Not thread safe.
+  const char* value = getenv(name);
+  if (value) {
+    result = value;
+  }
+#else
+  char* value = nullptr;
+  size_t valueLength = 0;
+  if (_dupenv_s(&value, &valueLength, name) != 0) {
+    abort();
+  }
+  if (value) {
+    result = value;
+    free(value);
+  }
+#endif
+  return result;
+}
+
 void
 MaybeLogOutput(const ByteString& result, const char* suffix)
 {
@ -447,8 +469,8 @@ MaybeLogOutput(const ByteString& result, const char* suffix)
  // This allows us to more easily debug the generated output, by creating a
  // file in the directory given by MOZILLA_PKIX_TEST_LOG_DIR for each
  // NOT THREAD-SAFE!!!
-  const char* logPath = getenv("MOZILLA_PKIX_TEST_LOG_DIR");
-  if (logPath) {
+  std::string logPath(GetEnv("MOZILLA_PKIX_TEST_LOG_DIR"));
+  if (!logPath.empty()) {
    static int counter = 0;

    std::ostringstream counterStream;
@ -829,7 +851,7 @@ BasicOCSPResponse(OCSPResponseContext& context)
 //   value            OCTET STRING
 // }
 static ByteString
-OCSPExtension(OCSPResponseContext& context, OCSPResponseExtension& extension)
+OCSPExtension(OCSPResponseExtension& extension)
 {
  ByteString encoded;
  encoded.append(extension.id);
@ -850,7 +872,7 @@ Extensions(OCSPResponseContext& context)
  ByteString value;
  for (OCSPResponseExtension* extension = context.extensions;
       extension; extension = extension->next) {
-    ByteString extensionEncoded(OCSPExtension(context, *extension));
+    ByteString extensionEncoded(OCSPExtension(*extension));
    if (ENCODING_FAILED(extensionEncoded)) {
      return ByteString();
    }
@ -915,8 +937,11 @@ ResponderID(OCSPResponseContext& context)
    responderIDType = 2; // byKey
  }

-  return TLV(der::CONSTRUCTED | der::CONTEXT_SPECIFIC | responderIDType,
-             contents);
+  // XXX: MSVC 2015 wrongly warns about signed/unsigned conversion without the
+  // static_cast.
+  uint8_t tag = static_cast<uint8_t>(der::CONSTRUCTED | der::CONTEXT_SPECIFIC |
+                                     responderIDType);
+  return TLV(tag, contents);
 }

 // KeyHash ::= OCTET STRING -- SHA-1 hash of responder's public key
@ -1047,7 +1072,10 @@ CertStatus(OCSPResponseContext& context)
    case 0:
    case 2:
    {
-      return TLV(der::CONTEXT_SPECIFIC | context.certStatus, ByteString());
+      // XXX: MSVC 2015 wrongly warns about signed/unsigned conversion without
+      // the static cast.
+      return TLV(static_cast<uint8_t>(der::CONTEXT_SPECIFIC |
+                                      context.certStatus), ByteString());
    }
    case 1:
    {
--- a/security/pkix/test/lib/pkixtestutil.h
+++ b/security/pkix/test/lib/pkixtestutil.h
@ -93,12 +93,8 @@ const ByteString md2WithRSAEncryption(alg_md2WithRSAEncryption,
  MOZILLA_PKIX_ARRAY_LENGTH(alg_md2WithRSAEncryption));

 // e.g. YMDHMS(2016, 12, 31, 1, 23, 45) => 2016-12-31:01:23:45 (GMT)
-mozilla::pkix::Time YMDHMS(int16_t year, int16_t month, int16_t day,
-                           int16_t hour, int16_t minutes, int16_t seconds);
-
-// e.g. YMDHMS(2016, 12, 31, 1, 23, 45) => 2016-12-31:01:23:45 (GMT)
-mozilla::pkix::Time YMDHMS(int16_t year, int16_t month, int16_t day,
-                           int16_t hour, int16_t minutes, int16_t seconds);
+mozilla::pkix::Time YMDHMS(uint16_t year, uint16_t month, uint16_t day,
+                           uint16_t hour, uint16_t minutes, uint16_t seconds);

 ByteString TLV(uint8_t tag, const ByteString& value);
 ByteString Boolean(bool value);
@ -168,7 +164,7 @@ template <size_t L>
 inline ByteString
 RFC822Name(const char (&bytes)[L])
 {
-  return RFC822Name(ByteString(reinterpret_cast<const uint8_t (&)[L]>(bytes),
+  return RFC822Name(ByteString(reinterpret_cast<const uint8_t*>(&bytes),
                               L - 1));
 }

@ -183,7 +179,7 @@ template <size_t L>
 inline ByteString
 DNSName(const char (&bytes)[L])
 {
-  return DNSName(ByteString(reinterpret_cast<const uint8_t (&)[L]>(bytes),
+  return DNSName(ByteString(reinterpret_cast<const uint8_t*>(&bytes),
                            L - 1));
 }

--- a/security/pkix/warnings.mozbuild
+++ b/security/pkix/warnings.mozbuild
@ -0,0 +1,43 @@
+FAIL_ON_WARNINGS = True
+
+if CONFIG['CLANG_CXX']:
+  CXXFLAGS += [
+    '-Weverything',
+
+    '-Wno-c++98-compat',
+    '-Wno-c++98-compat-pedantic',
+    '-Wno-missing-prototypes',
+    '-Wno-missing-variable-declarations',
+    '-Wno-padded',
+    '-Wno-reserved-id-macro', # XXX: Will be removed in bug 1128413, Part 4.
+    '-Wno-shadow', # XXX: Clang's rules are too strict for constructors.
+    '-Wno-undef', # XXX: Will be removed in bug 1128413, Part 4.
+    '-Wno-weak-vtables', # We rely on the linker to merge the duplicate vtables.
+  ]
+elif CONFIG['_MSC_VER']:
+  CXXFLAGS += [
+    '-sdl', # Enable additional security checks based on Microsoft's SDL.
+
+    '-Wall',
+
+    '-wd4514', # 'function': unreferenced inline function has been removed
+    '-wd4668', # warning C4668: 'X' is not defined as a preprocessor macro,
+               # replacing with '0' for '#if/#elif'.
+    '-wd4710', # 'function': function not inlined
+    '-wd4711', # function 'function' selected for inline expansion
+    '-wd4800', # forcing value to bool 'true' or 'false'
+    '-wd4820', # 'bytes' bytes padding added after construct 'member_name'
+
+    # XXX: We cannot use /Za (Disable Microsoft Extensions) because windows.h
+    # won't copmile with it.
+    '-Zc:forScope', # Standard C++ rules for variable scope in for loops.
+    '-Zc:inline', # Standard C++ rules requiring definition inline functions.
+    '-Zc:rvalueCast', # Standard C++ rules for result of cast being an rvalue.
+    '-Zc:strictStrings', # Standard C++ rule that string literals are const.
+  ]
+else:
+  CXXFLAGS += [
+    '-Wall',
+    '-Wextra',
+    '-pedantic-errors',
+  ]