Skip to content

Commit 0d9c2fc

Browse files
Abseil Teamcopybara-github
authored andcommitted
Replace signed integer overflow, since that's undefined behavior, with unsigned integer overflow.
PiperOrigin-RevId: 648730502 Change-Id: I662c365c59be9e51f565fd215d284a96b7bd8743
1 parent f36d333 commit 0d9c2fc

File tree

3 files changed

+34
-1
lines changed

3 files changed

+34
-1
lines changed

absl/synchronization/internal/graphcycles.cc

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -211,7 +211,7 @@ class NodeSet {
211211
Vec<int32_t> table_;
212212
uint32_t occupied_; // Count of non-empty slots (includes deleted slots)
213213

214-
static uint32_t Hash(int32_t a) { return static_cast<uint32_t>(a * 41); }
214+
static uint32_t Hash(int32_t a) { return static_cast<uint32_t>(a) * 41; }
215215

216216
// Return index for storing v. May return an empty index or deleted index
217217
uint32_t FindIndex(int32_t v) const {
@@ -365,6 +365,14 @@ static Node* FindNode(GraphCycles::Rep* rep, GraphId id) {
365365
return (n->version == NodeVersion(id)) ? n : nullptr;
366366
}
367367

368+
void GraphCycles::TestOnlyAddNodes(uint32_t n) {
369+
uint32_t old_size = rep_->nodes_.size();
370+
rep_->nodes_.resize(n);
371+
for (auto i = old_size; i < n; ++i) {
372+
rep_->nodes_[i] = nullptr;
373+
}
374+
}
375+
368376
GraphCycles::GraphCycles() {
369377
InitArenaIfNecessary();
370378
rep_ = new (base_internal::LowLevelAlloc::AllocWithArena(sizeof(Rep), arena))
@@ -373,6 +381,7 @@ GraphCycles::GraphCycles() {
373381

374382
GraphCycles::~GraphCycles() {
375383
for (auto* node : rep_->nodes_) {
384+
if (node == nullptr) { continue; }
376385
node->Node::~Node();
377386
base_internal::LowLevelAlloc::Free(node);
378387
}

absl/synchronization/internal/graphcycles.h

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -126,6 +126,11 @@ class GraphCycles {
126126
// Expensive: should only be called from graphcycles_test.cc.
127127
bool CheckInvariants() const;
128128

129+
// Test-only method to add more nodes. The nodes will not be valid, and this
130+
// method should only be used to test the behavior of the graph when it is
131+
// very full.
132+
void TestOnlyAddNodes(uint32_t n);
133+
129134
// ----------------------------------------------------
130135
struct Rep;
131136
private:

absl/synchronization/internal/graphcycles_test.cc

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@
1414

1515
#include "absl/synchronization/internal/graphcycles.h"
1616

17+
#include <climits>
1718
#include <map>
1819
#include <random>
1920
#include <unordered_set>
@@ -458,6 +459,24 @@ TEST_F(GraphCyclesTest, ManyEdges) {
458459
CheckInvariants(g_);
459460
}
460461

462+
TEST(GraphCycles, IntegerOverflow) {
463+
GraphCycles graph_cycles;
464+
char *buf = (char *)nullptr;
465+
GraphId prev_id = graph_cycles.GetId(buf);
466+
buf += 1;
467+
GraphId id = graph_cycles.GetId(buf);
468+
ASSERT_TRUE(graph_cycles.InsertEdge(prev_id, id));
469+
470+
// INT_MAX / 40 is enough to cause an overflow when multiplied by 41.
471+
graph_cycles.TestOnlyAddNodes(INT_MAX / 40);
472+
473+
buf += 1;
474+
GraphId newid = graph_cycles.GetId(buf);
475+
graph_cycles.HasEdge(prev_id, newid);
476+
477+
graph_cycles.RemoveNode(buf);
478+
}
479+
461480
} // namespace synchronization_internal
462481
ABSL_NAMESPACE_END
463482
} // namespace absl

0 commit comments

Comments
 (0)