Re: [Bug gas/m68k] parse error with m68k-aout targets

classic Classic list List threaded Threaded
8 messages Options
Reply | Threaded
Open this post in threaded view
|

Re: [Bug gas/m68k] parse error with m68k-aout targets

Alan Modra
On Thu, Nov 24, 2005 at 12:18:22PM +0100, Gunther Nikl wrote:

> Hello!
>
> There seems to be a bug for m68k-aout targets with any binutils version
> >= 2.10. The following testcase (extracted from bfd/libbfd.s output)
> triggers an error message:
>
> -- cut --
> #NO_APP
> .lcomm mask.0,4
> _foo:
> orl (mask.0),d0
> orl d2,(mask.0)
> rts
> -- cut --
>
> Assembling with:
>
>   ./gas/as-new bug.s -o bug.o
>
> yields:
>
>   gas214-bug.s: Assembler messages:
>   gas214-bug.s:4: Error: parse error -- statement `orl (mask.0),d0' ignored
>   gas214-bug.s:5: Error: parse error -- statement `orl d2,(mask.0)' ignored
>
> The testcase works fine with gas 2.9.1 and for m68k-elf. Note that for ELF
> a register prefix has to be added.

The trouble is that gas is seeing "(mask" and interpreting this as the
start of "(<reg>)".  "mask" was added as a reg on 1999-05-28, so this
has been around for a long time.

The following patch would fix this particular problem, but I'm not
applying it because it will break valid instructions like
"tstl (%d0.w*2)".  My 68k assembler is so rusty that I can't really help
more than just pointing someone else at the right area.  Sorry.
I suspect that the right fix would be to move quite a bit of code from
m68k-parse.y yylex to new parser rules to properly parse registers.

Index: gas/config/m68k-parse.y
===================================================================
RCS file: /cvs/src/src/gas/config/m68k-parse.y,v
retrieving revision 1.9
diff -u -p -r1.9 m68k-parse.y
--- gas/config/m68k-parse.y 23 Jun 2005 11:40:29 -0000 1.9
+++ gas/config/m68k-parse.y 28 Nov 2005 12:18:34 -0000
@@ -801,9 +801,13 @@ yylex ()
       if (str[1] == '\0' || (str[1] == '&' && str[2] == '\0'))
  return *str++;
       s = str + 1;
+      c = '\0';
       if (*s == '(')
- ++s;
-      if (m68k_reg_parse (&s) != 0)
+ {
+  ++s;
+  c = ')';
+ }
+      if (m68k_reg_parse (&s) != 0 && *s == c)
  return *str++;
       break;
     case '(':
@@ -816,7 +820,7 @@ yylex ()
   || str[-1] == ')')))
  return *str++;
       s = str + 1;
