On Wed, Apr 05, 2023 at 10:54:19AM +0200, Otto Moerbeek wrote:

> Hi,
> 
> This is work in progress. I have to think if the flags to kdump I'm
> introducing should be two or a single one.
> 
> Currently, malloc.c can be compiled with MALLOC_STATS defined. If run
> with option D it dumps its state to a malloc.out file at exit. This
> state can be used to find leaks amongst other things.
> 
> This is not ideal for pledged processes, as they often have no way to
> write files.
> 
> This changes malloc to use utrace(2) for that.
> 
> As kdump has no nice way to show those lines without all extras it
> normally shows, so add two options to it to just show the lines.
> 
> To use, compile and install libc with MALLOC_STATS defined.
> 
> Run :
> 
> $ MALLOC_OPTIONS=D ktrace -tu your_program
> ...
> $ kdump -hu
> 
> Feedback appreciated.

Second iteration. Main difference: docs and the leak report is greatly
enhanced by adding info that can be fed to addr2line. Exmaple:

...
Leak report:
                 f     sum      #    avg
     0x9fd0eb58abb   40960     10   4096 addr2line -e ./a.out 0x1abb
     0x9fd0eb58ae9    8192      1   8192 addr2line -e ./a.out 0x1ae9
     0x9ffa92d9daf   65536      1  65536 addr2line -e /usr/lib/libc.so.97.0 
0x87daf

$ addr2line -e /usr/lib/libc.so.97.0 0x87daf
/usr/src/lib/libc/stdio/makebuf.c:62

Note that only the top caller is reported. I once wrote code to record
more stack frames, but I do not think I want that in. More elaborate
memory allocation analysis is better done by external tools or
instrumentation wrappers.

But even them, the current report is very useful IMO. It might be even
usefull enough to compile it always #ifndef SMALL. The runtime
overhead (if malloc option D is not used) is small. I'll be thinking
about possible reasons not to compile it in by default.

        -Otto

Index: lib/libc/stdlib/malloc.3
===================================================================
RCS file: /home/cvs/src/lib/libc/stdlib/malloc.3,v
retrieving revision 1.130
diff -u -p -r1.130 malloc.3
--- lib/libc/stdlib/malloc.3    1 Apr 2023 18:47:51 -0000       1.130
+++ lib/libc/stdlib/malloc.3    7 Apr 2023 07:43:22 -0000
@@ -284,10 +284,13 @@ If it has been corrupted, the process is
 .It Cm D
 .Dq Dump .
 .Fn malloc
-will dump statistics to the file
-.Pa ./malloc.out ,
-if it already exists,
+will dump statistics using
+.Xr utrace 2
 at exit.
+To record the dump:
+.Dl $ MALLOC_OPTIONS=D ktrace -tu program ...
+To view the dump:
+.Dl $ kdump -hu ...
 This option requires the library to have been compiled with -DMALLOC_STATS in
 order to have any effect.
 .It Cm F
Index: lib/libc/stdlib/malloc.c
===================================================================
RCS file: /home/cvs/src/lib/libc/stdlib/malloc.c,v
retrieving revision 1.280
diff -u -p -r1.280 malloc.c
--- lib/libc/stdlib/malloc.c    5 Apr 2023 06:25:38 -0000       1.280
+++ lib/libc/stdlib/malloc.c    7 Apr 2023 07:43:22 -0000
@@ -23,7 +23,7 @@
  * can buy me a beer in return. Poul-Henning Kamp
  */
 
-/* #define MALLOC_STATS */
+#define MALLOC_STATS
 
 #include <sys/types.h>
 #include <sys/queue.h>
@@ -39,8 +39,10 @@
 #include <unistd.h>
 
 #ifdef MALLOC_STATS
+#include <sys/types.h>
 #include <sys/tree.h>
-#include <fcntl.h>
+#include <sys/ktrace.h>
+#include <dlfcn.h>
 #endif
 
 #include "thread_private.h"
