Fix NodeBuilder bug which could attempt to allocate beyond hard limit

diff --git a/src/expr/node_builder.h b/src/expr/node_builder.h
index f6aa1920d48ca66a93fc1768f606374e36719c24..458bd25faca2110738fe06ddc3d81cad8162f83e 100644 (file)
--- a/src/expr/node_builder.h
+++ b/src/expr/node_builder.h
@@ -1,11 +1,11 @@
 /*********************                                                        */
 /*! \file node_builder.h
  ** \verbatim
- ** Original author: mdeters
- ** Major contributors: dejan
- ** Minor contributors (to current version): taking, cconway
- ** This file is part of the CVC4 prototype.
- ** Copyright (c) 2009-2012  New York University and The University of Iowa
+ ** Original author: Dejan Jovanović <dejan.jovanovic@gmail.com>
+ ** Major contributors: Morgan Deters <mdeters@cs.nyu.edu>
+ ** Minor contributors (to current version): Tim King <taking@cs.nyu.edu>, Christopher L. Conway <christopherleeconway@gmail.com>
+ ** This file is part of the CVC4 project.
+ ** Copyright (c) 2009-2013  New York University and The University of Iowa
  ** See the file COPYING in the top-level source directory for licensing
  ** information.\endverbatim
  **
@@ -277,7 +277,9 @@ class NodeBuilder {
    * double-decremented later (on destruction/clear).
    */
   inline void realloc() {
-    realloc(d_nvMaxChildren * 2);
+    size_t newSize = 2 * size_t(d_nvMaxChildren);
+    size_t hardLimit = (1lu << __CVC4__EXPR__NODE_VALUE__NBITS__NCHILDREN) - 1;
+    realloc(EXPECT_FALSE( newSize > hardLimit ) ? hardLimit : newSize);
   }
 
   /**
@@ -662,6 +664,7 @@ public:
     expr::NodeValue* nv = n.d_nv;
     nv->inc();
     d_nv->d_children[d_nv->d_nchildren++] = nv;
+    Assert(d_nv->d_nchildren <= d_nvMaxChildren);
     return *this;
   }
 
@@ -674,6 +677,7 @@ public:
     expr::NodeValue* nv = typeNode.d_nv;
     nv->inc();
     d_nv->d_children[d_nv->d_nchildren++] = nv;
+    Assert(d_nv->d_nchildren <= d_nvMaxChildren);
     return *this;
   }
 
@@ -771,6 +775,9 @@ template <unsigned nchild_thresh>
 void NodeBuilder<nchild_thresh>::realloc(size_t toSize) {
   Assert( toSize > d_nvMaxChildren,
           "attempt to realloc() a NodeBuilder to a smaller/equal size!" );
+  Assert( toSize < (1lu << __CVC4__EXPR__NODE_VALUE__NBITS__NCHILDREN),
+          "attempt to realloc() a NodeBuilder to size %u (beyond hard limit of %u)",
+          toSize, (1lu << __CVC4__EXPR__NODE_VALUE__NBITS__NCHILDREN) - 1 );
 
   if(EXPECT_FALSE( nvIsAllocated() )) {
     // Ensure d_nv is not modified on allocation failure
@@ -783,6 +790,7 @@ void NodeBuilder<nchild_thresh>::realloc(size_t toSize) {
       throw std::bad_alloc();
     }
     d_nvMaxChildren = toSize;
+    Assert(d_nvMaxChildren == toSize);//overflow check
     // Here, the copy (between two heap-allocated buffers) has already
     // been done for us by the std::realloc().
     d_nv = newBlock;
@@ -795,6 +803,7 @@ void NodeBuilder<nchild_thresh>::realloc(size_t toSize) {
       throw std::bad_alloc();
     }
     d_nvMaxChildren = toSize;
+    Assert(d_nvMaxChildren == toSize);//overflow check
 
     d_nv = newBlock;
     d_nv->d_id = d_inlineNv.d_id;
@@ -1276,10 +1285,14 @@ void NodeBuilder<nchild_thresh>::internalCopy(const NodeBuilder<N>& nb) {
     return;
   }
 
+  bool realloced = false;
   if(nb.d_nvMaxChildren > d_nvMaxChildren) {
+    realloced = true;
     realloc(nb.d_nvMaxChildren);
   }
 
+  Assert(nb.d_nvMaxChildren <= d_nvMaxChildren);
+  Assert(nb.d_nv->nv_end() - nb.d_nv->nv_begin() <= d_nvMaxChildren, "realloced:%s, d_nvMax:%u, size:%u, nc:%u", realloced ? "true" : "false", d_nvMaxChildren, nb.d_nv->nv_end() - nb.d_nv->nv_begin(), nb.d_nv->getNumChildren());
   std::copy(nb.d_nv->nv_begin(),
             nb.d_nv->nv_end(),
             d_nv->nv_begin());

diff --git a/test/unit/expr/node_manager_white.h b/test/unit/expr/node_manager_white.h
index 3be95ad87f282167fa8fd2980578c57624b96be6..57fb8f445cfdbbd606d3876def0aa6530d0d5c05 100644 (file)
--- a/test/unit/expr/node_manager_white.h
+++ b/test/unit/expr/node_manager_white.h
@@ -54,4 +54,23 @@ public:
     Node m = d_nm->mkConst(i);
     TS_ASSERT_EQUALS(n.getId(), m.getId());
   }
+
+  void testOversizedNodeBuilder() {
+    NodeBuilder<> nb;
+
+    TS_ASSERT_THROWS_NOTHING(nb.realloc(15));
+    TS_ASSERT_THROWS_NOTHING(nb.realloc(25));
+    TS_ASSERT_THROWS_NOTHING(nb.realloc(256));
+#ifdef CVC4_ASSERTIONS
+    TS_ASSERT_THROWS(nb.realloc(100), AssertionException);
+#endif /* CVC4_ASSERTIONS */
+    TS_ASSERT_THROWS_NOTHING(nb.realloc(257));
+    TS_ASSERT_THROWS_NOTHING(nb.realloc(4000));
+    TS_ASSERT_THROWS_NOTHING(nb.realloc(20000));
+    TS_ASSERT_THROWS_NOTHING(nb.realloc(60000));
+    TS_ASSERT_THROWS_NOTHING(nb.realloc(65535));
+#ifdef CVC4_ASSERTIONS
+    TS_ASSERT_THROWS(nb.realloc(65536), AssertionException);
+#endif /* CVC4_ASSERTIONS */
+  }
 };