-      if (m68k_reg_parse (&s) != 0)
+      if (m68k_reg_parse (&s) != 0 && *s == ')')
  return *str++;
       /* Check for the case of '(expr,...' by scanning ahead.  If we
          find a comma outside of balanced parentheses, we return '('.

--
Alan Modra
IBM OzLabs - Linux Technology Centre
Reply | Threaded
Open this post in threaded view
|

Re: [Bug gas/m68k] parse error with m68k-aout targets

Gunther Nikl
On Mon, Nov 28, 2005 at 11:05:22PM +1030, Alan Modra wrote:

> On Thu, Nov 24, 2005 at 12:18:22PM +0100, Gunther Nikl wrote:
> >   gas214-bug.s: Assembler messages:
> >   gas214-bug.s:4: Error: parse error -- statement `orl (mask.0),d0' ignored
> >   gas214-bug.s:5: Error: parse error -- statement `orl d2,(mask.0)' ignored
> >
> > The testcase works fine with gas 2.9.1 and for m68k-elf. Note that for ELF
> > a register prefix has to be added.
>
> The trouble is that gas is seeing "(mask" and interpreting this as the
> start of "(<reg>)".

  Thank you for this explanation. I was a puzzled by the messages since I
  didn't know that GAS knows about a register called "mask". Even after
  looking at the source I have no idea what "mask" should be.

> "mask" was added as a reg on 1999-05-28, so this has been around for a
> long time.

  It is a hard to detect bug. It was now detected with binutils-2.14
  sources because bfd/libbfd.c/warn_deprecated has a static variable
  "mask". Since the variable is static it is emitted without a prepended
  underscore with a.out systems. Thus by renaming the variable the
  problem goes away for this particular source.
  FYI, the last usable binutils version for my target (m68k-amigaos) was
  2.9.1 since required modifications did only exist for that particular
  version. Thus it wasn't possible to run into that bug before.

> The following patch would fix this particular problem, but I'm
> not applying it because it will break valid instructions like
> "tstl (%d0.w*2)".

  Thats unfortunate :-(

> My 68k assembler is so rusty that I can't really help more than just
> pointing someone else at the right area.  Sorry. I suspect that the
> right fix would be to move quite a bit of code from m68k-parse.y yylex
> to new parser rules to properly parse registers.

  Maybe Ian Lance Taylor could help. I think he still knows m68k well
  enough.

  Thanks again for the insight and the patch.

  Gunther
Reply | Threaded
Open this post in threaded view
|

Re: [Bug gas/m68k] parse error with m68k-aout targets

Ian Lance Taylor
Gunther Nikl <[hidden email]> writes:

>   It is a hard to detect bug. It was now detected with binutils-2.14
>   sources because bfd/libbfd.c/warn_deprecated has a static variable
>   "mask". Since the variable is static it is emitted without a prepended
>   underscore with a.out systems. Thus by renaming the variable the
>   problem goes away for this particular source.

Even a static variable should have a prepended underscore.  Otherwise
it is impossible to say something like
    static int d0;
Not using an underscore is clearly a bug.  Are you using gcc?  If so,
which version?

Given that mask is a register name, this instruction
    orl (mask.0),d0
really is invalid.  We could fix it so that that works, but still
    orl (mask),d0
would not and could not work.

The compiler should not emit symbols which are in the register name
space.  The same applies to hand coded assembler.  Assembler code
should be unambiguous.  I don't think it is appropriate to add code to
the assembler to clarify ambiguity by guessing what the user means.

Ian
Reply | Threaded
Open this post in threaded view
|

Re: parse error with m68k-aout targets

Gunther Nikl
On Mon, Nov 28, 2005 at 12:02:32PM -0800, Ian Lance Taylor wrote:

> Gunther Nikl <[hidden email]> writes:
>
> >   It is a hard to detect bug. It was now detected with binutils-2.14
> >   sources because bfd/libbfd.c/warn_deprecated has a static variable
> >   "mask". Since the variable is static it is emitted without a prepended
> >   underscore with a.out systems. Thus by renaming the variable the
> >   problem goes away for this particular source.
>
> Even a static variable should have a prepended underscore.  Otherwise
> it is impossible to say something like
>     static int d0;
> Not using an underscore is clearly a bug.

  You are correct. I don't know why I didn't think about that myself.
  FWIW, no GAS2 version probably could assemble such code. GAS 1.38.1
  does.

> Are you using gcc?  If so, which version?

  Yes, the error surfaced with GCC. I was using GCC 3.4.3 to build
  binutils 2.14. Now I checked other GCC versions (2.95.x, 3.3.x,
  4.0.0 and an oldish 4.1.0) and all do add an underscore to the
  variable name. All GCC 3.4.x versions "forget" the underscore :-/
  It seems this is a GCC 3.4.x only bug affecting a.out targets: with
  i386-aout and m68k-aout the underscore is missing. The bug is still
  present with prerelease-3.4.5-20051128. I guess I should file a bug
  report. Can you give me a pointer where the bug might be?

> The compiler should not emit symbols which are in the register name
> space.  The same applies to hand coded assembler.  Assembler code
> should be unambiguous.  I don't think it is appropriate to add code to
> the assembler to clarify ambiguity by guessing what the user means.

  I agree that this isn't an assembler problem, its a GCC 3.4.x bug.
  Thank you for making me realize this fact.

  Gunther
Reply | Threaded
Open this post in threaded view
|

Re: [Bug gas/m68k] parse error with m68k-aout targets

Ian Lance Taylor
Gunther Nikl <[hidden email]> writes:

> > Are you using gcc?  If so, which version?
>
>   Yes, the error surfaced with GCC. I was using GCC 3.4.3 to build
>   binutils 2.14. Now I checked other GCC versions (2.95.x, 3.3.x,
>   4.0.0 and an oldish 4.1.0) and all do add an underscore to the
>   variable name. All GCC 3.4.x versions "forget" the underscore :-/
>   It seems this is a GCC 3.4.x only bug affecting a.out targets: with
>   i386-aout and m68k-aout the underscore is missing. The bug is still
>   present with prerelease-3.4.5-20051128. I guess I should file a bug
>   report. Can you give me a pointer where the bug might be?

This may be related to PR 14516 (http://gcc.gnu.org/PR14516).

In general, if USER_LABEL_PREFIX is defined correctly (which is
probably the case since otherwise no symbol would have a leading
underscore), this is some error in the use of user_label_prefix.

Ian
Reply | Threaded
Open this post in threaded view
|

Re: [Bug gas/m68k] parse error with m68k-aout targets

Gunther Nikl
On Tue, Nov 29, 2005 at 09:06:26AM -0800, Ian Lance Taylor wrote:

> Gunther Nikl <[hidden email]> writes:
>
> > > Are you using gcc?  If so, which version?
> >
> >   Yes, the error surfaced with GCC. I was using GCC 3.4.3 to build
> >   binutils 2.14. Now I checked other GCC versions (2.95.x, 3.3.x,
> >   4.0.0 and an oldish 4.1.0) and all do add an underscore to the
> >   variable name. All GCC 3.4.x versions "forget" the underscore :-/
> >   It seems this is a GCC 3.4.x only bug affecting a.out targets: with
> >   i386-aout and m68k-aout the underscore is missing. The bug is still
> >   present with prerelease-3.4.5-20051128. I guess I should file a bug
> >   report. Can you give me a pointer where the bug might be?
>
> This may be related to PR 14516 (http://gcc.gnu.org/PR14516).

  Yes, this pr seems to describe the problem. Unfortunately no solution
  is offered, worse the bug was classified as "no fix for 3.4" - thats
  a bit surprsining for me. And I don't want to switch to 4.0.
  What about your patch in the audit trail? What other problems does
  it cause?

> In general, if USER_LABEL_PREFIX is defined correctly (which is
> probably the case since otherwise no symbol would have a leading
> underscore), this is some error in the use of user_label_prefix.

  Only 3.4 has this problem, all other GCC versions are fine.

  Gunther
Reply | Threaded
Open this post in threaded view
|

Re: [Bug gas/m68k] parse error with m68k-aout targets

Ian Lance Taylor
Gunther Nikl <[hidden email]> writes:

>   Yes, this pr seems to describe the problem. Unfortunately no solution
>   is offered, worse the bug was classified as "no fix for 3.4" - thats
>   a bit surprsining for me. And I don't want to switch to 4.0.
>   What about your patch in the audit trail? What other problems does
>   it cause?

I have no idea.  Geoff didn't like the patch, but I don't know why.
It's possible that the patch only causes a problem when compiling
different source files at the same time.

Ian
Reply | Threaded
Open this post in threaded view
|

Re: [Bug gas/m68k] parse error with m68k-aout targets

Gunther Nikl
On Tue, Nov 29, 2005 at 09:35:50AM -0800, Ian Lance Taylor wrote:
> Gunther Nikl <[hidden email]> writes:
>
> >   What about your patch in the audit trail? What other problems does
> >   it cause?
>
> I have no idea.  Geoff didn't like the patch, but I don't know why.
> It's possible that the patch only causes a problem when compiling
> different source files at the same time.

  Then I should ask on the GCC ML. Thanks for helping,

  Gunther