From db168f7b75308bbc10e9571fd75cc34215fc8934 Mon Sep 17 00:00:00 2001 From: Denis Yaroshevskiy Date: Fri, 1 Dec 2023 11:16:48 -0800 Subject: [PATCH] memcpy empty string issue Summary: memcpying from a nullptr is a UB. UBSan caught it on one of the rollout tests This unfortunately has an overhead of an extra if but given that most strings are bigger than 8 chars, hopefully not a big one. Reviewed By: udippant, elalfer Differential Revision: D51754836 fbshipit-source-id: 50069195a3ac8121fc42f9310d43c49c17831917 --- mcrouter/lib/fbi/cpp/LowerBoundPrefixMap.h | 6 ++++-- mcrouter/lib/fbi/cpp/test/LowerBoundPrefixMapTest.cpp | 5 +++++ 2 files changed, 9 insertions(+), 2 deletions(-) diff --git a/mcrouter/lib/fbi/cpp/LowerBoundPrefixMap.h b/mcrouter/lib/fbi/cpp/LowerBoundPrefixMap.h index ffa887be5..d039296f2 100644 --- a/mcrouter/lib/fbi/cpp/LowerBoundPrefixMap.h +++ b/mcrouter/lib/fbi/cpp/LowerBoundPrefixMap.h @@ -40,8 +40,10 @@ class SmallPrefix { data_ = folly::Endian::swap(data_); return; } - std::memcpy(&data_, s.data(), s.size()); - data_ = folly::Endian::swap(data_); + if (FOLLY_LIKELY(s.data() != nullptr)) { + std::memcpy(&data_, s.data(), s.size()); + data_ = folly::Endian::swap(data_); + } } // default operator== and operator<=> are ok here diff --git a/mcrouter/lib/fbi/cpp/test/LowerBoundPrefixMapTest.cpp b/mcrouter/lib/fbi/cpp/test/LowerBoundPrefixMapTest.cpp index 6626a9a6d..31821da0d 100644 --- a/mcrouter/lib/fbi/cpp/test/LowerBoundPrefixMapTest.cpp +++ b/mcrouter/lib/fbi/cpp/test/LowerBoundPrefixMapTest.cpp @@ -205,5 +205,10 @@ TEST(LowerBoundPrefixMapTest, EmptyMap) { ASSERT_TRUE(lbMap.empty()); } +TEST(LowerBoundPrefixMapTest, NullKey) { + LowerBoundPrefixMap lbMap; + ASSERT_EQ(lbMap.findPrefix(std::string_view{}), lbMap.end()); +} + } // namespace } // namespace facebook::memcache