Skip to content

Commit cfbf5bf

Browse files
Abseil Teamdinord
authored andcommitted
Export of internal Abseil changes
-- 77e710b0ced5792a328e88bcb938a41484bf4cdc by Saleem Abdulrasool <[email protected]>: absl: add an implementation for UnscaledCycleClock on RISCV Add an implementation for UnscaledCycleClock on RISC-V targets. PiperOrigin-RevId: 395982312 -- 84430fce6760c488ca36401cd530f44268ac710d by Martijn Vels <[email protected]>: Harden CordRepBtreeReader against reading up to or beyond EOF This change hardens the reader to Next() calls on EOF situations. It changes the 'consumed()' property inside CordRepBtreeReader into a 'remaining()' property which is easier to understand and use than the 'consumed()' property QED the function documentation and use in cord.cc This change also adds the CharIterator test to the CordTest fixture enabling them to be run with btree cords. PiperOrigin-RevId: 395971732 -- 6557e628f2613169da8f693189223acb30e07833 by Martijn Vels <[email protected]>: Add AdvanceAndRead() test addressing the edge case surfaced in b/197776822 This adds a test explicitly exercising all possible AdvanceAndRead() calls on CharIterator. As per the linked bug, a partial or full small read ending exactly at the end of the last edge of a btree cord results in an attempt to read beyond that last edge and subsequent failure. We will fix the bug and enable these tests for btree in a subsequent change. PiperOrigin-RevId: 395958317 GitOrigin-RevId: 77e710b0ced5792a328e88bcb938a41484bf4cdc Change-Id: Ie6e21ce36980515165af7cf046cf199ecbe0ddb0
1 parent c22c032 commit cfbf5bf

File tree

6 files changed

+117
-70
lines changed

6 files changed

+117
-70
lines changed

absl/base/internal/unscaledcycleclock.cc

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -119,6 +119,18 @@ double UnscaledCycleClock::Frequency() {
119119
return aarch64_timer_frequency;
120120
}
121121

122+
#elif defined(__riscv)
123+
124+
int64_t UnscaledCycleClock::Now() {
125+
int64_t virtual_timer_value;
126+
asm volatile("rdcycle %0" : "=r"(virtual_timer_value));
127+
return virtual_timer_value;
128+
}
129+
130+
double UnscaledCycleClock::Frequency() {
131+
return base_internal::NominalCPUFrequency();
132+
}
133+
122134
#elif defined(_M_IX86) || defined(_M_X64)
123135

124136
#pragma intrinsic(__rdtsc)

absl/base/internal/unscaledcycleclock.h

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -46,8 +46,8 @@
4646

4747
// The following platforms have an implementation of a hardware counter.
4848
#if defined(__i386__) || defined(__x86_64__) || defined(__aarch64__) || \
49-
defined(__powerpc__) || defined(__ppc__) || \
50-
defined(_M_IX86) || defined(_M_X64)
49+
defined(__powerpc__) || defined(__ppc__) || defined(__riscv) || \
50+
defined(_M_IX86) || defined(_M_X64)
5151
#define ABSL_HAVE_UNSCALED_CYCLECLOCK_IMPLEMENTATION 1
5252
#else
5353
#define ABSL_HAVE_UNSCALED_CYCLECLOCK_IMPLEMENTATION 0
@@ -80,8 +80,8 @@
8080

8181
// This macro can be used to test if UnscaledCycleClock::Frequency()
8282
// is NominalCPUFrequency() on a particular platform.
83-
#if (defined(__i386__) || defined(__x86_64__) || \
84-
defined(_M_IX86) || defined(_M_X64))
83+
#if (defined(__i386__) || defined(__x86_64__) || defined(__riscv) || \
84+
defined(_M_IX86) || defined(_M_X64))
8585
#define ABSL_INTERNAL_UNSCALED_CYCLECLOCK_FREQUENCY_IS_CPU_FREQUENCY
8686
#endif
8787

absl/strings/cord_test.cc

Lines changed: 38 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1591,7 +1591,7 @@ TEST_P(CordTest, CordChunkIteratorOperations) {
15911591
VerifyChunkIterator(subcords, 128);
15921592
}
15931593

