Bug 1493226, part 2 - Statically prevent trivial calls to do_QueryInterface that returns an nsresult r=froydnj
authorAndrew McCreight <continuation@gmail.com>
Thu, 04 Oct 2018 19:16:30 +0000
changeset 495393 4bada21c38bf68253216b06b7f21e2beaf047f6e
parent 495392 a5ebcd6b0fbe7f84220120d60daac61cc9bc5eb9
child 495394 556b2f4cd653aa7642956ba66e60839b4ea9f474
push id9984
push userffxbld-merge
push dateMon, 15 Oct 2018 21:07:35 +0000
treeherdermozilla-beta@183d27ea8570 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersfroydnj
bugs1493226
milestone64.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 1493226, part 2 - Statically prevent trivial calls to do_QueryInterface that returns an nsresult r=froydnj This is the same as the other patch, except that it is applied to the case where the QI returns an nsresult. In addition, I marked the WithError helper class as being stack-only. Depends on D7553 Differential Revision: https://phabricator.services.mozilla.com/D7554
xpcom/base/nsCOMPtr.cpp
xpcom/base/nsCOMPtr.h
--- a/xpcom/base/nsCOMPtr.cpp
+++ b/xpcom/base/nsCOMPtr.cpp
@@ -15,17 +15,17 @@ nsQueryInterfaceISupports::operator()(co
   } else {
     status = NS_ERROR_NULL_POINTER;
   }
 
   return status;
 }
 
 nsresult
