base: Fix unsigned underflow mishandling.

The second argument in the std::max call is treated as an unsigned value
as all variables are unsigned as well. This will result in an
unsigned underflow, and as such the std::max is a no-op and will result
in the underflowed value.

The `start` and `used` value get corrupted after that, and checks for
`empty` and other stuff downstream break.

Change-Id: I00017e22ba84e65f6b1c596f47d348f342fbc304
Reviewed-on: https://gem5-review.googlesource.com/c/public/gem5/+/39496
Reviewed-by: Gabe Black <gabe.black@gmail.com>
Maintainer: Gabe Black <gabe.black@gmail.com>
Tested-by: kokoro <noreply+kokoro@google.com>

diff --git a/src/base/circlebuf.hh b/src/base/circlebuf.hh
index bcfa91a2771b09cec1194cc9777e160f2cb2ae47..77a05d7d35a1850e8969ce23695acc6b0da3c607 100644 (file)
--- a/src/base/circlebuf.hh
+++ b/src/base/circlebuf.hh
@@ -167,7 +167,9 @@ class CircleBuf
         }
 
         // How much existing data will be overwritten?
-        const size_t overflow = std::max<size_t>(0, used + len - maxSize);
+        const size_t total_bytes = used + len;
+        const size_t overflow = total_bytes > maxSize ?
+                                total_bytes - maxSize : 0;
         // The iterator of the next byte to add.
         auto next_it = buffer.begin() + (start + used) % maxSize;
         // How much there is to copy to the end of the buffer.

diff --git a/src/base/circlebuf.test.cc b/src/base/circlebuf.test.cc
index 6b81b61ff92454382c25977059b84dbb96650f36..2e1f6bd183738dd1524084e43070e8b27ace54af 100644 (file)
--- a/src/base/circlebuf.test.cc
+++ b/src/base/circlebuf.test.cc
@@ -130,3 +130,22 @@ TEST(CircleBufTest, PointerWrapAround)
     EXPECT_EQ(buf.size(), 0);
     EXPECT_THAT(subArr(foo, 12), ElementsAreArray(data, 12));
 }
+
+// Consume after produce empties queue
+TEST(CircleBufTest, ProduceConsumeEmpty)
+{
+    CircleBuf<char> buf(8);
+    char foo[1];
+
+    // buf is empty to begin with.
+    EXPECT_TRUE(buf.empty());
+    // Produce one element.
+    buf.write(foo, 1);
+    EXPECT_FALSE(buf.empty());
+
+    // Read it back out.
+    buf.read(foo, 1);
+
+    // Now the buffer should be empty again.
+    EXPECT_TRUE(buf.empty());
+}