This is the mail archive of the
mailing list for the elfutils project.
Re: [PATCH][v2] Support .debug_macro
- From: Petr Machata <pmachata at redhat dot com>
- To: elfutils-devel at lists dot fedorahosted dot org
- Date: Tue, 30 Sep 2014 18:06:08 +0200
- Subject: Re: [PATCH][v2] Support .debug_macro
Mark Wielaard <email@example.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,
... 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
> 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
>> 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.
> 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.