@@ -243,10 +245,8 @@ static __dead void wrterror(struct dir_i
     __attribute__((__format__ (printf, 2, 3)));
 
 #ifdef MALLOC_STATS
-void malloc_dump(int, int, struct dir_info *);
+void malloc_dump(void);
 PROTO_NORMAL(malloc_dump);
-void malloc_gdump(int);
-PROTO_NORMAL(malloc_gdump);
 static void malloc_exit(void);
 #define CALLER __builtin_return_address(0)
 #else
@@ -319,7 +319,7 @@ wrterror(struct dir_info *d, char *msg, 
 
 #ifdef MALLOC_STATS
        if (mopts.malloc_stats)
-               malloc_gdump(STDERR_FILENO);
+               malloc_dump();
 #endif /* MALLOC_STATS */
 
        errno = saved_errno;
@@ -2225,6 +2225,21 @@ aligned_alloc(size_t alignment, size_t s
 
 #ifdef MALLOC_STATS
 
+static void
+ulog(const char *format, ...)
+{
+       va_list ap;
+       char buf[100];
+       int len;
+
+       va_start(ap, format);
+       len = vsnprintf(buf, sizeof(buf), format, ap);
+       if (len > (int)sizeof(buf))
+               len = sizeof(buf);
+       utrace("mallocdumpline", buf, len);
+       va_end(ap);
+}
+
 struct malloc_leak {
        void *f;
        size_t total_size;
@@ -2280,21 +2295,32 @@ putleakinfo(void *f, size_t sz, int cnt)
 static struct malloc_leak *malloc_leaks;
 
 static void
-dump_leaks(int fd)
+dump_leaks(void)
 {
        struct leaknode *p;
        unsigned int i = 0;
 
-       dprintf(fd, "Leak report\n");
-       dprintf(fd, "                 f     sum      #    avg\n");
+       ulog("Leak report:\n");
+       ulog("                 f     sum      #    avg\n");
        /* XXX only one page of summary */
        if (malloc_leaks == NULL)
                malloc_leaks = MMAP(MALLOC_PAGESIZE, 0);
        if (malloc_leaks != MAP_FAILED)
                memset(malloc_leaks, 0, MALLOC_PAGESIZE);
        RBT_FOREACH(p, leaktree, &leakhead) {
-               dprintf(fd, "%18p %7zu %6u %6zu\n", p->d.f,
-                   p->d.total_size, p->d.count, p->d.total_size / p->d.count);
+               Dl_info info;
+               const char *caller = p->d.f;
+               const char *object = "";
+
+               if (caller != NULL) {
+                       if (dladdr(p->d.f, &info) != 0) {
+                               caller -= (uintptr_t)info.dli_fbase;
+                               object = info.dli_fname;
+                       }
+               }
+               ulog("%18p %7zu %6u %6zu addr2line -e %s %p\n", p->d.f,
+                   p->d.total_size, p->d.count, p->d.total_size / p->d.count,
+                   object, caller);
                if (malloc_leaks == MAP_FAILED ||
                    i >= MALLOC_PAGESIZE / sizeof(struct malloc_leak))
                        continue;
@@ -2306,10 +2332,10 @@ dump_leaks(int fd)
 }
 
 static void
-dump_chunk(int fd, struct chunk_info *p, void *f, int fromfreelist)
+dump_chunk(struct chunk_info *p, void *f, int fromfreelist)
 {
        while (p != NULL) {
-               dprintf(fd, "chunk %18p %18p %4zu %d/%d\n",
+               ulog("chunk %18p %18p %4zu %d/%d\n",
                    p->page, ((p->bits[0] & 1) ? NULL : f),
                    B2SIZE(p->bucket), p->free, p->total);
                if (!fromfreelist) {
@@ -2325,17 +2351,19 @@ dump_chunk(int fd, struct chunk_info *p,
                }
                p = LIST_NEXT(p, entries);
                if (p != NULL)
-                       dprintf(fd, "        ");
+                       ulog("       ->");
        }
 }
 
 static void
-dump_free_chunk_info(int fd, struct dir_info *d)
+dump_free_chunk_info(struct dir_info *d)
 {
        int i, j, count;
        struct chunk_info *p;
 
-       dprintf(fd, "Free chunk structs:\n");
+       ulog("Free chunk structs:\n");
+       ulog("Bkt) #CI                     page"
+           "                  f size free/n\n");
        for (i = 0; i <= BUCKETS; i++) {
                count = 0;
                LIST_FOREACH(p, &d->chunk_info_list[i], entries)
@@ -2345,99 +2373,98 @@ dump_free_chunk_info(int fd, struct dir_
                        if (p == NULL && count == 0)
                                continue;
                        if (j == 0)
-                               dprintf(fd, "%3d) %3d ", i, count);
+                               ulog("%3d) %3d ", i, count);
                        else
-                               dprintf(fd, "         ");
+                               ulog("         ");
                        if (p != NULL)
-                               dump_chunk(fd, p, NULL, 1);
+                               dump_chunk(p, NULL, 1);
                        else
-                               dprintf(fd, "\n");
+                               ulog(".\n");
                }
        }
 
 }
 
 static void
-dump_free_page_info(int fd, struct dir_info *d)
+dump_free_page_info(struct dir_info *d)
 {
        struct smallcache *cache;
        size_t i, total = 0;
 
-       dprintf(fd, "Cached in small cache:\n");
+       ulog("Cached in small cache:\n");
        for (i = 0; i < MAX_SMALLCACHEABLE_SIZE; i++) {
                cache = &d->smallcache[i];
                if (cache->length != 0)
-                       dprintf(fd, "%zu(%u): %u = %zu\n", i + 1, cache->max,
+                       ulog("%zu(%u): %u = %zu\n", i + 1, cache->max,
                            cache->length, cache->length * (i + 1));
                total += cache->length * (i + 1);
        }
 
-       dprintf(fd, "Cached in big cache: %zu/%zu\n", d->bigcache_used,
+       ulog("Cached in big cache: %zu/%zu\n", d->bigcache_used,
            d->bigcache_size);
        for (i = 0; i < d->bigcache_size; i++) {
                if (d->bigcache[i].psize != 0)
-                       dprintf(fd, "%zu: %zu\n", i, d->bigcache[i].psize);
+                       ulog("%zu: %zu\n", i, d->bigcache[i].psize);
                total += d->bigcache[i].psize;
        }
-       dprintf(fd, "Free pages cached: %zu\n", total);
+       ulog("Free pages cached: %zu\n", total);
 }
 
 static void
-malloc_dump1(int fd, int poolno, struct dir_info *d)
+malloc_dump1(int poolno, struct dir_info *d)
 {
        size_t i, realsize;
 
-       dprintf(fd, "Malloc dir of %s pool %d at %p\n", __progname, poolno, d);
+       ulog("Malloc dir of %s pool %d at %p\n", __progname, poolno, d);
        if (d == NULL)
                return;
-       dprintf(fd, "MT=%d J=%d Fl=%x\n", d->malloc_mt, d->malloc_junk, 
d->mmap_flag);
-       dprintf(fd, "Region slots free %zu/%zu\n",
+       ulog("MT=%d J=%d Fl=%x\n", d->malloc_mt, d->malloc_junk, d->mmap_flag);
+       ulog("Region slots free %zu/%zu\n",
                d->regions_free, d->regions_total);
-       dprintf(fd, "Finds %zu/%zu\n", d->finds, d->find_collisions);
-       dprintf(fd, "Inserts %zu/%zu\n", d->inserts, d->insert_collisions);
-       dprintf(fd, "Deletes %zu/%zu\n", d->deletes, d->delete_moves);
-       dprintf(fd, "Cheap reallocs %zu/%zu\n",
+       ulog("Finds %zu/%zu\n", d->finds, d->find_collisions);
+       ulog("Inserts %zu/%zu\n", d->inserts, d->insert_collisions);
+       ulog("Deletes %zu/%zu\n", d->deletes, d->delete_moves);
+       ulog("Cheap reallocs %zu/%zu\n",
            d->cheap_reallocs, d->cheap_realloc_tries);
-       dprintf(fd, "Other pool searches %zu/%zu\n",
+       ulog("Other pool searches %zu/%zu\n",
            d->other_pool, d->pool_searches);
-       dprintf(fd, "In use %zu\n", d->malloc_used);
-       dprintf(fd, "Guarded %zu\n", d->malloc_guarded);
-       dump_free_chunk_info(fd, d);
-       dump_free_page_info(fd, d);
-       dprintf(fd,
-           "slot)  hash d  type               page                  f "
+       ulog("In use %zu\n", d->malloc_used);
+       ulog("Guarded %zu\n", d->malloc_guarded);
+       dump_free_chunk_info(d);
+       dump_free_page_info(d);
+       ulog("Hash table:\n");
+       ulog("slot)  hash d  type               page                  f "
            "size [free/n]\n");
        for (i = 0; i < d->regions_total; i++) {
                if (d->r[i].p != NULL) {
                        size_t h = hash(d->r[i].p) &
                            (d->regions_total - 1);
-                       dprintf(fd, "%4zx) #%4zx %zd ",
+                       ulog("%4zx) #%4zx %zd ",
                            i, h, h - i);
                        REALSIZE(realsize, &d->r[i]);
                        if (realsize > MALLOC_MAXCHUNK) {
                                putleakinfo(d->r[i].f, realsize, 1);
-                               dprintf(fd,
-                                   "pages %18p %18p %zu\n", d->r[i].p,
+                               ulog("pages %18p %18p %zu\n", d->r[i].p,
                                    d->r[i].f, realsize);
                        } else
-                               dump_chunk(fd,
+                               dump_chunk(
                                    (struct chunk_info *)d->r[i].size,
                                    d->r[i].f, 0);
                }
        }
-       dump_leaks(fd);
-       dprintf(fd, "\n");
+       dump_leaks();
+       ulog("\n");
 }
 
-void
-malloc_dump(int fd, int poolno, struct dir_info *pool)
+static void
+malloc_dump0(int poolno, struct dir_info *pool)
 {
        int i;
        void *p;
        struct region_info *r;
        int saved_errno = errno;
 
-       if (pool == NULL)
+       if (pool == NULL || pool->r == NULL)
                return;
        for (i = 0; i < MALLOC_DELAYED_CHUNK_MASK + 1; i++) {
                p = pool->delayed_chunks[i];
@@ -2451,50 +2478,40 @@ malloc_dump(int fd, int poolno, struct d
        }
        /* XXX leak when run multiple times */
        RBT_INIT(leaktree, &leakhead);
-       malloc_dump1(fd, poolno, pool);
+       malloc_dump1(poolno, pool);
        errno = saved_errno;
 }
-DEF_WEAK(malloc_dump);
 
 void
-malloc_gdump(int fd)
+malloc_dump(void)
 {
        int i;
        int saved_errno = errno;
 
        for (i = 0; i < mopts.malloc_mutexes; i++)
-               malloc_dump(fd, i, mopts.malloc_pool[i]);
+               malloc_dump0(i, mopts.malloc_pool[i]);
 
        errno = saved_errno;
 }
-DEF_WEAK(malloc_gdump);
+DEF_WEAK(malloc_dump);
 
 static void
 malloc_exit(void)
 {
-       int save_errno = errno, fd;
-       unsigned i;
+       int save_errno = errno;
 
-       fd = open("malloc.out", O_RDWR|O_APPEND);
-       if (fd != -1) {
-               dprintf(fd, "******** Start dump %s *******\n", __progname);
-               dprintf(fd,
-                   "M=%u I=%d F=%d U=%d J=%d R=%d X=%d C=%d cache=%u "
-                   "G=%zu\n",
-                   mopts.malloc_mutexes,
-                   mopts.internal_funcs, mopts.malloc_freecheck,
-                   mopts.malloc_freeunmap, mopts.def_malloc_junk,
-                   mopts.malloc_realloc, mopts.malloc_xmalloc,
-                   mopts.chunk_canaries, mopts.def_maxcache,
-                   mopts.malloc_guard);
+       ulog("******** Start dump %s *******\n", __progname);
+       ulog("M=%u I=%d F=%d U=%d J=%d R=%d X=%d C=%d cache=%u "
+           "G=%zu\n",
+           mopts.malloc_mutexes,
+           mopts.internal_funcs, mopts.malloc_freecheck,
+           mopts.malloc_freeunmap, mopts.def_malloc_junk,
+           mopts.malloc_realloc, mopts.malloc_xmalloc,
+           mopts.chunk_canaries, mopts.def_maxcache,
+           mopts.malloc_guard);
 
-               for (i = 0; i < mopts.malloc_mutexes; i++)
-                       malloc_dump(fd, i, mopts.malloc_pool[i]);
-               dprintf(fd, "******** End dump %s *******\n", __progname);
-               close(fd);
-       } else
-               dprintf(STDERR_FILENO,
-                   "malloc() warning: Couldn't dump stats\n");
+       malloc_dump();
+       ulog("******** End dump %s *******\n", __progname);
        errno = save_errno;
 }
 
Index: usr.bin/kdump/kdump.1
===================================================================
RCS file: /home/cvs/src/usr.bin/kdump/kdump.1,v
retrieving revision 1.35
diff -u -p -r1.35 kdump.1
--- usr.bin/kdump/kdump.1       30 Jul 2022 07:19:30 -0000      1.35
+++ usr.bin/kdump/kdump.1       7 Apr 2023 07:43:22 -0000
@@ -66,6 +66,8 @@ Specifying
 will read from standard input.
 .It Fl H
 Display thread identifiers.
+.It Fl h
+Suppress header info.
 .It Fl l
 Loop reading the trace file, once the end-of-file is reached, waiting for
 more data.
@@ -106,6 +108,10 @@ See the
 option of
 .Xr ktrace 1
 for the meaning of the letters.
+.It Fl u
+Display I/O data as strings with
+.Xr vis 3
+escaping.
 .It Fl X
 Display I/O data with hexadecimal data and printable ASCII characters
 side by side.
Index: usr.bin/kdump/kdump.c
===================================================================
RCS file: /home/cvs/src/usr.bin/kdump/kdump.c,v
retrieving revision 1.156
diff -u -p -r1.156 kdump.c
--- usr.bin/kdump/kdump.c       17 Feb 2023 18:01:26 -0000      1.156
+++ usr.bin/kdump/kdump.c       7 Apr 2023 07:43:22 -0000
@@ -88,6 +88,8 @@ int needtid, tail, basecol;
 char *tracefile = DEF_TRACEFILE;
 struct ktr_header ktr_header;
 pid_t pid_opt = -1;
+int noheader;
+int utracelines;
 
 #define eqs(s1, s2)    (strcmp((s1), (s2)) == 0)
 
@@ -168,7 +170,7 @@ main(int argc, char *argv[])
                        screenwidth = 80;
        }
 
-       while ((ch = getopt(argc, argv, "f:dHlm:np:RTt:xX")) != -1)
+       while ((ch = getopt(argc, argv, "f:dhHlm:np:RTt:uxX")) != -1)
                switch (ch) {
                case 'f':
                        tracefile = optarg;
@@ -176,6 +178,9 @@ main(int argc, char *argv[])
                case 'd':
                        decimal = 1;
                        break;
+               case 'h':
+                       noheader = 1;
+                       break;
                case 'H':
                        needtid = 1;
                        break;
@@ -212,6 +217,9 @@ main(int argc, char *argv[])
                        if (trpoints < 0)
                                errx(1, "unknown trace point in %s", optarg);
                        break;
+               case 'u':
+                       utracelines = 1;
+                       break;
                case 'x':
                        iohex = 1;
                        break;
@@ -246,7 +254,7 @@ main(int argc, char *argv[])
                silent = 0;
                if (pid_opt != -1 && pid_opt != ktr_header.ktr_pid)
                        silent = 1;
-               if (silent == 0 && trpoints & (1<<ktr_header.ktr_type))
+               if (!noheader && silent == 0 && trpoints & 
(1<<ktr_header.ktr_type))
                        dumpheader(&ktr_header);
                ktrlen = ktr_header.ktr_len;
                if (ktrlen > size) {
@@ -1254,10 +1262,18 @@ showbufc(int col, unsigned char *dp, siz
 static void
 showbuf(unsigned char *dp, size_t datalen)
 {
-       int i, j;
+       size_t i, j;
        int col = 0, bpl;
        unsigned char c;
+       char visbuf[5];
 
+       if (utracelines) {
+               for (i = 0; i < datalen; i++) {
+                       vis(visbuf, dp[i], VIS_SAFE | VIS_OCTAL, 0);
+                       printf("%s", visbuf);
+               }
+               return;
+       }
        if (iohex == 1) {
                putchar('\t');
                col = 8;
@@ -1280,7 +1296,7 @@ showbuf(unsigned char *dp, size_t datale
                if (bpl <= 0)
                        bpl = 1;
                for (i = 0; i < datalen; i += bpl) {
-                       printf("   %04x:  ", i);
+                       printf("   %04zx:  ", i);
                        for (j = 0; j < bpl; j++) {
                                if (i+j >= datalen)
                                        printf("   ");
@@ -1413,8 +1429,10 @@ ktruser(struct ktr_user *usr, size_t len
        if (len < sizeof(struct ktr_user))
                errx(1, "invalid ktr user length %zu", len);
        len -= sizeof(struct ktr_user);
-       printf("%.*s:", KTR_USER_MAXIDLEN, usr->ktr_id);
-       printf(" %zu bytes\n", len);
+       if (!utracelines) {
+               printf("%.*s:", KTR_USER_MAXIDLEN, usr->ktr_id);
+               printf(" %zu bytes\n", len);
+       }
        showbuf((unsigned char *)(usr + 1), len);
 }
 

Reply via email to