This is the mail archive of the elfutils-devel@sourceware.org mailing list for the elfutils project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PATCH][v2] Support .debug_macro


Mark Wielaard <mjw@redhat.com> writes:

>> > Why you decide to a include the version and line_offset interfaces on
>> > Dwarf_Macro? Isn't that a CU DIE concept? (See also below, maybe I just
>> > misunderstand the data representation.)
>> 
>> Both version and line offset belong to macro units (not CU's, each macro
>> unit may set its own .debug_line offset, and macro units have their own
>> version).  But there's a question of how to get to that value.  You need
>> version to interpret the opcode, and you need .debug_line offset to
>> interpret file names.
>
> The question is does the user of Dwarf_Macro need them? The user isn't
> going to check against the version, they are just matching against the
> DW_MACRO constants and for interpreting the arguments and file names
> they will use the attribute and helper functions provided.

The user needs at least the .debug_line offset, if they are supposed to
handle any start_file-like opcodes.  But I noticed we don't even have a
function like dwarf_offline, or dwarf_offsrcfiles, which I just assumed
would be present.  So the offset itself is not really useful anyway.  I
think the way forward then is to have an interface like this:

extern int dwarf_macro_srcfiles (Dwarf_Macro *macro, Dwarf_Files **files,
				 size_t *nfiles)

... this would return the files-section associated with the macro unit.
The user could then use the usual interfaces to query the files section
however they like.  I would like such interface betten than one that
works akin to dwarf_macro_param*, because it's more general.  There's no
telling where the file number comes from, it might be implied by the
opcode for all I care.

> For the version I guess this depends on whether we can treat
> DW_MACINFO_vendor_ext specially or not.
>
> So if possible I wouldn't expose them at all in the interface and just
> treat them as implementation details. Does that make sense?

If 0xff is made reserved in Dwarf 5, then version is not necessary.
Other than that... I don't know.  I guess should it be needed in the
future, we could probably add bits of version-dependent logic where it's
needed.  After all, CU's do have versions, and users generally don't
need to care about them, as it's all hidden in the attribute decoding
routines.

> Can the table be setup by _init() by marking init_macinfo_table with
> __attribute__ ((constructor))? It seems small enough to not really
> matter that it is done eagerly instead of lazily when macros are used
> and solves all the locking trickery.

I don't see why not, the initialization is fairly minimal.  I'll do it
this way.

>> But I guess we should match the CU version to whatever corresponds to
>> the .debug_macro version that this macro comes from (the two versions
>> can in theory evolve independently.  It's hard to imagine something like
>> that happening, but there could be a version of Dwarf that doesn't
>> change .debug_info at all, but does change .debug_macro).
>
> I am fine with either hard coding 4 for now or pick the MU version.
> Please just add a comment for future reference.

OK.

> This is just an implementation detail, so it doesn't really matter that
> much. I just had a bit of a hard time keeping track of which value came
> from where and thought it was easier to understand (and less value
> copying) if the struct looked like:
>
>  struct Dwarf_Macro_s
>  {
>    Dwarf_Macro_Op_Table *table;
>    uint8_t opcode;
>    Dwarf_Attribute *attributes;
>  };
>
> And then explicitly lookup line_offset, version and nargs using the
> table when using a Dwarf_Macro_s later in the code. Certainly not a big
> deal, I don't think the data representation really matters here. Just
> wondering why you had chosen this one.

I can't recall doing this for any reason in particular.  I'll look into
this again and possibly rewrite to what you propose, I don't immediately
see why it wouldn't work.

Thank you,
Petr

Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]