1594-
TEST(CordCharIterator, Traits) {
1594+
TEST_P(CordTest, CharIteratorTraits) {
15951595
static_assert(std::is_copy_constructible<absl::Cord::CharIterator>::value,
15961596
"");
15971597
static_assert(std::is_copy_assignable<absl::Cord::CharIterator>::value, "");
@@ -1700,7 +1700,7 @@ static void VerifyCharIterator(const absl::Cord& cord) {
17001700
}
17011701
}
17021702

1703-
TEST(CordCharIterator, Operations) {
1703+
TEST_P(CordTest, CharIteratorOperations) {
17041704
absl::Cord empty_cord;
17051705
VerifyCharIterator(empty_cord);
17061706

@@ -1729,6 +1729,41 @@ TEST(CordCharIterator, Operations) {
17291729
VerifyCharIterator(subcords);
17301730
}
17311731

1732+
TEST_P(CordTest, CharIteratorAdvanceAndRead) {
1733+
// Create a Cord holding 6 flats of 2500 bytes each, and then iterate over it
1734+
// reading 150, 1500, 2500 and 3000 bytes. This will result in all possible
1735+
// partial, full and straddled read combinations including reads below
1736+
// kMaxBytesToCopy. b/197776822 surfaced a bug for a specific partial, small
1737+
// read 'at end' on Cord which caused a failure on attempting to read past the
1738+
// end in CordRepBtreeReader which was not covered by any existing test.
1739+
constexpr int kBlocks = 6;
1740+
constexpr size_t kBlockSize = 2500;
1741+
constexpr size_t kChunkSize1 = 1500;
1742+
constexpr size_t kChunkSize2 = 2500;
1743+
constexpr size_t kChunkSize3 = 3000;
1744+
constexpr size_t kChunkSize4 = 150;
1745+
RandomEngine rng;
1746+
std::string data = RandomLowercaseString(&rng, kBlocks * kBlockSize);
1747+
absl::Cord cord;
1748+
for (int i = 0; i < kBlocks; ++i) {
1749+
const std::string block = data.substr(i * kBlockSize, kBlockSize);
1750+
cord.Append(absl::Cord(block));
1751+
}
1752+
1753+
for (size_t chunk_size :
1754+
{kChunkSize1, kChunkSize2, kChunkSize3, kChunkSize4}) {
1755+
absl::Cord::CharIterator it = cord.char_begin();
1756+
size_t offset = 0;
1757+
while (offset < data.length()) {
1758+
const size_t n = std::min<size_t>(data.length() - offset, chunk_size);
1759+
absl::Cord chunk = cord.AdvanceAndRead(&it, n);
1760+
ASSERT_EQ(chunk.size(), n);
1761+
ASSERT_EQ(chunk.Compare(data.substr(offset, n)), 0);
1762+
offset += n;
1763+
}
1764+
}
1765+
}
1766+
17321767
TEST_P(CordTest, StreamingOutput) {
17331768
absl::Cord c =
17341769
absl::MakeFragmentedCord({"A ", "small ", "fragmented ", "Cord", "."});
@@ -1778,7 +1813,7 @@ TEST_P(CordTest, Format) {
17781813
EXPECT_EQ(c, "There were 0003 little pigs.And 1 bad wolf!");
17791814
}
17801815

1781-
TEST(CordDeathTest, Hardening) {
1816+
TEST_P(CordTest, Hardening) {
17821817
absl::Cord cord("hello");
17831818
// These statement should abort the program in all builds modes.
17841819
EXPECT_DEATH_IF_SUPPORTED(cord.RemovePrefix(6), "");

absl/strings/internal/cord_rep_btree_reader.cc

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -52,14 +52,14 @@ absl::string_view CordRepBtreeReader::Read(size_t n, size_t chunk_size,
5252
// data, calling `navigator_.Current()` is not safe before checking if we
5353
// already consumed all remaining data.
5454
const size_t consumed_by_read = n - chunk_size - result.n;
55-
if (consumed_ + consumed_by_read >= length()) {
56-
consumed_ = length();
55+
if (consumed_by_read >= remaining_) {
56+
remaining_ = 0;
5757
return {};
5858
}
5959

6060
// We did not read all data, return remaining data from current edge.
6161
edge = navigator_.Current();
62-
consumed_ += consumed_by_read + edge->length;
62+
remaining_ -= consumed_by_read + edge->length;
6363
return CordRepBtree::EdgeData(edge).substr(result.n);
6464
}
6565

absl/strings/internal/cord_rep_btree_reader.h

Lines changed: 25 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -31,9 +31,7 @@ namespace cord_internal {
3131
// References to the underlying data are returned as absl::string_view values.
3232
// The most typical use case is a forward only iteration over tree data.
3333
// The class also provides `Skip()`, `Seek()` and `Read()` methods similar to
34-
// CordRepBtreeNavigator that allow more advanced navigation. The class provides
35-
// a `consumed` property which contains the end offset of the chunk last
36-
// returned to the user which is useful in cord iteration logic.
34+
// CordRepBtreeNavigator that allow more advanced navigation.
3735
//
3836
// Example: iterate over all data inside a cord btree:
3937
//
@@ -61,24 +59,22 @@ namespace cord_internal {
6159
// absl::string_view sv = reader.Next();
6260
// }
6361
//
64-
// It is important to notice that `consumed` represents the end position of the
65-
// last data edge returned to the caller, not the cumulative data returned to
66-
// the caller which can be less in cases of skipping or seeking over data.
62+
// It is important to notice that `remaining` is based on the end position of
63+
// the last data edge returned to the caller, not the cumulative data returned
64+
// to the caller which can be less in cases of skipping or seeking over data.
6765
//
6866
// For example, consider a cord btree with five data edges: "abc", "def", "ghi",
6967
// "jkl" and "mno":
7068
//
7169
// absl::string_view sv;
7270
// CordRepBtreeReader reader;
7371
//
74-
// sv = reader.Init(tree); // sv = "abc", reader.consumed() = 3
75-
// sv = reader.Skip(4); // sv = "hi", reader.consumed() = 9
76-
// sv = reader.Skip(2); // sv = "l", reader.consumed() = 12
77-
// sv = reader.Next(); // sv = "mno", reader.consumed() = 15
72+
// sv = reader.Init(tree); // sv = "abc", remaining = 12
73+
// sv = reader.Skip(4); // sv = "hi", remaining = 6
74+
// sv = reader.Skip(2); // sv = "l", remaining = 3
75+
// sv = reader.Next(); // sv = "mno", remaining = 0
76+
// sv = reader.Seek(1); // sv = "bc", remaining = 12
7877
//
79-
// In the above example, `reader.consumed()` reflects the data edges iterated
80-
// over or skipped by the reader, not the amount of data 'consumed' by the
81-
// caller.
8278
class CordRepBtreeReader {
8379
public:
8480
using ReadResult = CordRepBtreeNavigator::ReadResult;
@@ -98,13 +94,14 @@ class CordRepBtreeReader {
9894
// Requires that the current instance is not empty.
9995
size_t length() const;
10096

101-
// Returns the end offset of the last navigated to chunk, which represents the
102-
// total bytes 'consumed' relative to the start of the tree. The returned
103-
// value is never zero. For example, initializing a reader with a tree with a
104-
// first data edge of 19 bytes will return `consumed() = 19`. See also the
105-
// class comments on the meaning of `consumed`.
106-
// Requires that the current instance is not empty.
107-
size_t consumed() const;
97+
// Returns the number of remaining bytes available for iteration, which is the
98+
// number of bytes directly following the end of the last chunk returned.
99+
// This value will be zero if we iterated over the last edge in the bound
100+
// tree, in which case any call to Next() or Skip() will return an empty
101+
// string_view reflecting the EOF state.
102+
// Note that a call to `Seek()` resets `remaining` to a value based on the
103+
// end position of the chunk returned by that call.
104+
size_t remaining() const { return remaining_; }
108105

109106
// Resets this instance to an empty value.
110107
void Reset() { navigator_.Reset(); }
@@ -157,7 +154,7 @@ class CordRepBtreeReader {
157154
absl::string_view Seek(size_t offset);
158155

159156
private:
160-
size_t consumed_;
157+
size_t remaining_ = 0;
161158
CordRepBtreeNavigator navigator_;
162159
};
163160

@@ -166,23 +163,18 @@ inline size_t CordRepBtreeReader::length() const {
166163
return btree()->length;
167164
}
168165

169-
inline size_t CordRepBtreeReader::consumed() const {
170-
assert(btree() != nullptr);
171-
return consumed_;
172-
}
173-
174166
inline absl::string_view CordRepBtreeReader::Init(CordRepBtree* tree) {
175167
assert(tree != nullptr);
176168
const CordRep* edge = navigator_.InitFirst(tree);
177-
consumed_ = edge->length;
169+
remaining_ = tree->length - edge->length;;
178170
return CordRepBtree::EdgeData(edge);
179171
}
180172

181173
inline absl::string_view CordRepBtreeReader::Next() {
182-
assert(consumed() < length());
174+
if (remaining_ == 0) return {};
183175
const CordRep* edge = navigator_.Next();
184176
assert(edge != nullptr);
185-
consumed_ += edge->length;
177+
remaining_ -= edge->length;
186178
return CordRepBtree::EdgeData(edge);
187179
}
188180

@@ -192,23 +184,23 @@ inline absl::string_view CordRepBtreeReader::Skip(size_t skip) {
192184
const size_t edge_length = navigator_.Current()->length;
193185
CordRepBtreeNavigator::Position pos = navigator_.Skip(skip + edge_length);
194186
if (ABSL_PREDICT_FALSE(pos.edge == nullptr)) {
195-
consumed_ = length();
187+
remaining_ = 0;
196188
return {};
197189
}
198190
// The combined length of all edges skipped before `pos.edge` is `skip -
199191
// pos.offset`, all of which are 'consumed', as well as the current edge.
200-
consumed_ += skip - pos.offset + pos.edge->length;
192+
remaining_ -= skip - pos.offset + pos.edge->length;
201193
return CordRepBtree::EdgeData(pos.edge).substr(pos.offset);
202194
}
203195

204196
inline absl::string_view CordRepBtreeReader::Seek(size_t offset) {
205197
const CordRepBtreeNavigator::Position pos = navigator_.Seek(offset);
206198
if (ABSL_PREDICT_FALSE(pos.edge == nullptr)) {
207-
consumed_ = length();
199+
remaining_ = 0;
208200
return {};
209201
}
210202
absl::string_view chunk = CordRepBtree::EdgeData(pos.edge).substr(pos.offset);
211-
consumed_ = offset + chunk.length();
203+
remaining_ = length() - offset - chunk.length();
212204
return chunk;
213205
}
214206

0 commit comments

Comments
 (0)