From 065c48527c462a1d1cc06717f762616d0fab6023 Mon Sep 17 00:00:00 2001
From: Martin Whitaker <icarus@martin-whitaker.me.uk>
Date: Fri, 28 Feb 2014 20:39:14 +0000
Subject: [PATCH] Fix for GitHub issue 19 : incorrect handling of large shift
 values.

For shift operations evaluated at compile time, the compiler was converting
the right operand to a native unsigned long value. If the operand exceeded
the size of an unsigned long, excess bits were discarded, which could lead
to an incorrect result.

The fix I've chosen is to add an as_unsigned() function to the verinum class
which returns the maximum unsigned value if the internal verinum value is
wider than the native unsigned type. This then naturally gives the correct
result for shifts, as the verinum bit width is also an unsigned value.

I've changed the as_ulong() and as_ulong64() functions to do likewise, as
this is more likely to either give the correct behaviour or to give some
indication that an overflow has occurred where these functions are used.
---
 eval_tree.cc |  2 +-
 verinum.cc   | 39 +++++++++++++++++++++++++++------------
 verinum.h    |  7 ++++++-
 3 files changed, 34 insertions(+), 14 deletions(-)

diff --git a/eval_tree.cc b/eval_tree.cc
index ac5d24ae4..ff224d909 100644
--- a/eval_tree.cc
+++ b/eval_tree.cc
@@ -1089,7 +1089,7 @@ NetEConst* NetEBShift::eval_arguments_(const NetExpr*l, const NetExpr*r) const
 
       verinum val;
       if (rv.is_defined()) {
-	    unsigned shift = rv.as_ulong();
+	    unsigned shift = rv.as_unsigned();
 
 	    switch (op_) {
 		case 'l':
diff --git a/verinum.cc b/verinum.cc
index 41efb99a4..250df8293 100644
--- a/verinum.cc
+++ b/verinum.cc
@@ -378,6 +378,25 @@ void verinum::set(unsigned off, const verinum&val)
 	    bits_[off+idx] = val[idx];
 }
 
+unsigned verinum::as_unsigned() const
+{
+      if (nbits_ == 0)
+	    return 0;
+
+      if (!is_defined())
+	    return 0;
+
+      unsigned val = 0;
+      unsigned mask = 1;
+      for (unsigned idx = 0 ;  idx < nbits_ ;  idx += 1, mask <<= 1)
+	    if (bits_[idx] == V1) {
+		  if (mask == 0) return ~mask;
+		  val |= mask;
+	    }
+
+      return val;
+}
+
 unsigned long verinum::as_ulong() const
 {
       if (nbits_ == 0)
@@ -386,15 +405,13 @@ unsigned long verinum::as_ulong() const
       if (!is_defined())
 	    return 0;
 
-      unsigned top = nbits_;
-      if (top >= (8 * sizeof(unsigned long)))
-	  top = 8 * sizeof(unsigned long);
-
       unsigned long val = 0;
       unsigned long mask = 1;
-      for (unsigned idx = 0 ;  idx < top ;  idx += 1, mask <<= 1)
-	    if (bits_[idx] == V1)
+      for (unsigned idx = 0 ;  idx < nbits_ ;  idx += 1, mask <<= 1)
+	    if (bits_[idx] == V1) {
+		  if (mask == 0) return ~mask;
 		  val |= mask;
+	    }
 
       return val;
 }
@@ -407,15 +424,13 @@ uint64_t verinum::as_ulong64() const
       if (!is_defined())
 	    return 0;
 
-      unsigned top = nbits_;
-      if (top >= (8 * sizeof(uint64_t)))
-	  top = 8 * sizeof(uint64_t);
-
       uint64_t val = 0;
       uint64_t mask = 1;
-      for (unsigned idx = 0 ;  idx < top ;  idx += 1, mask <<= 1)
-	    if (bits_[idx] == V1)
+      for (unsigned idx = 0 ;  idx < nbits_ ;  idx += 1, mask <<= 1)
+	    if (bits_[idx] == V1) {
+		  if (mask == 0) return ~mask;
 		  val |= mask;
+	    }
 
       return val;
 }
diff --git a/verinum.h b/verinum.h
index 41df9a90a..f6b3626c5 100644
--- a/verinum.h
+++ b/verinum.h
@@ -97,9 +97,14 @@ class verinum {
 
       V operator[] (unsigned idx) const { return get(idx); }
 
-
+	// Return the value as a native unsigned integer. If the value is
+	// larger than can be represented by the returned type, return
+	// the maximum value of that type. If the value has any x or z
+	// bits or has zero width, return the value 0.
       uint64_t as_ulong64() const;
+      unsigned as_unsigned() const;
       unsigned long as_ulong() const;
+
       signed long   as_long() const;
       double as_double() const;
       string as_string() const;