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

From: Jan Beulich <jbeulich@suse.com>
To: "Roger Pau Monné" <roger.pau@citrix.com>
Cc: "xen-devel@lists.xenproject.org" <xen-devel@lists.xenproject.org>,
	Andrew Cooper <andrew.cooper3@citrix.com>, Wei Liu <wl@xen.org>
Subject: Re: [PATCH 1/2] x86: improve .debug_line contents for assembly sources
Date: Thu, 14 Apr 2022 16:15:22 +0200	[thread overview]
Message-ID: <2eab851f-0fe3-8462-cdbf-b438dc01ade1@suse.com> (raw)
In-Reply-To: <YlgiLhvsKVYKKvrr@Air-de-Roger>

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@suse.com>
>>>>
>>>> [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).

Jan