Philip Metzler has submitted this change. (
https://gem5-review.googlesource.com/c/public/gem5/+/39496 )
Change subject: base: Fix unsigned underflow mishandling.
......................................................................
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.bl...@gmail.com>
Maintainer: Gabe Black <gabe.bl...@gmail.com>
Tested-by: kokoro <noreply+kok...@google.com>
---
M src/base/circlebuf.hh
M src/base/circlebuf.test.cc
2 files changed, 22 insertions(+), 1 deletion(-)
Approvals:
Gabe Black: Looks good to me, approved; Looks good to me, approved
kokoro: Regressions pass
diff --git a/src/base/circlebuf.hh b/src/base/circlebuf.hh
index bcfa91a..77a05d7 100644
--- a/src/base/circlebuf.hh
+++ b/src/base/circlebuf.hh
@@ -167,7 +167,9 @@
}
// 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 6b81b61..2e1f6bd 100644
--- a/src/base/circlebuf.test.cc
+++ b/src/base/circlebuf.test.cc
@@ -130,3 +130,22 @@
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());
+}
--
To view, visit https://gem5-review.googlesource.com/c/public/gem5/+/39496
To unsubscribe, or for help writing mail filters, visit
https://gem5-review.googlesource.com/settings
Gerrit-Project: public/gem5
Gerrit-Branch: develop
Gerrit-Change-Id: I00017e22ba84e65f6b1c596f47d348f342fbc304
Gerrit-Change-Number: 39496
Gerrit-PatchSet: 3
Gerrit-Owner: Philip Metzler <cpm...@google.com>
Gerrit-Reviewer: Andreas Diavastos <diavas...@gmail.com>
Gerrit-Reviewer: Bobby R. Bruce <bbr...@ucdavis.edu>
Gerrit-Reviewer: Daniel Carvalho <oda...@yahoo.com.br>
Gerrit-Reviewer: Gabe Black <gabe.bl...@gmail.com>
Gerrit-Reviewer: Gabe Black <gabebl...@google.com>
Gerrit-Reviewer: Jason Lowe-Power <power...@gmail.com>
Gerrit-Reviewer: Philip Metzler <cpm...@google.com>
Gerrit-Reviewer: kokoro <noreply+kok...@google.com>
Gerrit-MessageType: merged
_______________________________________________
gem5-dev mailing list -- gem5-dev@gem5.org
To unsubscribe send an email to gem5-dev-le...@gem5.org
%(web_page_url)slistinfo%(cgiext)s/%(_internal_name)s
