Grant Grundler | 24 Sep 20:54

Bug#329888: [parisc-linux] binutils breaks hppa kernel build, bad operands of fic, m

On Sat, Sep 24, 2005 at 10:45:40AM -0600, dann frazier wrote:
> As discussed on irc:
> http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=329888

binutils-2.15 has this in include/opcodes/hppa.h:
{ "fdc",        0x04001280, 0xfc003fdf, "cZx(s,b)", pa10, 0},
{ "fdc",        0x04001280, 0xfc003fdf, "cZx(b)", pa10, 0},
{ "fic",        0x04000280, 0xfc001fdf, "cZx(S,b)", pa10, 0},
{ "fic",        0x04000280, 0xfc001fdf, "cZx(b)", pa10, 0},

And the binutils_2.16.1cvs20050902 version has only:
{ "fdc",        0x04001280, 0xfc00ffdf, "cZx(b)", pa10, 0},
{ "fdc",        0x04001280, 0xfc003fdf, "cZx(s,b)", pa10, 0},
{ "fic",        0x04000280, 0xfc001fdf, "cZx(S,b)", pa10, 0},

The order of the with 's' vs without 's' was switched and
in the process dropped one "fic" line.

Can binutils maintainer/folks please just add it back
in the new location?

Is the fix obvious enough or is a patch necessary?

thanks,
grant

John David Anglin | 25 Sep 03:27
Picon

Bug#329888: [parisc-linux] binutils breaks hppa kernel build,

> binutils-2.15 has this in include/opcodes/hppa.h:
> { "fdc",        0x04001280, 0xfc003fdf, "cZx(s,b)", pa10, 0},
> { "fdc",        0x04001280, 0xfc003fdf, "cZx(b)", pa10, 0},
> { "fic",        0x04000280, 0xfc001fdf, "cZx(S,b)", pa10, 0},
> { "fic",        0x04000280, 0xfc001fdf, "cZx(b)", pa10, 0},

The last "fic" opcode entry is wrong.  It's using the wrong
instruction format, the mask is wrong, pa10 is wrong, etc.

> And the binutils_2.16.1cvs20050902 version has only:
> { "fdc",        0x04001280, 0xfc00ffdf, "cZx(b)", pa10, 0},
> { "fdc",        0x04001280, 0xfc003fdf, "cZx(s,b)", pa10, 0},
> { "fic",        0x04000280, 0xfc001fdf, "cZx(S,b)", pa10, 0},

Yes, I deleted the entry because there isn't implicit space
register selection for 'S' instructions.

> The order of the with 's' vs without 's' was switched and
> in the process dropped one "fic" line.

Changing the order of with 's' vs without 's' was intentional.
It generates better disassembly.  Give the new objdump a try
and you will see the difference.

> Can binutils maintainer/folks please just add it back
> in the new location?

We need a new pa20 format 24 entry.

> Is the fix obvious enough or is a patch necessary?
(Continue reading)

John David Anglin | 25 Sep 04:06
Picon

Bug#329888: [parisc-linux] binutils breaks hppa kernel build,

> > binutils-2.15 has this in include/opcodes/hppa.h:
> > { "fdc",        0x04001280, 0xfc003fdf, "cZx(s,b)", pa10, 0},
> > { "fdc",        0x04001280, 0xfc003fdf, "cZx(b)", pa10, 0},
> > { "fic",        0x04000280, 0xfc001fdf, "cZx(S,b)", pa10, 0},
> > { "fic",        0x04000280, 0xfc001fdf, "cZx(b)", pa10, 0},
> 
> The last "fic" opcode entry is wrong.  It's using the wrong
> instruction format, the mask is wrong, pa10 is wrong, etc.

The enclosed patch against binutils head adds the missing "fic"
entry.  I also noticed that we were missing a pa20 im5 variant for
"fdc".  This builds and checks with no regressions.  However,
the new entries aren't really tested.

Dave
--

