* [PATCH 4/5] Arm: avoid unhelpful use of .macro in testsuite
2022-11-29 10:34 [PATCH 0/5] gas: diagnostics for macros and some tidying Jan Beulich
` (2 preceding siblings ...)
2022-11-29 10:37 ` [PATCH 3/5] gas: add Dwarf line number test for .macro expansions Jan Beulich
@ 2022-11-29 10:38 ` Jan Beulich
2022-11-29 10:44 ` [PATCH 5/5] gas: re-work line number tracking for macros and their expansions Jan Beulich
4 siblings, 0 replies; 6+ messages in thread
From: Jan Beulich @ 2022-11-29 10:38 UTC (permalink / raw)
To: Binutils; +Cc: Nick Clifton, ramana.radhakrishnan, Richard Earnshaw
Macros with just a single use site are a little pointless to have.
Expand sp-usage-thumb2-relax'es inline, avoiding the need to touch the
testcase when diagnostics for code inside macros are changed.
While there also make what was "iter_mla" cover smlatt as well, rather
than testing smlabt twice.
---
If this removal of the use of .macro is deemed acceptable, I'd like to
extend it to some of the "[Bb]ad MVE ..." testcases as well (see also
the subsequent patch).
--- a/gas/testsuite/gas/arm/sp-usage-thumb2-relax-on-v7.l
+++ b/gas/testsuite/gas/arm/sp-usage-thumb2-relax-on-v7.l
@@ -1,17 +1,17 @@
[^:]*: Assembler messages:
-[^:]*:25: Error: r13 not allowed here -- `add.w sp,r7,#1'
-[^:]*:25: Error: r13 not allowed here -- `sub.w sp,r7,#1'
-[^:]*:25: Error: r13 not allowed here -- `addw sp,r7,#1'
-[^:]*:25: Error: r13 not allowed here -- `subw sp,r7,#1'
-[^:]*:26: Error: r13 not allowed here -- `bic r7,sp,r2'
-[^:]*:26: Error: r13 not allowed here -- `sbcs r7,sp,r2'
-[^:]*:26: Error: r13 not allowed here -- `and r7,sp,r2'
-[^:]*:26: Error: r13 not allowed here -- `eor r7,sp,r2'
-[^:]*:27: Error: r13 not allowed here -- `smlabb sp,sp,sp,sp'
-[^:]*:27: Error: r13 not allowed here -- `smlabb r0,sp,r3,r11'
-[^:]*:27: Error: r13 not allowed here -- `smlatb sp,sp,sp,sp'
-[^:]*:27: Error: r13 not allowed here -- `smlatb r0,sp,r3,r11'
-[^:]*:27: Error: r13 not allowed here -- `smlabt sp,sp,sp,sp'
-[^:]*:27: Error: r13 not allowed here -- `smlabt r0,sp,r3,r11'
-[^:]*:27: Error: r13 not allowed here -- `smlabt sp,sp,sp,sp'
-[^:]*:27: Error: r13 not allowed here -- `smlabt r0,sp,r3,r11'
+[^:]*:7: Error: r13 not allowed here -- `add.w sp,r7,#1'
+[^:]*:7: Error: r13 not allowed here -- `sub.w sp,r7,#1'
+[^:]*:7: Error: r13 not allowed here -- `addw sp,r7,#1'
+[^:]*:7: Error: r13 not allowed here -- `subw sp,r7,#1'
+[^:]*:11: Error: r13 not allowed here -- `bic r7,sp,r2'
+[^:]*:11: Error: r13 not allowed here -- `sbcs r7,sp,r2'
+[^:]*:11: Error: r13 not allowed here -- `and r7,sp,r2'
+[^:]*:11: Error: r13 not allowed here -- `eor r7,sp,r2'
+[^:]*:15: Error: r13 not allowed here -- `smlabb sp,sp,sp,sp'
+[^:]*:16: Error: r13 not allowed here -- `smlabb r0,sp,r3,r11'
+[^:]*:15: Error: r13 not allowed here -- `smlatb sp,sp,sp,sp'
+[^:]*:16: Error: r13 not allowed here -- `smlatb r0,sp,r3,r11'
+[^:]*:15: Error: r13 not allowed here -- `smlabt sp,sp,sp,sp'
+[^:]*:16: Error: r13 not allowed here -- `smlabt r0,sp,r3,r11'
+[^:]*:15: Error: r13 not allowed here -- `smlatt sp,sp,sp,sp'
+[^:]*:16: Error: r13 not allowed here -- `smlatt r0,sp,r3,r11'
--- a/gas/testsuite/gas/arm/sp-usage-thumb2-relax-on-v8.d
+++ b/gas/testsuite/gas/arm/sp-usage-thumb2-relax-on-v8.d
@@ -21,5 +21,5 @@ Disassembly of section \.text:
.*: fb1d b023 smlatb r0, sp, r3, fp
.*: fb1d dd1d smlabt sp, sp, sp, sp
.*: fb1d b013 smlabt r0, sp, r3, fp
-.*: fb1d dd1d smlabt sp, sp, sp, sp
-.*: fb1d b013 smlabt r0, sp, r3, fp
+.*: fb1d dd3d smlatt sp, sp, sp, sp
+.*: fb1d b033 smlatt r0, sp, r3, fp
--- a/gas/testsuite/gas/arm/sp-usage-thumb2-relax.s
+++ b/gas/testsuite/gas/arm/sp-usage-thumb2-relax.s
@@ -1,27 +1,17 @@
- .macro iter_addsub
+ .syntax unified
+ .text
+ .thumb
+ .global foo
+foo:
.irp m, add.w, sub.w, addw, subw
\m sp, r7, #1
.endr
- .endm
- .macro iter_arith3
.irp m, bic, sbcs, and, eor
\m r7, sp, r2
.endr
- .endm
- .macro iter_mla
- .irp m, smlabb, smlatb, smlabt, smlabt
+ .irp m, smlabb, smlatb, smlabt, smlatt
\m sp, sp, sp, sp
\m r0, sp, r3, r11
.endr
- .endm
-
- .syntax unified
- .text
- .thumb
- .global foo
-foo:
- iter_addsub
- iter_arith3
- iter_mla
^ permalink raw reply [flat|nested] 6+ messages in thread* [PATCH 5/5] gas: re-work line number tracking for macros and their expansions
2022-11-29 10:34 [PATCH 0/5] gas: diagnostics for macros and some tidying Jan Beulich
` (3 preceding siblings ...)
2022-11-29 10:38 ` [PATCH 4/5] Arm: avoid unhelpful use of .macro in testsuite Jan Beulich
@ 2022-11-29 10:44 ` Jan Beulich
4 siblings, 0 replies; 6+ messages in thread
From: Jan Beulich @ 2022-11-29 10:44 UTC (permalink / raw)
To: Binutils
Cc: Nick Clifton, ramana.radhakrishnan, Richard Earnshaw,
Palmer Dabbelt, Andrew Waterman, Jim Wilson, Nelson Chu,
Marcus Shawcroft, Alan Modra, Peter Bergner, Geoff Keating
[-- Attachment #1: Type: text/plain, Size: 16209 bytes --]
The PR gas/16908 workaround aimed at uniformly reporting line numbers
to reference macro invocation sites. As mentioned in a comment this may
be desirable for small macros, but often isn't for larger ones. As a
first step improve diagnostics to report both locations, while aiming at
leaving generated debug info unaltered.
Note that macro invocation context is lost for any diagnostics issued
only after all input was processed (or more generally for any use of
as_*_where(), as the functions can't know whether the passed in location
is related to [part of] the present stack of locations). To maintain the
intended workaround behavior for PR gas/16908, a new as_where() is
introduced to "look through" macro invocations, while the existing
as_where() is renamed (and used in only very few places for now). Down
the road as_where() will likely want to return a list of (file,line)
pairs.
---
Omitting testsuite adjustments from the inline patch, for its sheer size;
please see the attachment for the full (compressed) patch.
---
There are 86 more Arm "[Bb]ad MVE ..." testsuite failures, which I'm not
(yet) taking care of here. If the concept is not objected to, I'd go and
alter them all in the same mechanical way as done here for other
testcase adjustments. (In cases of macros which are used exactly once in
a testcase, of which there are a fair number, dropping the use of .macro
may also be an option, just like already done in "Arm: avoid unhelpful
use of .macro in testsuite" for another testcase.)
---
Adding
if (subseg_text_p (now_seg))
dwarf2_emit_insn (0);
to try_macro() (alongside using as_where_top() in dwarf2_where()) would
get .debug_line contents into reasonable shape (expressing both macro
invocation site and macro definition location). But that's likely
insufficient: We may want / need to represent macros as inline
subprograms and their expansions as inlined subroutines. Which in turn
looks cumbersome especially with the relatively recently added recording
of functions (with which macro expansions then would better be
associated, when marked as such).
--- a/gas/as.h
+++ b/gas/as.h
@@ -436,6 +436,10 @@ typedef struct _pseudo_type pseudo_typeS
#define PRINTF_WHERE_LIKE(FCN) \
void FCN (const char *file, unsigned int line, const char *format, ...) \
__attribute__ ((__format__ (__printf__, 3, 4)))
+#define PRINTF_INDENT_LIKE(FCN) \
+ void FCN (const char *file, unsigned int line, unsigned int indent, \
+ const char *format, ...) \
+ __attribute__ ((__format__ (__printf__, 4, 5)))
#else /* __GNUC__ < 2 || defined(VMS) */
@@ -443,6 +447,10 @@ typedef struct _pseudo_type pseudo_typeS
#define PRINTF_WHERE_LIKE(FCN) void FCN (const char *file, \
unsigned int line, \
const char *format, ...)
+#define PRINTF_INDENT_LIKE(FCN) void FCN (const char *file, \
+ unsigned int line, \
+ unsigned int indent, \
+ const char *format, ...)
#endif /* __GNUC__ < 2 || defined(VMS) */
@@ -452,6 +460,7 @@ PRINTF_LIKE (as_tsktsk);
PRINTF_LIKE (as_warn);
PRINTF_WHERE_LIKE (as_bad_where);
PRINTF_WHERE_LIKE (as_warn_where);
+PRINTF_INDENT_LIKE (as_info_where);
void as_abort (const char *, int, const char *) ATTRIBUTE_NORETURN;
void signal_init (void);
@@ -486,7 +495,9 @@ void cond_finish_check (int);
void cond_exit_macro (int);
int seen_at_least_1_file (void);
void app_pop (char *);
+void as_report_context (void);
const char * as_where (unsigned int *);
+const char * as_where_top (unsigned int *);
const char * as_where_physical (unsigned int *);
void bump_line_counters (void);
void do_scrub_begin (int);
--- a/gas/input-scrub.c
+++ b/gas/input-scrub.c
@@ -104,6 +104,9 @@ static const char *logical_input_file;
static unsigned int physical_input_line;
static unsigned int logical_input_line;
+/* Indicator whether the origin of an update was a .linefile directive. */
+static bool is_linefile;
+
/* Struct used to save the state of the input handler during include files */
struct input_save {
char * buffer_start;
@@ -115,6 +118,7 @@ struct input_save {
const char * logical_input_file;
unsigned int physical_input_line;
unsigned int logical_input_line;
+ bool is_linefile;
size_t sb_index;
sb from_sb;
enum expansion from_sb_expansion; /* Should we do a conditional check? */
@@ -166,6 +170,7 @@ input_scrub_push (char *saved_position)
saved->logical_input_file = logical_input_file;
saved->physical_input_line = physical_input_line;
saved->logical_input_line = logical_input_line;
+ saved->is_linefile = is_linefile;
saved->sb_index = sb_index;
saved->from_sb = from_sb;
saved->from_sb_expansion = from_sb_expansion;
@@ -193,6 +198,7 @@ input_scrub_pop (struct input_save *save
logical_input_file = saved->logical_input_file;
physical_input_line = saved->physical_input_line;
logical_input_line = saved->logical_input_line;
+ is_linefile = saved->is_linefile;
sb_index = saved->sb_index;
from_sb = saved->from_sb;
from_sb_expansion = saved->from_sb_expansion;
@@ -267,8 +273,6 @@ input_scrub_include_sb (sb *from, char *
as_fatal (_("macros nested too deeply"));
++macro_nest;
- gas_assert (expansion < expanding_nested);
-
#ifdef md_macro_start
if (expansion == expanding_macro)
{
@@ -283,8 +287,6 @@ input_scrub_include_sb (sb *from, char *
expansion. */
newline = from->len >= 1 && from->ptr[0] != '\n';
sb_build (&from_sb, from->len + newline + 2 * sizeof (".linefile") + 30);
- if (expansion == expanding_repeat && from_sb_expansion >= expanding_macro)
- expansion = expanding_nested;
from_sb_expansion = expansion;
if (newline)
{
@@ -437,10 +439,7 @@ bump_line_counters (void)
if (sb_index == (size_t) -1)
++physical_input_line;
- /* PR gas/16908 workaround: Don't bump logical line numbers while
- expanding macros, unless file (and maybe line; see as_where()) are
- used inside the macro. */
- if (logical_input_line != -1u && from_sb_expansion < expanding_macro)
+ if (logical_input_line != -1u)
++logical_input_line;
}
\f
@@ -471,10 +470,6 @@ new_logical_line_flags (const char *fnam
case 1 << 3:
if (line_number < 0 || fname != NULL)
abort ();
- /* PR gas/16908 workaround: Ignore updates when nested inside a macro
- expansion. */
- if (from_sb_expansion == expanding_nested)
- return;
if (next_saved_file == NULL)
fname = physical_input_file;
else if (next_saved_file->logical_input_file)
@@ -486,6 +481,8 @@ new_logical_line_flags (const char *fnam
abort ();
}
+ is_linefile = flags != 1 && (flags != 0 || fname);
+
if (line_number >= 0)
logical_input_line = line_number;
else if (line_number == -1 && fname && !*fname && (flags & (1 << 2)))
@@ -499,15 +496,6 @@ new_logical_line_flags (const char *fnam
&& (logical_input_file == NULL
|| filename_cmp (logical_input_file, fname)))
logical_input_file = fname;
-
- /* When encountering file or line changes inside a macro, arrange for
- bump_line_counters() to henceforth increment the logical line number
- again, just like it does when expanding repeats. See as_where() for
- why changing file or line alone doesn't alter expansion mode. */
- if (from_sb_expansion == expanding_macro
- && logical_input_file != NULL
- && logical_input_line != -1u)
- from_sb_expansion = expanding_repeat;
}
void
@@ -516,6 +504,33 @@ new_logical_line (const char *fname, int
new_logical_line_flags (fname, line_number, 0);
}
+void
+as_report_context (void)
+{
+ const struct input_save *saved = next_saved_file;
+ enum expansion expansion = from_sb_expansion;
+ int indent = 1;
+
+ if (!macro_nest)
+ return;
+
+ do
+ {
+ if (expansion != expanding_macro)
+ /* Nothing. */;
+ else if (saved->logical_input_file != NULL
+ && saved->logical_input_line != -1u)
+ as_info_where (saved->logical_input_file, saved->logical_input_line,
+ indent, _("macro invoked from here"));
+ else
+ as_info_where (saved->physical_input_file, saved->physical_input_line,
+ indent, _("macro invoked from here"));
+
+ expansion = saved->from_sb_expansion;
+ ++indent;
+ }
+ while ((saved = saved->next_saved_file) != NULL);
+}
\f
/* Return the current physical input file name and line number, if known */
@@ -534,11 +549,53 @@ as_where_physical (unsigned int *linep)
return NULL;
}
-/* Return the current file name and line number. */
+/* Return the file name and line number at the top most macro
+ invocation, unless .file / .line were used inside a macro. */
const char *
as_where (unsigned int *linep)
{
+ const char *file = as_where_top (linep);
+
+ if (macro_nest && is_linefile)
+ {
+ const struct input_save *saved = next_saved_file;
+ enum expansion expansion = from_sb_expansion;
+
+ do
+ {
+ if (!saved->is_linefile)
+ break;
+
+ if (expansion != expanding_macro)
+ /* Nothing. */;
+ else if (saved->logical_input_file != NULL
+ && (linep == NULL || saved->logical_input_line != -1u))
+ {
+ if (linep != NULL)
+ *linep = saved->logical_input_line;
+ file = saved->logical_input_file;
+ }
+ else if (saved->physical_input_file != NULL)
+ {
+ if (linep != NULL)
+ *linep = saved->physical_input_line;
+ file = saved->physical_input_file;
+ }
+
+ expansion = saved->from_sb_expansion;
+ }
+ while ((saved = saved->next_saved_file) != NULL);
+ }
+
+ return file;
+}
+
+/* Return the current file name and line number. */
+
+const char *
+as_where_top (unsigned int *linep)
+{
if (logical_input_file != NULL
&& (linep == NULL || logical_input_line != -1u))
{
@@ -549,4 +606,3 @@ as_where (unsigned int *linep)
return as_where_physical (linep);
}
-
--- a/gas/macro.c
+++ b/gas/macro.c
@@ -131,23 +131,21 @@ buffer_and_nest (const char *from, const
else
from_len = strlen (from);
- /* Except for macros record the present source position, such that
- diagnostics and debug info will be properly associated with the
- respective original lines, rather than with the line of the ending
- directive (TO). */
- if (from == NULL || strcasecmp (from, "MACRO") != 0)
- {
- unsigned int line;
- char *linefile;
-
- as_where (&line);
- if (!flag_m68k_mri)
- linefile = xasprintf ("\t.linefile %u .", line + 1);
- else
- linefile = xasprintf ("\tlinefile %u .", line + 1);
- sb_add_string (ptr, linefile);
- xfree (linefile);
- }
+ /* Record the present source position, such that diagnostics and debug info
+ can be properly associated with the respective original lines, rather
+ than with the line of the ending directive (TO). */
+ {
+ unsigned int line;
+ char *linefile;
+
+ as_where_top (&line);
+ if (!flag_m68k_mri)
+ linefile = xasprintf ("\t.linefile %u .", line + 1);
+ else
+ linefile = xasprintf ("\tlinefile %u .", line + 1);
+ sb_add_string (ptr, linefile);
+ xfree (linefile);
+ }
while (more)
{
@@ -249,14 +247,8 @@ buffer_and_nest (const char *from, const
}
/* PR gas/16908
- Apply and discard .linefile directives that appear within
- the macro. For long macros, one might want to report the
- line number information associated with the lines within
- the macro definition, but we would need more infrastructure
- to make that happen correctly (e.g. resetting the line
- number when expanding the macro), and since for short
- macros we clearly prefer reporting the point of expansion
- anyway, there's not an obviously better fix here. */
+ Apply .linefile directives that appear within the macro, alongside
+ keeping them for later expansion of the macro. */
if (from != NULL && strcasecmp (from, "MACRO") == 0
&& len >= 8 && strncasecmp (ptr->ptr + i, "linefile", 8) == 0)
{
@@ -267,7 +259,6 @@ buffer_and_nest (const char *from, const
s_linefile (0);
restore_ilp ();
ptr->ptr[ptr->len] = saved_eol_char;
- ptr->len = line_start;
}
}
--- a/gas/messages.c
+++ b/gas/messages.c
@@ -18,6 +18,7 @@
02110-1301, USA. */
#include "as.h"
+#include <limits.h>
#include <signal.h>
/* If the system doesn't provide strsignal, we get it defined in
@@ -119,7 +120,7 @@ as_show_where (void)
const char *file;
unsigned int line;
- file = as_where (&line);
+ file = as_where_top (&line);
identify (file);
if (file)
{
@@ -130,6 +131,25 @@ as_show_where (void)
}
}
+/* Send to stderr a string as information, with location data passed in.
+ Note that for now this is not intended for general use. */
+
+void
+as_info_where (const char *file, unsigned int line, unsigned int indent,
+ const char *format, ...)
+{
+ va_list args;
+ char buffer[2000];
+
+ gas_assert (file != NULL && line > 0 && indent <= INT_MAX);
+
+ va_start (args, format);
+ vsnprintf (buffer, sizeof (buffer), format, args);
+ va_end (args);
+ fprintf (stderr, "%s:%u: %*s%s%s\n",
+ file, line, (int)indent, "", _("Info: "), buffer);
+}
+
/* Send to stderr a string as a warning, and locate warning
in input file(s).
Please only use this for when we have some recovery action.
@@ -146,6 +166,7 @@ as_tsktsk (const char *format, ...)
vfprintf (stderr, format, args);
va_end (args);
(void) putc ('\n', stderr);
+ as_report_context ();
}
/* The common portion of as_warn and as_warn_where. */
@@ -153,10 +174,15 @@ as_tsktsk (const char *format, ...)
static void
as_warn_internal (const char *file, unsigned int line, char *buffer)
{
+ bool context = false;
+
++warning_count;
if (file == NULL)
- file = as_where (&line);
+ {
+ file = as_where_top (&line);
+ context = true;
+ }
identify (file);
if (file)
@@ -168,6 +194,10 @@ as_warn_internal (const char *file, unsi
}
else
fprintf (stderr, "%s%s\n", _("Warning: "), buffer);
+
+ if (context)
+ as_report_context ();
+
#ifndef NO_LISTING
listing_warning (buffer);
#endif
@@ -194,7 +224,7 @@ as_warn (const char *format, ...)
}
}
-/* Like as_bad but the file name and line number are passed in.
+/* Like as_warn but the file name and line number are passed in.
Unfortunately, we have to repeat the function in order to handle
the varargs correctly and portably. */
@@ -218,10 +248,15 @@ as_warn_where (const char *file, unsigne
static void
as_bad_internal (const char *file, unsigned int line, char *buffer)
{
+ bool context = false;
+
++error_count;
if (file == NULL)
- file = as_where (&line);
+ {
+ file = as_where_top (&line);
+ context = true;
+ }
identify (file);
if (file)
@@ -233,6 +268,10 @@ as_bad_internal (const char *file, unsig
}
else
fprintf (stderr, "%s%s\n", _("Error: "), buffer);
+
+ if (context)
+ as_report_context ();
+
#ifndef NO_LISTING
listing_error (buffer);
#endif
@@ -290,6 +329,7 @@ as_fatal (const char *format, ...)
vfprintf (stderr, format, args);
(void) putc ('\n', stderr);
va_end (args);
+ as_report_context ();
/* Delete the output file, if it exists. This will prevent make from
thinking that a file was created and hence does not need rebuilding. */
if (out_file_name != NULL)
@@ -312,6 +352,7 @@ as_abort (const char *file, int line, co
fprintf (stderr, _("Internal error in %s at %s:%d.\n"), fn, file, line);
else
fprintf (stderr, _("Internal error at %s:%d.\n"), file, line);
+ as_report_context ();
fprintf (stderr, _("Please report this bug.\n"));
--- a/gas/sb.h
+++ b/gas/sb.h
@@ -66,11 +66,9 @@ extern size_t sb_skip_comma (size_t, sb
/* Actually in input-scrub.c. */
enum expansion {
- /* Note: Order matters! */
expanding_none,
expanding_repeat,
expanding_macro,
- expanding_nested, /* Only for internal use of input-scrub.c. */
};
extern void input_scrub_include_sb (sb *, char *, enum expansion);
[-- Attachment #2: binutils-master-macro-location.patch.bz2 --]
[-- Type: application/octet-stream, Size: 26229 bytes --]
^ permalink raw reply [flat|nested] 6+ messages in thread