Backed out changesets acb4dd16755c and 40768f723990 (bug 867348) for static analysis bustage.
authorRyan VanderMeulen <ryanvm@gmail.com>
Thu, 18 Dec 2014 15:59:51 -0500
changeset 220384 29c50b8249238b436a17525ece13930cb92ac6ac
parent 220383 0f8c14c2cc1399a8371139cb91945deec001f419
child 220385 841573344c3f84db5b1782ad737d324c56eac2c0
push id53087
push userryanvm@gmail.com
push dateThu, 18 Dec 2014 20:59:58 +0000
treeherdermozilla-inbound@29c50b824923 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
bugs867348
milestone37.0a1
backs outacb4dd16755cc2c771268d05a40fa6190569a553
40768f72399053af7aca879d2fd7d5d93eeef8f7
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
Backed out changesets acb4dd16755c and 40768f723990 (bug 867348) for static analysis bustage. CLOSED TREE
build/clang-plugin/clang-plugin.cpp
build/clang-plugin/tests/TestNoArithmeticExprInArgument.cpp
build/clang-plugin/tests/moz.build
dom/media/ogg/OggCodecState.cpp
mfbt/Attributes.h
mfbt/CheckedInt.h
mfbt/tests/TestCheckedInt.cpp
--- a/build/clang-plugin/clang-plugin.cpp
+++ b/build/clang-plugin/clang-plugin.cpp
@@ -324,59 +324,16 @@ AST_MATCHER(QualType, nonheapClassAggreg
   return getClassAttrs(Node) == NonHeapClass;
 }
 
 /// This matcher will match any function declaration that is declared as a heap
 /// allocator.
 AST_MATCHER(FunctionDecl, heapAllocator) {
   return MozChecker::hasCustomAnnotation(&Node, "moz_heap_allocator");
 }