-- 
J. David Anglin                                  dave.anglin <at> nrc-cnrc.gc.ca
National Research Council of Canada              (613) 990-0752 (FAX: 952-6602)

Index: hppa.h
===================================================================
RCS file: /cvs/src/src/include/opcode/hppa.h,v
retrieving revision 1.59
diff -u -3 -p -r1.59 hppa.h
--- hppa.h	3 Aug 2005 15:08:52 -0000	1.59
+++ hppa.h	25 Sep 2005 01:58:35 -0000
@@ -755,6 +755,9 @@ static const struct pa_opcode pa_opcodes
 { "pdc",	0x04001380, 0xfc003fdf, "cZx(s,b)", pa10, 0},
 { "fdc",	0x04001280, 0xfc00ffdf, "cZx(b)", pa10, 0},
 { "fdc",	0x04001280, 0xfc003fdf, "cZx(s,b)", pa10, 0},
(Continue reading)

Matthew Wilcox | 25 Sep 03:49

Bug#329888: [parisc-linux] binutils breaks hppa kernel build,

On Sat, Sep 24, 2005 at 09:27:22PM -0400, John David Anglin wrote:
> The last "fic" opcode entry is wrong.  It's using the wrong
> instruction format, the mask is wrong, pa10 is wrong, etc.

I don't think we realised this until now.  I suspect we've been
silently emitting bad code up until this point, and given that it's
in flush_kernel_icache_page, maybe it explains some of our occasional
crashes.

The code was previously:

        fic,m           %r23(%r26)

We're trying to flush kernel space at this point, so we can use any of
sr4-sr7.   I suspect before we were putting sr0 in, which could have
been flushing any random space instead of the one we wanted.  I'll
commit this change to the kernel source now.

John David Anglin | 25 Sep 05:24
Picon

Bug#329888: [parisc-linux] binutils breaks hppa kernel build,

> On Sat, Sep 24, 2005 at 09:27:22PM -0400, John David Anglin wrote:
> > The last "fic" opcode entry is wrong.  It's using the wrong
> > instruction format, the mask is wrong, pa10 is wrong, etc.
> 
> I don't think we realised this until now.  I suspect we've been
> silently emitting bad code up until this point, and given that it's
> in flush_kernel_icache_page, maybe it explains some of our occasional
> crashes.
> 
> The code was previously:
> 
>         fic,m           %r23(%r26)

I've committed a fix to binutils head.  The above should now be
accepted when generating PA 2.0 code.  However, I'd probably
avoid using this feature for awhile.  It looks like the bug has
been present since ~ 1996.

Dave
--

-- 
J. David Anglin                                  dave.anglin <at> nrc-cnrc.gc.ca
National Research Council of Canada              (613) 990-0752 (FAX: 952-6602)

Matthew Wilcox | 25 Sep 05:45

Bug#329888: [parisc-linux] binutils breaks hppa kernel build,

On Sat, Sep 24, 2005 at 11:24:31PM -0400, John David Anglin wrote:
> > On Sat, Sep 24, 2005 at 09:27:22PM -0400, John David Anglin wrote:
> > > The last "fic" opcode entry is wrong.  It's using the wrong
> > > instruction format, the mask is wrong, pa10 is wrong, etc.
> > 
> > I don't think we realised this until now.  I suspect we've been
> > silently emitting bad code up until this point, and given that it's
> > in flush_kernel_icache_page, maybe it explains some of our occasional
> > crashes.
> > 
> > The code was previously:
> > 
> >         fic,m           %r23(%r26)
> 
> I've committed a fix to binutils head.  The above should now be
> accepted when generating PA 2.0 code.  However, I'd probably
> avoid using this feature for awhile.  It looks like the bug has
> been present since ~ 1996.

This is in a file that's used on pa1.1 processors too, so I've gone with
specifying sr4 explicitly.  Thanks for fixing this bug and exposing our
bug ...


Gmane