public inbox for binutils@sourceware.org
 help / color / mirror / Atom feed
* .macro bug in gas for IA64
@ 2003-03-01  3:23 Tim Connors
  2003-03-06 11:20 ` Nick Clifton
  0 siblings, 1 reply; 7+ messages in thread
From: Tim Connors @ 2003-03-01  3:23 UTC (permalink / raw)
  To: binutils

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;

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: .macro bug in gas for IA64
  2003-03-01  3:23 .macro bug in gas for IA64 Tim Connors
@ 2003-03-06 11:20 ` Nick Clifton
  2003-03-07  0:02   ` Jim Wilson
  2003-03-09 22:44   ` Jim Wilson
  0 siblings, 2 replies; 7+ messages in thread
From: Nick Clifton @ 2003-03-06 11:20 UTC (permalink / raw)
  To: connors; +Cc: wilson, binutils

Hi Tim,

> 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 :-(

It seems to me that your proposed patch would disallow multiple
instructions on a single line inside macro blocks (for the IA64 at least).
Instead would it not be easier to choose a different character to be
the line separator character ?  Then you would not have to add any new
code, and instead you could just change the initialisation value of
line_separator_chars[] in tc-ia64.c.

If you cannot change the line separator, then at the very least you
ought to change your new macro so that it takes the input line pointer
and then it can scan ahead.  ie something like:

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

  ...

  int
  ia64_end_of_line_char (char * ilp)
  {
    /* ';;' is the stop bit and should not be treated as a line separator.  */
    return (ilp[0]  == ';' && ilp[1] != ';');
  }

That way if there is only a single ';' it will be treated as a line
separator, but if there are two, then they will be treated as a stop
bit.

Cheers
        Nick

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: .macro bug in gas for IA64
  2003-03-06 11:20 ` Nick Clifton
@ 2003-03-07  0:02   ` Jim Wilson
  2003-03-09 22:44   ` Jim Wilson
  1 sibling, 0 replies; 7+ messages in thread
From: Jim Wilson @ 2003-03-07  0:02 UTC (permalink / raw)
  To: Nick Clifton; +Cc: connors, binutils, wilson

On Thu, 2003-03-06 at 06:22, Nick Clifton wrote:
> Instead would it not be easier to choose a different character to be
> the line separator character ?

The Intel Itanium Architecture Assembly Language Reference Guide
(IIAALRG) says that semicolon (;) is a statement separator and that
multiple statements can appear on the same line.  So we can't change
that.  It is likely that there is already code in glibc and the linux
kernel that makes use of ; as a statement separator.

The IIAALRG also says that ;; represents the stop bit, and that this can
appear between two instructions on the same line.  It is apparently not
valid for ;; to appear on the same line before or after something other
than an instruction, but this is a technicality that we can perhaps
continue to ignore for now.

So we need to be able to handle both.  If you see a single semicolon, it
is an end of line character that can be dropped, but if you see two
semicolons the line ends before the first semicolon so that
md_start_line_hook will see both semicolons.  Actually, I see that the
tc-ia64.c md_start_line_hook cheats, and looks at input_line_pointer[-1]
to see if this is a stop bit.  Also, it takes advantage of the fact that
; is a statement separator to allow the stop bit to appear in the middle
of a line.

I haven't looked at this patch or this bug report yet.  I think I can
find some time to take a closer look at this.

Jim


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: .macro bug in gas for IA64
  2003-03-06 11:20 ` Nick Clifton
  2003-03-07  0:02   ` Jim Wilson
@ 2003-03-09 22:44   ` Jim Wilson
  2003-03-11  0:56     ` Tim Connors
  2003-03-12 14:17     ` Nick Clifton
  1 sibling, 2 replies; 7+ messages in thread
From: Jim Wilson @ 2003-03-09 22:44 UTC (permalink / raw)
  To: Nick Clifton; +Cc: connors, binutils

On Thu, 2003-03-06 at 06:22, Nick Clifton wrote:
> It seems to me that your proposed patch would disallow multiple
> instructions on a single line inside macro blocks (for the IA64 at least).

This is correct.  For a testcase like this:
        .macro foo ; add r0 = r1, r2 ; .endm
        foo
I get with the patch
tmp3.s: Assembler messages:
tmp3.s:1: Error: unexpected end of file in macro definition

get_line_sb must exit when an end-of-line character is seen so that
buffer_and_nest can scan for the .endm.  The patch from Tim Connors
breaks this.

> If you cannot change the line separator, then at the very least you
> ought to change your new macro so that it takes the input line pointer
> and then it can scan ahead.  ie something like:

As Tim pointed out, this doesn't work, because the first ; gets treated
correctly, but the second one gets dropped.  We would need additional
changes to make this work.

get_line_sb inserts characters until it sees an end-of-line character,
and then returns to buffer_and_nest.  buffer_and_nest then inserts a
newline character.  We can get the right behavior for IA-64 if
buffer_and_nest inserts the original end-of-line character.  Also,
get_line_sb has to stop skipping multiple end-of-line characters.
That gives me the first following patch.  The fact that 0 is an end of
line character makes this a little ugly, but otherwise it seems OK.

If we want something like Tim's patch, then we need to recognize the
stop bit only in the new hook as Nick suggested, and then we also need
the loop in get_line_sb to copy two characters to the sb instead of
one.  I modified the hook to return the extra number of characters to
copy.  This gives me the second patch.  I didn't flesh this one out
further since the first patch seems like a better idea.

Jim

First patch:

2003-03-09  James E Wilson  <wilson@tuliptree.org>

	* macro.c (buffer_and_nest): Store more to sb instead of '\n'.
	* read.c (get_line_sb): Return end of line character or '\n' if
	it is zero or non-existent.

Index: macro.c
===================================================================
RCS file: /cvs/src/src/gas/macro.c,v
retrieving revision 1.20
diff -p -r1.20 macro.c
*** macro.c	3 Jan 2003 21:47:20 -0000	1.20
--- macro.c	9 Mar 2003 21:58:50 -0000
*************** buffer_and_nest (from, to, ptr, get_line
*** 222,229 ****
  	    }
  	}
  
