[PATCH 0/6] gas: (mostly) further file/line handling adjustments

[PATCH 0/6] gas: (mostly) further file/line handling adjustments

[Jan Beulich]

The 1st patch addresses an oversight of mine in earlier related work,
while the latter address further observations made in the process. The
last patch is slightly RFC, as there might be reservations. Comments
appreciated, in particular on that last patch.

1: tweak .irp and alike file/line handling for M68K/MRI
2: simplify ignore_input()
3: don't ignore .linefile inside false conditionals
4: fold do_repeat{,_with_expander}()
5: avoid bignum related errors when processing .linefile
6: avoid octal numbers being accepted when processing .linefile

Jan

[PATCH 1/6] gas: tweak .irp and alike file/line handling for M68K/MRI

[Jan Beulich]

In commit 2ee1792bec22 ("gas: further adjust file/line handling for .irp
and alike") I neglected the need to omit the leading . in M68K/MRI mode.

--- a/gas/macro.c
+++ b/gas/macro.c
@@ -136,7 +136,10 @@ buffer_and_nest (const char *from, const
       char *linefile;
 
       as_where (&line);
-      linefile = xasprintf ("\t.linefile %u .\n", line);
+      if (!flag_m68k_mri)
+	linefile = xasprintf ("\t.linefile %u .\n", line);
+      else
+	linefile = xasprintf ("\tlinefile %u .\n", line);
       sb_add_buffer (ptr, linefile, strlen (linefile));
       xfree (linefile);
     }

[PATCH 2/6] gas: simplify ignore_input()

[Jan Beulich]

First of all convert to switch(), in preparation of adding another
directive here which may not be ignored. While doing so drop dead code:
A string the first two characters of which do not match "if" also wont
match "ifdef" or "ifndef".
---
I was tempted to drop the leading 'e' from the strings compared against,
but I then decided against because doing so might not be liked by
everyone.

--- a/gas/cond.c
+++ b/gas/cond.c
@@ -513,17 +513,19 @@ ignore_input (void)
     }
 
   /* We cannot ignore certain pseudo ops.  */
-  if (((s[0] == 'i'
-	|| s[0] == 'I')
-       && (!strncasecmp (s, "if", 2)
-	   || !strncasecmp (s, "ifdef", 5)
-	   || !strncasecmp (s, "ifndef", 6)))
-      || ((s[0] == 'e'
-	   || s[0] == 'E')
-	  && (!strncasecmp (s, "else", 4)
-	      || !strncasecmp (s, "endif", 5)
-	      || !strncasecmp (s, "endc", 4))))
-    return 0;
+  switch (s[0])
+    {
+    case 'i': case  'I':
+      if (s[1] == 'f' || s[1] == 'F')
+	return 0;
+      break;
+    case 'e': case 'E':
+      if (!strncasecmp (s, "else", 4)
+	  || !strncasecmp (s, "endif", 5)
+	  || !strncasecmp (s, "endc", 4))
+	return 0;
+      break;
+    }
 
   return (current_cframe != NULL) && (current_cframe->ignoring);
 }

[PATCH 3/6] gas: don't ignore .linefile inside false conditionals

[Jan Beulich]

When assembling code previously pre-processed by a C compiler, long
enough comments may have been collapsed into "# <line> <file>"
constructs. If we skip these, line numbers (and possibly even file
names) will be off / wrong in both diagnostics and debug info.

--- a/gas/cond.c
+++ b/gas/cond.c
@@ -525,6 +525,10 @@ ignore_input (void)
 	  || !strncasecmp (s, "endc", 4))
 	return 0;
       break;
+    case 'l': case 'L':
+      if (!strncasecmp (s, "linefile", 8))
+	return 0;
+      break;
     }
 
   return (current_cframe != NULL) && (current_cframe->ignoring);
