.macro bug in gas for IA64

[Tim Connors <connors at hpltsc dot hpl dot hp dot com>]

I've come across a bug in the assembler (binutils-2.13) for IA64.  I
have a solution and I'm looking for feedback as to whether it is
palatable.  (And if so, a champion to fix it.  diffs at the end.)

Suppose that the stop bit construct (;;) is used in the body of a
macro definition (.macro).  At macro invocation, the stop bit
construct does not appear in the macro expansion :-(

The root cause of this problem derives from the fact that the IA64
line separator char (;) is a prefix of the stop bit construct.  When
the macro-body is scanned at definition time, the scanning code
(get_line_sb in read.c) is too aggressive about removing line
separators which really aren't.  Thus the stop bit construct never
makes it into the stored macro-body template.

The scanning code, get_line_sb(), is used for the following pseudo
ops:
	.irp
	.irpc
	.rept
	.macro

Each of these pseudo ops involve building a program-fragment template
and then expanding that template.  This bug affects all of them.

The fix I have used involves three files.
	binutils-2.13/gas/read.c                add  6 lines, replace 1
	binutils-2.13/gas/config/tc-ia64.h      add  2 lines (decls)
	binutils-2.13/gas/config/tc-ia64.c      add 11 lines (1 function)

It uses a machine dependent function called
	md_keep_end_line_char_sb(char c);
to control the aggressive behavior of get_line_sb().  The function
returns 1, if the input char should be kept as part of a
program-fragment template and 0 otherwise.  An #ifndef in read.c
prevents the fix from having any impact when the machine dependent
function is not supplied.

In particular, the current code for get_line_sb() has the following
loop:

    while (!is_end_of_line[(unsigned char) *input_line_pointer]
           || (inquote != '\0' && *input_line_pointer != '\n'))
      {
        ...
        sb_add_char (line, *input_line_pointer++);
      }

It scans the character string, called input_line_pointer, and inserts
the acceptable characters into the program-fragment template called
line.  I propose the following replacement code:

  #ifndef md_keep_end_line_char_sb
  #define md_keep_end_line_char_sb(c) 0
  #endif

    while (!is_end_of_line[(unsigned char) *input_line_pointer]
  	   || (inquote != '\0' && *input_line_pointer != '\n')
  	   || md_keep_end_line_char_sb(*input_line_pointer)
          )
      {
        ...
        sb_add_char (line, *input_line_pointer++);
      }

Note that if md_keep_end_line_char_sb is not defined, the boolean
expression controlling the loop is "... || 0".  The "|| 0" part would
be eliminated by the compiler, so there's no runtime impact in this
case.  When md_keep_end_line_char_sb is defined, this function is the
third operand in a multi-operand "||" expression.  Due to short
circuiting, the function is only called when the first two || operands
are false.

The code changes to tc-ia64.h and tc-ia64.c are pretty simple, so I
won't explain them in detail.  Below are some actual diffs.

Thanks for your attention,
-Tim

----------------------------------------------------------------------
$ diff -u binutils-2.13/gas/read.c binutils-2.13-tsc/gas/read.c 
--- binutils-2.13/gas/read.c	Fri Feb 28 13:57:48 2003
+++ binutils-2.13-tsc/gas/read.c	Fri Feb 28 18:24:50 2003
@@ -2269,8 +2269,14 @@
 
   inquote = '\0';
 
+#ifndef md_keep_end_line_char_sb
+#define md_keep_end_line_char_sb(c) 0
+#endif
+
   while (!is_end_of_line[(unsigned char) *input_line_pointer]
-        || (inquote != '\0' && *input_line_pointer != '\n'))
+        || (inquote != '\0' && *input_line_pointer != '\n')
+        || md_keep_end_line_char_sb(*input_line_pointer)
+        )
     {
       if (inquote == *input_line_pointer)
        inquote = '\0';
$ 
$ 
$ 
$ 
$ diff -u binutils-2.13/gas/config/tc-ia64.h binutils-2.13-tsc/gas/config/tc-ia64.h 
--- binutils-2.13/gas/config/tc-ia64.h	Fri Feb 28 13:59:20 2003
+++ binutils-2.13-tsc/gas/config/tc-ia64.h	Fri Feb 28 18:01:02 2003
@@ -68,6 +68,7 @@
 extern void ia64_do_align PARAMS((int n));
 extern void ia64_end_of_source PARAMS((void));
 extern void ia64_start_line PARAMS((void));
+extern int ia64_keep_end_line_char_sb PARAMS((char c));
 extern int ia64_unrecognized_line PARAMS((int ch));
 extern void ia64_frob_label PARAMS((struct symbol *sym));
 extern void ia64_flush_pending_output PARAMS((void));
@@ -92,6 +93,7 @@
 
 #define md_end()                               ia64_end_of_source ()
 #define md_start_line_hook()           ia64_start_line ()
+#define md_keep_end_line_char_sb(c)    ia64_keep_end_line_char_sb (c)
 #define tc_unrecognized_line(ch)       ia64_unrecognized_line (ch)
 #define tc_frob_label(s)               ia64_frob_label (s)
 #define md_flush_pending_output()      ia64_flush_pending_output ()
$ 
$ 
$ 
$ 
$ diff -u binutils-2.13/gas/config/tc-ia64.c binutils-2.13-tsc/gas/config/tc-ia64.c 
--- binutils-2.13/gas/config/tc-ia64.c	Fri Feb 28 13:58:07 2003
+++ binutils-2.13-tsc/gas/config/tc-ia64.c	Fri Feb 28 17:01:20 2003
@@ -6853,6 +6853,17 @@
     }
 }
 
+int
+ia64_keep_end_line_char_sb (c)
+     char c;
+{
+  if (c  == ';')
+    {
+      return 1;
+    }
+  return 0;
+}
+
 /* This is a hook for ia64_frob_label, so that it can distinguish tags from
    labels.  */
 static int defining_tag = 0;