!       /* Add a CR to the end and keep running.  */
!       sb_add_char (ptr, '\n');
        line_start = ptr->len;
        more = get_line (ptr);
      }
--- 222,229 ----
  	    }
  	}
  
!       /* Add the original end-of-line char to the end and keep running.  */
!       sb_add_char (ptr, more);
        line_start = ptr->len;
        more = get_line (ptr);
      }
Index: read.c
===================================================================
RCS file: /cvs/src/src/gas/read.c,v
retrieving revision 1.58
diff -p -r1.58 read.c
*** read.c	11 Jan 2003 06:24:12 -0000	1.58
--- read.c	9 Mar 2003 21:58:54 -0000
*************** get_line_sb (line)
*** 2285,2299 ****
        sb_add_char (line, *input_line_pointer++);
      }
  
!   while (input_line_pointer < buffer_limit
  	 && is_end_of_line[(unsigned char) *input_line_pointer])
      {
!       if (input_line_pointer[-1] == '\n')
! 	bump_line_counters ();
!       ++input_line_pointer;
      }
! 
!   return 1;
  }
  
  /* Define a macro.  This is an interface to macro.c.  */
--- 2285,2305 ----
        sb_add_char (line, *input_line_pointer++);
      }
  
!   /* Don't skip multiple end-of-line characters, because that breaks support
!      for the IA-64 stop bit (;;) which looks like two consecutive end-of-line
!      characters but isn't.  Return the end-of-line character so that the
!      caller can insert it if necessary.  */
!   if (input_line_pointer < buffer_limit
  	 && is_end_of_line[(unsigned char) *input_line_pointer])
      {
!       char c = *input_line_pointer++;
!       if (c != 0)
! 	return c;
!       else
! 	return '\n';
      }
