Re: [Crash-utility] Speed up list/tree '-s' output.

Hi Dave, here is patch with arguments instead of static variables:

--- gdb-7.6.orig/gdb/symtab.c 2016-08-18 09:06:33.000000000 +0300 +++ gdb-7.6/gdb/symtab.c 2016-08-24 08:40:45.700772046 +0300 @@ -5122,7 +5122,7 @@ #define GDB_COMMON #include "../../defs.h" -static void get_member_data(struct gnu_request *, struct type *); +static void get_member_data(struct gnu_request *, struct type *, long, char); static void dump_enum(struct type *, struct gnu_request *); static void eval_enum(struct type *, struct gnu_request *); static void gdb_get_line_number(struct gnu_request *); @@ -5327,7 +5327,7 @@ req->typecode = TYPE_CODE(sym->type); req->length = TYPE_LENGTH(sym->type); if (req->member) - get_member_data(req, sym->type); + get_member_data(req, sym->type, 0, 1); if (TYPE_CODE(sym->type) == TYPE_CODE_ENUM) { if (req->flags & GNU_PRINT_ENUMERATORS) @@ -5397,7 +5397,7 @@ } if (req->member) - get_member_data(req, type); + get_member_data(req, type, 0, 1); break; @@ -5480,7 +5480,7 @@ * member field, and when found, return its relevant data. */ static void -get_member_data(struct gnu_request *req, struct type *type) +get_member_data(struct gnu_request *req, struct type *type, long offset, char is_first) { register short i; struct field *nextfield; @@ -5492,7 +5492,7 @@ nfields = TYPE_MAIN_TYPE(type)->nfields; nextfield = TYPE_MAIN_TYPE(type)->flds_bnds.fields; - if (nfields == 0) { + if (nfields == 0 && is_first /* The first call */) { struct type *newtype; newtype = lookup_transparent_type(req->name); if (newtype) { @@ -5505,13 +5505,18 @@ for (i = 0; i < nfields; i++) { if (STREQ(req->member, nextfield->name)) { - req->member_offset = nextfield->loc.bitpos; + req->member_offset = offset + nextfield->loc.bitpos; req->member_length = TYPE_LENGTH(nextfield->type); req->member_typecode = TYPE_CODE(nextfield->type); if ((req->member_typecode == TYPE_CODE_TYPEDEF) && (typedef_type = check_typedef(nextfield->type))) req->member_length = TYPE_LENGTH(typedef_type); return; + } else if (*nextfield->name == 0) { /* Anonymous struct/union */ + get_member_data(req, nextfield->type, + offset + nextfield->loc.bitpos, 0); + if (req->member_offset != -1) + return; } nextfield++; } @@ -5706,7 +5711,7 @@ } if (req->member) - get_member_data(req, type); + get_member_data(req, type, 0, 1); do_cleanups (old_chain); } Thanks, Alexandr ________________________________________ From: crash-utility-bounces(a)redhat.com <crash-utility-bounces(a)redhat.com&gt; on behalf of Dave Anderson <anderson(a)redhat.com&gt; Sent: Tuesday, August 23, 2016 23:17 To: Discussion list for crash utility usage, maintenance and development Subject: Re: [Crash-utility] Speed up list/tree '-s' output. Hi Alexandr, I'm still testing and sanity-checking the symtab.c patch given that it's modifying such a commonly-called function. Interestingly enough, it's actually uncovered a bug with the current determination of the page.list members, which I found by saving the output of "help -o" into a file, both without and then with your patch, into /tmp/old and /tmp/new, and then diff'ed the two files: # diff /tmp/old /tmp/new 221,223c221,223 < page_list: -1 < page_list_next: -1 < page_list_prev: -1 --- > page_list: 32 > page_list_next: 32 > page_list_prev: 40 # Fortunately those fields aren't used by the crash code for more recent kernels, which have gone crazy with the anonymous structures in the page structure. But anyway, that was a good catch by your patch. On the other hand, I don't like the use of the deep and offset static variables, because there's always the remote possibility that a CTRL-C or other function failure may happen while they are set, and then leave them in a bogus state. So I would rather they get passed as arguments. Thanks, Dave ----- Original Message ----- > Hi Dave, > > here is updated patch for gdb/symtab.c: > > --- gdb-7.6.orig/gdb/symtab.c 2016-08-18 09:06:33.000000000 +0300 > +++ gdb-7.6/gdb/symtab.c 2016-08-23 14:28:45.456510348 +0300 > @@ -5486,13 +5486,15 @@ > struct field *nextfield; > short nfields; > struct type *typedef_type; > + static int deep = 0; > + static long offset = 0; > > req->member_offset = -1; > > nfields = TYPE_MAIN_TYPE(type)->nfields; > nextfield = TYPE_MAIN_TYPE(type)->flds_bnds.fields; > > - if (nfields == 0) { > + if (nfields == 0 && deep == 0 /* The first call */) { > struct type *newtype; > newtype = lookup_transparent_type(req->name); > if (newtype) { > @@ -5505,13 +5507,21 @@ > > for (i = 0; i < nfields; i++) { > if (STREQ(req->member, nextfield->name)) { > - req->member_offset = nextfield->loc.bitpos; > + req->member_offset = offset + nextfield->loc.bitpos; > req->member_length = TYPE_LENGTH(nextfield->type); > req->member_typecode = TYPE_CODE(nextfield->type); > if ((req->member_typecode == TYPE_CODE_TYPEDEF) && > (typedef_type = check_typedef(nextfield->type))) > req->member_length = > TYPE_LENGTH(typedef_type); > return; > + } else if (*nextfield->name == 0) { /* Anonymous struct/union > */ > + deep++; > + offset += nextfield->loc.bitpos; > + get_member_data(req, nextfield->type); > + offset -= nextfield->loc.bitpos; > + deep--; > + if (req->member_offset != -1) > + return; > } > nextfield++; > } > @@ -5705,7 +5715,7 @@ > req->target_length = target_type->length; > } > > - if (req->member) > + if (req->member) > get_member_data(req, type); > > do_cleanups (old_chain); > > The patch for crash tool can be found on github: > https://raw.githubusercontent.com/hziSot/crash-patches/41abc6f9c91c2a6029... > > Thanks, > Alexandr > > -- > Crash-utility mailing list > Crash-utility(a)redhat.com > https://www.redhat.com/mailman/listinfo/crash-utility > -- Crash-utility mailing list Crash-utility(a)redhat.com https://www.redhat.com/mailman/listinfo/crash-utility -- Crash-utility mailing list Crash-utility(a)redhat.com https://www.redhat.com/mailman/listinfo/crash-utility