This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: [PATCH 1/2] btrace: only search for lines in current symtab
- From: Jan Kratochvil <jan dot kratochvil at redhat dot com>
- To: Markus Metzger <markus dot t dot metzger at intel dot com>
- Cc: gdb-patches at sourceware dot org
- Date: Fri, 21 Mar 2014 18:43:22 +0100
- Subject: Re: [PATCH 1/2] btrace: only search for lines in current symtab
- Authentication-results: sourceware.org; auth=none
- References: <1394182665-14164-1-git-send-email-markus dot t dot metzger at intel dot com> <1394182665-14164-2-git-send-email-markus dot t dot metzger at intel dot com>
On Fri, 07 Mar 2014 09:57:44 +0100, Markus Metzger wrote:
[...]
> @@ -543,32 +532,39 @@ ftrace_update_function (struct gdbarch *gdbarch,
> static void
> ftrace_update_lines (struct btrace_function *bfun, CORE_ADDR pc)
> {
> - struct symtab_and_line sal;
> - const char *fullname;
> + struct linetable *lines;
> + struct symtab *symtab;
> + int i;
>
> - sal = find_pc_line (pc, 0);
> - if (sal.symtab == NULL || sal.line == 0)
> - {
> - DEBUG_FTRACE ("no lines at %s", core_addr_to_string_nz (pc));
> - return;
> - }
> + symtab = bfun->symtab;
> + if (symtab == NULL)
> + return;
> +
> + lines = LINETABLE (symtab);
> + if (lines == NULL)
> + return;
>
> - /* Check if we switched files. This could happen if, say, a macro that
> - is defined in another file is expanded here. */
> - fullname = symtab_to_fullname (sal.symtab);
> - if (ftrace_skip_file (bfun, fullname))
> + for (i = 0; i < lines->nitems; ++i)
The line table has ordered PC values so one can find the right line in
logarithmic time.
> {
> - DEBUG_FTRACE ("ignoring file at %s, file=%s",
> - core_addr_to_string_nz (pc), fullname);
> - return;
> - }
> + struct linetable_entry *entry;
>
> - /* Update the line range. */
> - bfun->lbegin = min (bfun->lbegin, sal.line);
> - bfun->lend = max (bfun->lend, sal.line);
> + entry = &lines->item[i];
> + if (entry->pc == pc)
This is not right, PC can be between two ENTRY->PC values and it should match
the former line. This implementation would not find a matching line.
> + {
> + int line;
>
> - if (record_debug > 1)
> - ftrace_debug (bfun, "update lines");
> + line = entry->line;
> +
> + /* Update the line range. */
> + bfun->lbegin = min (bfun->lbegin, line);
> + bfun->lend = max (bfun->lend, line);
> +
> + if (record_debug > 1)
> + ftrace_debug (bfun, "update lines");
> +
> + break;
> + }
> + }
> }
>
> /* Add the instruction at PC to BFUN's instructions. */
I do not think this is the right approach, find_pc_sect_line() can be improved
to at least look up the lines in logarithmic time by bisecting the range.
It could also use some cache of recent lookups.
Also I am not sure this new implementation handles correctly include files and
their boundaries, which find_pc_sect_line() has to take care of.
This is my personal opinion unrelated to GDB project maintainers' opinion.
Regards,
Jan