On 14/11/18 10:31 +0100, Christophe Lyon wrote:
On Tue, 13 Nov 2018 at 23:58, Jonathan Wakely <jwak...@redhat.com> wrote:
Since a big_block rounds up the size to a multiple of big_block::min it
is wrong to assert that the supplied number of bytes equals the
big_block's size(). Add big_block::alloc_size(size_t) to calculate the
allocated size consistently, and add comments to the code.
* src/c++17/memory_resource.cc (big_block): Improve comments.
(big_block::all_ones): Remove.
(big_block::big_block(size_t, size_t)): Use alloc_size.
(big_block::size()): Add comment, replace all_ones with equivalent
expression.
(big_block::align()): Shift value of correct type.
(big_block::alloc_size(size_t)): New function to round up size.
(__pool_resource::allocate(size_t, size_t)): Add comment.
(__pool_resource::deallocate(void*, size_t, size_t)): Likewise. Fix
incorrect assertion by using big_block::alloc_size(size_t).
* testsuite/20_util/unsynchronized_pool_resource/allocate.cc: Add
more tests for unpooled allocations.
Hi Jonathan,
I've noticed that the updated test fails on arm*:
FAIL: 20_util/unsynchronized_pool_resource/allocate.cc execution test
the log says:
/libstdc++-v3/testsuite/20_util/unsynchronized_pool_resource/allocate.cc:232:
void test06(): Assertion 'false' failed.
The same happens on aarch64-elf with -mabi=ilp32
Should be fixed by this patch, committed to trunk.
commit b9aea25cc625b0ee3322d8185c2fc9354a800ebb
Author: Jonathan Wakely <jwak...@redhat.com>
Date: Wed Nov 14 20:23:58 2018 +0000
Fix test that does undefined shifts greater than width of size_t
* testsuite/20_util/unsynchronized_pool_resource/allocate.cc: Fix
test for 32-bit targets. Test additional allocation sizes.
diff --git a/libstdc++-v3/testsuite/20_util/unsynchronized_pool_resource/allocate.cc b/libstdc++-v3/testsuite/20_util/unsynchronized_pool_resource/allocate.cc
index 749655b63c7..0325a4358b6 100644
--- a/libstdc++-v3/testsuite/20_util/unsynchronized_pool_resource/allocate.cc
+++ b/libstdc++-v3/testsuite/20_util/unsynchronized_pool_resource/allocate.cc
@@ -170,7 +170,7 @@ test05()
void
test06()
{
- struct custom_mr : std::pmr::memory_resource
+ struct checking_mr : std::pmr::memory_resource
{
size_t expected_size = 0;
size_t expected_alignment = 0;
@@ -178,29 +178,30 @@ test06()
struct bad_size { };
struct bad_alignment { };
- void* do_allocate(std::size_t b, std::size_t a)
+ void* do_allocate(std::size_t bytes, std::size_t align)
{
- if (expected_size != 0)
- {
- if (b < expected_size)
- throw bad_size();
- else if (a != expected_alignment)
- throw bad_alignment();
- // Else just throw, don't try to allocate:
- throw std::bad_alloc();
- }
+ // Internal data structures in unsynchronized_pool_resource need to
+ // allocate memory, so handle those normally:
+ if (align <= alignof(std::max_align_t))
+ return std::pmr::new_delete_resource()->allocate(bytes, align);
- return std::pmr::new_delete_resource()->allocate(b, a);
+ // This is a large, unpooled allocation. Check the arguments:
+ if (bytes < expected_size)
+ throw bad_size();
+ else if (align != expected_alignment)
+ throw bad_alignment();
+ // Else just throw, don't really try to allocate:
+ throw std::bad_alloc();
}
- void do_deallocate(void* p, std::size_t b, std::size_t a)
- { std::pmr::new_delete_resource()->deallocate(p, b, a); }
+ void do_deallocate(void* p, std::size_t bytes, std::size_t align)
+ { std::pmr::new_delete_resource()->deallocate(p, bytes, align); }
bool do_is_equal(const memory_resource& r) const noexcept
{ return false; }
};
- custom_mr c;
+ checking_mr c;
std::pmr::unsynchronized_pool_resource r({1, 1}, &c);
std::pmr::pool_options opts = r.options();
const std::size_t largest_pool = opts.largest_required_pool_block;
@@ -214,23 +215,26 @@ test06()
// Try allocating various very large sizes and ensure the size requested
// from the upstream allocator is at least as large as needed.
- for (int i = 1; i < 64; ++i)
+ for (int i = 0; i < std::numeric_limits<std::size_t>::digits; ++i)
{
- for (auto b : { -1, 0, 1, 3 })
+ for (auto b : { -63, -5, -1, 0, 1, 3, std::numeric_limits<int>::max() })
{
std::size_t bytes = std::size_t(1) << i;
- bytes += b;
+ bytes += b; // For negative b this can wrap to a large positive value.
c.expected_size = bytes;
c.expected_alignment = large_alignment;
+ bool caught_bad_alloc = false;
try {
(void) r.allocate(bytes, large_alignment);
} catch (const std::bad_alloc&) {
// expect to catch bad_alloc
- } catch (custom_mr::bad_size) {
- VERIFY(false);
- } catch (custom_mr::bad_alignment) {
- VERIFY(false);
+ caught_bad_alloc = true;
+ } catch (checking_mr::bad_size) {
+ VERIFY( ! "allocation from upstream resource had expected size" );
+ } catch (checking_mr::bad_alignment) {
+ VERIFY( ! "allocation from upstream resource had expected alignment" );
}
+ VERIFY( caught_bad_alloc );
}
}
}