!   else
!     return '\n';
  }
  
  /* Define a macro.  This is an interface to macro.c.  */

Second patch, based on Tim's patch:

Index: read.c
===================================================================
RCS file: /cvs/src/src/gas/read.c,v
retrieving revision 1.58
diff -p -r1.58 read.c
*** read.c	11 Jan 2003 06:24:12 -0000	1.58
--- read.c	9 Mar 2003 22:38:18 -0000
*************** get_line_sb (line)
*** 2242,2247 ****
--- 2242,2248 ----
       sb *line;
  {
    char quote1, quote2, inquote;
+   int count;
  
    if (input_line_pointer[-1] == '\n')
      bump_line_counters ();
*************** get_line_sb (line)
*** 2269,2276 ****
  
    inquote = '\0';
  
    while (!is_end_of_line[(unsigned char) *input_line_pointer]
! 	 || (inquote != '\0' && *input_line_pointer != '\n'))
      {
        if (inquote == *input_line_pointer)
  	inquote = '\0';
--- 2270,2284 ----
  
    inquote = '\0';
  
+   /* This returns the number of extra characters to copy to the sb.  */
+ #ifndef md_keep_end_line_char_sb
+ #define md_keep_end_line_char_sb(c) 0
+ #endif
+ 
+   count = 0;
    while (!is_end_of_line[(unsigned char) *input_line_pointer]
! 	 || (inquote != '\0' && *input_line_pointer != '\n')
! 	 || (count = md_keep_end_line_char_sb (input_line_pointer)))
      {
        if (inquote == *input_line_pointer)
  	inquote = '\0';
*************** get_line_sb (line)
*** 2280,2285 ****
--- 2288,2299 ----
  	    inquote = quote1;
  	  else if (*input_line_pointer == quote2)
  	    inquote = quote2;
+ 	}
+ 
+       while (count)
+ 	{
+ 	  sb_add_char (line, *input_line_pointer++);
+ 	  count--;
  	}
  
        sb_add_char (line, *input_line_pointer++);
Index: config/tc-ia64.c
===================================================================
RCS file: /cvs/src/src/gas/config/tc-ia64.c,v
retrieving revision 1.80
diff -p -r1.80 tc-ia64.c
*** config/tc-ia64.c	28 Jan 2003 03:24:12 -0000	1.80
--- config/tc-ia64.c	9 Mar 2003 22:36:10 -0000
*************** ia64_start_line ()
*** 6877,6882 ****
--- 6877,6893 ----
      }
  }
  
+ int
+ ia64_keep_end_line_char_sb (ptr)
+      char *ptr;
+ {
+   if (ptr[0] == ';' && ptr[1] == ';')
+     {
+       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;
Index: config/tc-ia64.h
===================================================================
RCS file: /cvs/src/src/gas/config/tc-ia64.h,v
retrieving revision 1.21
diff -p -r1.21 tc-ia64.h
*** config/tc-ia64.h	5 Sep 2002 00:01:18 -0000	1.21
--- config/tc-ia64.h	9 Mar 2003 22:36:10 -0000
*************** struct ia64_fix
*** 68,73 ****
--- 68,74 ----
  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 *ptr));
  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));
