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

Reply via email to