Re: [PATCH] libdw: dwarf_getsrcfiles should not imply dwarf_getsrclines

[Aaron Merey <amerey@redhat.com>]

Hi Mark,

On Wed, Apr 10, 2024 at 3:48 PM Mark Wielaard <mark@klomp.org> wrote:
> > v2 changes:
> > Restored support for DW_LNE_define_file.
>
> Great. And sorry I first suggested to just drop it and then said I
> would like it back. This was more work than I though.

No problem, better if we support this just in case.

> This one line uses tabs for indentation, all others spaces.
> [...]
> And this last block uses tabs again.
> [...]
> Here tabs again, then the rest spaces.
> [...]
> So inconsistent indentation, but the code looks good.

Fixed the indentation.

> > -  /* Pass the file data structure to the caller.  */
> > -  if (filesp != NULL)
> > -    *filesp = files;
> > +      const char **newdirs = (void *) &newfiles->info[nnewfiles];
> > +      const char **prevdirs = (void *) &prevfiles->info[nprevfiles];
> > +
> > +      /* Copy prevdirs to newdirs.  */
> > +      for (size_t n = 0; n < ndirs; n++)
> > +     newdirs[n] = prevdirs[n];
>
> Again slightly off indentation.
> But I also don't fully follow the prevdirs/newdirs copying.
> Why is this? No newdirs are defined here, are there?
> Maybe I don't understand the data-structure used here.

The directories are the same but we still need to copy them so that
dwarf_getsrcdirs can find newfiles' dir names.

Dwarf_Files is an unusual structure since it doesn't contain a member
specifically for the array of dirnames.  Instead they're stored at
the end of the Dwarf_Fileinfo array member.

> So testfile-define-file is actually testfile36.debug but with a new
> line program? How did you edit/insert that one?

I used xxd to create a hexdump of testfile36.debug and modified the line
program by hand with a text editor.  I converted the modified hexdump
back to a binary with xxd -r.

I chose testfile36.debug because its .debug_line includes multiple
directory and file names.  It also contains a single short line program
that was easy to replace with two DW_LNE_define_file opcodes without
corrupting things.

Aaron