*************** extern void ia64_after_parse_args PARAMS
*** 92,97 ****
--- 93,99 ----
  
  #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 ()

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: .macro bug in gas for IA64
  2003-03-09 22:44   ` Jim Wilson
@ 2003-03-11  0:56     ` Tim Connors
  2003-03-12 14:17     ` Nick Clifton
  1 sibling, 0 replies; 7+ messages in thread
From: Tim Connors @ 2003-03-11  0:56 UTC (permalink / raw)
  To: Jim Wilson; +Cc: binutils

On Sun, Mar 09, 2003 at 05:44:54PM -0500, Jim Wilson wrote:
> On Thu, 2003-03-06 at 06:22, Nick Clifton wrote:
> > It seems to me that your proposed patch would disallow multiple
> > instructions on a single line inside macro blocks (for the IA64 at least).
> 
> This is correct.  For a testcase like this:
>         .macro foo ; add r0 = r1, r2 ; .endm
>         foo
> I get with the patch
> tmp3.s: Assembler messages:
> tmp3.s:1: Error: unexpected end of file in macro definition

Ah, so Nick was right.  I had tested multple instructions, but not the
" .... ; .endm" case.

> . . . . The patch from Tim Connors
> breaks this.

It sure does.

> That gives me the first following patch. 
> (snip)
> First patch:
> (snip)
> Second patch, based on Tim's patch:
> (snip)
> This gives me the second patch.  I didn't flesh this one out
> further since the first patch seems like a better idea.

As I explained in my reply to Nick, I have no preference as to what
the fix is.  Jim's first patch looks good to me - eg, it doesn't
require an md hook.  For now, I have a locally patched assembler that
meets my needs.  I'd appreciate it, Jim, if you could eventually apply
your patch.  No hurry, but I'd like to know that this bug will be
fixed sometime so that I can use macros with confidence.

Thanks for your attention,
-Tim

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: .macro bug in gas for IA64
  2003-03-09 22:44   ` Jim Wilson
  2003-03-11  0:56     ` Tim Connors
@ 2003-03-12 14:17     ` Nick Clifton
  1 sibling, 0 replies; 7+ messages in thread
From: Nick Clifton @ 2003-03-12 14:17 UTC (permalink / raw)
  To: Jim Wilson; +Cc: connors, binutils

Hi Jim,

> get_line_sb inserts characters until it sees an end-of-line
> character, and then returns to buffer_and_nest.  buffer_and_nest
> then inserts a newline character.  We can get the right behaviour for
> IA-64 if buffer_and_nest inserts the original end-of-line character.
> Also, get_line_sb has to stop skipping multiple end-of-line
> characters.  That gives me the first following patch.  The fact that
> 0 is an end of line character makes this a little ugly, but
> otherwise it seems OK.

I like this first patch, but I am a little confused by part of it:

> *************** get_line_sb (line)
> *** 2285,2299 ****
>         sb_add_char (line, *input_line_pointer++);
>       }
>   
> !   while (input_line_pointer < buffer_limit
>   	 && is_end_of_line[(unsigned char) *input_line_pointer])
>       {
> !       if (input_line_pointer[-1] == '\n')
> ! 	bump_line_counters ();
> !       ++input_line_pointer;
>       }
> ! 
> !   return 1;
>   }
>   
>   /* Define a macro.  This is an interface to macro.c.  */
> --- 2285,2305 ----
>         sb_add_char (line, *input_line_pointer++);
>       }
>   
> !   /* Don't skip multiple end-of-line characters, because that breaks support
> !      for the IA-64 stop bit (;;) which looks like two consecutive end-of-line
> !      characters but isn't.  Return the end-of-line character so that the
> !      caller can insert it if necessary.  */
> !   if (input_line_pointer < buffer_limit
>   	 && is_end_of_line[(unsigned char) *input_line_pointer])
>       {
> !       char c = *input_line_pointer++;
> !       if (c != 0)
> ! 	return c;
> !       else
> ! 	return '\n';
>       }
> !   else
> !     return '\n';
>   }

It seems to me that the patched version will return the character
*after* the end-of-line character, but only if there were characters
left in the input buffer.  To get the behaviour you desire, I think
that all that you need to do is to return *input_line_pointer.  Here
is a slightly modified version of your patch which does this.  What do
you think ?

