Bug 1128413, Part 1: Fix switch-related warnings, r=mmc
authorBrian Smith <brian@briansmith.org>
Mon, 02 Feb 2015 14:21:27 -0800
changeset 228528 e9f7e2678bfb4437ec88669970d20a6f7c8aba07
parent 228527 ed3e996b3805c641dbda53c83907f87d3302995a
child 228529 09c407963c95928f8c5e8742256a3f740edcb06b
push id28264
push usercbook@mozilla.com
push dateWed, 11 Feb 2015 13:58:35 +0000
treeherdermozilla-central@38058cb42a0e [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersmmc
bugs1128413
milestone38.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1128413, Part 1: Fix switch-related warnings, r=mmc
security/pkix/lib/pkixcheck.cpp
security/pkix/lib/pkixnames.cpp
security/pkix/lib/pkixnss.cpp
security/pkix/lib/pkixocsp.cpp
security/pkix/lib/pkixresult.cpp
security/pkix/lib/pkixutil.h
--- a/security/pkix/lib/pkixcheck.cpp
+++ b/security/pkix/lib/pkixcheck.cpp
@@ -437,20 +437,16 @@ MatchEKU(Reader& value, KeyPurposeId req
 
       case KeyPurposeId::id_kp_OCSPSigning:
         match = value.MatchRest(ocsp);
         break;
 
       case KeyPurposeId::anyExtendedKeyUsage:
         return NotReached("anyExtendedKeyUsage should start with found==true",
                           Result::FATAL_ERROR_LIBRARY_FAILURE);
-
-      default:
-        return NotReached("unrecognized EKU",
-                          Result::FATAL_ERROR_LIBRARY_FAILURE);
     }
   }
 
   if (match) {
     found = true;
     if (requiredEKU == KeyPurposeId::id_kp_OCSPSigning) {
       foundOCSPSigning = true;
     }
--- a/security/pkix/lib/pkixnames.cpp
+++ b/security/pkix/lib/pkixnames.cpp
@@ -252,19 +252,17 @@ CheckCertHostname(Input endEntityCertDER
     return rv;
   }
   switch (match) {
     case MatchResult::NoNamesOfGivenType: // fall through
     case MatchResult::Mismatch:
       return Result::ERROR_BAD_CERT_DOMAIN;
     case MatchResult::Match:
       return Success;
-    default:
-      return NotReached("Invalid match result",
-                        Result::FATAL_ERROR_LIBRARY_FAILURE);
+    MOZILLA_PKIX_UNREACHABLE_DEFAULT_ENUM
   }
 }
 
 // 4.2.1.10. Name Constraints
 Result
 CheckNameConstraints(Input encodedNameConstraints,
                      const BackCert& firstChild,
                      KeyPurposeId requiredEKUIfPresent)
@@ -716,20 +714,18 @@ MatchPresentedIDWithReferenceID(GeneralN
     case GeneralNameType::x400Address: // fall through
     case GeneralNameType::ediPartyName: // fall through
     case GeneralNameType::uniformResourceIdentifier: // fall through
     case GeneralNameType::registeredID: // fall through
     case GeneralNameType::nameConstraints:
       return NotReached("unexpected nameType for SearchType::Match",
                         Result::FATAL_ERROR_INVALID_ARGS);
 
-    default:
-      return NotReached("Invalid nameType for MatchPresentedIDWithReferenceID",
-                        Result::FATAL_ERROR_INVALID_ARGS);
-  }
+    MOZILLA_PKIX_UNREACHABLE_DEFAULT_ENUM
+ }
 
   if (rv != Success) {
     return rv;
   }
   matchResult = foundMatch ? MatchResult::Match : MatchResult::Mismatch;
   return Success;
 }
 
@@ -895,38 +891,36 @@ CheckPresentedIDConformsToNameConstraint
         // forms."
         case GeneralNameType::otherName: // fall through
         case GeneralNameType::x400Address: // fall through
         case GeneralNameType::ediPartyName: // fall through
         case GeneralNameType::uniformResourceIdentifier: // fall through
         case GeneralNameType::registeredID: // fall through
           return Result::ERROR_CERT_NOT_IN_NAME_SPACE;
 
