Re: [Crash-utility] [PATCH v2] struct: Allow -r with a single member-specific output

2020-04-02 Thread Dave Anderson 



- Original Message -
> Hi Dave,
> 
> Dave Anderson wrote on Wed, Apr 01, 2020:
> > > I didn't post that v2 back in Feb because I wasn't totally happy with
> > > it; I can't say I now am but might as well get your take on it...
> > 
> > What part of this patch aren't you happy about?
> 
> It's mostly style really - I don't like that we're calling in twice in
> datatype_info(), because member_to_datatype() doesn't really fill in the
> datatype_member struct and only fills in dm->member and offset.
> At a naive read, I would expect member_to_datatype to fill in the whole dm...
> 
> Functionally I tested it, it's a bit slower than my original version but
> not enough to be a valid argument here; it's much better than nothing so
> if you're happy with this let's go with it :)

Works for me -- queued for crash-7.2.9:
  
  
https://github.com/crash-utility/crash/commit/42fba6524ce01b6cecb4cd2cac8f0a50d79b1420

Thanks,
  Dave


> 
> I will probably want to follow up with a second patch for raw_data_dump
> to add DISPLAY_32/16 flags if the len requested is < word size but it's
> not directly related to this patch...
> 
> 
> Cheers,
> --
> Dominique
> 
> 
> --
> Crash-utility mailing list
> Crash-utility@redhat.com
> https://www.redhat.com/mailman/listinfo/crash-utility
> 

--
Crash-utility mailing list
Crash-utility@redhat.com
https://www.redhat.com/mailman/listinfo/crash-utility

Re: [Crash-utility] [PATCH v2] struct: Allow -r with a single member-specific output

2020-04-01 Thread Dominique Martinet 
Hi Dave,

Dave Anderson wrote on Wed, Apr 01, 2020:
> > I didn't post that v2 back in Feb because I wasn't totally happy with
> > it; I can't say I now am but might as well get your take on it...
> 
> What part of this patch aren't you happy about?

It's mostly style really - I don't like that we're calling in twice in
datatype_info(), because member_to_datatype() doesn't really fill in the
datatype_member struct and only fills in dm->member and offset.
At a naive read, I would expect member_to_datatype to fill in the whole
dm...

Functionally I tested it, it's a bit slower than my original version but
not enough to be a valid argument here; it's much better than nothing so
if you're happy with this let's go with it :)


I will probably want to follow up with a second patch for raw_data_dump
to add DISPLAY_32/16 flags if the len requested is < word size but it's
not directly related to this patch...


Cheers,
-- 
Dominique


--
Crash-utility mailing list
Crash-utility@redhat.com
https://www.redhat.com/mailman/listinfo/crash-utility

Re: [Crash-utility] [PATCH v2] struct: Allow -r with a single member-specific output

2020-04-01 Thread Dave Anderson 



- Original Message -
> Using struct -r allows much faster execution when iterating through a file;
> on 3909 elements struct struct.member takes 192s vs. 3s with -r
> ---
> Sorry for the delay, here's another take at allowing struct -r with a
> specific field specified.
> 
> I didn't post that v2 back in Feb because I wasn't totally happy with
> it; I can't say I now am but might as well get your take on it...

What part of this patch aren't you happy about?

Dave