Index: gas/read.c
===================================================================
RCS file: /cvs/src/src/gas/read.c,v
retrieving revision 1.58
diff -c -3 -p -w -r1.58 read.c
*** gas/read.c	11 Jan 2003 06:24:12 -0000	1.58
--- gas/read.c	12 Mar 2003 14:11:09 -0000
*************** s_lsym (ignore)
*** 2235,2247 ****
    demand_empty_rest_of_line ();
  }
  
! /* Read a line into an sb.  */
  
  static int
  get_line_sb (line)
       sb *line;
  {
    char quote1, quote2, inquote;
  
    if (input_line_pointer[-1] == '\n')
      bump_line_counters ();
--- 2235,2249 ----
    demand_empty_rest_of_line ();
  }
  
! /* Read a line into an sb.  Returns the character that ended the line
!    or zero if there are no more lines.  */
  
  static int
  get_line_sb (line)
       sb *line;
  {
    char quote1, quote2, inquote;
+   unsigned char c;
  
    if (input_line_pointer[-1] == '\n')
      bump_line_counters ();
*************** get_line_sb (line)
*** 2269,2299 ****
  
    inquote = '\0';
  
!   while (!is_end_of_line[(unsigned char) *input_line_pointer]
! 	 || (inquote != '\0' && *input_line_pointer != '\n'))
      {
!       if (inquote == *input_line_pointer)
  	inquote = '\0';
        else if (inquote == '\0')
  	{
! 	  if (*input_line_pointer == quote1)
  	    inquote = quote1;
! 	  else if (*input_line_pointer == quote2)
  	    inquote = quote2;
  	}
  
!       sb_add_char (line, *input_line_pointer++);
      }
  
!   while (input_line_pointer < buffer_limit
! 	 && is_end_of_line[(unsigned char) *input_line_pointer])
!     {
!       if (input_line_pointer[-1] == '\n')
! 	bump_line_counters ();
!       ++input_line_pointer;
!     }
! 
!   return 1;
  }
  
  /* Define a macro.  This is an interface to macro.c.  */
--- 2271,2299 ----
  
    inquote = '\0';
  
!   while ((c = * input_line_pointer ++) != 0
! 	 && (!is_end_of_line[c]
! 	     || (inquote != '\0' && c != '\n')))
      {
!       if (inquote == c)
  	inquote = '\0';
        else if (inquote == '\0')
  	{
! 	  if (c == quote1)
  	    inquote = quote1;
! 	  else if (c == quote2)
  	    inquote = quote2;
  	}
  
!       sb_add_char (line, c);
      }
  
!   /* Don't skip multiple end-of-line characters, because that breaks support
!      for the IA-64 stop bit (;;) which looks like two consecutive end-of-line
!      characters but isn't.  Instead just skip one end of line character and
!      return the character skipped so that the caller can re-insert it if
!      necessary.   */
!   return c;
  }
  
  /* Define a macro.  This is an interface to macro.c.  */
Index: gas/macro.c
===================================================================
RCS file: /cvs/src/src/gas/macro.c,v
retrieving revision 1.20
diff -c -3 -p -w -r1.20 macro.c
*** gas/macro.c	3 Jan 2003 21:47:20 -0000	1.20
--- gas/macro.c	12 Mar 2003 14:12:21 -0000
*************** buffer_and_nest (from, to, ptr, get_line
*** 222,229 ****
  	    }
  	}
  
!       /* Add a CR to the end and keep running.  */
!       sb_add_char (ptr, '\n');
        line_start = ptr->len;
        more = get_line (ptr);
      }
--- 222,229 ----
  	    }
  	}
  
!       /* Add the original end-of-line char to the end and keep running.  */
!       sb_add_char (ptr, more);
        line_start = ptr->len;
        more = get_line (ptr);
      }


I tested it with this test case:

  .macro foo ; add r0 = r1, r2 ; .endm
  .macro bar ; add r0 = r1, r2 ;; ; .endm

        foo
	bar

