Bug 1128413, Part 1: Fix switch-related warnings, r=mmc

6 files changed, 74 insertions, 46 deletions

diff --git a/lib/mozpkix/lib/pkixcheck.cpp b/lib/mozpkix/lib/pkixcheck.cpp
index a7f1dccfc..3524b2b14 100644
--- a/lib/mozpkix/lib/pkixcheck.cpp
+++ b/lib/mozpkix/lib/pkixcheck.cpp
@@ -442,10 +442,6 @@ MatchEKU(Reader& value, KeyPurposeId requiredEKU,
       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);
     }
   }
 
diff --git a/lib/mozpkix/lib/pkixnames.cpp b/lib/mozpkix/lib/pkixnames.cpp
index 69dd3751e..8517adfdf 100644
--- a/lib/mozpkix/lib/pkixnames.cpp
+++ b/lib/mozpkix/lib/pkixnames.cpp
@@ -257,9 +257,7 @@ CheckCertHostname(Input endEntityCertDER, Input hostname)
       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
   }
 }
 
@@ -721,10 +719,8 @@ MatchPresentedIDWithReferenceID(GeneralNameType presentedIDType,
       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;
@@ -900,10 +896,11 @@ CheckPresentedIDConformsToNameConstraintsSubtrees(
         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) {
@@ -919,9 +916,6 @@ CheckPresentedIDConformsToNameConstraintsSubtrees(
             return Result::ERROR_CERT_NOT_IN_NAME_SPACE;
           }
           break;
-        default:
-          return NotReached("unexpected subtreesType",
-                            Result::FATAL_ERROR_INVALID_ARGS);
       }
     }
   } while (!subtrees.AtEnd());
@@ -1142,8 +1136,7 @@ MatchPresentedDNSIDWithReferenceDNSID(
     }
 
     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);
   }
 
@@ -1348,8 +1341,6 @@ MatchPresentedDirectoryNameWithConstraint(NameConstraintsSubtrees subtreesType,
       }
       matches = true;
       return Success;
-    default:
-      return NotReached("invalid subtrees", Result::FATAL_ERROR_INVALID_ARGS);
   }
 
   for (;;) {
@@ -1509,10 +1500,6 @@ MatchPresentedRFC822NameWithReferenceRFC822Name(Input presentedRFC822Name,
                AllowDotlessSubdomainMatches::No, IDRole::NameConstraint,
                referenceRFC822Name, matches);
     }
-
-    default:
-      return NotReached("invalid referenceRFC822NameRole",
-                        Result::FATAL_ERROR_INVALID_ARGS);
   }
 
   if (!IsValidRFC822Name(referenceRFC822Name)) {
diff --git a/lib/mozpkix/lib/pkixnss.cpp b/lib/mozpkix/lib/pkixnss.cpp
index aea39d614..72187cf69 100644
--- a/lib/mozpkix/lib/pkixnss.cpp
+++ b/lib/mozpkix/lib/pkixnss.cpp
@@ -33,6 +33,7 @@
 #include "pkix/pkix.h"
 #include "pkix/ScopedPtr.h"
 #include "pkixder.h"
+#include "pkixutil.h"
 #include "secerr.h"
 #include "sslerr.h"
 
@@ -56,6 +57,17 @@ CheckPublicKeySize(Input subjectPublicKeyInfo, unsigned int minimumNonECCBits,
     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:
     {
@@ -87,28 +99,27 @@ CheckPublicKeySize(Input subjectPublicKeyInfo, unsigned int minimumNonECCBits,
         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;
 }
 
@@ -160,9 +171,9 @@ VerifySignedDataNSS(const SignedDataWithSignature& sd,
       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;
@@ -244,9 +255,7 @@ MapResultToPRErrorCode(Result result)
 
 #undef MOZILLA_PKIX_MAP
 
-    default:
-      PR_NOT_REACHED("Unknown error code in MapResultToPRErrorCode");
-      return SEC_ERROR_LIBRARY_FAILURE;
+    MOZILLA_PKIX_UNREACHABLE_DEFAULT_ENUM
   }
 }
 
diff --git a/lib/mozpkix/lib/pkixocsp.cpp b/lib/mozpkix/lib/pkixocsp.cpp
index 89f8a0b30..d58a51672 100644
--- a/lib/mozpkix/lib/pkixocsp.cpp
+++ b/lib/mozpkix/lib/pkixocsp.cpp
@@ -195,8 +195,7 @@ MatchResponderID(TrustDomain& trustDomain,
                           potentialSignerSubjectPublicKeyInfo, match);
     }
 
-    default:
-      return Result::ERROR_OCSP_MALFORMED_RESPONSE;
+    MOZILLA_PKIX_UNREACHABLE_DEFAULT_ENUM
   }
 }
 
@@ -320,9 +319,8 @@ VerifyEncodedOCSPResponse(TrustDomain& trustDomain, const struct CertID& certID,
       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 {
diff --git a/lib/mozpkix/lib/pkixresult.cpp b/lib/mozpkix/lib/pkixresult.cpp
index a4ad14851..670642de8 100644
--- a/lib/mozpkix/lib/pkixresult.cpp
+++ b/lib/mozpkix/lib/pkixresult.cpp
@@ -23,8 +23,7 @@
  */
 
 #include "pkix/Result.h"
-
-#include "pkix/stdkeywords.h"
+#include "pkixutil.h"
 
 namespace mozilla { namespace pkix {
 
@@ -40,9 +39,7 @@ MapResultToName(Result result)
 
 #undef MOZILLA_PKIX_MAP
 
-    default:
-      assert(false);
-      return nullptr;
+    MOZILLA_PKIX_UNREACHABLE_DEFAULT_ENUM
   }
 }
 
diff --git a/lib/mozpkix/lib/pkixutil.h b/lib/mozpkix/lib/pkixutil.h
index 238bd62b2..eac104dc0 100644
--- a/lib/mozpkix/lib/pkixutil.h
+++ b/lib/mozpkix/lib/pkixutil.h
@@ -211,6 +211,47 @@ WrappedVerifySignedData(TrustDomain& trustDomain,
   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