From: DJ Delorie <dj@redhat.com>
To: libc-alpha@sourceware.org
Subject: [patch v3] malloc: set NON_MAIN_ARENA flag for reclaimed memalign chunk (BZ #30101)
Date: Wed, 12 Apr 2023 00:00:15 -0400 [thread overview]
Message-ID: <xnleixyckw.fsf@greed.delorie.com> (raw)
In-Reply-To: <xnsfdfvz9y.fsf@greed.delorie.com>
changes since v2:
* Use rounded-up size in chunk_ok_for_memalign() to make sure size
checks pass later on.
From e7fca683c719cb6e1f9f4f47f76f1550c76d3c3c Mon Sep 17 00:00:00 2001
From: DJ Delorie <dj@redhat.com>
Date: Mon, 3 Apr 2023 17:33:03 -0400
Subject: malloc: set NON_MAIN_ARENA flag for reclaimed memalign chunk (BZ
#30101)
Based on these comments in malloc.c:
size field is or'ed with NON_MAIN_ARENA if the chunk was obtained
from a non-main arena. This is only set immediately before handing
the chunk to the user, if necessary.
The NON_MAIN_ARENA flag is never set for unsorted chunks, so it
does not have to be taken into account in size comparisons.
When we pull a chunk off the unsorted list (or any list) we need to
make sure that flag is set properly before returning the chunk.
Also, use the rounded-up size for chunk_ok_for_memalign()
diff --git a/malloc/Makefile b/malloc/Makefile
index f49675845e..e66247ed01 100644
--- a/malloc/Makefile
+++ b/malloc/Makefile
@@ -43,7 +43,8 @@ tests := mallocbug tst-malloc tst-valloc tst-calloc tst-obstack \
tst-tcfree1 tst-tcfree2 tst-tcfree3 \
tst-safe-linking \
tst-mallocalign1 \
- tst-memalign-2
+ tst-memalign-2 \
+ tst-memalign-3
tests-static := \
tst-interpose-static-nothread \
@@ -71,7 +72,7 @@ test-srcs = tst-mtrace
# with MALLOC_CHECK_=3 because they expect a specific failure.
tests-exclude-malloc-check = tst-malloc-check tst-malloc-usable \
tst-mxfast tst-safe-linking \
- tst-compathooks-off tst-compathooks-on tst-memalign-2
+ tst-compathooks-off tst-compathooks-on tst-memalign-2 tst-memalign-3
# Run all tests with MALLOC_CHECK_=3
tests-malloc-check = $(filter-out $(tests-exclude-malloc-check) \
diff --git a/malloc/malloc.c b/malloc/malloc.c
index 0315ac5d16..8ed2ec553b 100644
--- a/malloc/malloc.c
+++ b/malloc/malloc.c
@@ -5084,7 +5084,7 @@ _int_memalign (mstate av, size_t alignment, size_t bytes)
fwd = bck->fd;
while (fwd != bck)
{
- if (chunk_ok_for_memalign (fwd, alignment, bytes) > 0)
+ if (chunk_ok_for_memalign (fwd, alignment, nb) > 0)
{
victim = fwd;
@@ -5114,7 +5114,7 @@ _int_memalign (mstate av, size_t alignment, size_t bytes)
if (chunksize (fwd) < nb)
break;
- extra = chunk_ok_for_memalign (fwd, alignment, bytes);
+ extra = chunk_ok_for_memalign (fwd, alignment, nb);
if (extra > 0
&& (extra <= best_size || best == NULL))
{
@@ -5147,6 +5147,8 @@ _int_memalign (mstate av, size_t alignment, size_t bytes)
p = victim;
m = chunk2mem (p);
set_inuse (p);
+ if (av != &main_arena)
+ set_non_main_arena (p);
}
else
{
diff --git a/malloc/tst-memalign-2.c b/malloc/tst-memalign-2.c
index 4996578e9f..f229283dbf 100644
--- a/malloc/tst-memalign-2.c
+++ b/malloc/tst-memalign-2.c
@@ -33,9 +33,10 @@ typedef struct TestCase {
} TestCase;
static TestCase tcache_allocs[] = {
- { 24, 8, NULL, NULL },
- { 24, 16, NULL, NULL },
- { 128, 32, NULL, NULL }
+ { 24, 32, NULL, NULL },
+ { 24, 64, NULL, NULL },
+ { 128, 128, NULL, NULL },
+ { 500, 128, NULL, NULL }
};
#define TN array_length (tcache_allocs)
@@ -70,11 +71,15 @@ do_test (void)
for (i = 0; i < TN; ++ i)
{
+ size_t sz2;
+
tcache_allocs[i].ptr1 = memalign (tcache_allocs[i].alignment, tcache_allocs[i].size);
CHECK (tcache_allocs[i].ptr1, tcache_allocs[i].alignment);
+ sz2 = malloc_usable_size (tcache_allocs[i].ptr1);
free (tcache_allocs[i].ptr1);
+
/* This should return the same chunk as was just free'd. */
- tcache_allocs[i].ptr2 = memalign (tcache_allocs[i].alignment, tcache_allocs[i].size);
+ tcache_allocs[i].ptr2 = memalign (tcache_allocs[i].alignment, sz2);
CHECK (tcache_allocs[i].ptr2, tcache_allocs[i].alignment);
free (tcache_allocs[i].ptr2);
diff --git a/malloc/tst-memalign-3.c b/malloc/tst-memalign-3.c
new file mode 100644
index 0000000000..ab90d6ca9b
--- /dev/null
+++ b/malloc/tst-memalign-3.c
@@ -0,0 +1,173 @@
+/* Test for memalign chunk reuse.
+ Copyright (C) 2022 Free Software Foundation, Inc.
+ This file is part of the GNU C Library.
+
+ The GNU C Library is free software; you can redistribute it and/or
+ modify it under the terms of the GNU Lesser General Public
+ License as published by the Free Software Foundation; either
+ version 2.1 of the License, or (at your option) any later version.
+
+ The GNU C Library is distributed in the hope that it will be useful,
+ but WITHOUT ANY WARRANTY; without even the implied warranty of
+ MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ Lesser General Public License for more details.
+
+ You should have received a copy of the GNU Lesser General Public
+ License along with the GNU C Library; if not, see
+ <https://www.gnu.org/licenses/>. */
+
+#include <errno.h>
+#include <malloc.h>
+#include <stdio.h>
+#include <pthread.h>
+#include <string.h>
+#include <unistd.h>
+#include <array_length.h>
+#include <libc-pointer-arith.h>
+#include <support/check.h>
+#include <support/xthread.h>
+
+
+typedef struct TestCase {
+ size_t size;
+ size_t alignment;
+ void *ptr1;
+ void *ptr2;
+} TestCase;
+
+static TestCase tcache_allocs[] = {
+ { 24, 32, NULL, NULL },
+ { 24, 64, NULL, NULL },
+ { 128, 128, NULL, NULL },
+ { 500, 128, NULL, NULL }
+};
+#define TN array_length (tcache_allocs)
+
+static TestCase large_allocs[] = {
+ { 23450, 64, NULL, NULL },
+ { 23450, 64, NULL, NULL },
+ { 23550, 64, NULL, NULL },
+ { 23550, 64, NULL, NULL },
+ { 23650, 64, NULL, NULL },
+ { 23650, 64, NULL, NULL },
+ { 33650, 64, NULL, NULL },
+ { 33650, 64, NULL, NULL }
+};
+#define LN array_length (large_allocs)
+
+void *p;
+
+/* Sanity checks, ancillary to the actual test. */
+#define CHECK(p,a) \
+ if (p == NULL || !PTR_IS_ALIGNED (p, a)) \
+ FAIL_EXIT1 ("NULL or misaligned memory detected.\n");
+
+static void *
+mem_test (void *closure)
+{
+ int i;
+ int j;
+ int count;
+ void *ptr[10];
+ void *p;
+
+ /* TCache test. */
+ for (i = 0; i < TN; ++ i)
+ {
+ size_t sz2;
+
+ tcache_allocs[i].ptr1 = memalign (tcache_allocs[i].alignment, tcache_allocs[i].size);
+ CHECK (tcache_allocs[i].ptr1, tcache_allocs[i].alignment);
+ sz2 = malloc_usable_size (tcache_allocs[i].ptr1);
+ free (tcache_allocs[i].ptr1);
+
+ /* This should return the same chunk as was just free'd. */
+ tcache_allocs[i].ptr2 = memalign (tcache_allocs[i].alignment, sz2);
+ CHECK (tcache_allocs[i].ptr2, tcache_allocs[i].alignment);
+ free (tcache_allocs[i].ptr2);
+
+ TEST_VERIFY (tcache_allocs[i].ptr1 == tcache_allocs[i].ptr2);
+ }
+
+ /* Test for non-head tcache hits. */
+ for (i = 0; i < array_length (ptr); ++ i)
+ {
+ if (i == 4)
+ {
+ ptr[i] = memalign (64, 256);
+ CHECK (ptr[i], 64);
+ }
+ else
+ {
+ ptr[i] = malloc (256);
+ CHECK (ptr[i], 4);
+ }
+ }
+ for (i = 0; i < array_length (ptr); ++ i)
+ free (ptr[i]);
+
+ p = memalign (64, 256);
+ CHECK (p, 64);
+
+ count = 0;
+ for (i = 0; i < 10; ++ i)
+ if (ptr[i] == p)
+ ++ count;
+ free (p);
+ TEST_VERIFY (count > 0);
+
+ /* Large bins test. */
+
+ for (i = 0; i < LN; ++ i)
+ {
+ large_allocs[i].ptr1 = memalign (large_allocs[i].alignment, large_allocs[i].size);
+ CHECK (large_allocs[i].ptr1, large_allocs[i].alignment);
+ /* Keep chunks from combining by fragmenting the heap. */
+ p = malloc (512);
+ CHECK (p, 4);
+ }
+
+ for (i = 0; i < LN; ++ i)
+ free (large_allocs[i].ptr1);
+
+ /* Force the unsorted bins to be scanned and moved to small/large
+ bins. */
+ p = malloc (60000);
+
+ for (i = 0; i < LN; ++ i)
+ {
+ large_allocs[i].ptr2 = memalign (large_allocs[i].alignment, large_allocs[i].size);
+ CHECK (large_allocs[i].ptr2, large_allocs[i].alignment);
+ }
+
+ count = 0;
+ for (i = 0; i < LN; ++ i)
+ {
+ int ok = 0;
+ for (j = 0; j < LN; ++ j)
+ if (large_allocs[i].ptr1 == large_allocs[j].ptr2)
+ ok = 1;
+ if (ok == 1)
+ count ++;
+ }
+
+ /* The allocation algorithm is complicated outside of the memalign
+ logic, so just make sure it's working for most of the
+ allocations. This avoids possible boundary conditions with
+ empty/full heaps. */
+ TEST_VERIFY (count > LN / 2);
+
+ return 0;
+}
+
+static int
+do_test (void)
+{
+ pthread_t p;
+
+ p = xpthread_create (NULL, mem_test, NULL);
+ xpthread_join (p);
+ return 0;
+}
+
+#include <support/test-driver.c>
next prev parent reply other threads:[~2023-04-12 4:00 UTC|newest]
Thread overview: 10+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-04-03 22:12 [patch v1] " DJ Delorie
2023-04-04 10:26 ` Florian Weimer
2023-04-04 17:54 ` Carlos O'Donell
2023-04-05 2:27 ` [patch v2] " DJ Delorie
2023-04-05 6:14 ` Carlos O'Donell
2023-04-05 17:23 ` DJ Delorie
2023-04-06 17:09 ` Florian Weimer
2023-04-12 4:00 ` DJ Delorie [this message]
2023-04-12 13:11 ` [patch v3] " Cristian Rodríguez
2023-04-12 16:46 ` DJ Delorie
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=xnleixyckw.fsf@greed.delorie.com \
--to=dj@redhat.com \
--cc=libc-alpha@sourceware.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).