-
-/// This matcher will match any declaration that is marked as not accepting
-/// arithmetic expressions in its arguments.
-AST_MATCHER(Decl, noArithmeticExprInArgs) {
-  return MozChecker::hasCustomAnnotation(&Node, "moz_no_arith_expr_in_arg");
-}
-
-/// This matcher will match all arithmetic binary operators.
-AST_MATCHER(BinaryOperator, binaryArithmeticOperator) {
-  BinaryOperatorKind opcode = Node.getOpcode();
-  return opcode == BO_Mul ||
-         opcode == BO_Div ||
-         opcode == BO_Rem ||
-         opcode == BO_Add ||
-         opcode == BO_Sub ||
-         opcode == BO_Shl ||
-         opcode == BO_Shr ||
-         opcode == BO_And ||
-         opcode == BO_Xor ||
-         opcode == BO_Or ||
-         opcode == BO_MulAssign ||
-         opcode == BO_DivAssign ||
-         opcode == BO_RemAssign ||
-         opcode == BO_AddAssign ||
-         opcode == BO_SubAssign ||
-         opcode == BO_ShlAssign ||
-         opcode == BO_ShrAssign ||
-         opcode == BO_AndAssign ||
-         opcode == BO_XorAssign ||
-         opcode == BO_OrAssign;
-}
-
-/// This matcher will match all arithmetic unary operators.
-AST_MATCHER(UnaryOperator, unaryArithmeticOperator) {
-  UnaryOperatorKind opcode = Node.getOpcode();
-  return opcode == UO_PostInc ||
-         opcode == UO_PostDec ||
-         opcode == UO_PreInc ||
-         opcode == UO_PreDec ||
-         opcode == UO_Plus ||
-         opcode == UO_Minus ||
-         opcode == UO_Not;
-}
 }
 }
 
 namespace {
 
 bool isPlacementNew(const CXXNewExpr *expr) {
   // Regular new expressions aren't placement new
   if (expr->getNumPlacementArgs() == 0)
@@ -405,43 +362,16 @@ DiagnosticsMatcher::DiagnosticsMatcher()
   // Any heap allocation function that returns a non-heap or a stack class is
   // definitely doing something wrong
   astMatcher.addMatcher(callExpr(callee(functionDecl(allOf(heapAllocator(),
       returns(pointerType(pointee(nonheapClassAggregate()))))))).bind("node"),
     &nonheapClassChecker);
   astMatcher.addMatcher(callExpr(callee(functionDecl(allOf(heapAllocator(),
       returns(pointerType(pointee(stackClassAggregate()))))))).bind("node"),
     &stackClassChecker);
-
-  astMatcher.addMatcher(callExpr(allOf(hasDeclaration(noArithmeticExprInArgs()),
-          anyOf(
-              hasDescendant(binaryOperator(allOf(binaryArithmeticOperator(),
-                  hasLHS(hasDescendant(declRefExpr())),
-                  hasRHS(hasDescendant(declRefExpr()))
-              )).bind("node")),
-              hasDescendant(unaryOperator(allOf(unaryArithmeticOperator(),
-                  hasUnaryOperand(allOf(hasType(builtinType()),
-                                        anyOf(hasDescendant(declRefExpr()), declRefExpr())))
-              )).bind("node"))
-          )
-      )).bind("call"),
-    &arithmeticArgChecker);
-  astMatcher.addMatcher(constructExpr(allOf(hasDeclaration(noArithmeticExprInArgs()),
-          anyOf(
-              hasDescendant(binaryOperator(allOf(binaryArithmeticOperator(),
-                  hasLHS(hasDescendant(declRefExpr())),
-                  hasRHS(hasDescendant(declRefExpr()))
-              )).bind("node")),
-              hasDescendant(unaryOperator(allOf(unaryArithmeticOperator(),
-                  hasUnaryOperand(allOf(hasType(builtinType()),
-                                        anyOf(hasDescendant(declRefExpr()), declRefExpr())))
-              )).bind("node"))
-          )
-      )).bind("call"),
-    &arithmeticArgChecker);
 }
 
 void DiagnosticsMatcher::StackClassChecker::run(
     const MatchFinder::MatchResult &Result) {
   DiagnosticsEngine &Diag = Result.Context->getDiagnostics();
   unsigned stackID = Diag.getDiagnosticIDs()->getCustomDiagID(
     DiagnosticIDs::Error, "variable of type %0 only valid on the stack");
   if (const VarDecl *d = Result.Nodes.getNodeAs<VarDecl>("node")) {
@@ -537,29 +467,16 @@ void DiagnosticsMatcher::NonHeapClassChe
   } else if (const FieldDecl *FD = dyn_cast<FieldDecl>(cause)) {
     Diag.Report(FD->getLocation(), memberID) << T << FD << FD->getType();
   }
   
   // Recursively follow this back.
   noteInferred(cast<ValueDecl>(cause)->getType(), Diag);
 }
 
-void DiagnosticsMatcher::ArithmeticArgChecker::run(
-    const MatchFinder::MatchResult &Result) {
-  DiagnosticsEngine &Diag = Result.Context->getDiagnostics();
-  unsigned errorID = Diag.getDiagnosticIDs()->getCustomDiagID(
-      DiagnosticIDs::Error, "cannot pass an arithmetic expression of built-in types to %0");
-  const Expr *expr = Result.Nodes.getNodeAs<Expr>("node");
-  if (const CallExpr *call = Result.Nodes.getNodeAs<CallExpr>("call")) {
-    Diag.Report(expr->getLocStart(), errorID) << call->getDirectCallee();
-  } else if (const CXXConstructExpr *ctr = Result.Nodes.getNodeAs<CXXConstructExpr>("call")) {
-    Diag.Report(expr->getLocStart(), errorID) << ctr->getConstructor();
-  }
-}
-
 class MozCheckAction : public PluginASTAction {
 public:
   ASTConsumerPtr CreateASTConsumer(CompilerInstance &CI, StringRef fileName) override {
 #if CLANG_VERSION_FULL >= 306
     std::unique_ptr<MozChecker> checker(make_unique<MozChecker>(CI));
 
     std::vector<std::unique_ptr<ASTConsumer>> consumers;
     consumers.push_back(std::move(checker));
deleted file mode 100644
--- a/build/clang-plugin/tests/TestNoArithmeticExprInArgument.cpp
+++ /dev/null
@@ -1,32 +0,0 @@
-#define MOZ_NO_ARITHMETIC_EXPR_IN_ARGUMENT __attribute__((annotate("moz_no_arith_expr_in_arg")))
-
-struct X {
-  explicit X(int) MOZ_NO_ARITHMETIC_EXPR_IN_ARGUMENT;
-  void baz(int) MOZ_NO_ARITHMETIC_EXPR_IN_ARGUMENT;
-};
-
-int operator+(int, X);
-int operator+(X, int);
-int operator++(X);
-
-void badArithmeticsInArgs() {
-  int a;
-  typedef int myint;
-  myint b;
-  X goodObj1(a);
-  goodObj1.baz(b);
-  X badObj1(a + b); // expected-error{{cannot pass an arithmetic expression of built-in types to 'X'}}
-  X badObj2 = X(a ? 0 : ++a); // expected-error{{cannot pass an arithmetic expression of built-in types to 'X'}}
-  X badObj3(~a); // expected-error{{cannot pass an arithmetic expression of built-in types to 'X'}}
-  badObj1.baz(a - 1 - b); // expected-error{{cannot pass an arithmetic expression of built-in types to 'baz'}}
-  badObj1.baz(++a); // expected-error{{cannot pass an arithmetic expression of built-in types to 'baz'}}
-  badObj1.baz(a++); // expected-error{{cannot pass an arithmetic expression of built-in types to 'baz'}}
-  badObj1.baz(a || b);
-  badObj1.baz(a + goodObj1);
-  badObj1.baz(goodObj1 + a);
-  badObj1.baz(++goodObj1);
-  badObj1.baz(-1);
-  badObj1.baz(-1.0);
-  badObj1.baz(1 + 2);
-  badObj1.baz(1 << (sizeof(int)/2));
-}
--- a/build/clang-plugin/tests/moz.build
+++ b/build/clang-plugin/tests/moz.build
@@ -3,15 +3,14 @@
 # This Source Code Form is subject to the terms of the Mozilla Public
 # License, v. 2.0. If a copy of the MPL was not distributed with this
 # file, You can obtain one at http://mozilla.org/MPL/2.0/.
 
 SOURCES += [
     'TestBadImplicitConversionCtor.cpp',
     'TestCustomHeap.cpp',
     'TestMustOverride.cpp',
-    'TestNoArithmeticExprInArgument.cpp',
     'TestNonHeapClass.cpp',
     'TestStackClass.cpp',
 ]
 
 DISABLE_STL_WRAPPING = True
 NO_VISIBILITY_FLAGS = True
--- a/dom/media/ogg/OggCodecState.cpp
+++ b/dom/media/ogg/OggCodecState.cpp
@@ -931,17 +931,17 @@ int64_t OpusState::Time(int64_t aGranule
 }
 
 int64_t OpusState::Time(int aPreSkip, int64_t aGranulepos)
 {
   if (aGranulepos < 0)
     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;
 }
 
 bool OpusState::IsHeader(ogg_packet* aPacket)
 {
   return aPacket->bytes >= 16 &&
          (!memcmp(aPacket->packet, "OpusHead", 8) ||
           !memcmp(aPacket->packet, "OpusTags", 8));
@@ -1192,27 +1192,25 @@ bool SkeletonState::DecodeIndex(ogg_pack
   int64_t timeDenom = LittleEndian::readInt64(aPacket->packet + INDEX_TIME_DENOM_OFFSET);
   if (timeDenom == 0) {
     LOG(PR_LOG_DEBUG, ("Ogg Skeleton Index packet for stream %u has 0 "
                        "timestamp denominator.", serialno));
     return (mActive = false);
   }
 
   // Extract the start time.
-  int64_t timeRawInt = LittleEndian::readInt64(p + INDEX_FIRST_NUMER_OFFSET);
-  CheckedInt64 t = CheckedInt64(timeRawInt) * USECS_PER_S;
+  CheckedInt64 t = CheckedInt64(LittleEndian::readInt64(p + INDEX_FIRST_NUMER_OFFSET)) * USECS_PER_S;
   if (!t.isValid()) {
     return (mActive = false);
   } else {
     startTime = t.value() / timeDenom;
   }
 
   // Extract the end time.
-  timeRawInt = LittleEndian::readInt64(p + INDEX_LAST_NUMER_OFFSET);
-  t = CheckedInt64(timeRawInt) * USECS_PER_S;
+  t = LittleEndian::readInt64(p + INDEX_LAST_NUMER_OFFSET) * USECS_PER_S;
   if (!t.isValid()) {
     return (mActive = false);
   } else {
     endTime = t.value() / timeDenom;
   }
 
   // Check the numKeyPoints value read, ensure we're not going to run out of
   // memory while trying to decode the index packet.
--- a/mfbt/Attributes.h
+++ b/mfbt/Attributes.h
@@ -497,41 +497,37 @@
  *   need not be provided in such cases.
  * MOZ_HEAP_ALLOCATOR: Applies to any function. This indicates that the return
  *   value is allocated on the heap, and will as a result check such allocations
  *   during MOZ_STACK_CLASS and MOZ_NONHEAP_CLASS annotation checking.
  * MOZ_IMPLICIT: Applies to constructors. Implicit conversion constructors
  *   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
  * anyways, so the warning is safe to ignore.
  */
 #  define MOZ_HEAP_ALLOCATOR \
     _Pragma("clang diagnostic push") \
     _Pragma("clang diagnostic ignored \"-Wgcc-compat\"") \
     __attribute__((annotate("moz_heap_allocator"))) \
     _Pragma("clang diagnostic pop")
 #else
 #  define MOZ_MUST_OVERRIDE /* nothing */
 #  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 */
 
 /*
  * MOZ_THIS_IN_INITIALIZER_LIST is used to avoid a warning when we know that
  * it's safe to use 'this' in an initializer list.
  */
 #ifdef _MSC_VER
--- a/mfbt/CheckedInt.h
+++ b/mfbt/CheckedInt.h
@@ -6,17 +6,16 @@
 
 /* Provides checked integers, detecting integer overflow and divide-by-0. */
 
 #ifndef mozilla_CheckedInt_h
 #define mozilla_CheckedInt_h
 
 #include <stdint.h>
 #include "mozilla/Assertions.h"
-#include "mozilla/Attributes.h"
 #include "mozilla/IntegerTypeTraits.h"
 
 namespace mozilla {
 
 template<typename T> class CheckedInt;
 
 namespace detail {
 
@@ -521,17 +520,17 @@ public:
    *
    * This constructor is not explicit. Instead, the type of its argument is a
    * separate template parameter, ensuring that no conversion is performed
    * before this constructor is actually called. As explained in the above
    * documentation for class CheckedInt, this constructor checks that its
    * argument is valid.
    */
   template<typename U>
-  CheckedInt(U aValue) MOZ_NO_ARITHMETIC_EXPR_IN_ARGUMENT
+  CheckedInt(U aValue)
     : mValue(T(aValue)),
       mIsValid(detail::IsInRange<T>(aValue))
   {
     static_assert(detail::IsSupported<T>::value &&
                   detail::IsSupported<U>::value,
                   "This type is not supported by CheckedInt");
   }
 
--- a/mfbt/tests/TestCheckedInt.cpp
+++ b/mfbt/tests/TestCheckedInt.cpp
@@ -516,17 +516,17 @@ void test()
     VERIFY_IS_VALID_IF(CheckedInt<T>(MaxValue<U>::value), \
       (sizeof(T) > sizeof(U) || ((sizeof(T) == sizeof(U)) && (isUSigned || !isTSigned)))); \
     VERIFY_IS_VALID_IF(CheckedInt<T>(MinValue<U>::value), \
       isUSigned == false ? 1 \
                          : bool(isTSigned) == false ? 0 \
                                                     : sizeof(T) >= sizeof(U)); \
   }
   #define VERIFY_CONSTRUCTION_FROM_INTEGER_TYPE(U) \
-    VERIFY_CONSTRUCTION_FROM_INTEGER_TYPE2(U,U,+zero) \
+    VERIFY_CONSTRUCTION_FROM_INTEGER_TYPE2(U,U,+0) \
     VERIFY_CONSTRUCTION_FROM_INTEGER_TYPE2(U,CheckedInt<U>,.toChecked<T>())
 
   VERIFY_CONSTRUCTION_FROM_INTEGER_TYPE(int8_t)
   VERIFY_CONSTRUCTION_FROM_INTEGER_TYPE(uint8_t)
   VERIFY_CONSTRUCTION_FROM_INTEGER_TYPE(int16_t)
   VERIFY_CONSTRUCTION_FROM_INTEGER_TYPE(uint16_t)
   VERIFY_CONSTRUCTION_FROM_INTEGER_TYPE(int32_t)
   VERIFY_CONSTRUCTION_FROM_INTEGER_TYPE(uint32_t)