Message ID | xnr22tevec.fsf@greed.delorie.com |
---|---|
State | Committed |
Commit | ff12e0fb91b9072800f031cb21fb2651ee7b6251 |
Headers |
Received: (qmail 24426 invoked by alias); 30 Oct 2019 22:11:32 -0000 Mailing-List: contact libc-alpha-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: <libc-alpha.sourceware.org> List-Unsubscribe: <mailto:libc-alpha-unsubscribe-##L=##H@sourceware.org> List-Subscribe: <mailto:libc-alpha-subscribe@sourceware.org> List-Archive: <http://sourceware.org/ml/libc-alpha/> List-Post: <mailto:libc-alpha@sourceware.org> List-Help: <mailto:libc-alpha-help@sourceware.org>, <http://sourceware.org/ml/#faqs> Sender: libc-alpha-owner@sourceware.org Delivered-To: mailing list libc-alpha@sourceware.org Received: (qmail 24418 invoked by uid 89); 30 Oct 2019 22:11:32 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-17.8 required=5.0 tests=AWL, BAYES_00, GIT_PATCH_0, GIT_PATCH_1, GIT_PATCH_2, GIT_PATCH_3 autolearn=ham version=3.3.1 spammy= X-HELO: us-smtp-1.mimecast.com DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1572473488; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:content-type:content-type: content-transfer-encoding:content-transfer-encoding; bh=QJcU6WIiUIHIXSrvwhJE8KkeVEqrZyVB+2P57drfEGA=; b=SxOnF66J8lbdoOG53NPeaTZZ8hL/Av11LeUzWAHmfj3VOn5ZqjDVL31UxkYLauQ07tv4si DZVjgI0g+3viC1/1A+VHEZbllacGXRH72s9SNfKHQsNFjIozwhlhLczKYsP3tdSCyjYuqx jiBg18Krkkcmjms7nsTc48dpFFKG/zY= Date: Wed, 30 Oct 2019 18:11:23 -0400 Message-Id: <xnr22tevec.fsf@greed.delorie.com> From: DJ Delorie <dj@redhat.com> To: libc-alpha@sourceware.org Subject: [patch v1] malloc: fix set_max_fast "impossibly small" value X-Mimecast-Spam-Score: 0 Content-Type: text/plain; charset=WINDOWS-1252 Content-Transfer-Encoding: quoted-printable |
Commit Message
DJ Delorie
Oct. 30, 2019, 10:11 p.m. UTC
From 83035919da9208a7e6666bdde60e110e2e301553 Mon Sep 17 00:00:00 2001
From: DJ Delorie <dj@redhat.com>
Date: Wed, 30 Oct 2019 18:03:14 -0400
Subject: Base max_fast on alignment, not width, of bins
set_max_fast set the "impossibly small" value based on,
eventually, MALLOC_ALIGNMENT. The comparisons for the smallest
chunk used, eventuall, MIN_CHUNK_SIZE. Note that i386
is the only platform where these are the same, so a smallest
chunk *would* be put in a no-fastbins fastbin.
This change calculates the "impossibly small" value
based on MIN_CHUNK_SIZE instead, so that we can know it will
always be impossibly small.
Comments
On 10/30/19 6:11 PM, DJ Delorie wrote: > > From 83035919da9208a7e6666bdde60e110e2e301553 Mon Sep 17 00:00:00 2001 > From: DJ Delorie <dj@redhat.com> > Date: Wed, 30 Oct 2019 18:03:14 -0400 > Subject: Base max_fast on alignment, not width, of bins Does this fix bug 24903? If it does please add "(Bug 24903)" to the first line of your git commit. I assume this message is your git format-patch HEAD~1 output. OK for master with the following fixed: - Add "(Bug 24903)" to the git commit first line. - Apply both commit message changes. > set_max_fast set the "impossibly small" value based on, s/set/sets/g > eventually, MALLOC_ALIGNMENT. The comparisons for the smallest > chunk used, eventuall, MIN_CHUNK_SIZE. Note that i386 s/, eventuall/is, eventually/g > is the only platform where these are the same, so a smallest > chunk *would* be put in a no-fastbins fastbin. > > This change calculates the "impossibly small" value > based on MIN_CHUNK_SIZE instead, so that we can know it will > always be impossibly small. > > diff --git a/malloc/malloc.c b/malloc/malloc.c > index 5d3e82a8f6..70cc35a473 100644 > --- a/malloc/malloc.c > +++ b/malloc/malloc.c > @@ -1621,7 +1621,7 @@ static INTERNAL_SIZE_T global_max_fast; > > #define set_max_fast(s) \ > global_max_fast = (((s) == 0) \ > - ? SMALLBIN_WIDTH : ((s + SIZE_SZ) & ~MALLOC_ALIGN_MASK)) > + ? MIN_CHUNK_SIZE / 2 : ((s + SIZE_SZ) & ~MALLOC_ALIGN_MASK)) OK. My analysis: SMALLBIN_WIDTH is MALLOC_ALIGNMENT: 1403 #define SMALLBIN_WIDTH MALLOC_ALIGNMENT Which on i386 is overriden here: sysdeps/i386/malloc-alignment.h: 22 #define MALLOC_ALIGNMENT 16 This is because we have to account for 16-byte aligned vector types. This means SMALLBIN_WIDTH is 16, which for 32-bit targets is an aligned and perfectly viable chunk size. 1178 /* The smallest possible chunk */ 1179 #define MIN_CHUNK_SIZE (offsetof(struct malloc_chunk, fd_nextsize)) This is because MIN_CHUNK_SIZE is: 1048 struct malloc_chunk { 1049 1050 INTERNAL_SIZE_T mchunk_prev_size; /* Size of previous chunk (if free). */ 1051 INTERNAL_SIZE_T mchunk_size; /* Size in bytes, including overhead. */ 1052 1053 struct malloc_chunk* fd; /* double links -- used only if free. */ 1054 struct malloc_chunk* bk; 1055 1056 /* Only used for large blocks: pointer to next larger size. */ 1057 struct malloc_chunk* fd_nextsize; /* double links -- used only if free. */ 1058 struct malloc_chunk* bk_nextsize; 1059 }; The offsetof returns 16-bytes. On x86_64 we have SMALLBIN_WIDTH of 16, but the MIN_CHUNK_SIZE is 32, so it works. It's the increase in MALLOC_ALIGNMENT to 16 for i686 which breaks this. Your solution of MIN_CHUNK_SIZE/2 works. On i686 it yields 8, a size that is too small. On x86_64 it yields 16, a size that is too small. Thus it looks like the new calculation in set_max_fast yields an impossibly small value that we can use that no result of request2size() will return. > > static inline INTERNAL_SIZE_T > get_max_fast (void) >
On Wed, 30 Oct 2019, Carlos O'Donell wrote: > Which on i386 is overriden here: > sysdeps/i386/malloc-alignment.h: > 22 #define MALLOC_ALIGNMENT 16 > > This is because we have to account for 16-byte aligned vector types. Rather, to account for 16-byte-aligned _Float128 and _Decimal128. Vector types are expected to require aligned_alloc, but any type that's a basic type as defined in ISO C has a fundamental alignment and so malloc must allocate memory suitably aligned for it.
On 10/31/19 12:56 PM, Joseph Myers wrote: > On Wed, 30 Oct 2019, Carlos O'Donell wrote: > >> Which on i386 is overriden here: >> sysdeps/i386/malloc-alignment.h: >> 22 #define MALLOC_ALIGNMENT 16 >> >> This is because we have to account for 16-byte aligned vector types. > > Rather, to account for 16-byte-aligned _Float128 and _Decimal128. Vector > types are expected to require aligned_alloc, but any type that's a basic > type as defined in ISO C has a fundamental alignment and so malloc must > allocate memory suitably aligned for it. Thanks for the clarification.
diff --git a/malloc/malloc.c b/malloc/malloc.c index 5d3e82a8f6..70cc35a473 100644 --- a/malloc/malloc.c +++ b/malloc/malloc.c @@ -1621,7 +1621,7 @@ static INTERNAL_SIZE_T global_max_fast; #define set_max_fast(s) \ global_max_fast = (((s) == 0) \ - ? SMALLBIN_WIDTH : ((s + SIZE_SZ) & ~MALLOC_ALIGN_MASK)) + ? MIN_CHUNK_SIZE / 2 : ((s + SIZE_SZ) & ~MALLOC_ALIGN_MASK)) static inline INTERNAL_SIZE_T get_max_fast (void)