Skip to content

Conversation

vitalybuka
Copy link
Collaborator

@vitalybuka vitalybuka commented Sep 27, 2024

This is regression test for #90292.

Allocator used in test is very similar to test_allocator.
However, reproducer requires size_type of the string
to be 64bit, but test_allocator uses 32bit.

32bit size_type makes sizeof(string::__long) to be 16,
but the alignment issue fixed with #90292 is only triggered
with default sizeof(string::__long) which is 24.

Fixes #92128.

Created using spr 1.3.4
@vitalybuka vitalybuka requested a review from a team as a code owner September 27, 2024 06:09
@llvmbot llvmbot added the libc++ libc++ C++ Standard Library. Not GNU libstdc++. Not libc++abi. label Sep 27, 2024
@vitalybuka vitalybuka requested a review from ldionne September 27, 2024 06:09
@llvmbot
Copy link
Member

llvmbot commented Sep 27, 2024

@llvm/pr-subscribers-libcxx

Author: Vitaly Buka (vitalybuka)

Changes

This is regression test for #90292.

Allocator used in test is very similar to
test_allocator. However reproducer requires
size_type of the string to be 64bit, and
test_allocator has 32bit.

Fixes #92128.


Full diff: https://github.com/llvm/llvm-project/pull/110210.diff

1 Files Affected:

  • (added) libcxx/test/std/strings/basic.string/string.capacity/deallocate_size.pass.cpp (+63)
diff --git a/libcxx/test/std/strings/basic.string/string.capacity/deallocate_size.pass.cpp b/libcxx/test/std/strings/basic.string/string.capacity/deallocate_size.pass.cpp
new file mode 100644
index 00000000000000..612b66195141c5
--- /dev/null
+++ b/libcxx/test/std/strings/basic.string/string.capacity/deallocate_size.pass.cpp
@@ -0,0 +1,63 @@
+//===----------------------------------------------------------------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+// <string>
+
+// allocate/deallocate must match.
+
+#include <string>
+#include <cassert>
+#include <type_traits>
+
+#include "test_macros.h"
+
+static int allocated_;
+
+template <class T, class Sz>
+struct test_alloc {
+  typedef Sz size_type;
+  typedef std::make_signed<Sz>::type difference_type;
+  typedef T value_type;
+  typedef value_type* pointer;
+  typedef const value_type* const_pointer;
+  typedef typename std::add_lvalue_reference<value_type>::type reference;
+  typedef typename std::add_lvalue_reference<const value_type>::type const_reference;
+
+  template <class U>
+  struct rebind {
+    typedef test_alloc<U, Sz> other;
+  };
+
+  TEST_CONSTEXPR_CXX14 pointer allocate(size_type n, const void* = nullptr) {
+    allocated_ += n;
+    return std::allocator<value_type>().allocate(n);
+  }
+
+  TEST_CONSTEXPR_CXX14 void deallocate(pointer p, size_type s) {
+    allocated_ -= s;
+    std::allocator<value_type>().deallocate(p, s);
+  }
+};
+
+template <class Sz>
+void test() {
+  for (int i = 1; i < 1000; ++i) {
+    using Str = std::basic_string<char, std::char_traits<char>, test_alloc<char, Sz> >;
+    {
+      Str s(i, 't');
+      assert(allocated_ == 0 || allocated_ >= i);
+    }
+  }
+  assert(allocated_ == 0);
+}
+
+int main(int, char**) {
+  test<uint32_t>();
+  test<uint64_t>();
+  test<size_t>();
+}

Created using spr 1.3.4
Created using spr 1.3.4
Copy link
Member

@ldionne ldionne left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM with nitpicks, thanks!

Copy link

github-actions bot commented Sep 30, 2024

✅ With the latest revision this PR passed the C/C++ code formatter.

@vitalybuka vitalybuka merged commit eea5e7e into main Oct 1, 2024
64 checks passed
@vitalybuka vitalybuka deleted the users/vitalybuka/spr/libcstring-add-regression-test-for-sized-newdelete-bug branch October 1, 2024 05:29
Sterling-Augustine pushed a commit to Sterling-Augustine/llvm-project that referenced this pull request Oct 3, 2024
…10210)

This is regression test for llvm#90292.

Allocator used in test is very similar to test_allocator.
However, reproducer requires size_type of the string
to be 64bit, but test_allocator uses 32bit.

32bit size_type makes `sizeof(string::__long)` to be 16,
but the alignment issue fixed with llvm#90292 is only triggered
with default `sizeof(string::__long)` which is 24.

Fixes llvm#92128.

---------

Co-authored-by: Louis Dionne <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
libc++ libc++ C++ Standard Library. Not GNU libstdc++. Not libc++abi.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add tests for #90292
3 participants