-nsQueryInterfaceWithError::operator()(const nsIID& aIID, void** aAnswer) const
+nsQueryInterfaceISupportsWithError::operator()(const nsIID& aIID, void** aAnswer) const
 {
   nsresult status;
   if (mRawPtr) {
     status = mRawPtr->QueryInterface(aIID, aAnswer);
   } else {
     status = NS_ERROR_NULL_POINTER;
   }
 
@@ -50,17 +50,17 @@ nsCOMPtr_base::assign_from_qi(const nsQu
   void* newRawPtr;
   if (NS_FAILED(aQI(aIID, &newRawPtr))) {
     newRawPtr = nullptr;
   }
   assign_assuming_AddRef(static_cast<nsISupports*>(newRawPtr));
 }
 
 void
-nsCOMPtr_base::assign_from_qi_with_error(const nsQueryInterfaceWithError& aQI,
+nsCOMPtr_base::assign_from_qi_with_error(const nsQueryInterfaceISupportsWithError& aQI,
                                          const nsIID& aIID)
 {
   void* newRawPtr;
   if (NS_FAILED(aQI(aIID, &newRawPtr))) {
     newRawPtr = nullptr;
   }
   assign_assuming_AddRef(static_cast<nsISupports*>(newRawPtr));
 }
--- a/xpcom/base/nsCOMPtr.h
+++ b/xpcom/base/nsCOMPtr.h
@@ -176,40 +176,62 @@ public:
   nsQueryInterface(T* aRawPtr) : nsQueryInterfaceISupports(aRawPtr) {}
 
   nsresult NS_FASTCALL operator()(const nsIID& aIID, void** aAnswer) const {
     return nsQueryInterfaceISupports::operator()(aIID, aAnswer);
   }
 };
 #endif // #ifndef NSCAP_FEATURE_USE_BASE
 
-class nsQueryInterfaceWithError final
+class MOZ_STACK_CLASS nsQueryInterfaceISupportsWithError
 {
 public:
-  nsQueryInterfaceWithError(nsISupports* aRawPtr, nsresult* aError)
+  nsQueryInterfaceISupportsWithError(nsISupports* aRawPtr, nsresult* aError)
     : mRawPtr(aRawPtr)
     , mErrorPtr(aError)
   {
   }
 
   nsresult NS_FASTCALL operator()(const nsIID& aIID, void**) const;
 
 private:
   nsISupports* MOZ_OWNING_REF mRawPtr;
   nsresult* mErrorPtr;
 };
 
+#ifndef NSCAP_FEATURE_USE_BASE
+template<typename T>
+class MOZ_STACK_CLASS nsQueryInterfaceWithError final : public nsQueryInterfaceISupportsWithError
+{
+public:
+  explicit
+  nsQueryInterfaceWithError(T* aRawPtr, nsresult* aError)
+    : nsQueryInterfaceISupportsWithError(aRawPtr, aError)
+  {}
+
+  nsresult NS_FASTCALL operator()(const nsIID& aIID, void** aAnswer) const {
+    return nsQueryInterfaceISupportsWithError::operator()(aIID, aAnswer);
+  }
+};
+#endif // #ifndef NSCAP_FEATURE_USE_BASE
+
 #ifdef NSCAP_FEATURE_USE_BASE
 
 inline nsQueryInterfaceISupports
 do_QueryInterface(nsISupports* aRawPtr)
 {
   return nsQueryInterfaceISupports(aRawPtr);
 }
 
+inline nsQueryInterfaceISupportsWithError
+do_QueryInterface(nsISupports* aRawPtr, nsresult* aError)
+{
+  return nsQueryInterfaceISupportsWithError(aRawPtr, aError);
+}
+
 #else
 
 namespace mozilla {
 // PointedToType<> is needed so that do_QueryInterface() will work with a
 // variety of smart pointer types in addition to raw pointers. These types
 // include RefPtr<>, nsCOMPtr<>, and OwningNonNull<>.
 template<class T>
 using PointedToType = typename mozilla::RemovePointer<decltype(&*mozilla::DeclVal<T>())>::Type;
@@ -217,23 +239,24 @@ using PointedToType = typename mozilla::
 
 template<class T>
 inline nsQueryInterface<mozilla::PointedToType<T>>
 do_QueryInterface(T aPtr)
 {
   return nsQueryInterface<mozilla::PointedToType<T>>(aPtr);
 }
 
+template<class T>
+inline nsQueryInterfaceWithError<mozilla::PointedToType<T>>
+do_QueryInterface(T aRawPtr, nsresult* aError)
+{
+  return nsQueryInterfaceWithError<mozilla::PointedToType<T>>(aRawPtr, aError);
+}
 #endif // ! #ifdef NSCAP_FEATURE_USE_BASE
 
-inline nsQueryInterfaceWithError
-do_QueryInterface(nsISupports* aRawPtr, nsresult* aError)
-{
-  return nsQueryInterfaceWithError(aRawPtr, aError);
-}
 
 template<class T>
 inline void
 do_QueryInterface(already_AddRefed<T>&)
 {
   // This signature exists solely to _stop_ you from doing the bad thing.
   // Saying |do_QueryInterface()| on a pointer that is not otherwise owned by
   // someone else is an automatic leak. See bug 8221.
@@ -349,17 +372,17 @@ public:
     }
   }
 
   void NS_FASTCALL
   assign_with_AddRef(nsISupports*);
   void NS_FASTCALL
   assign_from_qi(const nsQueryInterfaceISupports, const nsIID&);
   void NS_FASTCALL
-  assign_from_qi_with_error(const nsQueryInterfaceWithError&, const nsIID&);
+  assign_from_qi_with_error(const nsQueryInterfaceISupportsWithError&, const nsIID&);
   void NS_FASTCALL
   assign_from_gs_cid(const nsGetServiceByCID, const nsIID&);
   void NS_FASTCALL
   assign_from_gs_cid_with_error(const nsGetServiceByCIDWithError&, const nsIID&);
   void NS_FASTCALL
   assign_from_gs_contractid(const nsGetServiceByContractID, const nsIID&);
   void NS_FASTCALL
   assign_from_gs_contractid_with_error(const nsGetServiceByContractIDWithError&,
@@ -411,17 +434,18 @@ class MOZ_IS_REFPTR nsCOMPtr final
   #define NSCAP_CTOR_BASE(x) nsCOMPtr_base(x)
 #else
   #define NSCAP_CTOR_BASE(x) mRawPtr(x)
 
 private:
   void assign_with_AddRef(nsISupports*);
   template<typename U>
   void assign_from_qi(const nsQueryInterface<U>, const nsIID&);
-  void assign_from_qi_with_error(const nsQueryInterfaceWithError&, const nsIID&);
+  template<typename U>
+  void assign_from_qi_with_error(const nsQueryInterfaceWithError<U>&, const nsIID&);
   void assign_from_gs_cid(const nsGetServiceByCID, const nsIID&);
   void assign_from_gs_cid_with_error(const nsGetServiceByCIDWithError&,
                                      const nsIID&);
   void assign_from_gs_contractid(const nsGetServiceByContractID, const nsIID&);
   void assign_from_gs_contractid_with_error(
     const nsGetServiceByContractIDWithError&, const nsIID&);
   void assign_from_query_referent(const nsQueryReferent&, const nsIID&);
   void assign_from_helper(const nsCOMPtr_helper&, const nsIID&);
@@ -609,17 +633,22 @@ public:
     : NSCAP_CTOR_BASE(nullptr)
   {
     assert_validity();
     NSCAP_LOG_ASSIGNMENT(this, nullptr);
     assign_from_qi(aQI, NS_GET_TEMPLATE_IID(T));
   }
 
   // Construct from |do_QueryInterface(expr, &rv)|.
-  MOZ_IMPLICIT nsCOMPtr(const nsQueryInterfaceWithError& aQI)
+#ifdef NSCAP_FEATURE_USE_BASE
+  MOZ_IMPLICIT nsCOMPtr(const nsQueryInterfaceISupportsWithError& aQI)
+#else
+  template<typename U>
+  MOZ_IMPLICIT nsCOMPtr(const nsQueryInterfaceWithError<U>& aQI)
+#endif // ! #ifdef NSCAP_FEATURE_USE_BASE
     : NSCAP_CTOR_BASE(nullptr)
   {
     assert_validity();
     NSCAP_LOG_ASSIGNMENT(this, nullptr);
     assign_from_qi_with_error(aQI, NS_GET_TEMPLATE_IID(T));
   }
 
   // Construct from |do_GetService(cid_expr)|.
@@ -764,17 +793,22 @@ public:
   nsCOMPtr<T>& operator=(const nsQueryInterface<U> aRhs)
 #endif // ! #ifdef NSCAP_FEATURE_USE_BASE
   {
     assign_from_qi(aRhs, NS_GET_TEMPLATE_IID(T));
     return *this;
   }
 
   // Assign from |do_QueryInterface(expr, &rv)|.
-  nsCOMPtr<T>& operator=(const nsQueryInterfaceWithError& aRhs)
+#ifdef NSCAP_FEATURE_USE_BASE
+  nsCOMPtr<T>& operator=(const nsQueryInterfaceISupportsWithError& aRhs)
+#else
+  template<typename U>
+  nsCOMPtr<T>& operator=(const nsQueryInterfaceWithError<U>& aRhs)
+#endif // ! #ifdef NSCAP_FEATURE_USE_BASE
   {
     assign_from_qi_with_error(aRhs, NS_GET_TEMPLATE_IID(T));
     return *this;
   }
 
   // Assign from |do_GetService(cid_expr)|.
   nsCOMPtr<T>& operator=(const nsGetServiceByCID aRhs)
   {
@@ -997,17 +1031,17 @@ public:
   MOZ_IMPLICIT nsCOMPtr(const nsQueryInterfaceISupports aQI)
     : nsCOMPtr_base(nullptr)
   {
     NSCAP_LOG_ASSIGNMENT(this, nullptr);
     assign_from_qi(aQI, NS_GET_IID(nsISupports));
   }
 
   // Construct from |do_QueryInterface(expr, &rv)|.
-  MOZ_IMPLICIT nsCOMPtr(const nsQueryInterfaceWithError& aQI)
+  MOZ_IMPLICIT nsCOMPtr(const nsQueryInterfaceISupportsWithError& aQI)
     : nsCOMPtr_base(nullptr)
   {
     NSCAP_LOG_ASSIGNMENT(this, nullptr);
     assign_from_qi_with_error(aQI, NS_GET_IID(nsISupports));
   }
 
   // Construct from |do_GetService(cid_expr)|.
   MOZ_IMPLICIT nsCOMPtr(const nsGetServiceByCID aGS)
@@ -1096,17 +1130,17 @@ public:
   // Assign from |do_QueryInterface(expr)|.
   nsCOMPtr<nsISupports>& operator=(const nsQueryInterfaceISupports aRhs)
   {
     assign_from_qi(aRhs, NS_GET_IID(nsISupports));
     return *this;
   }
 
   // Assign from |do_QueryInterface(expr, &rv)|.
-  nsCOMPtr<nsISupports>& operator=(const nsQueryInterfaceWithError& aRhs)
+  nsCOMPtr<nsISupports>& operator=(const nsQueryInterfaceISupportsWithError& aRhs)
   {
     assign_from_qi_with_error(aRhs, NS_GET_IID(nsISupports));
     return *this;
   }
 
   // Assign from |do_GetService(cid_expr)|.
   nsCOMPtr<nsISupports>& operator=(const nsGetServiceByCID aRhs)
   {
@@ -1276,20 +1310,24 @@ nsCOMPtr<T>::assign_from_qi(const nsQuer
   void* newRawPtr;
   if (NS_FAILED(aQI(aIID, &newRawPtr))) {
     newRawPtr = nullptr;
   }
   assign_assuming_AddRef(static_cast<T*>(newRawPtr));
 }
 
 template<class T>
+template<typename U>
 void
-nsCOMPtr<T>::assign_from_qi_with_error(const nsQueryInterfaceWithError& aQI,
+nsCOMPtr<T>::assign_from_qi_with_error(const nsQueryInterfaceWithError<U>& aQI,
                                        const nsIID& aIID)
 {
+  static_assert(!(mozilla::IsSame<T, U>::value ||
+                  mozilla::IsBaseOf<T, U>::value),
+                "don't use do_QueryInterface for compile-time-determinable casts");
   void* newRawPtr;
   if (NS_FAILED(aQI(aIID, &newRawPtr))) {
     newRawPtr = nullptr;
   }
   assign_assuming_AddRef(static_cast<T*>(newRawPtr));
 }
 
 template<class T>