On Mon, Jun 26, 2023 at 04:53:45PM +0000, Klemens Nanni wrote:
> On Mon, Jun 26, 2023 at 05:24:53PM +0200, Claudio Jeker wrote:
> > I created this simple btrace script to help find malloc(9) leaks but
> > it did not work. First step was adding kstack support to the map
> > implementation. But then it still did not work because btrace did not
> > enable the kstack reporting for the probe. This turned out to be an
> > issue with the if () condition I added to filter out uninteresting events.
> > The problem is that the statements in the if block are not scanned for
> > possible extra probe arguments. The below diff fixes all of that.
> >
> > Here is the btrace script:
> > tracepoint:uvm:malloc {
> > if (arg0 == 127 && arg2 <= 64) {
> > @mem[arg1] = kstack
> > }
> > }
> > tracepoint:uvm:free {
> > if (arg0 == 127 && arg2 <= 64) {
> > delete(@mem[arg1])
> > }
> > }
> > END {
> > printf("Possible memory leaks\n");
> > print(@mem)
> > }
> >
> > --
> > :wq Claudio
> >
> > Index: bt.5
> > ===================================================================
> > RCS file: /cvs/src/usr.sbin/btrace/bt.5,v
> > retrieving revision 1.14
> > diff -u -p -r1.14 bt.5
> > --- bt.5 31 Mar 2022 17:27:29 -0000 1.14
> > +++ bt.5 26 Jun 2023 15:16:25 -0000
> > @@ -120,6 +120,11 @@ Functions:
> > .It Fn clear "@map"
> > Delete all (key, value) pairs from
> > .Va @map .
> > +.It "@map[key]" = Fn count
> > +Increment the value of
> > +.Va key
> > +in
> > +.Va @map .
>
> Would it make more sense to document count() like min() and max()?
> Then it doesn't look like a special case.
>
> Looks like min/max functions can be used in map key assignments as well,
> although I'm not super familar with btrace internals yet.
count() is strange since it only works on maps (at least from what I
figured out). I need to double check how min() and max() work. Since the
usage also seems non-intuitive.
I find the documentation of bt(5) rather weak. So more is needed for sure.
> > .It Fn delete "@map[key]"
> > Delete the pair indexed by
> > .Va key
> > Index: btrace.c
> > ===================================================================
> > RCS file: /cvs/src/usr.sbin/btrace/btrace.c,v
> > retrieving revision 1.70
> > diff -u -p -r1.70 btrace.c
> > --- btrace.c 12 May 2023 14:14:16 -0000 1.70
> > +++ btrace.c 26 Jun 2023 15:12:18 -0000
> > @@ -450,6 +450,37 @@ rules_do(int fd)
> > }
> > }
> >
> > +static uint64_t
> > +rules_action_scan(struct bt_stmt *bs)
> > +{
> > + struct bt_arg *ba;
> > + uint64_t evtflags = 0;
> > +
> > + while (bs != NULL) {
> > + SLIST_FOREACH(ba, &bs->bs_args, ba_next)
> > + evtflags |= ba2dtflags(ba);
> > +
> > + /* Also check the value for map/hist insertion */
> > + switch (bs->bs_act) {
> > + case B_AC_BUCKETIZE:
> > + case B_AC_INSERT:
> > + ba = (struct bt_arg *)bs->bs_var;
> > + evtflags |= ba2dtflags(ba);
> > + break;
> > + case B_AC_TEST:
> > + evtflags |= rules_action_scan(
> > + (struct bt_stmt *)bs->bs_var);
> > + break;
> > + default:
> > + break;
> > + }
> > +
> > + bs = SLIST_NEXT(bs, bs_next);
> > + }
> > +
> > + return evtflags;
> > +}
> > +
> > void
> > rules_setup(int fd)
> > {
> > @@ -474,21 +505,7 @@ rules_setup(int fd)
> > evtflags |= ba2dtflags(ba);
> > }
> >
> > - SLIST_FOREACH(bs, &r->br_action, bs_next) {
> > - SLIST_FOREACH(ba, &bs->bs_args, ba_next)
> > - evtflags |= ba2dtflags(ba);
> > -
> > - /* Also check the value for map/hist insertion */
> > - switch (bs->bs_act) {
> > - case B_AC_BUCKETIZE:
> > - case B_AC_INSERT:
> > - ba = (struct bt_arg *)bs->bs_var;
> > - evtflags |= ba2dtflags(ba);
> > - break;
> > - default:
> > - break;
> > - }
> > - }
> > + evtflags |= rules_action_scan(SLIST_FIRST(&r->br_action));
> >
> > SLIST_FOREACH(bp, &r->br_probes, bp_next) {
> > debug("parsed probe '%s'", debug_probe_name(bp));
> > @@ -1685,10 +1702,17 @@ ba2dtflags(struct bt_arg *ba)
> > long
> > bacmp(struct bt_arg *a, struct bt_arg *b)
> > {
> > - assert(a->ba_type == b->ba_type);
> > - assert(a->ba_type == B_AT_LONG);
> > + if (a->ba_type != b->ba_type)
> > + return a->ba_type - b->ba_type;
> >
> > - return ba2long(a, NULL) - ba2long(b, NULL);
> > + switch (a->ba_type) {
> > + case B_AT_LONG:
> > + return ba2long(a, NULL) - ba2long(b, NULL);
> > + case B_AT_STR:
> > + return strcmp(ba2str(a, NULL), ba2str(b, NULL));
> > + default:
> > + errx(1, "no compare support for type %d", a->ba_type);
> > + }
> > }
> >
> > __dead void
> > Index: map.c
> > ===================================================================
> > RCS file: /cvs/src/usr.sbin/btrace/map.c,v
> > retrieving revision 1.20
> > diff -u -p -r1.20 map.c
> > --- map.c 30 Apr 2022 01:29:05 -0000 1.20
> > +++ map.c 24 Jun 2023 10:27:59 -0000
> > @@ -176,6 +176,11 @@ map_insert(struct map *map, const char *
> > val += ba2long(bval->ba_value, dtev);
> > mep->mval->ba_value = (void *)val;
> > break;
> > + case B_AT_BI_KSTACK:
> > + case B_AT_BI_USTACK:
> > + free(mep->mval);
> > + mep->mval = ba_new(ba2str(bval, dtev), B_AT_STR);
> > + break;
> > default:
> > errx(1, "no insert support for type %d", bval->ba_type);
> > }
> >
>
--
:wq Claudio