| Message ID | 23509d85-8a73-4d81-7ade-435daf46fcd6@suse.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | x86: aid debug info generation in assembly files | expand |
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__ Thanks, Roger.
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(). Jan
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? Thanks, Roger.
On Thu, Apr 14, 2022 at 03:31:26PM +0200, 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 ^n't Sorry.
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
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@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). 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. Acked-by: roger Pau Monné <roger.pau@citrix.com> Thanks, Roger.
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@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). > > 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@citrix.com> Thanks. Given the above, I guess I'll apply this only provisionally. Jan
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@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). > > 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. Actually I think I'll withdraw this patch. After quite a bit of further consideration, it should really be the assembler to get this right, and once properly working there the directives added here may actually get in the way. Jan
--- 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__ + /* * Saving and restoring MSR_SPEC_CTRL state is a little tricky. * @@ -89,6 +91,7 @@ */ .macro DO_OVERWRITE_RSB tmp=rax + FILE_AND_LINE /* * Requires nothing * Clobbers \tmp (%rax by default), %rcx @@ -137,6 +140,7 @@ .endm .macro DO_SPEC_CTRL_ENTRY maybexen:req + FILE_AND_LINE /* * Requires %rsp=regs (also cpuinfo if !maybexen) * Requires %r14=stack_end (if maybexen) @@ -171,6 +175,7 @@ .endm .macro DO_SPEC_CTRL_EXIT_TO_XEN + FILE_AND_LINE /* * Requires %rbx=stack_end * Clobbers %rax, %rcx, %rdx @@ -192,6 +197,7 @@ .endm .macro DO_SPEC_CTRL_EXIT_TO_GUEST + FILE_AND_LINE /* * Requires %eax=spec_ctrl, %rsp=regs/cpuinfo * Clobbers %rcx, %rdx @@ -241,6 +247,7 @@ * been reloaded. */ .macro SPEC_CTRL_ENTRY_FROM_INTR_IST + FILE_AND_LINE /* * Requires %rsp=regs, %r14=stack_end * Clobbers %rax, %rcx, %rdx @@ -288,6 +295,7 @@ UNLIKELY_DISPATCH_LABEL(\@_serialise): /* Use when exiting to Xen in IST context. */ .macro SPEC_CTRL_EXIT_TO_XEN_IST + FILE_AND_LINE /* * Requires %rbx=stack_end * Clobbers %rax, %rcx, %rdx --- a/xen/arch/x86/indirect-thunk.S +++ b/xen/arch/x86/indirect-thunk.S @@ -12,6 +12,7 @@ #include <asm/asm_defns.h> .macro IND_THUNK_RETPOLINE reg:req + .line __LINE__ call 2f 1: lfence
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