On 2011-08-11 17:06:21 Thu, Ken'ichi Ohmichi wrote:
>
> Hi Mahesh,
>
> On Wed, 18 May 2011 01:35:19 +0530
> Mahesh J Salgaonkar <mahesh at linux.vnet.ibm.com> wrote:
> >
> > From: Mahesh Salgaonkar <mahesh at linux.vnet.ibm.com>
> >
> > This patch adds support to read and process 'for' command from config file
> > to filter multiple memory locations that are accessible through an array,
> > link list or list_head.
> >
> > The syntax for 'for' (loop construct) filter command is:
> >
> > for <id> in {<ArrayVar> |
> > <StructVar> via <NextMember> |
> > <ListHeadVar> within <StructName>:<ListHeadMember>}
> > erase <id>[.MemberExpression] [size <SizeExpression>|nullify]
> > [erase <id> ...]
> > [...]
> > endfor
> >
> > Updated filter.conf(8) man page that describes the syntax for loop construct.
> >
> > Signed-off-by: Mahesh Salgaonkar <mahesh at linux.vnet.ibm.com>
> > Signed-off-by: Prerna Saxena <prerna at linux.vnet.ibm.com>
> > ---
>
> Thank you for the patch.
> I think this patch is good.
>
> Acked-by: Ken'ichi Ohmichi <oomichi at mxs.nes.nec.co.jp>
>
>
> Thanks
> Ken'ichi Ohmichi
Hi Ken'ichi,
As I told you earlier I found few BUGs w.r.t patch 6/8 that processes loop
construct. Please find the patch below that fixes those BUGs. Please review.
Thanks,
-Mahesh.
-------
[PATCH] makedumpfile: Fix array traversal for array of structure and char type.
From: Mahesh Salgaonkar <mahesh@xxxxxxxxxxxxxxxxxx>
Introduce new function get_next_list_entry() that returns config entry
for each element in the LIST entry. This approach improves the handling
of LIST entry. With this change the function get_config_symbol_addr()
no longer required to handle LIST entry separately.
This patch fixes following BUGs:
- If the loop construct is used for array of structures (non-pointer), then
the array index value is not incremented resulting in an infinite loop. This
patch fixes this BUG.
- The loop construct used for array of char* (pointer) silently fails and
does not filter the strings.
- Corrected 2nd argument while calling get_structure_size().
- Fixed dwarf_info.c:get_data_array_length() to handle array of const data
type.
e.g.
<1><1a49df3>: Abbrev Number: 90 (DW_TAG_variable)
<1a49df4> DW_AT_name : policycap_names
<1a49df8> DW_AT_decl_file : 1
<1a49df9> DW_AT_decl_line : 43
<1a49dfa> DW_AT_type : <0x1a49e08>
<1><1a49e08>: Abbrev Number: 7 (DW_TAG_const_type)
<1a49e09> DW_AT_type : <0x1a49de3>
<1><1a49de3>: Abbrev Number: 4 (DW_TAG_array_type)
<1a49de4> DW_AT_type : <0x1a3b276>
<1a49de8> DW_AT_sibling : <0x1a49df3>
Signed-off-by: Mahesh Salgaonkar <mahesh at linux.vnet.ibm.com>
---
dwarf_info.c | 8 ++
makedumpfile.c | 292 +++++++++++++++++++++++++++++++++++---------------------
2 files changed, 188 insertions(+), 112 deletions(-)
diff --git a/dwarf_info.c b/dwarf_info.c
index 744ecf7..46dcd8e 100644
--- a/dwarf_info.c
+++ b/dwarf_info.c
@@ -328,6 +328,14 @@ get_data_array_length(Dwarf_Die *die)
return FALSE;
}
tag = dwarf_tag(&die_type);
+ if (tag == DW_TAG_const_type) {
+ /* This array is of const type. Get the die type again */
+ if (!get_die_type(&die_type, &die_type)) {
+ ERRMSG("Can't get CU die of DW_AT_type.\n");
+ return FALSE;
+ }
+ tag = dwarf_tag(&die_type);
+ }
if (tag != DW_TAG_array_type) {
/*
* This kernel doesn't have the member of array.
diff --git a/makedumpfile.c b/makedumpfile.c
index 599950d..1a88171 100644
--- a/makedumpfile.c
+++ b/makedumpfile.c
@@ -6763,11 +6763,27 @@ read_pointer_value(unsigned long long addr)
return val;
}
+static long
+get_strlen(unsigned long long addr)
+{
+ char buf[BUFSIZE + 1];
+ long len = 0;
+
+ /*
+ * Determine the string length for 'char' pointer.
+ * BUFSIZE(1024) is the upper limit for string length.
+ */
+ if (readmem(VADDR, addr, buf, BUFSIZE)) {
+ buf[BUFSIZE] = '\0';
+ len = strlen(buf);
+ }
+ return len;
+}
+
int
resolve_config_entry(struct config_entry *ce, unsigned long long base_addr,
char *base_struct_name)
{
- char buf[BUFSIZE + 1];
unsigned long long symbol;
if (ce->flag & SYMBOL_ENTRY) {
@@ -6866,7 +6882,7 @@ resolve_config_entry(struct config_entry *ce, unsigned long long base_addr,
* If this is a struct or list_head data type then
* create a leaf node entry with 'next' member.
*/
- if ((ce->type_flag & TYPE_BASE)
+ if (((ce->type_flag & (TYPE_BASE | TYPE_ARRAY)) == TYPE_BASE)
&& (strcmp(ce->type_name, "void")))
return FALSE;
@@ -6905,17 +6921,14 @@ resolve_config_entry(struct config_entry *ce, unsigned long long base_addr,
ce->size = 0;
}
- if ((ce->type_flag & TYPE_BASE) && (ce->type_flag & TYPE_PTR)) {
+ if ((ce->type_flag & TYPE_BASE) && (ce->type_flag & TYPE_PTR)
+ && !(ce->type_flag & TYPE_ARRAY)) {
/*
* Determine the string length for 'char' pointer.
* BUFSIZE(1024) is the upper limit for string length.
*/
- if (!strcmp(ce->type_name, "char")) {
- if (readmem(VADDR, ce->addr, buf, BUFSIZE)) {
- buf[BUFSIZE] = '\0';
- ce->size = strlen(buf);
- }
- }
+ if (!strcmp(ce->type_name, "char"))
+ ce->size = get_strlen(ce->addr);
}
if (!ce->next && (ce->flag & SIZE_ENTRY)) {
void *val;
@@ -6968,80 +6981,11 @@ get_config_symbol_addr(struct config_entry *ce,
unsigned long long base_addr,
char *base_struct_name)
{
- unsigned long addr = 0;
-
if (!(ce->flag & ENTRY_RESOLVED)) {
if (!resolve_config_entry(ce, base_addr, base_struct_name))
return 0;
}
- if ((ce->flag & LIST_ENTRY)) {
- /* handle List entry differently */
- if (!ce->next) {
- /* leaf node. */
- if (ce->type_flag & TYPE_ARRAY) {
- if (ce->index == ce->array_length)
- return 0;
- if (!(ce->type_flag & TYPE_PTR))
- return (ce->addr +
- (ce->index * ce->size));
- /* Array of pointers.
- *
- * Array may contain NULL pointers at some
- * indexes. Hence return the next non-null
- * address value.
- */
- while (ce->index < ce->array_length) {
- addr = read_pointer_value(ce->addr +
- (ce->index * get_pointer_size()));
- ce->index++;
- if (addr)
- break;
- }
- return addr;
- }
- else {
- if (ce->addr == ce->cmp_addr)
- return 0;
-
- /* Set the leaf node as unresolved, so that
- * it will be resolved every time when
- * get_config_symbol_addr is called untill
- * it hits the exit condiftion.
- */
- ce->flag &= ~ENTRY_RESOLVED;
- }
- }
- else if ((ce->next->next == NULL) &&
- !(ce->next->type_flag & TYPE_ARRAY)) {
- /* the next node is leaf node. for non-array element
- * Set the sym_addr and addr of this node with that of
- * leaf node.
- */
- addr = ce->addr;
- ce->addr = ce->next->addr;
-
- if (!(ce->type_flag & TYPE_LIST_HEAD)) {
- if (addr == ce->next->cmp_addr)
- return 0;
-
- if (!ce->next->cmp_addr) {
- /* safeguard against circular
- * link-list
- */
- ce->next->cmp_addr = addr;
- }
-
- /* Force resolution of traversal node */
- if (ce->addr && !resolve_config_entry(ce->next,
- ce->addr, ce->type_name))
- return 0;
-
- return addr;
- }
- }
- }
-
if (ce->next && ce->addr) {
/* Populate nullify flag down the list */
ce->next->nullify = ce->nullify;
@@ -7083,6 +7027,116 @@ get_config_symbol_size(struct config_entry *ce,
}
}
+int
+get_next_list_entry(struct config_entry *ce, unsigned long long base_addr,
+ char *base_struct_name, struct config_entry *out_ce)
+{
+ unsigned long addr = 0;
+
+ /* This function only deals with LIST_ENTRY config entry. */
+ if (!(ce->flag & LIST_ENTRY))
+ return FALSE;
+
+ if (!(ce->flag & ENTRY_RESOLVED)) {
+ if (!resolve_config_entry(ce, base_addr, base_struct_name))
+ return FALSE;
+ }
+
+ if (!ce->next) {
+ /* leaf node. */
+ if (ce->type_flag & TYPE_ARRAY) {
+ if (ce->index == ce->array_length)
+ return FALSE;
+
+ if (ce->type_flag & TYPE_PTR) {
+ /* Array of pointers.
+ *
+ * Array may contain NULL pointers at some
+ * indexes. Hence jump to the next non-null
+ * address value.
+ */
+ while (ce->index < ce->array_length) {
+ addr = read_pointer_value(ce->addr +
+ (ce->index * get_pointer_size()));
+ if (addr)
+ break;
+ ce->index++;
+ }
+ if (ce->index == ce->array_length)
+ return FALSE;
+ out_ce->sym_addr = ce->addr + (ce->index *
+ get_pointer_size());
+ out_ce->addr = addr;
+ if (!strcmp(ce->type_name, "char"))
+ out_ce->size = get_strlen(addr);
+ else
+ out_ce->size = ce->size;
+ }
+ else {
+ out_ce->sym_addr = ce->addr +
+ (ce->index * ce->size);
+ out_ce->addr = out_ce->sym_addr;
+ out_ce->size = ce->size;
+ }
+ ce->index++;
+ }
+ else {
+ if (ce->addr == ce->cmp_addr)
+ return FALSE;
+
+ out_ce->addr = ce->addr;
+ /* Set the leaf node as unresolved, so that
+ * it will be resolved every time when
+ * get_next_list_entry is called untill
+ * it hits the exit condiftion.
+ */
+ ce->flag &= ~ENTRY_RESOLVED;
+ }
+ return TRUE;
+ }
+ else if ((ce->next->next == NULL) &&
+ !(ce->next->type_flag & TYPE_ARRAY)) {
+ /* the next node is leaf node. for non-array element
+ * Set the sym_addr and addr of this node with that of
+ * leaf node.
+ */
+ if (!(ce->type_flag & TYPE_LIST_HEAD)) {
+ if (!ce->addr || ce->addr == ce->next->cmp_addr)
+ return FALSE;
+
+ if (!ce->next->cmp_addr) {
+ /* safeguard against circular
+ * link-list
+ */
+ ce->next->cmp_addr = ce->addr;
+ }
+
+ out_ce->addr = ce->addr;
+ out_ce->sym_addr = ce->sym_addr;
+ out_ce->size = ce->size;
+
+ ce->sym_addr = ce->next->sym_addr;
+ ce->addr = ce->next->addr;
+
+ /* Force resolution of traversal node */
+ if (ce->addr && !resolve_config_entry(ce->next,
+ ce->addr, ce->type_name))
+ return FALSE;
+
+ return TRUE;
+ }
+ else {
+ ce->sym_addr = ce->next->sym_addr;
+ ce->addr = ce->next->addr;
+ }
+ }
+
+ if (ce->next && ce->addr)
+ return get_next_list_entry(ce->next, ce->addr,
+ ce->type_name, out_ce);
+ return FALSE;
+}
+
static int
resolve_list_entry(struct config_entry *ce, unsigned long long base_addr,
char *base_struct_name, char **out_type_name,
@@ -7308,31 +7362,14 @@ initialize_iteration_entry(struct config_entry *ie,
ie->line);
ie->next->flag &= ~SYMBOL_ENTRY;
}
- }
- else {
- if (ie->type_name) {
- /* looks like user has used 'within' keyword for
- * non-list_head VAR. Print the warning and continue.
- */
- ERRMSG("Warning: line %d: 'within' keyword not "
- "required for ArrayVar/StructVar.\n", ie->line);
- free(ie->type_name);
-
- /* remove the next list_head member from iteration
- * entry that would have added as part of 'within'
- * keyword processing.
- */
- if (ie->next) {
- free_config_entry(ie->next);
- ie->next = NULL;
- }
- }
- ie->type_name = strdup(type_name);
- }
- if (!ie->size) {
- ie->size = get_structure_size(ie->type_name,
- DWARF_INFO_GET_STRUCT_SIZE);
+ /*
+ * For list_head find out the size of the StructName and
+ * populate ie->size now. For array and link list we get the
+ * size info from config entry returned by
+ * get_next_list_entry().
+ */
+ ie->size = get_structure_size(ie->type_name, 0);
if (ie->size == FAILED_DWARFINFO) {
ERRMSG("Config error at %d: "
"Can't get size for type: %s.\n",
@@ -7345,8 +7382,7 @@ initialize_iteration_entry(struct config_entry *ie,
ie->line, ie->type_name);
return FALSE;
}
- }
- if (type_flag & TYPE_LIST_HEAD) {
+
if (!resolve_config_entry(ie->next, 0, ie->type_name))
return FALSE;
@@ -7356,6 +7392,34 @@ initialize_iteration_entry(struct config_entry *ie,
ie->next->line, ie->next->name);
return FALSE;
}
+ ie->type_flag = TYPE_STRUCT;
+ }
+ else {
+ if (ie->type_name) {
+ /* looks like user has used 'within' keyword for
+ * non-list_head VAR. Print the warning and continue.
+ */
+ ERRMSG("Warning: line %d: 'within' keyword not "
+ "required for ArrayVar/StructVar.\n", ie->line);
+ free(ie->type_name);
+
+ /* remove the next list_head member from iteration
+ * entry that would have added as part of 'within'
+ * keyword processing.
+ */
+ if (ie->next) {
+ free_config_entry(ie->next);
+ ie->next = NULL;
+ }
+ }
+ /*
+ * Set type flag for iteration entry. The iteration entry holds
+ * individual element from array/list, hence strip off the
+ * array type flag bit.
+ */
+ ie->type_name = strdup(type_name);
+ ie->type_flag = type_flag;
+ ie->type_flag &= ~TYPE_ARRAY;
}
return TRUE;
}
@@ -7363,25 +7427,29 @@ initialize_iteration_entry(struct config_entry *ie,
int
list_entry_empty(struct config_entry *le, struct config_entry *ie)
{
- unsigned long long addr;
+ struct config_entry ce;
/* Error out if arguments are not correct */
if (!(le->flag & LIST_ENTRY) || !(ie->flag & ITERATION_ENTRY)) {
ERRMSG("Invalid arguments\n");
return TRUE;
}
- addr = get_config_symbol_addr(le, 0, NULL);
- if (!addr)
+
+ memset(&ce, 0, sizeof(struct config_entry));
+ /* get next available entry from LIST entry. */
+ if (!get_next_list_entry(le, 0, NULL, &ce))
return TRUE;
if (ie->next) {
/* we are dealing with list_head */
- ie->next->addr = addr;
- ie->addr = addr - ie->next->offset;
- //resolve_iteration_entry(ie, addr);
+ ie->next->addr = ce.addr;
+ ie->addr = ce.addr - ie->next->offset;
+ }
+ else {
+ ie->addr = ce.addr;
+ ie->sym_addr = ce.sym_addr;
+ ie->size = ce.size;
}
- else
- ie->addr = addr;
return FALSE;
}
--
Mahesh J Salgaonkar
