From 0d9c2fc763dd766b868665a302ff4526748c4b36 Mon Sep 17 00:00:00 2001 From: Abseil Team Date: Tue, 2 Jul 2024 08:40:43 -0700 Subject: Replace signed integer overflow, since that's undefined behavior, with unsigned integer overflow. PiperOrigin-RevId: 648730502 Change-Id: I662c365c59be9e51f565fd215d284a96b7bd8743 --- absl/synchronization/internal/graphcycles.cc | 11 ++++++++++- absl/synchronization/internal/graphcycles.h | 5 +++++ absl/synchronization/internal/graphcycles_test.cc | 19 +++++++++++++++++++ 3 files changed, 34 insertions(+), 1 deletion(-) diff --git a/absl/synchronization/internal/graphcycles.cc b/absl/synchronization/internal/graphcycles.cc index 61b4ec05..129067c1 100644 --- a/absl/synchronization/internal/graphcycles.cc +++ b/absl/synchronization/internal/graphcycles.cc @@ -211,7 +211,7 @@ class NodeSet { Vec table_; uint32_t occupied_; // Count of non-empty slots (includes deleted slots) - static uint32_t Hash(int32_t a) { return static_cast(a * 41); } + static uint32_t Hash(int32_t a) { return static_cast(a) * 41; } // Return index for storing v. May return an empty index or deleted index uint32_t FindIndex(int32_t v) const { @@ -365,6 +365,14 @@ static Node* FindNode(GraphCycles::Rep* rep, GraphId id) { return (n->version == NodeVersion(id)) ? n : nullptr; } +void GraphCycles::TestOnlyAddNodes(uint32_t n) { + uint32_t old_size = rep_->nodes_.size(); + rep_->nodes_.resize(n); + for (auto i = old_size; i < n; ++i) { + rep_->nodes_[i] = nullptr; + } +} + GraphCycles::GraphCycles() { InitArenaIfNecessary(); rep_ = new (base_internal::LowLevelAlloc::AllocWithArena(sizeof(Rep), arena)) @@ -373,6 +381,7 @@ GraphCycles::GraphCycles() { GraphCycles::~GraphCycles() { for (auto* node : rep_->nodes_) { + if (node == nullptr) { continue; } node->Node::~Node(); base_internal::LowLevelAlloc::Free(node); } diff --git a/absl/synchronization/internal/graphcycles.h b/absl/synchronization/internal/graphcycles.h index ceba33e4..08f304bc 100644 --- a/absl/synchronization/internal/graphcycles.h +++ b/absl/synchronization/internal/graphcycles.h @@ -126,6 +126,11 @@ class GraphCycles { // Expensive: should only be called from graphcycles_test.cc. bool CheckInvariants() const; + // Test-only method to add more nodes. The nodes will not be valid, and this + // method should only be used to test the behavior of the graph when it is + // very full. + void TestOnlyAddNodes(uint32_t n); + // ---------------------------------------------------- struct Rep; private: diff --git a/absl/synchronization/internal/graphcycles_test.cc b/absl/synchronization/internal/graphcycles_test.cc index 3c6ef798..47410aad 100644 --- a/absl/synchronization/internal/graphcycles_test.cc +++ b/absl/synchronization/internal/graphcycles_test.cc @@ -14,6 +14,7 @@ #include "absl/synchronization/internal/graphcycles.h" +#include #include #include #include @@ -458,6 +459,24 @@ TEST_F(GraphCyclesTest, ManyEdges) { CheckInvariants(g_); } +TEST(GraphCycles, IntegerOverflow) { + GraphCycles graph_cycles; + char *buf = (char *)nullptr; + GraphId prev_id = graph_cycles.GetId(buf); + buf += 1; + GraphId id = graph_cycles.GetId(buf); + ASSERT_TRUE(graph_cycles.InsertEdge(prev_id, id)); + + // INT_MAX / 40 is enough to cause an overflow when multiplied by 41. + graph_cycles.TestOnlyAddNodes(INT_MAX / 40); + + buf += 1; + GraphId newid = graph_cycles.GetId(buf); + graph_cycles.HasEdge(prev_id, newid); + + graph_cycles.RemoveNode(buf); +} + } // namespace synchronization_internal ABSL_NAMESPACE_END } // namespace absl -- cgit v1.2.3