6:31 p.m.
Alex,
Just send me a full patch against symbols.c, since that's the only file that
you'll need to patch from this point on. I'm happy with the state of the other
files.
Thanks,
Dave
Sent from my ASUS Pad
Alexandr Terekhov <Alexandr_Terekhov(a)epam.com> wrote:
On Tue, May 19, 2015 at 12:14:49PM -0400, Dave Anderson wrote:
> I've run into three problems dealing with embedded arrays.
>
> First, should array elements inside an embedded structure be
> selectable? For example, the task_struct has an embedded
> task_rss_stat structure:
>
.........
> crash> task -R rss_stat.count[1]
> PID: 11722 TASK: ffff8801073da5d0 CPU: 2 COMMAND: "crash"
> task: invalid structure member reference: rss_stat.count[1]
> crash>
>
> Is that expected behavior?
Yes, the new logic consider structure
count = {0, 0, 0}
in the same way as scalar (you can find the corresponding comment within
parser branches.
+ if ( p && (*s_e != '{' || (*s_e == '{' &&
buf[len] == '}'))) {
+ /* Scalar or one-line array
+ * next = 0x0
+ * or
+ * files = {0x0, 0x0}
+ */
+ strcpy(current->value, s_e);
+ if (trailing_comma) ALLOC_NEXT_ELEMENT;
+ }
Since this type of array is short, I wasn't sure whether it's necessary to
access its elements or not.
> But for some reason, it fails to accept the "pid_link.pid" member:
>
> crash> task -R pids[1].pid
> PID: 11722 TASK: ffff8801073da5d0 CPU: 0 COMMAND: "crash"
> task: invalid structure member reference: pids[1].pid
This behavior is incorrent, I will check it.
> And third, this behavior also doesn't seem correct. Here again I select
> just the first element of the task_struct.pids[] array:
>
> crash> task -R pids[0].node
> PID: 11722 TASK: ffff8801073da5d0 CPU: 2 COMMAND: "crash"
> pids[0].node = {
> next = 0x0,
> pprev = 0xffff880105fc9888
> },
>
> But it allows me to leave off the "pids[x]" reference, and if so, it
defaults
> to the first member of the array:
>
> crash> task -R pids.node
> PID: 11722 TASK: ffff8801073da5d0 CPU: 0 COMMAND: "crash"
> pids.node = {
> next = 0x0,
> pprev = 0xffff880105fc9888
> },
>
> Should it reject that syntax?
Yes, it should. I will fix it.
And again, since you've started updating my patch, what is the best way
to provide these fixes?
Thanks,
Alexandr
--
Crash-utility mailing list
Crash-utility(a)redhat.com
https://www.redhat.com/mailman/listinfo/crash-utility
7:05 a.m.
New subject: [PATCH 1/1] Resend CLI list command, print deep structure members
Dave,
here is the patch.
Thanks,
Alexandr.
--- a/symbols.c 2015-05-13 11:39:12.401474372 +0300
+++ b/symbols.c 2015-05-20 12:42:53.838600738 +0300
@@ -94,6 +94,11 @@
#define PARSE_FOR_DECLARATION (2)
static void parse_for_member(struct datatype_member *, ulong);
static int show_member_offset(FILE *, struct datatype_member *, char *);
+struct struct_elem;
+static void free_structure(struct struct_elem *);
+static unsigned char is_right_brace(const char *);
+static struct struct_elem *find_node(struct struct_elem *, char *);
+static void dump_node(struct struct_elem *, char *, unsigned char, unsigned char);
/*
@@ -5576,17 +5581,18 @@
FREEBUF(buf);
error(FATAL, "invalid structure name: %s\n", dm->name);
}
- if (!MEMBER_EXISTS(dm->name, dm->member)) {
- FREEBUF(buf);
- error(FATAL, "invalid structure member name: %s\n",
- dm->member);
- }
set_temporary_radix(radix, &restore_radix);
open_tmpfile();
print_struct(dm->name, addr);
- parse_for_member(dm, PARSE_FOR_DATA);
+
+ if (MEMBER_EXISTS(dm->name, dm->member)) {
+ parse_for_member(dm, PARSE_FOR_DATA);
+ } else {
+ parse_for_member_new(dm, PARSE_FOR_DATA);
+ }
+
close_tmpfile();
restore_current_radix(restore_radix);
@@ -6026,8 +6032,7 @@
if ((count_chars(args[optind], ',')+1) > MAXARGS)
error(FATAL, "too many members in comma-separated list!\n");
- if ((count_chars(args[optind], '.') > 1) ||
- (LASTCHAR(args[optind]) == ',') ||
+ if ((LASTCHAR(args[optind]) == ',') ||
(LASTCHAR(args[optind]) == '.'))
error(FATAL, "invalid format: %s\n", args[optind]);
@@ -6219,6 +6224,10 @@
i = 0;
do {
if (argc_members) {
+ /* This call works fine with fields
+ * of the second, third, ... levels.
+ * There is no need to fix it
+ */
if (!member_to_datatype(memberlist[i], dm,
ANON_MEMBER_QUERY))
error(FATAL, "invalid data structure reference: %s.%s\n",
@@ -6250,7 +6259,12 @@
if (dm->member) {
if (!((flags & DEREF_POINTERS) &&
dereference_pointer(addr, dm, flags)))
- parse_for_member(dm, PARSE_FOR_DATA);
+ {
+ if (count_chars(dm->member, '.') || count_chars(dm->member,
'['))
+ parse_for_member_new(dm, PARSE_FOR_DATA);
+ else
+ parse_for_member(dm, PARSE_FOR_DATA);
+ }
close_tmpfile();
}
@@ -6275,6 +6289,10 @@
if (CRASHDEBUG(1))
dump_datatype_member(fp, dm);
+ if (dm->member && count_chars(dm->member, '.'))
+ error(FATAL, "invalid data structure reference: %s.%s\n",
+ dm->name, dm->member);
+
open_tmpfile();
whatis_datatype(dm->name, flags, pc->tmpfile);
rewind(pc->tmpfile);
@@ -7383,6 +7401,283 @@
}
}
+struct struct_elem {
+ char field_name[BUFSIZE];
+ unsigned char field_len;
+ char value[BUFSIZE];
+ unsigned char is_array_root:1;
+
+ struct struct_elem *parent;
+ struct struct_elem *inner;
+ struct struct_elem *next;
+ struct struct_elem *prev;
+};
+
+#define ALLOC_XXX_ELEMENT(xxx, clone_parent) \
+{ \
+ if (NULL == current) { \
+ error(FATAL, "Internal error while parsing structure %s\n", dm->name); \
+ } \
+ current->xxx = (struct struct_elem *)GETBUF(sizeof(struct struct_elem)); \
+ if (clone_parent) current->xxx->parent = current->parent; \
+ else current->xxx->parent = current; \
+ current = current->xxx; \
+}
+
+#define ALLOC_INNER_ELEMENT { ALLOC_XXX_ELEMENT(inner, 0) }
+#define ALLOC_NEXT_ELEMENT { ALLOC_XXX_ELEMENT(next, 1) }
+
+static void
+free_structure(struct struct_elem *p)
+{
+ if (p == NULL)
+ return;
+ free_structure(p->inner);
+ free_structure(p->next);
+ FREEBUF(p);
+}
+
+static unsigned char
+is_right_brace(const char *b)
+{
+ unsigned char r = 0;
+ for (; *b == ' '; b++);
+ if (*b == '}') {
+ b++;
+ r = 1;
+ if (*b == '}') {
+ r = 2;
+ b++;
+ }
+ }
+
+ if (*b == ',')
+ b++;
+
+ if (*b == '\0')
+ return r;
+ else
+ return 0;
+}
+
+static struct struct_elem *
+find_node(struct struct_elem *s, char *n)
+{
+ char *p, *b, *e;
+ struct struct_elem *t = s;
+ unsigned i;
+
+ if (('\0' == *n) || (NULL == s))
+ return s;
+
+ /* [n .. p) - struct member with index*/
+ if (NULL == (p = strstr(n, ".")))
+ p = n + strlen(n);
+
+ /* [n .. b) - struct member without index*/
+ for (b = n; (b < p) && (*b != '['); b++);
+
+ /* s - is the current level of items [s, s->next, ..., s->...->next] */
+ for (; s; s = s->next) {
+ if (*s->field_name == '\0')
+ continue;
+
+ /* `field_name` doesn't match */
+ if (((b - n) != s->field_len) || memcmp(s->field_name, n, b - n))
+ continue;
+
+ // For case like `pids.node` where pids is an array
+ if (s->is_array_root && *b != '[' && *p)
+ return NULL;
+
+ if (*b == '[') { /* Array */
+ i = strtol(b + 1, &e, 10);
+ /* Check if the current node is array and
+ * we've parsed index more or less correctly
+ */
+ if (!(s->is_array_root && *e == ']' && (e != b + 1)))
+ return NULL;
+
+ /* Look for the i-th element */
+ for (s = s->inner; s && i; s = s->next, i--);
+ if (i || (NULL == s))
+ return NULL;
+ }
+
+ /* Ok. We've found node, it's - the last member
+ * in our search string, let's return it.
+ */
+ if ('\0' == *p)
+ return s;
+ else
+ return find_node(s->inner, p + 1);
+ }
+
+ // We haven't found any field.
+ // Might happen, we've encountered anonymous structure
+ // of union. Lets try every record without `field_name`
+ s = t;
+ t = NULL;
+ for (; s; s = s->next) {
+ if (*s->field_name)
+ continue;
+ t = find_node(s->inner, n);
+ if (t)
+ break;
+ }
+
+ return t;
+}
+
+static void
+dump_node(struct struct_elem *p, char *f, unsigned char level, unsigned char is_array)
+{
+ unsigned int i;
+ if (p == NULL)
+ return;
+ do {
+#define PUT_INDENTED_STRING(m, ...) { \
+ for (i = 0; i++ < 2 + 2 * (m * is_array + level); fprintf(pc->saved_fp, "
")); \
+ fprintf(pc->saved_fp, __VA_ARGS__); }
+
+ if (p->inner) {
+ if (*p->field_name) {
+ PUT_INDENTED_STRING(1, "%s = %s\n", f ? f : p->field_name,
p->inner->is_array_root ? "{{" : "{");
+ } else {
+ if (f) /* For union */
+ PUT_INDENTED_STRING(1, "%s = ", f);
+ PUT_INDENTED_STRING(1, "%s\n", p->inner->is_array_root ?
"{{" : "{");
+ }
+ dump_node(p->inner, NULL, is_array + level + 1, p->inner->is_array_root);
+ PUT_INDENTED_STRING(1, "%s%s\n", p->inner->is_array_root ?
"}}" : "}", (p->next && !p->next->is_array_root) ?
"," : "");
+ } else {
+ PUT_INDENTED_STRING(1, "%s = %s%s", f ? f : p->field_name, p->value,
p->next ? ",\n" : "\n");
+ }
+ if (level) {
+ p = p->next;
+ if (p && p->is_array_root)
+ PUT_INDENTED_STRING(0, "}, {\n");
+ }
+ } while (p && level);
+}
+
+void
+parse_for_member_new(struct datatype_member *dm,
+ ulong __attribute__ ((unused)) flag)
+{
+ struct struct_elem *i, *current = NULL, *root = NULL;
+
+ char buf[BUFSIZE];
+ char *p, *p1;
+ char *s_e; // structure_element
+ unsigned int len;
+ unsigned char trailing_comma, braces, found = 0;
+
+ rewind(pc->tmpfile);
+
+ root = (struct struct_elem *)GETBUF(sizeof(struct struct_elem));
+ current = root;
+ ALLOC_INNER_ELEMENT;
+
+ while (fgets(buf, BUFSIZE, pc->tmpfile)) {
+ len = strlen(buf) - 1;
+ for (; buf[len] <= ' '; buf[len--] = '\0');
+ if ((trailing_comma = (buf[len] == ',')))
+ buf[len--] = '\0';
+
+ if ((braces = is_right_brace(buf))) {
+ for (; braces && current; braces--)
+ current = current->parent;
+
+ if ((current->parent == root) || trailing_comma)
+ ALLOC_NEXT_ELEMENT;
+ continue;
+ }
+
+ for (p1 = buf; *p1 == ' '; p1++);
+
+ if (NULL != (p = strstr(buf, " = "))) {
+ s_e = p + 3;
+ } else {
+ s_e = p1;
+ }
+
+ /*
+ * After that we have pointers:
+ * foobar = bazzz
+ * -----^ ^ ^
+ * | ------| |
+ * | | |
+ * p1 p s_e
+ *
+ * OR
+ *
+ * {
+ * ^
+ * |
+ * ---------
+ * | |
+ * p1 s_e
+ *
+ * p == NULL
+ *
+ *
+ * p1 - the first non-whitespace symbol in line
+ * p - pointer to line ' = '.
+ * If not NULL, there is identifier
+ * s_e - element of structure (brace / double brace / array separator / scalar)
+ *
+ */
+
+ if (current && p && (p - p1 < BUFSIZE)) {
+ strncpy(current->field_name, p1, p - p1);
+ current->field_len = p - p1;
+ }
+
+ if ( p && (*s_e != '{' || (*s_e == '{' && buf[len] ==
'}') )) {
+ /* Scalar or one-line array
+ * next = 0x0
+ * or
+ * files = {0x0, 0x0}
+ */
+ strcpy(current->value, s_e);
+ if (trailing_comma) ALLOC_NEXT_ELEMENT;
+ }
+ else if ( *s_e == '{' ) {
+ ALLOC_INNER_ELEMENT;
+ if (*(s_e + 1) == '{') {
+ current->parent->is_array_root = 1;
+ ALLOC_INNER_ELEMENT;
+ }
+ }
+ else if (strstr(s_e, "}, {")) {
+ /* Next array element */
+ current = current->parent;
+ ALLOC_NEXT_ELEMENT;
+ ALLOC_INNER_ELEMENT;
+ }
+ else if (buf == (p = strstr(buf, "struct "))) {
+ p += 7; /* strlen "struct " */
+ p1 = strstr(buf, " {");
+ strncpy(current->field_name, p, p1 - p);
+ ALLOC_INNER_ELEMENT;
+ }
+ }
+
+ for (i = root->inner; i; i = i->next) {
+ if ((current = find_node(i->inner, dm->member))) {
+ dump_node(current, dm->member, 0, 0);
+ found = 1;
+ break;
+ }
+ }
+
+ free_structure(root);
+
+ if (!found)
+ error(WARNING, "invalid structure reference: %s\n", dm->member);
+}
+
/*
* Dig out a member name from a formatted gdb structure declaration dump,
* and print its offset from the named structure passed in.
3332
days inactive
3333
days old
devel@lists.crash-utility.osci.io
1 comments
2 participants
participants (2)
-
Alexandr Terekhov -
Dave Anderson