--- /dev/null
+++ b/gas/testsuite/gas/all/cond-2.l
@@ -0,0 +1,12 @@
+# This should match the output of gas -al cond-2.s.
+.*: Assembler messages:
+.*:1005: Warning: line 5
+.*cond-2.s.*
+
+
+[ 	]*[1-9][0-9]*[ 	]+\.if[ 	]+0[ 	]*
+[ 	]*[1-9][0-9]*[ 	]+# 1003 "cond-2\.s"
+[ 	]*[1-9][0-9]*[ 	]+\.endif[ 	]*
+[ 	]*[1-9][0-9]*[ 	]*
+[ 	]*[1-9][0-9]*[ 	]+\.warning[ 	].*
+#pass
--- /dev/null
+++ b/gas/testsuite/gas/all/cond-2.s
@@ -0,0 +1,5 @@
+	.if 0
+# 1003 "cond-2.s"
+	.endif
+
+	.warning "line 5"
--- a/gas/testsuite/gas/all/gas.exp
+++ b/gas/testsuite/gas/all/gas.exp
@@ -466,6 +466,11 @@ if [is_elf_format] {
 
 run_dump_test quoted-sym-names
 
+# Targets where # is not a line comment character don't transform
+# "# <line> <file>" into .linefile (PR gas/29120).
+setup_xfail "tic30-*-*"
+run_list_test cond-2 "-al"
+
 run_list_test macro "-alm"
 
 run_list_test pr20312

[PATCH 4/6] gas: fold do_repeat{,_with_expander}()

[Jan Beulich]

do_repeat_with_expander() already deals with the "no expander" case
quite fine, so there's really little point having two functions. What it
lacks compared with do_repeat() is a call to sb_build(), which can
simply be moved (and the then redundant sb_new() be avoided). Along with
this moving also flip if the main if()'s condition such that the "no
expander" case is handled first.
---
The "with-expander" case has a number of issues, though:
- use of literal 8 instead of using the actual size of "expander",
- assertion on user controlled value,
- subsequent code nevertheless depends on that assertion,
- only a single instance of the pattern is being replaced (despite a
  comment saying otherwise),
- no separators are required (checked for) around the found "expander"
  instance,
- "count" is silently truncated from size_t to unsigned long.
Furthermore .mrepeat doesn't properly nest with itself or inside e.g.
.irp, as the inner .endr will be taken as terminating the other
construct.

--- a/gas/config/tc-rx.c
+++ b/gas/config/tc-rx.c
@@ -566,7 +566,7 @@ rx_rept (int ignore ATTRIBUTE_UNUSED)
 {
   size_t count = get_absolute_expression ();
 
-  do_repeat_with_expander (count, "MREPEAT", "ENDR", "..MACREP");
+  do_repeat (count, "MREPEAT", "ENDR", "..MACREP");
 }
 
 /* Like cons() accept that strings are allowed.  */
--- a/gas/config/tc-tic54x.c
+++ b/gas/config/tc-tic54x.c
@@ -2034,7 +2034,7 @@ tic54x_loop (int count)
   if (!is_end_of_line[(unsigned char) *input_line_pointer])
     count = get_absolute_expression ();
 
-  do_repeat ((size_t) count, "LOOP", "ENDLOOP");
+  do_repeat ((size_t) count, "LOOP", "ENDLOOP", NULL);
 }
 
 /* Normally, endloop gets eaten by the preceding loop.  */
--- a/gas/read.c
+++ b/gas/read.c
@@ -3094,14 +3094,17 @@ s_rept (int ignore ATTRIBUTE_UNUSED)
 
   count = (size_t) get_absolute_expression ();
 
-  do_repeat (count, "REPT", "ENDR");
+  do_repeat (count, "REPT", "ENDR", NULL);
 }
 
 /* This function provides a generic repeat block implementation.   It allows
-   different directives to be used as the start/end keys.  */
+   different directives to be used as the start/end keys.  Any text matching
+   the optional EXPANDER in the block is replaced by the remaining iteration
+   count.  */
 
 void
