PATCH: Fix 2.6 kernel ia64 directives

PATCH: Fix 2.6 kernel ia64 directives

[H. J. Lu]

The new ia64 assembler caught a few directive bugs. Here is the patch.
 

H.J.
--- linux-2.6.9/arch/ia64/kernel/ivt.S.unwind	2004-10-18 14:53:22.000000000 -0700
+++ linux-2.6.9/arch/ia64/kernel/ivt.S	2005-02-02 12:00:27.562232125 -0800
@@ -547,7 +547,7 @@ ENTRY(dirty_bit)
 #endif
 	mov pr=r31,-1				// restore pr
 	rfi
-END(idirty_bit)
+END(dirty_bit)
 
 	.org ia64_ivt+0x2400
 /////////////////////////////////////////////////////////////////////////////////////////

Re: PATCH: Fix 2.6 kernel ia64 directives

[David Mosberger]

>>>>> On Wed, 2 Feb 2005 14:19:18 -0800, "H. J. Lu" <hjl@lucon.org> said:

  HJ> The new ia64 assembler caught a few directive bugs.

  HJ> -END(idirty_bit)
  HJ> +END(dirty_bit)

While I'm in favor of fixing such typos, the Itanium Assembly Language
Reference Manual states:

	"The assembler ignores the name operands of the
	 directive."

(page 35 of document 248801-004).

In my opinion, a warning might be in place, but a hard error for
something that the spec explicitly allows seems like a bad idea to me.

	--david

Re: PATCH: Fix 2.6 kernel ia64 directives

[Jan Beulich]