-        case GeneralNameType::nameConstraints: // fall through
-        default:
+        case GeneralNameType::nameConstraints:
           return NotReached("invalid presentedIDType",
                             Result::FATAL_ERROR_LIBRARY_FAILURE);
+
+        MOZILLA_PKIX_UNREACHABLE_DEFAULT_ENUM
       }
 
       switch (subtreesType) {
         case NameConstraintsSubtrees::permittedSubtrees:
           if (matches) {
             hasPermittedSubtreesMatch = true;
           } else {
             hasPermittedSubtreesMismatch = true;
           }
           break;
         case NameConstraintsSubtrees::excludedSubtrees:
           if (matches) {
             return Result::ERROR_CERT_NOT_IN_NAME_SPACE;
           }
           break;
-        default:
-          return NotReached("unexpected subtreesType",
-                            Result::FATAL_ERROR_INVALID_ARGS);
       }
     }
   } while (!subtrees.AtEnd());
 
   if (hasPermittedSubtreesMismatch && !hasPermittedSubtreesMatch) {
     // If there was any entry of the given type in permittedSubtrees, then it
     // required that at least one of them must match. Since none of them did,
     // we have a failure.
@@ -1137,18 +1131,17 @@ MatchPresentedDNSIDWithReferenceDNSID(
             return Success;
           }
         }
       }
       break;
     }
 
     case IDRole::PresentedID: // fall through
-    default:
-      return NotReached("invalid or unknown referenceDNSIDRole",
+      return NotReached("IDRole::PresentedID is not a valid referenceDNSIDRole",
                         Result::FATAL_ERROR_INVALID_ARGS);
   }
 
   // We only allow wildcard labels that consist only of '*'.
   if (presented.Peek('*')) {
     if (presented.Skip(1) != Success) {
       return NotReached("Skipping '*' failed",
                         Result::FATAL_ERROR_LIBRARY_FAILURE);
@@ -1343,18 +1336,16 @@ MatchPresentedDirectoryNameWithConstrain
     case NameConstraintsSubtrees::permittedSubtrees:
       break; // dealt with below
     case NameConstraintsSubtrees::excludedSubtrees:
       if (!constraintRDNs.AtEnd() || !presentedRDNs.AtEnd()) {
         return Result::ERROR_CERT_NOT_IN_NAME_SPACE;
       }
       matches = true;
       return Success;
-    default:
-      return NotReached("invalid subtrees", Result::FATAL_ERROR_INVALID_ARGS);
   }
 
   for (;;) {
     // The AVAs have to be fully equal, but the constraint RDNs just need to be
     // a prefix of the presented RDNs.
     if (constraintRDNs.AtEnd()) {
       matches = true;
       return Success;
@@ -1504,20 +1495,16 @@ MatchPresentedRFC822NameWithReferenceRFC
         return Result::FATAL_ERROR_LIBRARY_FAILURE;
       }
 
       return MatchPresentedDNSIDWithReferenceDNSID(
                presentedDNSID, AllowWildcards::No,
                AllowDotlessSubdomainMatches::No, IDRole::NameConstraint,
                referenceRFC822Name, matches);
     }
-
-    default:
-      return NotReached("invalid referenceRFC822NameRole",
-                        Result::FATAL_ERROR_INVALID_ARGS);
   }
 
   if (!IsValidRFC822Name(referenceRFC822Name)) {
     return Result::ERROR_BAD_DER;
   }
 
   Reader reference(referenceRFC822Name);
 
--- a/security/pkix/lib/pkixnss.cpp
+++ b/security/pkix/lib/pkixnss.cpp
@@ -28,16 +28,17 @@
 
 #include "cert.h"
 #include "cryptohi.h"
 #include "keyhi.h"
 #include "pk11pub.h"
 #include "pkix/pkix.h"
 #include "pkix/ScopedPtr.h"
 #include "pkixder.h"
