Re: [PATCH 1/2] btrace: only search for lines in current symtab

[Jan Kratochvil <jan.kratochvil@redhat.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