> Thanks!
> 
>  defs.h|  2 ++
>  symbols.c | 69 ---
>  2 files changed, 63 insertions(+), 8 deletions(-)
> 
> diff --git a/defs.h b/defs.h
> index e852ddf..a3f828d 100644
> --- a/defs.h
> +++ b/defs.h
> @@ -2283,6 +2283,7 @@ struct array_table {
>  #define MEMBER_TYPE_REQUEST ((struct datatype_member *)(-3))
>  #define STRUCT_SIZE_REQUEST ((struct datatype_member *)(-4))
>  #define MEMBER_TYPE_NAME_REQUEST ((struct datatype_member *)(-5))
> +#define ANON_MEMBER_SIZE_REQUEST ((struct datatype_member *)(-6))
>  
>  #define STRUCT_SIZE(X)  datatype_info((X), NULL, STRUCT_SIZE_REQUEST)
>  #define UNION_SIZE(X)   datatype_info((X), NULL, STRUCT_SIZE_REQUEST)
> @@ -2294,6 +2295,7 @@ struct array_table {
>  #define MEMBER_TYPE(X,Y)datatype_info((X), (Y), MEMBER_TYPE_REQUEST)
>  #define MEMBER_TYPE_NAME(X,Y)((char *)datatype_info((X), (Y),
>  MEMBER_TYPE_NAME_REQUEST))
>  #define ANON_MEMBER_OFFSET(X,Y)datatype_info((X), (Y),
>  ANON_MEMBER_OFFSET_REQUEST)
> +#define ANON_MEMBER_SIZE(X,Y)datatype_info((X), (Y),
> ANON_MEMBER_SIZE_REQUEST)
>  
>  /*
>   *  The following set of macros can only be used with pre-intialized fields
> diff --git a/symbols.c b/symbols.c
> index 9c3032d..b32e480 100644
> --- a/symbols.c
> +++ b/symbols.c
> @@ -145,6 +145,7 @@ static void print_union(char *, ulong);
>  static void dump_datatype_member(FILE *, struct datatype_member *);
>  static void dump_datatype_flags(ulong, FILE *);
>  static long anon_member_offset(char *, char *);
> +static long anon_member_size(char *, char *);
>  static int gdb_whatis(char *);
>  static void do_datatype_declaration(struct datatype_member *, ulong);
>  static int member_to_datatype(char *, struct datatype_member *, ulong);
> @@ -5604,14 +5605,17 @@ long
>  datatype_info(char *name, char *member, struct datatype_member *dm)
>  {
>   struct gnu_request *req;
> -long offset, size, member_size;
> + long offset, size, member_size;
>   int member_typecode;
> -ulong type_found;
> + ulong type_found;
>   char buf[BUFSIZE];
>  
> -if (dm == ANON_MEMBER_OFFSET_REQUEST)
> + if (dm == ANON_MEMBER_OFFSET_REQUEST)
>   return anon_member_offset(name, member);
>  
> + if (dm == ANON_MEMBER_SIZE_REQUEST)
> + return anon_member_size(name, member);
> +
>   strcpy(buf, name);
>  
>   req = (struct gnu_request *)GETBUF(sizeof(struct gnu_request));
> @@ -5828,6 +5832,46 @@ retry:
>   return value;
>  }
>  
> +/*
> + *  Determine the size of a member in an anonymous union
> + *  in a structure or union.
> + */
> +static long
> +anon_member_size(char *name, char *member)
> +{
> + char buf[BUFSIZE];
> + ulong value;
> + int type;
> +
> + value = -1;
> + type = STRUCT_REQUEST;
> + sprintf(buf, "printf \"%%ld\", (u64)(&((struct %s*)0)->%s + 1) -
> (u64)&((struct %s*)0)->%s",
> + name, member, name, member);
> + open_tmpfile2();
> +retry:
> + if (gdb_pass_through(buf, pc->tmpfile2, GNU_RETURN_ON_ERROR)) {
> + rewind(pc->tmpfile2);
> + if (fgets(buf, BUFSIZE, pc->tmpfile2)) {
> + if (hexadecimal(buf, 0))
> + value = htol(buf, RETURN_ON_ERROR|QUIET, NULL);
> + else if (STRNEQ(buf, "(nil)"))
> + value = 0;
> + }
> + }
> +
> + if ((value == -1) && (type == STRUCT_REQUEST)) {
> + type = UNION_REQUEST;
> + sprintf(buf, "printf \"%%ld\", (u64)(&((union %s*)0)->%s + 1) -
> (u64)&((union %s*)0)->%s",
> + name, member, name, member);
> + rewind(pc->tmpfile2);
> + goto retry;
> + }
> +
> + close_tmpfile2();
> +
> + return value;
> +}
> +
>  /*
>   *  Get the basic type info for a symbol.  Let the caller pass in the
>   *  gnu_request structure to have access to the full response; in either
> @@ -6617,6 +6661,9 @@ do_datatype_addr(struct datatype_member *dm, ulong
> addr, int count,
>   i = 0;
>   do {
>   if (argc_members) {
> + if (argc_members > 1 && flags & SHOW_RAW_DATA)
> + error(FATAL,
> +   "only up to one member-specific 
> output allowed with -r\n");
>   /* This call works fine with fields
>

[Crash-utility] [PATCH v2] struct: Allow -r with a single member-specific output

2020-04-01 Thread Dominique Martinet 
Using struct -r allows much faster execution when iterating through a file;
on 3909 elements struct struct.member takes 192s vs. 3s with -r
---
Sorry for the delay, here's another take at allowing struct -r with a
specific field specified.

I didn't post that v2 back in Feb because I wasn't totally happy with
it; I can't say I now am but might as well get your take on it...
Thanks!

 defs.h|  2 ++
 symbols.c | 69 ---
 2 files changed, 63 insertions(+), 8 deletions(-)

diff --git a/defs.h b/defs.h
index e852ddf..a3f828d 100644
--- a/defs.h
+++ b/defs.h
@@ -2283,6 +2283,7 @@ struct array_table {
 #define MEMBER_TYPE_REQUEST ((struct datatype_member *)(-3))
 #define STRUCT_SIZE_REQUEST ((struct datatype_member *)(-4))
 #define MEMBER_TYPE_NAME_REQUEST ((struct datatype_member *)(-5))
+#define ANON_MEMBER_SIZE_REQUEST ((struct datatype_member *)(-6))
 
 #define STRUCT_SIZE(X)  datatype_info((X), NULL, STRUCT_SIZE_REQUEST)
 #define UNION_SIZE(X)   datatype_info((X), NULL, STRUCT_SIZE_REQUEST)
@@ -2294,6 +2295,7 @@ struct array_table {
 #define MEMBER_TYPE(X,Y)datatype_info((X), (Y), MEMBER_TYPE_REQUEST)
 #define MEMBER_TYPE_NAME(X,Y)((char *)datatype_info((X), (Y), 
MEMBER_TYPE_NAME_REQUEST))
 #define ANON_MEMBER_OFFSET(X,Y)datatype_info((X), (Y), 
ANON_MEMBER_OFFSET_REQUEST)
+#define ANON_MEMBER_SIZE(X,Y)datatype_info((X), (Y), 
ANON_MEMBER_SIZE_REQUEST)
 
 /*
  *  The following set of macros can only be used with pre-intialized fields
diff --git a/symbols.c b/symbols.c
index 9c3032d..b32e480 100644
--- a/symbols.c
+++ b/symbols.c
@@ -145,6 +145,7 @@ static void print_union(char *, ulong);
 static void dump_datatype_member(FILE *, struct datatype_member *);
 static void dump_datatype_flags(ulong, FILE *);
 static long anon_member_offset(char *, char *);
+static long anon_member_size(char *, char *);
 static int gdb_whatis(char *);
 static void do_datatype_declaration(struct datatype_member *, ulong);
 static int member_to_datatype(char *, struct datatype_member *, ulong);
@@ -5604,14 +5605,17 @@ long
 datatype_info(char *name, char *member, struct datatype_member *dm)
 {
struct gnu_request *req;
-long offset, size, member_size;
+   long offset, size, member_size;
int member_typecode;
-ulong type_found;
+   ulong type_found;
char buf[BUFSIZE];
 
-if (dm == ANON_MEMBER_OFFSET_REQUEST)
+   if (dm == ANON_MEMBER_OFFSET_REQUEST)
return anon_member_offset(name, member);
 
+   if (dm == ANON_MEMBER_SIZE_REQUEST)
+   return anon_member_size(name, member);
+
strcpy(buf, name);
 
req = (struct gnu_request *)GETBUF(sizeof(struct gnu_request));
@@ -5828,6 +5832,46 @@ retry:
return value;
 }
 
+/*
+ *  Determine the size of a member in an anonymous union
+ *  in a structure or union.
+ */
+static long
+anon_member_size(char *name, char *member)
+{
+   char buf[BUFSIZE];
+   ulong value;
+   int type;
+
+   value = -1;
+   type = STRUCT_REQUEST;
+   sprintf(buf, "printf \"%%ld\", (u64)(&((struct %s*)0)->%s + 1) - 
(u64)&((struct %s*)0)->%s",
+   name, member, name, member);
+   open_tmpfile2();
+retry:
+   if (gdb_pass_through(buf, pc->tmpfile2, GNU_RETURN_ON_ERROR)) {
+   rewind(pc->tmpfile2);
+   if (fgets(buf, BUFSIZE, pc->tmpfile2)) {
+   if (hexadecimal(buf, 0))
+   value = htol(buf, RETURN_ON_ERROR|QUIET, NULL);
+   else if (STRNEQ(buf, "(nil)"))
+   value = 0;
+   }
+   }
+
+   if ((value == -1) && (type == STRUCT_REQUEST)) {
+   type = UNION_REQUEST;
+   sprintf(buf, "printf \"%%ld\", (u64)(&((union %s*)0)->%s + 1) - 
(u64)&((union %s*)0)->%s",
+   name, member, name, member);
+   rewind(pc->tmpfile2);
+   goto retry;
+   }
+
+   close_tmpfile2();
+
+   return value;
+}
+
 /*
  *  Get the basic type info for a symbol.  Let the caller pass in the 
  *  gnu_request structure to have access to the full response; in either
@@ -6617,6 +6661,9 @@ do_datatype_addr(struct datatype_member *dm, ulong addr, 
int count,
i = 0;
do {
if (argc_members) {
+   if (argc_members > 1 && flags & SHOW_RAW_DATA)
+   error(FATAL,
+ "only up to one member-specific 
output allowed with -r\n");
/* This call works fine with fields
 * of the second, third, ... levels.
 * There is no need to fix it
@@ -6625,9 +6672,6 @@ do_datatype_addr(struct datatype_member *dm, ulong addr, 
int count,