+#include "pkixutil.h"
 #include "secerr.h"
 #include "sslerr.h"
 
 namespace mozilla { namespace pkix {
 
 typedef ScopedPtr<SECKEYPublicKey, SECKEY_DestroyPublicKey> ScopedSECKeyPublicKey;
 
 static Result
@@ -51,16 +52,27 @@ CheckPublicKeySize(Input subjectPublicKe
   if (!spki) {
     return MapPRErrorCodeToResult(PR_GetError());
   }
   publicKey = SECKEY_ExtractPublicKey(spki.get());
   if (!publicKey) {
     return MapPRErrorCodeToResult(PR_GetError());
   }
 
+  // Some compilers complain if if we don't explicitly list every case. That is
+  // usually what we want, but in this case we really want to support an
+  // open-ended set of key types that might be expanded by future NSS versions.
+#if defined(__clang__)
+#pragma clang diagnostic push
+#pragma clang diagnostic ignored "-Wswitch-enum"
+#elif defined(_MSC_VER)
+#pragma warning(push)
+#pragma warning(disable: 4061)
+#endif
+
   switch (publicKey.get()->keyType) {
     case ecKey:
     {
       SECKEYECParams* encodedParams = &publicKey.get()->u.ec.DEREncodedParams;
       if (!encodedParams) {
         return Result::ERROR_UNSUPPORTED_ELLIPTIC_CURVE;
       }
 
@@ -82,38 +94,37 @@ CheckPublicKeySize(Input subjectPublicKe
         return rv;
       }
 
       switch (namedCurve) {
         case NamedCurve::secp256r1: // fall through
         case NamedCurve::secp384r1: // fall through
         case NamedCurve::secp521r1:
           break;
-        default:
-          return Result::ERROR_UNSUPPORTED_ELLIPTIC_CURVE;
       }
 
       return Success;
     }
+
     case rsaKey:
       if (SECKEY_PublicKeyStrengthInBits(publicKey.get()) < minimumNonECCBits) {
         return Result::ERROR_INADEQUATE_KEY_SIZE;
       }
       break;
-    case dsaKey: // fall through
-    case nullKey: // fall through
-    case fortezzaKey: // fall through
-    case dhKey: // fall through
-    case keaKey: // fall through
-    case rsaPssKey: // fall through
-    case rsaOaepKey: // fall through
+
     default:
       return Result::ERROR_UNSUPPORTED_KEYALG;
   }
 
+#if defined(__clang__)
+#pragma clang diagnostic pop
+#elif defined(_MSC_VER)
+#pragma warning(pop)
+#endif
+
   return Success;
 }
 
 Result
 CheckPublicKeyNSS(Input subjectPublicKeyInfo, unsigned int minimumNonECCBits)
 {
   ScopedSECKeyPublicKey unused;
   return CheckPublicKeySize(subjectPublicKeyInfo, minimumNonECCBits, unused);
@@ -155,19 +166,19 @@ VerifySignedDataNSS(const SignedDataWith
       pubKeyAlg = SEC_OID_PKCS1_RSA_ENCRYPTION;
       digestAlg = SEC_OID_SHA256;
       break;
     case SignatureAlgorithm::rsa_pkcs1_with_sha1:
       pubKeyAlg = SEC_OID_PKCS1_RSA_ENCRYPTION;
       digestAlg = SEC_OID_SHA1;
       break;
     case SignatureAlgorithm::unsupported_algorithm: // fall through
-    default:
       return NotReached("unknown signature algorithm",
                         Result::ERROR_CERT_SIGNATURE_ALGORITHM_DISABLED);
+    MOZILLA_PKIX_UNREACHABLE_DEFAULT_ENUM
   }
 
   Result rv;
   ScopedSECKeyPublicKey pubKey;
   rv = CheckPublicKeySize(subjectPublicKeyInfo, minimumNonECCBits, pubKey);
   if (rv != Success) {
     return rv;
   }
@@ -239,19 +250,17 @@ MapResultToPRErrorCode(Result result)
   {
 #define MOZILLA_PKIX_MAP(mozilla_pkix_result, value, nss_result) \
     case Result::mozilla_pkix_result: return nss_result;
 
     MOZILLA_PKIX_MAP_LIST
 
 #undef MOZILLA_PKIX_MAP
 
-    default:
-      PR_NOT_REACHED("Unknown error code in MapResultToPRErrorCode");
-      return SEC_ERROR_LIBRARY_FAILURE;
+    MOZILLA_PKIX_UNREACHABLE_DEFAULT_ENUM
   }
 }
 
 void
 RegisterErrorTable()
 {
   // Note that these error strings are not localizable.
   // When these strings change, update the localization information too.
--- a/security/pkix/lib/pkixocsp.cpp
+++ b/security/pkix/lib/pkixocsp.cpp
@@ -190,18 +190,17 @@ MatchResponderID(TrustDomain& trustDomai
       Result rv = der::ExpectTagAndGetValue(input, der::OCTET_STRING, keyHash);
       if (rv != Success) {
         return rv;
       }
       return MatchKeyHash(trustDomain, keyHash,
                           potentialSignerSubjectPublicKeyInfo, match);
     }
 
-    default:
-      return Result::ERROR_OCSP_MALFORMED_RESPONSE;
+    MOZILLA_PKIX_UNREACHABLE_DEFAULT_ENUM
   }
 }
 
 static Result
 VerifyOCSPSignedData(TrustDomain& trustDomain,
                      const SignedDataWithSignature& signedResponseData,
                      Input spki)
 {
@@ -315,19 +314,18 @@ VerifyEncodedOCSPResponse(TrustDomain& t
       if (expired) {
         return Result::ERROR_OCSP_OLD_RESPONSE;
       }
       return Success;
     case CertStatus::Revoked:
       return Result::ERROR_REVOKED_CERTIFICATE;
     case CertStatus::Unknown:
       return Result::ERROR_OCSP_UNKNOWN_CERT;
+     MOZILLA_PKIX_UNREACHABLE_DEFAULT_ENUM
   }
-
-  return NotReached("unknown CertStatus", Result::ERROR_OCSP_UNKNOWN_CERT);
 }
 
 // OCSPResponse ::= SEQUENCE {
 //       responseStatus         OCSPResponseStatus,
 //       responseBytes          [0] EXPLICIT ResponseBytes OPTIONAL }
 //
 static inline Result
 OCSPResponse(Reader& input, Context& context)
--- a/security/pkix/lib/pkixresult.cpp
+++ b/security/pkix/lib/pkixresult.cpp
@@ -18,32 +18,29 @@
  * Unless required by applicable law or agreed to in writing, software
  * distributed under the License is distributed on an "AS IS" BASIS,
  * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
  * See the License for the specific language governing permissions and
  * limitations under the License.
  */
 
 #include "pkix/Result.h"
-
-#include "pkix/stdkeywords.h"
+#include "pkixutil.h"
 
 namespace mozilla { namespace pkix {
 
 const char*
 MapResultToName(Result result)
 {
   switch (result)
   {
 #define MOZILLA_PKIX_MAP(mozilla_pkix_result, value, nss_result) \
     case Result::mozilla_pkix_result: return "Result::" #mozilla_pkix_result;
 
     MOZILLA_PKIX_MAP_LIST
 
 #undef MOZILLA_PKIX_MAP
 
-    default:
-      assert(false);
-      return nullptr;
+    MOZILLA_PKIX_UNREACHABLE_DEFAULT_ENUM
   }
 }
 
 } } // namespace mozilla::pkix
--- a/security/pkix/lib/pkixutil.h
+++ b/security/pkix/lib/pkixutil.h
@@ -206,11 +206,52 @@ WrappedVerifySignedData(TrustDomain& tru
 {
   if (signedData.algorithm == SignatureAlgorithm::unsupported_algorithm) {
     return Result::ERROR_CERT_SIGNATURE_ALGORITHM_DISABLED;
   }
 
   return trustDomain.VerifySignedData(signedData, subjectPublicKeyInfo);
 }
 
+// In a switch over an enum, sometimes some compilers are not satisfied that
+// all control flow paths have been considered unless there is a default case.
+// However, in our code, such a default case is almost always unreachable dead
+// code. That can be particularly problematic when the compiler wants the code
+// to choose a value, such as a return value, for the default case, but there's
+// no appropriate "impossible case" value to choose.
+//
+// MOZILLA_PKIX_UNREACHABLE_DEFAULT_ENUM accounts for this. Example:
+//
+//     // In xy.cpp
+//     #include "xt.h"
+//
+//     enum class XY { X, Y };
+//
+//     int func(XY xy) {
+//       switch (xy) {
+//         case XY::X: return 1;
+//         case XY::Y; return 2;
+//         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();
+#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);
+#else
+# error Unsupported compiler for MOZILLA_PKIX_UNREACHABLE_DEFAULT.
+#endif
+
 } } // namespace mozilla::pkix
 
 #endif // mozilla_pkix__pkixutil_h