Re: [PATCH v5 1/1] memalign: Support scanning for aligned chunks.

[Xi Ruoyao <xry111@xry111.site>]

On Wed, 2023-04-12 at 13:16 -0400, DJ Delorie wrote:
> Xi Ruoyao <xry111@xry111.site> writes:
> > On LoongArch (with a GCC 12.2 but LoongArch backend patched to match
> > GCC
> > trunk), this causes an almost deterministic failure of malloc/tst-
> > malloc-thread-fail-malloc-hugetlb2:
> 
> (1) What's in malloc/tst-malloc-thread-fail-malloc-hugetlb2.out ?
> 
> (2) Have you tried with "export TIMEOUTFACTOR=20" ? some tests fail
>     reliably on slower systems due to timeouts.

No, it's not a timeout, but a segment fault.  The output is just "error:
exit status 139 from child process".

I'm not an expert with ptmalloc, but there is some code like:

  arena_get (ar_ptr, bytes + alignment + MINSIZE);

  p = _int_memalign (ar_ptr, alignment, bytes);
  if (!p && ar_ptr != NULL)
    {
      LIBC_PROBE (memory_memalign_retry, 2, bytes, alignment);
      ar_ptr = arena_get_retry (ar_ptr, bytes);
      p = _int_memalign (ar_ptr, alignment, bytes);
    }

arena_get can set ar_ptr to NULL (at least when the system memory is not
enough).

Then _int_memalign is dereferencing ar_ptr w/o any nullity check (bin_at
seems a "fancy" dereference operation to me).

Then we test ar_ptr != NULL in the if statement.

Now it looks like a notorious "NULL check after dereferencing" pattern.
So I added a nullity check:

diff --git a/malloc/malloc.c b/malloc/malloc.c
index 0315ac5d16..ed10b6b0e3 100644
--- a/malloc/malloc.c
+++ b/malloc/malloc.c
@@ -5025,7 +5025,7 @@ _int_memalign (mstate av, size_t alignment, size_t
bytes)
   mchunkptr victim;
 
   nb = checked_request2size (bytes);
-  if (nb == 0)
+  if (nb == 0 || !av)
     {
       __set_errno (ENOMEM);
       return NULL;

And it indeed fixed the test for me.  But I'm not sure if it's the
correct solution and I've not ran the complete test suite with the
change yet.


-- 
Xi Ruoyao <xry111@xry111.site>
School of Aerospace Science and Technology, Xidian University