From 587d4663a339200a82b518d74c9400f4b77c8380 Mon Sep 17 00:00:00 2001 From: Timothy Flynn Date: Sat, 28 Aug 2021 11:19:21 -0400 Subject: [PATCH] AK: Return early from swap() when swapping the same object When swapping the same object, we could end up with a double-free error. This was found while quick-sorting a Vector of Variants holding complex types, reproduced by the new swap_same_complex_object test case. --- AK/StdLibExtras.h | 2 ++ Tests/AK/CMakeLists.txt | 1 + Tests/AK/TestStdLibExtras.cpp | 56 +++++++++++++++++++++++++++++++++++ 3 files changed, 59 insertions(+) create mode 100644 Tests/AK/TestStdLibExtras.cpp diff --git a/AK/StdLibExtras.h b/AK/StdLibExtras.h index d93ff3834c0..39263ece026 100644 --- a/AK/StdLibExtras.h +++ b/AK/StdLibExtras.h @@ -99,6 +99,8 @@ constexpr T ceil_div(T a, U b) template inline void swap(T& a, U& b) { + if (&a == &b) + return; U tmp = move((U&)a); a = (T &&) move(b); b = move(tmp); diff --git a/Tests/AK/CMakeLists.txt b/Tests/AK/CMakeLists.txt index 61cb63a14ef..ba5f1fb7c02 100644 --- a/Tests/AK/CMakeLists.txt +++ b/Tests/AK/CMakeLists.txt @@ -51,6 +51,7 @@ set(AK_TEST_SOURCES TestSourceLocation.cpp TestSpan.cpp TestStack.cpp + TestStdLibExtras.cpp TestString.cpp TestStringUtils.cpp TestStringView.cpp diff --git a/Tests/AK/TestStdLibExtras.cpp b/Tests/AK/TestStdLibExtras.cpp new file mode 100644 index 00000000000..6a371cfcacf --- /dev/null +++ b/Tests/AK/TestStdLibExtras.cpp @@ -0,0 +1,56 @@ +/* + * Copyright (c) 2021, Tim Flynn + * + * SPDX-License-Identifier: BSD-2-Clause + */ + +#include + +#include +#include +#include +#include +#include + +TEST_CASE(swap) +{ + int i = 4; + int j = 6; + + swap(i, j); + + EXPECT_EQ(i, 6); + EXPECT_EQ(j, 4); +} + +TEST_CASE(swap_same_value) +{ + + int i = 4; + swap(i, i); + EXPECT_EQ(i, 4); +} + +TEST_CASE(swap_same_complex_object) +{ + struct Type1 { + StringView foo; + }; + struct Type2 { + Optional foo; + Vector bar; + }; + + Variant value1 { Type1 { "hello"sv } }; + Variant value2 { Type2 { {}, { { "goodbye"sv } } } }; + + swap(value1, value2); + + EXPECT(value1.has()); + EXPECT(value2.has()); + + swap(value1, value1); + + EXPECT(value1.has()); + EXPECT(value2.has()); +}