Re: Introduce dwarf-abbrev.pk pickle for decoding the .debug_abbrev section

[Martin Cermak]

On  Wed  2023-09-20  18:20 , Martin Cermak wrote:
> Hello Jose, folks,
>
> On  Tue  2023-09-19  11:29 , Jose E. Marchesi wrote:
> > > Hello folks,
> > >
> > > I suggest to introduce the dwarf-abbrev.pk pickle for decoding
> > > the .debug_abbrev section.  The patch comes with a testcase.
> >
> > Hello Martin.
> >
> > Thank you very much for working on this :)
> >
> > Please see some comments below.
> >
> > > -TESTS = test-dwarf.pk
> > > +TESTS = test-dwarf.pk test-dwarf-abbrev.pk
> >
> > I think it would be better to just put the new etsts in test-dwarf.pk.
> > Is a new file really necessary?
> >
> > > +var DW_CHILDREN_no = 0x00,
> > > +    DW_CHILDREN_yes = 0x01;
> > > +
> > > +type Pretty_DW_CHILDREN_T = struct
> > > +  {
> > > +    type Entry = struct
> > > +    {
> > > +        uint<64> tag;
> > > +        string value;
> > > +    };
> > > +
> > > +    Entry[] entries = Entry[]();
> > > +
> > > +    method add = (uint<64> tag, string value) void:
> > > +    {
> > > +      apush (entries, Entry { tag = tag, value = value });
> > > +    }
> > > +
> > > +    method get = (uint<64> tag) string:
> > > +    {
> > > +      for(entry in entries where entry.tag ==  tag)
> > > +        return entry.value;
> > > +      raise E_inval;
> > > +    }
> > > +  };
> >
> > I see these Pretty* data structures act like registries for DWARF
> > configuration parameters, a very similar approach that I used in the ELF
> > pickle.  I have some suggestions:
> >
> > - I would put the DWARF contiguration registry in its own pickle:
> >   dwarf-config.pk.  Similar to elf-config.pk in the ELF pickle.
> >
> > - I would use a single type Dwarf_Config, containing entries for all
> >   different kinds of parameters.   Something like this:
> >
> >     /* Valid classes are: dw-form, dw-at, ... */
> >
> >     type Dwarf_Config =
> >       struct
> >       {
> >         type Entry =
> >           struct
> >           {
> >             string class;
> >             uint<64> tag;
> >             string name;
> >             string doc;
> >           };
> >
> >         method add = (int<32> class, uint<64> tag, string name) void: { ... 
> > }
> >         method get_name = (int<32> class, uint<64> tag) string: { ... }
> >       };
> >
> > - Then, in dwarf-common.pk or perhaps in dwarf.pk, you can have a single
> >   variable for the entire registry:
> >
> >   Dwarf_Config dwarf_config;
> >
> > - Then in dwarf-info.pk or dwarf-abbrev.pk or wherever makes more sense,
> >   you populate the registry with invocations like:
> >
> >   dwarf_config.add
> >     :class "dw-at"
> >     :tag DW_AT_string_length
> >     :doc "This DIE attribute reflects the length in bytes of blah blah";
> >
> > - The Dwarf_Config type can have methods for checking whether a given
> >   value for some given CLASS is valid or not.  You can then use it for
> >   the constraints in the pickle structs.  Also, you can use it for
> >   pretty printing using field pretty-printers.  This is an example from
> >   elf-64.pk:
> >
> >   type Elf64_RelInfo =
> >     struct Elf64_Xword
> >     {
> >       uint<32> r_sym;
> >       uint<32> r_type : elf_config.check_enum ("reloc-types", elf_mach, 
> > r_type);
> >
> >       method _print_r_type = void:
> >       {
> >         printf "#<%s>", elf_config.format_enum ("reloc-types", elf_mach, 
> > r_type);
> >       }
> >     };
> >
> >   Note how this type uses services of the ELF configuration registry
> >   (elf_config) in order to both check the integrity of the field r_type
> >   and to pretty-print it.
> >
> > WDYT?
>
> Thank you for the suggestions!  Absolutely, all that makes sense.  Please,
> check the attached patch hopefully addressing the aforementioned isues.
>
> Does it look good to you now?

I've found a typo in the previously attached patch.
Please, consider this updated my.patch2.

Thoughts?

Martin

my.patch2
Description: Text document