Re: [PATCH 1/2] x86: improve .debug_line contents for assembly sources

[Jan Beulich <jbeulich@xxxxxxxx>]

On 14.04.2022 18:02, Roger Pau Monné wrote:
> On Thu, Apr 14, 2022 at 04:15:22PM +0200, Jan Beulich wrote:
>> On 14.04.2022 15:31, Roger Pau Monné wrote:
>>> On Thu, Apr 14, 2022 at 02:52:47PM +0200, Jan Beulich wrote:
>>>> On 14.04.2022 14:40, Roger Pau Monné wrote:
>>>>> On Tue, Apr 12, 2022 at 12:27:34PM +0200, Jan Beulich wrote:
>>>>>> While future gas versions will allow line number information to be
>>>>>> generated for all instances of .irp and alike [1][2], the same isn't
>>>>>> true (nor immediately intended) for .macro [3]. Hence macros, when they
>>>>>> do more than just invoke another macro or issue an individual insn, want
>>>>>> to have .line directives (in header files also .file ones) in place.
>>>>>>
>>>>>> Signed-off-by: Jan Beulich <jbeulich@xxxxxxxx>
>>>>>>
>>>>>> [1] 
>>>>>> https://sourceware.org/git?p=binutils-gdb.git;a=commitdiff;h=7992631e8c0b0e711fbaba991348ef6f6e583725
>>>>>> [2] 
>>>>>> https://sourceware.org/git?p=binutils-gdb.git;a=commitdiff;h=2ee1792bec225ea19c71095cee5a3a9ae6df7c59
>>>>>> [3] 
>>>>>> https://sourceware.org/git?p=binutils-gdb.git;a=commitdiff;h=6d1ace6861e999361b30d1bc27459ab8094e0d4a
>>>>>> ---
>>>>>> Using .file has the perhaps undesirable side effect of generating a fair
>>>>>> amount of (all identical) STT_FILE entries in the symbol table. We also
>>>>>> can't use the supposedly assembler-internal (and hence undocumented)
>>>>>> .appfile anymore, as it was removed [4]. Note that .linefile (also
>>>>>> internal/undocumented) as well as the "# <line> <file>" constructs the
>>>>>> compiler emits, leading to .linefile insertion by the assembler, aren't
>>>>>> of use anyway as these are processed and purged when processing .macro
>>>>>> [3].
>>>>>>
>>>>>> [4] 
>>>>>> https://sourceware.org/git?p=binutils-gdb.git;a=commitdiff;h=c39e89c3aaa3a6790f85e80f2da5022bc4bce38b
>>>>>>
>>>>>> --- a/xen/arch/x86/include/asm/spec_ctrl_asm.h
>>>>>> +++ b/xen/arch/x86/include/asm/spec_ctrl_asm.h
>>>>>> @@ -24,6 +24,8 @@
>>>>>>  #include <asm/msr-index.h>
>>>>>>  #include <asm/spec_ctrl.h>
>>>>>>  
>>>>>> +#define FILE_AND_LINE .file __FILE__; .line __LINE__
>>>>>
>>>>> Seeing as this seems to get added to all macros below, I guess you did
>>>>> consider (and discarded) introducing a preprocessor macro do to the
>>>>> asm macro definitons:
>>>>>
>>>>> #define DECLARE_MACRO(n, ...) \
>>>>> .macro n __VA_ARGS__ \
>>>>>     .file __FILE__; .line __LINE__
>>>>
>>>> No, I didn't even consider that. I view such as too obfuscating - there's
>>>> then e.g. no visual match with the .endm. Furthermore, as outlined in the
>>>> description, I don't think this wants applying uniformly. There are
>>>> macros which better don't have this added. Yet I also would prefer to not
>>>> end up with a mix of .macro and DECLARE_MACRO().
>>>
>>> I think it's a dummy question, but why would we want to add this to
>>> some macros?
>>>
>>> Isn't it better to always have the file and line reference where the
>>> macro gets used?
>>
>> Like said in the description, a macro simply invoking another macro,
>> or a macro simply wrapping a single insn, is likely better to have
>> its generated code associated with the original line number. Complex
>> macros, otoh, are imo often better to have line numbers associated
>> with actual macro contents. IOW to some degree I support the cited
>> workaround in binutils (which has been there for many years).
> 
> Seems a bit ad-hoc policy, but it's you and Andrew that mostly deal
> with this stuff, so if you are fine with it.

What other rule of thumb would you suggest? I'd be happy to take
suggestions rather than force in something which looks to be not
entirely uncontroversial.

> Acked-by: roger Pau Monné <roger.pau@xxxxxxxxxx>

Thanks. Given the above, I guess I'll apply this only provisionally.

Jan