| Message ID | xnv8icwr6j.fsf@greed.delorie.com |
|---|---|
| State | New |
| Headers | show |
| Series | [v1] malloc: set NON_MAIN_ARENA flag for reclaimed memalign chunk (BZ #30101) | expand |
* DJ Delorie via Libc-alpha: > From 61bd502ecac4d63f04c74bfc491ca675660d26b7 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. > > diff --git a/malloc/malloc.c b/malloc/malloc.c > index 0315ac5d16..66e7ca57dd 100644 > --- a/malloc/malloc.c > +++ b/malloc/malloc.c > @@ -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 > { The change looks reasonable. Can we add a test for this? Maybe run the existing memalign tests on a second thread as well? Thanks, Florian
On 4/3/23 18:12, DJ Delorie via Libc-alpha wrote: > > From 61bd502ecac4d63f04c74bfc491ca675660d26b7 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. > > diff --git a/malloc/malloc.c b/malloc/malloc.c > index 0315ac5d16..66e7ca57dd 100644 > --- a/malloc/malloc.c > +++ b/malloc/malloc.c > @@ -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); My preference is to: (a) Fix both cases where this happens. The other is here: 5199 /* Also give back spare room at the end */ 5200 if (!chunk_is_mmapped (p)) 5201 { 5202 size = chunksize (p); 5203 if ((unsigned long) (size) > (unsigned long) (nb + MINSIZE)) 5204 { 5205 remainder_size = size - nb; 5206 remainder = chunk_at_offset (p, nb); 5207 set_head (remainder, remainder_size | PREV_INUSE | 5208 (av != &main_arena ? NON_MAIN_ARENA : 0)); 5209 set_head_size (p, nb); 5210 _int_free (av, remainder, 1); 5211 } 5212 } (b) Remove the comment that says NON_MAIN_ARENA flag is never set, and adjust the comment to say it's always set. I want a *strong* invariant here that the chunks have their flags set correctly when placed into any of the lists, to do otherwise is incredibly confusing and is the root cause of the assertion triggering (very good of you to add it in the first place). > } > else > { >
diff --git a/malloc/malloc.c b/malloc/malloc.c index 0315ac5d16..66e7ca57dd 100644 --- a/malloc/malloc.c +++ b/malloc/malloc.c @@ -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 {
From 61bd502ecac4d63f04c74bfc491ca675660d26b7 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.