This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: [RFA] Cleanup: Use add_sal_to_sals for expressions
- From: Keith Seitz <keiths at redhat dot com>
- To: Tom Tromey <tromey at redhat dot com>
- Cc: "gp >> \"gdb-patches at sourceware dot org ml\"" <gdb-patches at sourceware dot org>
- Date: Wed, 18 Jul 2012 12:14:21 -0700
- Subject: Re: [RFA] Cleanup: Use add_sal_to_sals for expressions
- References: <5005D93D.7020808@redhat.com> <878veh3u4z.fsf@fleche.redhat.com>
On 07/18/2012 09:34 AM, Tom Tromey wrote:
"Keith" == Keith Seitz <keiths@redhat.com> writes:
Keith> This patch causes expressions to use add_sal_to_sals, which will then
Keith> set a canonical name for the linespec, thereby negating the need to
Keith> double-check for missing linespecs later (in decode_line_full).
Keith> @@ -842,6 +842,8 @@ add_sal_to_sals (struct linespec_state *self,
Keith> else
Keith> canonical_name = xstrprintf ("%s:%d", filename, sal->line);
Keith> }
Keith> + else if (symname != NULL)
Keith> + canonical_name = xstrdup (symname);
[...]
Keith> + sal = find_pc_line (ls->expr_pc, 0);
Keith> + sal.pc = ls->expr_pc;
Keith> + sal.section = find_pc_overlay (ls->expr_pc);
Keith> + sal.explicit_pc = 1;
Keith> + add_sal_to_sals (state, &sals, &sal, ls->expression);
It seems to me that find_pc_line can return a sal with symtab set;
search for "val.symtab = " in find_pc_sect_line. In this case it seems
that you'd wind up with the wrong canonical form.
Yes, you are correct. [As if you didn't know that already!]
As much as I hate to extend the API for add_sal_to_sals (especially for
a special case like this), but I really want [to try?] to isolate name
canonicalization in one place, and add_sal_to_sals seems like the best
candidate for that.
So, what do you think of this revised patch? [No regressions on x86-64
linux.]
Keith
ChangeLog
2012-07-18 Keith Seitz <keiths@redhat.com>
* linespec.c (add_sal_to_sals): Add LITERAL_CANONICAL
parameter. If non-zero, use SYMNAME as the canonical name
for the SaL.
Update all callers.
(convert_linespec_to_sals): Use add_sal_to_sals for
expressions, too.
(decode_line_full): No need to "fill in missing canonical names"
anymore. Simply make cleanups for the allocated names.
diff --git a/gdb/linespec.c b/gdb/linespec.c
index 4156694..4c9c110 100644
--- a/gdb/linespec.c
+++ b/gdb/linespec.c
@@ -810,13 +810,16 @@ add_sal_to_sals_basic (struct symtabs_and_lines *sals,
/* Add SAL to SALS, and also update SELF->CANONICAL_NAMES to reflect
the new sal, if needed. If not NULL, SYMNAME is the name of the
- symbol to use when constructing the new canonical name. */
+ symbol to use when constructing the new canonical name.
+
+ If LITERAL_CANONICAL is non-zero, SYMNAME will be used as the
+ canonical name for the SAL. */
static void
add_sal_to_sals (struct linespec_state *self,
struct symtabs_and_lines *sals,
struct symtab_and_line *sal,
- const char *symname)
+ const char *symname, int literal_canonical)
{
add_sal_to_sals_basic (sals, sal);
@@ -826,7 +829,7 @@ add_sal_to_sals (struct linespec_state *self,
self->canonical_names = xrealloc (self->canonical_names,
sals->nelts * sizeof (char *));
- if (sal->symtab && sal->symtab->filename)
+ if (!literal_canonical && sal->symtab && sal->symtab->filename)
{
char *filename = sal->symtab->filename;
@@ -842,6 +845,8 @@ add_sal_to_sals (struct linespec_state *self,
else
canonical_name = xstrprintf ("%s:%d", filename, sal->line);
}
+ else if (symname != NULL)
+ canonical_name = xstrdup (symname);
self->canonical_names[sals->nelts - 1] = canonical_name;
}
@@ -1809,7 +1814,7 @@ create_sals_line_offset (struct linespec_state *self,
found. */
intermediate_results.sals[i].line = val.line;
add_sal_to_sals (self, &values, &intermediate_results.sals[i],
- sym ? SYMBOL_NATURAL_NAME (sym) : NULL);
+ sym ? SYMBOL_NATURAL_NAME (sym) : NULL, 0);
}
do_cleanups (cleanup);
@@ -1837,13 +1842,14 @@ convert_linespec_to_sals (struct linespec_state *state, linespec_p ls)
if (ls->expression != NULL)
{
+ struct symtab_and_line sal;
+
/* We have an expression. No other attribute is allowed. */
- sals.sals = XMALLOC (struct symtab_and_line);
- sals.nelts = 1;
- sals.sals[0] = find_pc_line (ls->expr_pc, 0);
- sals.sals[0].pc = ls->expr_pc;
- sals.sals[0].section = find_pc_overlay (ls->expr_pc);
- sals.sals[0].explicit_pc = 1;
+ sal = find_pc_line (ls->expr_pc, 0);
+ sal.pc = ls->expr_pc;
+ sal.section = find_pc_overlay (ls->expr_pc);
+ sal.explicit_pc = 1;
+ add_sal_to_sals (state, &sals, &sal, ls->expression, 1);
}
else if (ls->labels.label_symbols != NULL)
{
@@ -1856,7 +1862,7 @@ convert_linespec_to_sals (struct linespec_state *state, linespec_p ls)
{
symbol_to_sal (&sal, state->funfirstline, sym);
add_sal_to_sals (state, &sals, &sal,
- SYMBOL_NATURAL_NAME (sym));
+ SYMBOL_NATURAL_NAME (sym), 0);
}
}
else if (ls->function_symbols != NULL || ls->minimal_symbols != NULL)
@@ -1882,7 +1888,8 @@ convert_linespec_to_sals (struct linespec_state *state, linespec_p ls)
set_current_program_space (pspace);
symbol_to_sal (&sal, state->funfirstline, sym);
if (maybe_add_address (state->addr_set, pspace, sal.pc))
- add_sal_to_sals (state, &sals, &sal, SYMBOL_NATURAL_NAME (sym));
+ add_sal_to_sals (state, &sals, &sal,
+ SYMBOL_NATURAL_NAME (sym), 0);
}
}
@@ -2284,19 +2291,15 @@ decode_line_full (char **argptr, int flags,
gdb_assert (canonical->addr_string != NULL);
canonical->pre_expanded = 1;
- /* Fill in the missing canonical names. */
+ /* Arrange for allocated canonical names to be freed. */
if (result.nelts > 0)
{
int i;
- if (state->canonical_names == NULL)
- state->canonical_names = xcalloc (result.nelts, sizeof (char *));
make_cleanup (xfree, state->canonical_names);
for (i = 0; i < result.nelts; ++i)
{
- if (state->canonical_names[i] == NULL)
- state->canonical_names[i] = savestring (arg_start,
- *argptr - arg_start);
+ gdb_assert (state->canonical_names[i] != NULL);
make_cleanup (xfree, state->canonical_names[i]);
}
}
@@ -3110,7 +3113,7 @@ decode_digits_list_mode (struct linespec_state *self,
val.pc = 0;
val.explicit_line = 1;
- add_sal_to_sals (self, values, &val, NULL);
+ add_sal_to_sals (self, values, &val, NULL, 0);
}
}
@@ -3254,7 +3257,7 @@ minsym_found (struct linespec_state *self, struct objfile *objfile,
skip_prologue_sal (&sal);
if (maybe_add_address (self->addr_set, objfile->pspace, sal.pc))
- add_sal_to_sals (self, result, &sal, SYMBOL_NATURAL_NAME (msymbol));
+ add_sal_to_sals (self, result, &sal, SYMBOL_NATURAL_NAME (msymbol), 0);
}
/* A helper struct to pass some data through