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

login
register
mail settings
Submitter Doug Evans via gdb-patches
Date Feb. 13, 2020, 5:39 a.m.
Message ID <20200213053920.144599-1-tamur@google.com>
Download mbox | patch
Permalink /patch/38016/
State New
Headers show

Comments

Doug Evans via gdb-patches - Feb. 13, 2020, 5:39 a.m.
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(-)
Tom Tromey - Feb. 13, 2020, 2:24 p.m.
>>>>> "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
Doug Evans via gdb-patches - Feb. 16, 2020, 6:21 a.m.
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

Patch

diff --git a/gdb/dwarf2/read.c b/gdb/dwarf2/read.c
index 7edbd9d7df..988d44a714 100644
--- a/gdb/dwarf2/read.c
+++ b/gdb/dwarf2/read.c
@@ -3160,8 +3160,9 @@  dw2_get_file_names_reader (const struct die_reader_specs *reader,
     XOBNEWVEC (&objfile->objfile_obstack, const char *, qfn->num_file_names);
   if (offset != 0)
     qfn->file_names[0] = xstrdup (fnd.name);
+  int index_offset = lh->version < 5 ? 1 : 0;
   for (int i = 0; i < lh->file_names_size (); ++i)
-    qfn->file_names[i + offset] = lh->file_full_name (i + 1,
+    qfn->file_names[i + offset] = lh->file_full_name (i + index_offset,
 						      fnd.comp_dir).release ();
   qfn->real_names = NULL;