commit f2f108254546136b476af5dcfe77fa8120adc5bc
author Matthew Bentham <matthew.bentham@arm.com> Wed Jun 30 12:00:05 2021 +0100
committer Matthew Bentham <matthew.bentham@arm.com> Thu Jul 01 15:56:11 2021 +0000
tree f9d944ef67ff95481b8d7a7ee5cb5681bdb4d716
parent 3ab85485968c0e70d0378de3243119fb7b2f7e94

Fix undefined behaviour in PolymorphicDowncast

In the assertion part of PolymorphicDowncast (and PolymorphicPointerDowncast)
there was an unnecessary static_cast of the source pointer to the destination
type. This is unnecessary because the implicit conversion of the result pointer
back to the base type is sufficient to check for correctness of the downcast,
and potentially harmful because if the downcast is actually incorrect the
behaviour of static_cast is undefined (not actually known to be a problem
in any tested implementation).

Fixes warnings detected by ubsan.

Signed-off-by: Matthew Bentham <matthew.bentham@arm.com>
Change-Id: I9126288cbb06564009f94e57f6ca4688fc3b53c4

include/armnn/utility/PolymorphicDowncast.hpp

diff --git a/include/armnn/utility/PolymorphicDowncast.hpp b/include/armnn/utility/PolymorphicDowncast.hpp
index 8f05237..76b00fa 100644
--- a/include/armnn/utility/PolymorphicDowncast.hpp
+++ b/include/armnn/utility/PolymorphicDowncast.hpp
@@ -48,14 +48,14 @@
 
 // static_pointer_cast overload for raw pointers
 template<class T1, class T2>
-inline T1* StaticPointerCast(T2 *ptr)
+inline T1* StaticPointerCast(T2* ptr)
 {
     return static_cast<T1*>(ptr);
 }
 
 // dynamic_pointer_cast overload for raw pointers
 template<class T1, class T2>
-inline T1* DynamicPointerCast(T2 *ptr)
+inline T1* DynamicPointerCast(T2* ptr)
 {
     return dynamic_cast<T1*>(ptr);
 }
@@ -71,13 +71,12 @@
 /// \param value        Pointer to the source object
 /// \return             Pointer of type DestType (Pointer of type child)
 template<typename DestType, typename SourceType>
-DestType PolymorphicDowncast(SourceType value)
+DestType PolymorphicDowncast(SourceType* value)
 {
-    static_assert(std::is_pointer<SourceType>::value &&
-                  std::is_pointer<DestType>::value,
+    static_assert(std::is_pointer<DestType>::value,
                   "PolymorphicDowncast only works with pointer types.");
 
-    ARMNN_POLYMORPHIC_CAST_CHECK(dynamic_cast<DestType>(value) == static_cast<DestType>(value));
+    ARMNN_POLYMORPHIC_CAST_CHECK(dynamic_cast<DestType>(value) == value);
     return static_cast<DestType>(value);
 }
 
@@ -94,8 +93,8 @@
 auto PolymorphicPointerDowncast(const SourceType& value)
 {
     ARMNN_POLYMORPHIC_CAST_CHECK(utility::DynamicPointerCast<DestType>(value)
-                                 == utility::StaticPointerCast<DestType>(value));
+                                 == value);
     return utility::StaticPointerCast<DestType>(value);
 }
 
-} //namespace armnn
\ No newline at end of file
+} //namespace armnn