Bug 506967 - Implement and override mallinfo2
Summary: Implement and override mallinfo2
Status: RESOLVED FIXED
Alias: None
Product: valgrind
Classification: Developer tools
Component: memcheck (other bugs)
Version First Reported In: 3.25.0
Platform: Other Linux
: NOR normal
Target Milestone: ---
Assignee: mcermak
URL:
Keywords:
Depends on:
Blocks: 506971
  Show dependency treegraph
 
Reported: 2025-07-12 20:15 UTC by Mark Wielaard
Modified: 2025-07-31 11:29 UTC (History)
2 users (show)

See Also:
Latest Commit:
Version Fixed In:
Sentry Crash Report:


Attachments
WIP patch (6.12 KB, patch)
2025-07-17 12:10 UTC, mcermak
Details
proposed patch (13.11 KB, patch)
2025-07-18 15:18 UTC, mcermak
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Mark Wielaard 2025-07-12 20:15:22 UTC
There is an implementation and override of mallinfo. See:
coregrind/m_mallocfree.c
coregrind/m_replacemalloc/vg_replace_malloc.c

mallinfo2 is the same, but the struct fields are size_t instead of int.

There are two LTP testcases, mallinfo02 and mallinfo2_01.

https://www.man7.org/linux/man-pages/man3/mallinfo.3.html
Comment 1 Paul Floyd 2025-07-15 09:46:26 UTC
Solaris has mallinfo which was fixed to use 64bit fields.

I don't think that FreeBSD has it by default. No idea about Darwin but probably the same as FreeBSD.
Comment 2 mcermak 2025-07-17 12:10:21 UTC
Created attachment 183301 [details]
WIP patch

I've attempted to model mallinfo2() after mallinfo().  My attached WIP patch does compile fine.  But running the following test:

> $ ./vg-in-place ./auxprogs/auxchecks/ltp-full-20250530/testcases/kernel/syscalls/mallinfo2/mallinfo2_01  |& fgrep overflow
> mallinfo2_01.c:35: TFAIL: hblkhd member of struct mallinfo2 overflow?
> $

Makes me think that the new override function mallinfo2() still somehow isn't properly wired in.

Without valgrind, the testcase does pass:

> $ ./auxprogs/auxchecks/ltp-full-20250530/testcases/kernel/syscalls/mallinfo2/mallinfo2_01 
> tst_test.c:1953: TINFO: LTP version: VALGRIND_3_25_0-117-gbd1e857cd
> tst_test.c:1956: TINFO: Tested kernel: 6.12.11-200.fc41.x86_64 #1 SMP PREEMPT_DYNAMIC Fri Jan 24 04:59:58 UTC 2025 x86_64
> tst_kconfig.c:88: TINFO: Parsing kernel config '/lib/modules/6.12.11-200.fc41.x86_64/config'
> tst_test.c:1774: TINFO: Overall timeout per run is 0h 00m 30s
> mallinfo2_01.c:37: TPASS: hblkhd member of struct mallinfo2 doesn't overflow
>
> Summary:
> passed   1
> failed   0
> broken   0
> skipped  0
> warnings 0
> $

Any thoughts?
Comment 3 Mark Wielaard 2025-07-17 13:16:18 UTC
(In reply to mcermak from comment #2)
> Created attachment 183301 [details]
> WIP patch
> 
> I've attempted to model mallinfo2() after mallinfo().  My attached WIP patch
> does compile fine.

Patch looks good, just one issue:

+/* This struct definition MUST match the system one. */
+/* SVID2/XPG mallinfo structure */
+#include <stddef.h> /* for size_t */
+struct vg_mallinfo2 {
+   size_t arena;    /* total space allocated from system */
+   size_t ordblks;  /* number of non-inuse chunks */
+   size_t smblks;   /* unused -- always zero */
+   size_t hblks;    /* number of mmapped regions */
+   size_t hblkhd;   /* total space in mmapped regions */
+   size_t usmblks;  /* unused -- always zero */
+   size_t fsmblks;  /* unused -- always zero */
+   size_t uordblks; /* total allocated space */
+   size_t fordblks; /* total non-inuse space */
+   size_t keepcost; /* top-most, releasable (via malloc_trim) space */
+};

Please don't #include <stddef.h> and use SizeT instead of size_t.
In general we try to not use libc or kernel headers but define our own types.

>  But running the following test:
> 
> > $ ./vg-in-place ./auxprogs/auxchecks/ltp-full-20250530/testcases/kernel/syscalls/mallinfo2/mallinfo2_01  |& fgrep overflow
> > mallinfo2_01.c:35: TFAIL: hblkhd member of struct mallinfo2 overflow?
> > $
> 
> Makes me think that the new override function mallinfo2() still somehow
> isn't properly wired in.
> 
> Any thoughts?

You comment explains I think:

+   // We don't have fastbins so smblks & fsmblks are always 0. Also we don't
+   // have a separate mmap allocator so set hblks & hblkhd to 0.

So the testcase tests something we don't support. It seems to test an glibc implementation detail.

Note that something similar is true for testcases/kernel/syscalls/mallinfo/mallinfo02
mallinfo02.c:41: TFAIL: malloc uses sbrk when size >= 128k
Which also testes info.hblks > 0.
Comment 4 mcermak 2025-07-18 15:18:07 UTC
Created attachment 183324 [details]
proposed patch

The attached patch tries to address the review notes.  It also adds a new testcase to the memcheck testsuite.  Finally, it excludes LTP tests that are not relevant, because they implement test scenario unsupported by valgrind.
Comment 5 Paul Floyd 2025-07-19 13:04:46 UTC
Looks OK.

I'll add illumos support when this lands.
Comment 6 Mark Wielaard 2025-07-31 11:26:40 UTC
Yes, looks good. I added a bug NEWS entry and pushed as:

commit ab551753fad6a87acbb8a87a80ed5f5578bfd29c
Author: Martin Cermak <mcermak@redhat.com>
Date:   Fri Jul 18 17:11:49 2025 +0200

    Implement and override mallinfo2
    
    Implement and override mallinfo2.  Add a testcase covering mallinfo2.
    Exclude irrelevant LTP tests trying to cover mallinfo2.
    
    https://bugs.kde.org/show_bug.cgi?id=506967