Bug 867348 - Part 2: Apply MOZ_NO_ARITHMETIC_EXPR_IN_ARGUMENT to CheckedInt's constructor; r=jrmuizel,cpearce

Note that the analysis currently just looks at the AST subtree of the
function call site and is therefore unable to correctly deal with cases
such as the last two hunks of the change to OggCodecState.cpp.  Fixing
the analysis to deal with that would be very difficult, so we currently
adjust the code so that it compiles.  The first hunk in that file though
is a real bug that this analysis found.

dom/media/ogg/OggCodecState.cpp

@@ -936,7 +936,7 @@ int64_t OpusState::Time(int aPreSkip, int64_t aGranulepos)
     return -1;
 
   // Ogg Opus always runs at a granule rate of 48 kHz.
-  CheckedInt64 t = CheckedInt64(aGranulepos - aPreSkip) * USECS_PER_S;
+  CheckedInt64 t = (CheckedInt64(aGranulepos) - aPreSkip) * USECS_PER_S;
   return t.isValid() ? t.value() / 48000 : -1;
 }
 
@@ -1197,7 +1197,8 @@ bool SkeletonState::DecodeIndex(ogg_packet* aPacket)
   }
 
   // Extract the start time.
-  CheckedInt64 t = CheckedInt64(LittleEndian::readInt64(p + INDEX_FIRST_NUMER_OFFSET)) * USECS_PER_S;
+  int64_t timeRawInt = LittleEndian::readInt64(p + INDEX_FIRST_NUMER_OFFSET);
+  CheckedInt64 t = CheckedInt64(timeRawInt) * USECS_PER_S;
   if (!t.isValid()) {
     return (mActive = false);
   } else {
@@ -1205,7 +1206,8 @@ bool SkeletonState::DecodeIndex(ogg_packet* aPacket)
   }
 
   // Extract the end time.
-  t = LittleEndian::readInt64(p + INDEX_LAST_NUMER_OFFSET) * USECS_PER_S;
+  timeRawInt = LittleEndian::readInt64(p + INDEX_LAST_NUMER_OFFSET);
+  t = CheckedInt64(timeRawInt) * USECS_PER_S;
   if (!t.isValid()) {
     return (mActive = false);
   } else {

mfbt/Attributes.h

@@ -502,12 +502,15 @@
  *   are disallowed by default unless they are marked as MOZ_IMPLICIT. This
  *   attribute must be used for constructors which intend to provide implicit
  *   conversions.
+ * MOZ_NO_ARITHMETIC_EXPR_IN_ARGUMENT: Applies to functions. Makes it a compile
+ *   time error to path arithmetic expressions on variables to the function.
  */
 #ifdef MOZ_CLANG_PLUGIN
 #  define MOZ_MUST_OVERRIDE __attribute__((annotate("moz_must_override")))
 #  define MOZ_STACK_CLASS __attribute__((annotate("moz_stack_class")))
 #  define MOZ_NONHEAP_CLASS __attribute__((annotate("moz_nonheap_class")))
 #  define MOZ_IMPLICIT __attribute__((annotate("moz_implicit")))
+#  define MOZ_NO_ARITHMETIC_EXPR_IN_ARGUMENT __attribute__((annotate("moz_no_arith_expr_in_arg")))
 /*
  * It turns out that clang doesn't like void func() __attribute__ {} without a
  * warning, so use pragmas to disable the warning. This code won't work on GCC
@@ -523,6 +526,7 @@
 #  define MOZ_STACK_CLASS /* nothing */
 #  define MOZ_NONHEAP_CLASS /* nothing */
 #  define MOZ_IMPLICIT /* nothing */
+#  define MOZ_NO_ARITHMETIC_EXPR_IN_ARGUMENT /* nothing */
 #  define MOZ_HEAP_ALLOCATOR /* nothing */
 #endif /* MOZ_CLANG_PLUGIN */
 

mfbt/CheckedInt.h

@@ -11,6 +11,7 @@
 
 #include <stdint.h>
 #include "mozilla/Assertions.h"
+#include "mozilla/Attributes.h"
 #include "mozilla/IntegerTypeTraits.h"
 
 namespace mozilla {
@@ -525,7 +526,7 @@ public:
    * argument is valid.
    */
   template<typename U>
-  CheckedInt(U aValue)
+  CheckedInt(U aValue) MOZ_NO_ARITHMETIC_EXPR_IN_ARGUMENT
     : mValue(T(aValue)),
       mIsValid(detail::IsInRange<T>(aValue))
   {

mfbt/tests/TestCheckedInt.cpp

@@ -521,7 +521,7 @@ void test()
                                                     : sizeof(T) >= sizeof(U)); \
   }
   #define VERIFY_CONSTRUCTION_FROM_INTEGER_TYPE(U) \
-    VERIFY_CONSTRUCTION_FROM_INTEGER_TYPE2(U,U,+0) \
+    VERIFY_CONSTRUCTION_FROM_INTEGER_TYPE2(U,U,+zero) \
     VERIFY_CONSTRUCTION_FROM_INTEGER_TYPE2(U,CheckedInt<U>,.toChecked<T>())
 
   VERIFY_CONSTRUCTION_FROM_INTEGER_TYPE(int8_t)