From: Philip Metzler <cpmetz@google.com>
Date: Wed, 20 Jan 2021 20:59:48 +0000 (-0800)
Subject: base: Fix unsigned underflow mishandling.
X-Git-Tag: develop-gem5-snapshot~263
X-Git-Url: https://git.libre-soc.org/?a=commitdiff_plain;h=927cd10cb48c73e5a4a092ec42d36674c73892fd;p=gem5.git

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 bcfa91a27..77a05d7d3 100644
--- 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 6b81b61ff..2e1f6bd18 100644
--- 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());
+}