[Crash-utility] [PATCH v2] struct: Fix handing of percpu symbols (when symbolic argument only)

[Aaron Tomlin]

Hi Dave,

Sorry about the delay. I completely forgot about this one!
Please note I have only addressed the case when the argument to the
'struct' command is not an address:

When a percpu symbol is of type pointer, the 'struct' command does
not generate the expected output. For example:

Note: The correct value of 'exec_start' should be
      0x15b070b28b0c27 not 0x0

  crash> struct task_struct.se.exec_start softlockup_watchdog:0
  [0]: ffff880214e55a00
    se.exec_start = 0x0,

  crash> px softlockup_watchdog:0-1
  per_cpu(softlockup_watchdog, 0) = $1 = (struct task_struct *) 0xffff880fe97e2e00

  crash> px ((struct task_struct *)0xffff880fe97e2e00)->se.exec_start
  $2 = 0x15b070b28b0c27

Currently, the 'struct' and 'p' command simply calculates 'cpuaddr' as
follows -- where 'addr' is a percpu's value:

        cpuaddr = addr + __per_cpu_offset

This is correct if the percpu symbol or offset
(i.e. [percpu symbol:cpu-specifier] or [percpu value:cpu-specifier])
is not of type pointer. If a given percpu symbol is of type pointer such as
in 'static DEFINE_PER_CPU(struct task_struct *, softlockup_watchdog)', we
need to dereference the pointer to obtain the above correct kernel virtual
address.

For instance, using the above example, we need to pass the following to gdb
to resolve appropriately:

kernel virtual addresses for the cpus specified.

  crash> set gdb on
  gdb: on

  gdb> p *(struct task_struct **) 0xffff880fffc0ddc0
  $1 = (struct task_struct *) 0xffff880fe97e2e00
  gdb>

With this patch, we now obtain the expected output:

  crash> struct task_struct.se.exec_start softlockup_watchdog:0
  [0]: ffff880214e55a00
    se.exec_start = 0x15b070b28b0c27,

Signed-off-by: Aaron Tomlin <atomlin at redhat.com>
---
 symbols.c | 29 +++++++++++++++++++++++++++++
 1 file changed, 29 insertions(+)

diff --git a/symbols.c b/symbols.c
index 8b0d8b9..b68d0d3 100644
--- a/symbols.c
+++ b/symbols.c
@@ -81,6 +81,7 @@ static void cmd_datatype_common(ulong);
 static void do_datatype_addr(struct datatype_member *, ulong, int,
 			     ulong, char **, int);
 static void process_gdb_output(char *, unsigned, const char *, int);
+static char *expr_type_name(const char *);
 static int display_per_cpu_info(struct syment *, int, char *);
 static struct load_module *get_module_percpu_sym_owner(struct syment *);
 static int is_percpu_symbol(struct syment *);
@@ -6112,6 +6113,11 @@ cmd_datatype_common(ulong flags)
         char *separator;
         char *structname, *members;
         char *memberlist[MAXARGS];
+        char *typename;
+        char buf[BUFSIZE];
+        char *argv[MAXARGS];
+        FILE *tmpfp;
+        ushort ptype;
 
         dm = &datatype_member;
 	count = 0xdeadbeef;
@@ -6122,6 +6128,7 @@ cmd_datatype_common(ulong flags)
 	separator = members = NULL;
 	cpuspec = NULL;
 	cpus = NULL;
+	ptype = 0;
 
         while ((c = getopt(argcnt, args, "pxdhfuc:rvol:")) != EOF) {
                 switch (c)
@@ -6262,6 +6269,12 @@ cmd_datatype_common(ulong flags)
 			SET_BIT(cpus, CURRENT_CONTEXT()->processor);
 		else
 			make_cpumask(cpuspec, cpus, FAULT_ON_ERROR, NULL);
+
+		if (sp) {
+			typename = expr_type_name(sp->name);
+			ptype = typename &&
+				LASTCHAR(typename) == '*' ? 1 : 0;
+		}
 	}
 
 	optind = optind_save;
@@ -6346,6 +6359,19 @@ cmd_datatype_common(ulong flags)
 				continue;
 			}
 
+			if (ptype) {
+				open_tmpfile();
+				snprintf(buf, sizeof buf, "p *(%s*) 0x%lx",
+						typename, cpuaddr);
+				gdb_pass_through(buf, pc->tmpfile, GNU_RETURN_ON_ERROR);
+
+				rewind(pc->tmpfile);
+				fgets(buf, BUFSIZE, pc->tmpfile);
+				parse_line(buf, argv);
+				cpuaddr = htol(argv[3], FAULT_ON_ERROR, NULL);
+				close_tmpfile();
+			}
+
 			fprintf(fp, "%lx\n", cpuaddr);
 			do_datatype_addr(dm, cpuaddr , count,
 					 flags, memberlist, argc_members);
@@ -6364,6 +6390,9 @@ freebuf:
 
 	if (cpus)
 		FREEBUF(cpus);
+
+	if (typename)
+		FREEBUF(typename);
 }
 
 static void
-- 
2.5.5