On Jan 24, 2018, Jakub Jelinek <ja...@redhat.com> wrote:
> On Tue, Dec 12, 2017 at 12:52:18AM -0200, Alexandre Oliva wrote:
>> --- a/include/dwarf2.h
>> +++ b/include/dwarf2.h
>> @@ -298,6 +298,14 @@ enum dwarf_location_list_entry_type
>> DW_LLE_start_end = 0x07,
>> DW_LLE_start_length = 0x08,
>>
>> + /*
>> <http://lists.dwarfstd.org/private.cgi/dwarf-discuss-dwarfstd.org/2017-April/004347.html>
>> + has the proposal for now; only available to list members.
>> +
>> + A (possibly updated) copy of the proposal is available at
>> + <http://people.redhat.com/aoliva/papers/sfn/dwarf6-sfn-lvu.txt>. */
>> + DW_LLE_GNU_view_pair = 0x09,
>> +#define DW_LLE_view_pair DW_LLE_GNU_view_pair
>> +
> This looks wrong. The proposal has not been accepted yet, so you
> really can't know if DW_LLE_view_pair will be like that or whether it
> will have value of 9. Unfortunately, the DWARF standard doesn't specify a
> vendor range for DW_LLE_* values. I'd use 0xf0 or so, and don't pretend
> there is DW_LLE_view_pair at all, just use DW_LLE_GNU_view_pair everywhere.
> Jason, what do you think?
My reasoning was that, since we'd only emit this as an
explicitly-requested backward-incompatible extension to DWARF-5 (by
specifying -gdwarf-6 in this patch, but the option spelling could be
changed), any LLE number whatsoever would do. The point of the #define
was to write the code in GCC so that, once DW_LLE_view_pair was
standardized (if it ever was), we'd just set up an enum for it and we'd
be done, but that would only happen at DWARF6+ anyway, so we'd be able
to tell, since we'd then have actual DWARF6, rather than DWARF5 with an
incompatible extensions (which is what we emit with the current
-gdwarf-6 option; see below).
>> --- a/gcc/dwarf2asm.c
>> +++ b/gcc/dwarf2asm.c
>> @@ -767,6 +767,33 @@ dw2_asm_output_data_sleb128 (HOST_WIDE_INT value,
>> va_end (ap);
>> }
>>
>> +/* output symbol LAB1 as an unsigned LEB128 quantity. */
> Capital O in Output please.
Thanks, fixed.
>> +static inline bool
>> +dwarf2out_locviews_in_attribute ()
>> +{
>> + return debug_variable_location_views
>> + && dwarf_version <= 5;
> Formatting, should be
> return debug_variable_location_views && dwarf_version <= 5;
> or
> return (debug_variable_location_views
> && dwarf_version <= 5);
> if it wouldn't fit (but it does).
Hmm... Where does that mandate come from?, if you don't mind my asking.
I have just revised both GNU Coding Standards, and GCC-specific
conventions, to make sure I hadn't missed a change or some long-ago
established convention, and I couldn't find anything that supports that
"should".
I don't mind adjusting code to match your preferences (if that's what it
is), especially in modules you maintain, but I'd like to make sure I
haven't missed some requirement in the coding standards. Or maybe, if
it's more than a personal preference and rather a group preference, we
should add that at least to the GCC coding conventions, if most of us
agree with it.
Personally, I don't see that line breaks before operators are only
permitted when the operand that follows wouldn't fit; the standards are
more permissive than that. And I don't see that the parentheses are
mandatory either; GNU recommends them so that one doesn't have to indent
the subsequent lines by hand (or risk automatically reintendation to
undo that manual work), but it doesn't seem to mandate the indentation
or the paretheses, and the indentation one gets in their absence is IIUC
permitted and occasionally even desirable. Am I missing something?
Anyway, consider the requested changes done.
> Do we really need to introduce -gdwarf-6 at this point?
> -gdwarf-5 -gvariable-location-views should be sufficient, isn't it?
No, that means something else, namely emit location view lists in a
separate attribute, matching corresponding ranges.
Maybe we shouldn't even have an option to emit that at this point, or
make it a very different spelling. But I'd argue there's no point in
discarding the code that implements the format that was proposed for
standardization; at the very least it serves as a reference. That's
also an argument for retaining the ability to emit it somehow, even if
with an option that we document as to-be-dropped if/when there's a
standard representation.
--
Alexandre Oliva, freedom fighter http://FSFLA.org/~lxoliva/
You must be the change you wish to see in the world. -- Gandhi
Be Free! -- http://FSFLA.org/ FSF Latin America board member
Free Software Evangelist|Red Hat Brasil GNU Toolchain Engineer