and got this output:

   0:   0d 00 04 04 00 20       [MFI]       add r0=r1,r2
   6:   00 00 00 02 00 00                   nop.f 0x0
   c:   10 10 00 80                         add r0=r1,r2;;

I am not an IA-64 instruction set expert, but this looks about right
to me.

Cheers
        Nick

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: .macro bug in gas for IA64
@ 2003-03-07 17:55 Tim Connors
  0 siblings, 0 replies; 7+ messages in thread
From: Tim Connors @ 2003-03-07 17:55 UTC (permalink / raw)
  To: binutils

Nick,

Thanks for looking at my submission to binutils@sources.redhat.com.

> It seems to me that your proposed patch would disallow multiple
> instructions on a single line inside macro blocks (for the IA64 at least).

Actually, it doesn't do that.  It simply makes sure that ALL ';'
characters in a macro definition find there way into the stored macro
body.  That includes single runs of ';', as well as double runs ";;",
and even longer runs ";;;;;;;".  Then, after macro expansion, the
normal parsing routines determine what's a separator and what's a stop
bit.  In fact part of the goal, was to make sure that this determination
only occurs in one place.

> Instead would it not be easier to choose a different character to be
> the line separator character ?  Then you would not have to add any new
> code, and instead you could just change the initialisation value of
> line_separator_chars[] in tc-ia64.c.

I thought of that.  But I wanted to provide a fix that most people
would consider acceptable.  Changing the syntax of the input seemed
like it would be a harder sell.  Other non-gas assemblers for IA64 use
';' as a separator as well as ";;" for stop bit.

> If you cannot change the line separator, then at the very least you
> ought to change your new macro so that it takes the input line pointer
> and then it can scan ahead.  ie something like:

I don't see how scanning ahead helps any (perhaps scanning "nearby"
might, more below).  In particular,

>     while (!is_end_of_line[(unsigned char) *input_line_pointer]
> #ifdef md_end_of_line_char
>   	   || ! md_end_of_line_char (input_line_pointer)
> #endif
>   	   || (inquote != '\0' && *input_line_pointer != '\n'))
>       {
>         ...
>         sb_add_char (line, *input_line_pointer++);
>       }
> 
>   ...
> 
>   int
>   ia64_end_of_line_char (char * ilp)
>   {
>     /* ';;' is the stop bit and should not be treated as a line separator. */
>     return (ilp[0]  == ';' && ilp[1] != ';');
>   }

Note that in your suggested code, the first ';' in a double run would
get true for md_end_of_line_char() and this first ';' would be
inserted into the stored macro body :-)  But the second ';' in a double
run would give a false for the test and the second ';' would not make
it into the stored macro body.  This has the affect of converting a
stop bit into a line separator :-( although a triple run would be
converted to a double.  To fix this one might replace 

>     return (ilp[0]  == ';' && ilp[1] != ';');

with

    return ( (ilp[0]   == ';' && ilp[1] != ';') ||
	     (ilp[-1]  == ';' && ilp[0] != ';')    );

ie, scan nearby.  This test would have the affect of preserving all
double or greater runs while removing single runs :-).  The downside
is that it puts the knowledge of what token is a stop bit in more
places in the tc-ia64 code.

Even though I've tried to explain how I came up with my fix, I have no
preference as to what the fix is.  For now I've patched my assembler.
But unitl some fix ends up in the official code, I can't share my
sources with anyone.

Thanks for taking the time to look at this,
-Tim

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2003-03-12 14:17 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2003-03-01  3:23 .macro bug in gas for IA64 Tim Connors
2003-03-06 11:20 ` Nick Clifton
2003-03-07  0:02   ` Jim Wilson
2003-03-09 22:44   ` Jim Wilson
2003-03-11  0:56     ` Tim Connors
2003-03-12 14:17     ` Nick Clifton
2003-03-07 17:55 Tim Connors

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).