-do_repeat (size_t count, const char *start, const char *end)
+do_repeat (size_t count, const char *start, const char *end,
+	   const char *expander)
 {
   sb one;
   sb many;
@@ -3119,46 +3122,16 @@ do_repeat (size_t count, const char *sta
       return;
     }
 
-  sb_build (&many, count * one.len);
-  while (count-- > 0)
-    sb_add_sb (&many, &one);
-
-  sb_kill (&one);
-
-  input_scrub_include_sb (&many, input_line_pointer, expanding_repeat);
-  sb_kill (&many);
-  buffer_limit = input_scrub_next_buffer (&input_line_pointer);
-}
-
-/* Like do_repeat except that any text matching EXPANDER in the
-   block is replaced by the iteration count.  */
-
-void
-do_repeat_with_expander (size_t count,
-			 const char * start,
-			 const char * end,
-			 const char * expander)
-{
-  sb one;
-  sb many;
-
-  if (((ssize_t) count) < 0)
+  if (expander == NULL || strstr (one.ptr, expander) == NULL)
     {
-      as_bad (_("negative count for %s - ignored"), start);
-      count = 0;
+      sb_build (&many, count * one.len);
+      while (count-- > 0)
+	sb_add_sb (&many, &one);
     }
-
-  sb_new (&one);
-  if (!buffer_and_nest (start, end, &one, get_non_macro_line_sb))
+  else
     {
-      as_bad (_("%s without %s"), start, end);
-      return;
-    }
-
-  sb_new (&many);
+      sb_new (&many);
 
-  if (expander != NULL && strstr (one.ptr, expander) != NULL)
-    {
       while (count -- > 0)
 	{
 	  int len;
@@ -3177,9 +3150,6 @@ do_repeat_with_expander (size_t count,
 	  sb_kill (& processed);
 	}
     }
-  else
-    while (count-- > 0)
-      sb_add_sb (&many, &one);
 
   sb_kill (&one);
 
--- a/gas/read.h
+++ b/gas/read.h
@@ -148,8 +148,7 @@ extern void stabs_generate_asm_file (voi
 extern void stabs_generate_asm_lineno (void);
 extern void stabs_generate_asm_func (const char *, const char *);
 extern void stabs_generate_asm_endfunc (const char *, const char *);
-extern void do_repeat (size_t, const char *, const char *);
-extern void do_repeat_with_expander (size_t, const char *, const char *, const char *);
+extern void do_repeat (size_t, const char *, const char *, const char *);
 extern void end_repeat (int);
 extern void do_parse_cons_expression (expressionS *, int);
 

[PATCH 5/6] gas: avoid bignum related errors when processing .linefile

[Jan Beulich]

Any construct which to the scrubber looks like a C preprocessor
line/file "directive" is converted to .linefile, but the amount of
checking the scrubber does is minimal (albeit it does let through only
decimal digits for the line part of the contruct). Since the scrubber
conversion is further tied to # being a line comment character, anything
which upon closer inspection turns out not to be a line/file "directive"
is supposed to be treated as a comment, i.e. ignored. Therefore we
cannot use get_absolute_expression(), as this may raise errors. Open-
code the function instead, treating everything not resulting in
O_constant as a comment as well.

Furthermore also bounds-check the parsed value. This bounds check tries
to avoid implementation defined behavior (which may be the raising of an
implementation defined signal), but for now makes the assumption that
int has less than 64 bits. The way bfd_signed_vma (which is what offsetT
aliases) is defined in bfd.h for the BFD64 case I cannot really see a
clean way of avoiding this assumption. Omitting the #ifdef, otoh, would
risk "condition is always false" warnings by compilers.

Convert get_linefile_number() to return bool at this occasion as well.

--- a/gas/read.c
+++ b/gas/read.c
@@ -2037,17 +2037,28 @@ s_file (int ignore ATTRIBUTE_UNUSED)
     }
 }
 
-static int
+static bool
 get_linefile_number (int *flag)
 {
+  expressionS exp;
+
   SKIP_WHITESPACE ();
 
   if (*input_line_pointer < '0' || *input_line_pointer > '9')
-    return 0;
+    return false;
 
-  *flag = get_absolute_expression ();
+  expression_and_evaluate (&exp);
+  if (exp.X_op != O_constant)
+    return false;
 
-  return 1;
+#if defined (BFD64) || LONG_MAX > INT_MAX
+  if (exp.X_add_number < INT_MIN || exp.X_add_number > INT_MAX)
+    return false;
+#endif
+
+  *flag = exp.X_add_number;
+
+  return true;
 }
 
 /* Handle the .linefile pseudo-op.  This is automatically generated by
--- a/gas/testsuite/gas/all/gas.exp
+++ b/gas/testsuite/gas/all/gas.exp
@@ -465,6 +465,8 @@ run_dump_test quoted-sym-names
 # "# <line> <file>" into .linefile (PR gas/29120).
 setup_xfail "tic30-*-*"
 run_list_test cond-2 "-al"
+setup_xfail "tic30-*-*"
+run_list_test linefile ""
 
 run_list_test macro "-alm"
 
--- /dev/null
+++ b/gas/testsuite/gas/all/linefile.l
@@ -0,0 +1,5 @@
+# This should match the output of gas linefile.s.
+.*linefile\.s: Assembler messages:
+.*linefile\.s:2: Warning: line 2
+.*linefile\.s:5: Warning: line 5
+#pass
--- /dev/null
+++ b/gas/testsuite/gas/all/linefile.s
@@ -0,0 +1,5 @@
+# 12345678900 "LineFile.s"
+	.warning "line 2"
+
+# 123456789123456789123456789 "LINEfile.s"
+	.warning "line 5"

[PATCH 6/6] gas: avoid octal numbers being accepted when processing .linefile

[Jan Beulich]

Compilers would put decimal numbers there, so I think we should treat
finding octal numbers the same as finding bignums - ignore them as
actually being comments of some very specific form.
---
With the bignum and this change in place, I'm still worried about the
secondary use of get_linefile_number() - afaict we might still
mistakenly try to process special form comments as line/file
"directives", and under the right conditions this might also still
result in unde diagnostics.

--- a/gas/read.c
+++ b/gas/read.c
@@ -2047,6 +2047,14 @@ get_linefile_number (int *flag)
   if (*input_line_pointer < '0' || *input_line_pointer > '9')
     return false;
 
+  /* Don't mistakenly interpret octal numbers as line numbers.  */
+  if (*input_line_pointer == '0')
+    {
+      *flag = 0;
+      ++input_line_pointer;
+      return true;
+    }
+
   expression_and_evaluate (&exp);
   if (exp.X_op != O_constant)
     return false;
--- a/gas/testsuite/gas/all/linefile.l
+++ b/gas/testsuite/gas/all/linefile.l
@@ -2,4 +2,5 @@
 .*linefile\.s: Assembler messages:
 .*linefile\.s:2: Warning: line 2
 .*linefile\.s:5: Warning: line 5
+.*linefile\.s:8: Warning: line 8
 #pass
--- a/gas/testsuite/gas/all/linefile.s
+++ b/gas/testsuite/gas/all/linefile.s
@@ -3,3 +3,6 @@
 
 # 123456789123456789123456789 "LINEfile.s"
 	.warning "line 5"
+
+# 0123456789 "lineFILE.s"
+	.warning "line 8"