Fix off-by-one bug in dwarf 5 file indexes.

  Fix dw2_get_file_names_reader behavior where it treated file names in
line header as one indexed.

gdb/ChangeLog:

	* dwarf2/read.c (dw2_get_file_names_reader): Bug fix.
---
 gdb/dwarf2/read.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)
  

>>>>> "Ali" == Ali Tamur via gdb-patches <gdb-patches@sourceware.org> writes:

Ali> Fix dw2_get_file_names_reader behavior where it treated file names in
Ali> line header as one indexed.

Ali> gdb/ChangeLog:

Ali> 	* dwarf2/read.c (dw2_get_file_names_reader): Bug fix.

Normally you should mention how the patch was tested.  If it triggers
for some existing test, then you can just note that.  Otherwise, it's
normal to add a new test, unless there's some reason that this is too
difficult.

thanks,
Tom
  

Hi,

Sorry I have tried more than a day and I can neither find a test whose
success depends solely on this change, nor was I able to construct
one. It seems to me gdb is too resilient for small errors in debug
sections. Current code ignores the first file name entry in the line
header for dwarf 5, when constructing
dwarf2_per_cu_data.v.quick.file_names:
- Suppose the line header has two file name entries in m_file_names[0]
and m_file_names[1]. A possible current code flow for the first
iteration is:
file_full_name(i + 1) => fule_file_name(1) => lh->file_name_at (1) =>
(because version >= 5) return &m_file_names[1];
So, m_file_names[0] is never used.. Can we apply the 'obvious rule' to
submit this?

Thanks,
Ali



On Thu, Feb 13, 2020 at 6:24 AM Tom Tromey <tom@tromey.com> wrote:
>
> >>>>> "Ali" == Ali Tamur via gdb-patches <gdb-patches@sourceware.org> writes:
>
> Ali> Fix dw2_get_file_names_reader behavior where it treated file names in
> Ali> line header as one indexed.
>
> Ali> gdb/ChangeLog:
>
> Ali>    * dwarf2/read.c (dw2_get_file_names_reader): Bug fix.
>
> Normally you should mention how the patch was tested.  If it triggers
> for some existing test, then you can just note that.  Otherwise, it's
> normal to add a new test, unless there's some reason that this is too
> difficult.
>
> thanks,
> Tom
  

On 2020-02-16 1:21 a.m., Ali Tamur via gdb-patches wrote:
> Hi,
> 
> Sorry I have tried more than a day and I can neither find a test whose
> success depends solely on this change, nor was I able to construct
> one. It seems to me gdb is too resilient for small errors in debug
> sections. Current code ignores the first file name entry in the line
> header for dwarf 5, when constructing
> dwarf2_per_cu_data.v.quick.file_names:
> - Suppose the line header has two file name entries in m_file_names[0]
> and m_file_names[1]. A possible current code flow for the first
> iteration is:
> file_full_name(i + 1) => fule_file_name(1) => lh->file_name_at (1) =>
> (because version >= 5) return &m_file_names[1];
> So, m_file_names[0] is never used.. Can we apply the 'obvious rule' to
> submit this?
> 
> Thanks,
> Ali

Please describe what is the impact of the bug.  I presume the consequence is that
quick_file_names::file_names array will be not be constructed correctly?  What does
it contain in a gdb pre-your patch, and what does it contain in a gdb post-your
patch?  Already, it would help convince the reader (the human reader, not the dwarf
reader) that there is indeed a bug an that your fix is right.

If that array is not constructed correctly, then surely something will not work
correctly down the line, like finding a symbol by file name or something like that?
Unless the DWARF5 support of GDB is not evolved enough yet to get to the point where
that array would be used?

Simon