That is correct. However, to comply with this *AND* emit an error when a
symbol mentioned with .proc wasn't defined before the .endp, more
substantial changes to the assembler would be necessary (it would have
to keep track of all the symbols mentioned in .proc, which it currently
doesn't). I can certainly do such a change, but I think ignoring the
name arguments of .endp is a bad idea. Jan

>>> davidm@napali.hpl.hp.com 02.02.05 23:50:45 >>>
[Hi Jan, I sent this to HJ, assuming that he changed the assembler,
 but looking through the ChangeLog, I now believe that you added the
 .endp checking.  I see you also added some unwind-directive sanity
 checking and other useful improvements, which should be very
helpful.]

>>>>> On Wed, 2 Feb 2005 14:19:18 -0800, "H. J. Lu" <hjl@lucon.org>
said:

  HJ> The new ia64 assembler caught a few directive bugs.

  HJ> -END(idirty_bit)
  HJ> +END(dirty_bit)

While I'm in favor of fixing such typos, the Itanium Assembly Language
Reference Manual states:

	"The assembler ignores the name operands of the
	 directive."

(page 35 of document 248801-004).

In my opinion, a warning might be in place, but a hard error for
something that the spec explicitly allows seems like a bad idea to me.

	--david

Re: PATCH: Fix 2.6 kernel ia64 directives

[James E Wilson]

On Wed, 2005-02-02 at 14:50, David Mosberger wrote:
> While I'm in favor of fixing such typos, the Itanium Assembly Language
> Reference Manual states:

The Itanium Assembly Language Reference Manual is badly out of date, and
can no longer be trusted to be accurate.  It hasn't been updated since
2001, but IAS has apparently been updated every year, and no longer
behaves the same as described in the out dated manual.

There are people porting code from the dead Itanium Windows OS to linux
that need gas to behave more like the current IAS.  This is why gas
behaviour is changing.  I'm being careful to try to avoid breaking
anything linux depends on, but the changes include some extra error
checking, which is going to expose some latent problems.

> 	"The assembler ignores the name operands of the
> 	 directive."

I'd call this is a bug in the manual.  Just because one implementation,
at the time the manual was written, ignored the name operand, does not
mean that all assemblers should ignore it.

Also, I'm told that current IAS gives an error in this case, which is
why gas was modified to do likewise.  Unfortunately, I don't have access
to IAS, or to the HP assembler, so I can not check that.  If this is
wrong, then changing it to a warning certainly makes sense.
Otherwise, I don't see the problem with an error here, as the intent of
the syntax is clearly that the .proc/.endp names are supposed to match.

Re: PATCH: Fix 2.6 kernel ia64 directives

[James E Wilson]

On Mon, 2005-02-07 at 12:11, James E Wilson wrote:
> On Wed, 2005-02-02 at 14:50, David Mosberger wrote:
> > While I'm in favor of fixing such typos, the Itanium Assembly Language
> > Reference Manual states:

I see Jan responded to this already.  I missed it earlier because it
didn't thread with the rest of the discussion.  I also see that this
discussion has moved to the binutils list, so I will continue it there.

I see I was wrong about IAS giving an error for mismatched proc/end, I
misunderstood why that change was made.  The rest of the message should
still be accurate.
-- 
Jim Wilson, GNU Tools Support, http://www.SpecifixInc.com

Re: PATCH: Fix 2.6 kernel ia64 directives

[David Mosberger]

>>>>> On Mon, 07 Feb 2005 12:11:48 -0800, James E Wilson <wilson@tuliptree.org> said:

  >> "The assembler ignores the name operands of the
  >> directive."

  Jim> I'd call this is a bug in the manual.

Perhaps.

  Jim> Also, I'm told that current IAS gives an error in this case, which is
  Jim> why gas was modified to do likewise.  Unfortunately, I don't have access
  Jim> to IAS, or to the HP assembler, so I can not check that.  If this is
  Jim> wrong, then changing it to a warning certainly makes sense.

Why should it depend on IAS as to whether GAS does or does not produce
an error?  The fact is that GAS has _not_ in the past issued an error
and it's not acceptable to break existing code for no good reason.
Making it a warning is fine, of course.  That will alert people to the
fact that they may have an unintentional inconsistency in their code
and will let them fix the error at their convenience.  After enough
time has elapsed (at least a year), the severity could (and probably
should) be raised to an error.

Thanks,

	--david

Re: PATCH: Fix 2.6 kernel ia64 directives

[H. J. Lu]

On Wed, Feb 09, 2005 at 11:35:32AM -0800, David Mosberger wrote:
> >>>>> On Mon, 07 Feb 2005 12:11:48 -0800, James E Wilson <wilson@tuliptree.org> said:
> 
>   >> "The assembler ignores the name operands of the
>   >> directive."
> 
>   Jim> I'd call this is a bug in the manual.
> 
> Perhaps.
> 
>   Jim> Also, I'm told that current IAS gives an error in this case, which is
>   Jim> why gas was modified to do likewise.  Unfortunately, I don't have access
>   Jim> to IAS, or to the HP assembler, so I can not check that.  If this is
>   Jim> wrong, then changing it to a warning certainly makes sense.
> 
> Why should it depend on IAS as to whether GAS does or does not produce
> an error?  The fact is that GAS has _not_ in the past issued an error

FYI, I checked the current IAS, the name after endp IS ignored by IAS.

> and it's not acceptable to break existing code for no good reason.
> Making it a warning is fine, of course.  That will alert people to the
> fact that they may have an unintentional inconsistency in their code
> and will let them fix the error at their convenience.  After enough
> time has elapsed (at least a year), the severity could (and probably
> should) be raised to an error.
> 

I guess Jim is still catching his emails. I will give him a few days
to go though my ia64 gas patches which deal this very issues.


H.J.

Re: PATCH: Fix 2.6 kernel ia64 directives

[James E Wilson]

On Wed, 2005-02-09 at 11:35, David Mosberger wrote:
> Why should it depend on IAS as to whether GAS does or does not produce
> an error?

I was wrong about IAS giving an error in this case (.proc/.endp).  I
mentioned this in an earlier mail.  See Jan's response for the
clarification.

It has always been the goal since the beginning to be compatible with
IAS.  Unfortunately, the gas IA-64 port was never finished.  There is
some stuff that was never implemented, some stuff that was incorrectly
and/or incompletely implemented, and some stuff that is known to be
buggy.  The gas IA-64 port is suffering from a lack of attention. 
Fortunately, there is only one major bug, the DV support is fatally
flawed and has to be rewritten, and at the moment this problem is
limited by the fact that the gcc predication support is not smart enough
to emit code that gas can't handle correctly.

Anyways, these gas problems all need to be fixed eventually.  I do want
to avoid breaking existing code unnecessarily, but it may not be
possible in all cases.

> The fact is that GAS has _not_ in the past issued an error
> and it's not acceptable to break existing code for no good reason.
> Making it a warning is fine, of course.

I believe there is a good reason for this.  We are talking about
directives related to unwind info.  Unwind info only gets tested when
exceptional conditions occur, at which point it is too late to recover. 
It is important that the assembler give an error if a problem is
detected, as otherwise a serious latent bug may go unfixed.  I we give a
warning, then people may not notice it and fix their code.

However, I do understand that there is some inconvenience here.  I have
no problem with emitting a warning temporarily to give people a chance
to migrate.  In fact, I believe I will have to use a warning for now
because these changes break gcc -pg, and this is not easy to fix as it
will also require glibc changes.  I already have a patch from HJ to do
this, I just need to review it.

Re: PATCH: Fix 2.6 kernel ia64 directives

[David Mosberger]

>>>>> On Wed, 09 Feb 2005 12:10:13 -0800, James E Wilson <wilson@tuliptree.org> said:

  Jim> It is important that the assembler give an error if a problem is
  Jim> detected, as otherwise a serious latent bug may go unfixed.  I we give a
  Jim> warning, then people may not notice it and fix their code.

  Jim> However, I do understand that there is some inconvenience here.  I have
  Jim> no problem with emitting a warning temporarily to give people a chance
  Jim> to migrate.

Yes, that's really all I'm asking for.

I'm _very_ happy to see that the unwind directives are getting
sanity-checking.  Not having this has bitten me before on more than
one occasion and we all know that the number of people that check
unwind-info with readelf -u can be counted on one hand, so like you
say, that's very important.  But we can't expect existing code to get
fixed over night, hence we should allow for a grace period.  If the
code doesn't get fixed during that period, at least we can say "you
were warned"...

  Jim> In fact, I believe I will have to use a warning for now because
  Jim> these changes break gcc -pg, and this is not easy to fix as it
  Jim> will also require glibc changes.  I already have a patch from
  Jim> HJ to do this, I just need to review it.

OK, that sounds great.

Thanks,

	--david