[comp.sys.amiga] Lattice C 5.04 bug

SUGGS@freddy.hac.com (11/30/89)

I've discovered the following bug in Lattice C 5.04:

When register variables are declared with an initial value, that
value is also written onto the stack.  That is, written over existing
values on the stack, not pushed on the stack.

Actually, the values are written to (A5), 4(A5) ..., but when a function
reserves variable space on the stack via   LINK A5,#nnnn   the values that
get overwritten are the saved value of A5 (for one register variable with
an initial value) and the return address of the subroutine call (with a
second register variable).

source:

test()
{
   register int i = 1234;
   register int j = 5678;
   int k; /* so the function does a LINK A5,#nnnn 
             if this line isn't here, A5 could be pointing
             anywhere, so you don't always see what gets trashed */
}

generates the following assembly:
LINK     A5,#FFF4
CMPA.L   __base(A4),A7   \ stack
BCS.W    __xcovf         /      overflow checking
MOVEM.L  D6-D7,-(A7)
MOVE.L   #4D2,D7
LEA      0(A5),A0        \  this code shouldn't be here!
MOVE.L   D7,(A0)         /      (overwrites saved value of A5)     
MOVE.L   #162E,D6
LEA      4(A5),A1        \  ditto for this!
MOVE.L   D6,(A1)         /      (overwrites subroutine call return address) 
MOVEM.L  (A7)+,D6-D7
UNLK     A5
RTS

It seems that the compiler doesn't take into account the fact that the variable
is a register variable when handling the initial value of a declared variable.
I say this because the code is similar to what is done when the variable is
*not* a register variable (except the offset from A5 is negative to put
it into the space reserved by the LINK instruction.)

As I'm writing this, I just noticed that the LINK instruction also seems
to be reserving space for all three variables, it should only need to be
LINK A5,#-4 instead of -12.

Hope this helps someone avoid the frustration I've been through.
-Brian

P.S. Why do they do 
         LEA      n(A5),A0
         MOVE.L   D7,(A0)
     instead of just
         MOVE.L   D7,n(A5)

mcw@wet.UUCP (Martin Warnett) (12/06/89)

There appears to be a bug in the chdir() function in Lattice C 5.04.
When I run my program from the CLI everything works fine. However when
the same program ran from the WB, it ran fine and terminated but the
system locked up and I had to reboot. I couldn't even click on a disk
icon.

When I replaced the chdir() with the AmigaDOS CurrentDir function 
everything worked fine.

Martin.
-- 
+====================================================================+
| Martin Warnett          | uucp:   ...sun!claris!wet!mcw            |
| San Francisco, CA       |         claris!wet!mcw@ames.arc.nasa.gov |
+====================================================================+

SUGGS%freddy.hac.com@cunyvm.cuny.edu (12/06/89)

I've discovered the following bug in Lattice C 5.04:

When register variables are declared with an initial value, that
value is also written onto the stack.  That is, written over existing
values on the stack, not pushed on the stack.

Actually, the values are written to (A5), 4(A5) ..., but when a function
reserves variable space on the stack via   LINK A5,#nnnn   the values that
get overwritten are the saved value of A5 (for one register variable with
an initial value) and the return address of the subroutine call (with a
second register variable).

source:

test()
{
   register int i = 1234;
   register int j = 5678;
   int k; /* so the function does a LINK A5,#nnnn
             if this line isn't here, A5 could be pointing
             anywhere, so you don't always see what gets trashed */
}

generates the following assembly:
LINK     A5,#FFF4
CMPA.L   __base(A4),A7   \ stack
BCS.W    __xcovf         /      overflow checking
MOVEM.L  D6-D7,-(A7)
MOVE.L   #4D2,D7
LEA      0(A5),A0        \  this code shouldn't be here!
MOVE.L   D7,(A0)         /      (overwrites saved value of A5)
MOVE.L   #162E,D6
LEA      4(A5),A1        \  ditto for this!
MOVE.L   D6,(A1)         /      (overwrites subroutine call return address)
MOVEM.L  (A7)+,D6-D7
UNLK     A5
RTS

It seems that the compiler doesn't take into account the fact that the variable
is a register variable when handling the initial value of a declared variable.
I say this because the code is similar to what is done when the variable is
*not* a register variable (except the offset from A5 is negative to put
it into the space reserved by the LINK instruction.)

As I'm writing this, I just noticed that the LINK instruction also seems
to be reserving space for all three variables, it should only need to be
LINK A5,#-4 instead of -12.

Hope this helps someone avoid the frustration I've been through.
-Brian

P.S. Why do they do
         LEA      n(A5),A0
         MOVE.L   D7,(A0)
     instead of just
         MOVE.L   D7,n(A5)

gregg@cbnewsc.ATT.COM (gregg.g.wonderly) (12/12/89)

From article <828@wet.UUCP>, by mcw@wet.UUCP (Martin Warnett):
> There appears to be a bug in the chdir() function in Lattice C 5.04.
> When I run my program from the CLI everything works fine. However when
> the same program ran from the WB, it ran fine and terminated but the
> system locked up and I had to reboot. I couldn't even click on a disk
> icon.

Which brings me to my favorite gripe.  Why do people insist on implementing
compatibility libraries from the lowest level?  It seems really rediculous
to try and invent new bugs in software.  My favorite is the sleep()
implementation in C-Kermit.  Instead of 

sleep(n)
	unsigned n;
{
	Delay (60*n);
}

the code was written with huge amounts of StartIO/WaitIO and other such
nonsense.  This is really why the Exec functions were written.  Once
these routines are written and debugged (most were long ago), there are
no more worries.

Please folks use the highest level of interface, not the lowest, even though
you know how to do it.  I look much more highly on those that do it the
easiest way rather than the "trickest" way.  Remember, the code has to
work right before being faster makes a difference...

-- 
-----
gregg.g.wonderly@att.com   (AT&T bell laboratories)

bader+@andrew.cmu.edu (Miles Bader) (12/12/89)

gregg@cbnewsc.ATT.COM (gregg.g.wonderly) writes:
> From article <828@wet.UUCP>, by mcw@wet.UUCP (Martin Warnett):
> > There appears to be a bug in the chdir() function in Lattice C 5.04.
> > ...
> Which brings me to my favorite gripe.  Why do people insist on implementing
> compatibility libraries from the lowest level?  It seems really rediculous
> to try and invent new bugs in software.  My favorite is the sleep()
> implementation in C-Kermit.  Instead of 
> 
> sleep(n)
>         unsigned n;
> {
>         Delay (60*n);
> }
> 
> the code was written with huge amounts of StartIO/WaitIO and other such
> nonsense.
> ...
> Please folks use the highest level of interface, not the lowest, even though
> you know how to do it.  I look much more highly on those that do it the
> easiest way rather than the "trickest" way.  Remember, the code has to
> work right before being faster makes a difference...

I was under the impression that "Delay" was a pretty nasty thing to use, that
it stopped the whole system dead while it was delaying or something.
Whatever the reason, I remember seeing posts to the effect of "Don't use
Delay."  Anyone remember why?

-Miles

usenet@cps3xx.UUCP (Usenet file owner) (12/12/89)

>> From article <828@wet.UUCP>, by mcw@wet.UUCP (Martin Warnett):
>> work right before being faster makes a difference...

Especially in a "PLease waste some time" function :-)

>
>I was under the impression that "Delay" was a pretty nasty thing to use, that
>it stopped the whole system dead while it was delaying or something.
>Whatever the reason, I remember seeing posts to the effect of "Don't use
>Delay."  Anyone remember why?

Yes. Bad things happen to the nearest filesystem if you  do a Delay(0);
This is a bug in the timer.device i believe, something about
it having trouble with very short intervals.

You get the same effect with WaitForChar(handle,0);


So, sllep really ought to be:
sleep(n)
         unsigned n;
 {
	 if(n==0 || n>(MAXINT/60) ) return; /* No delay, and too big
					       delays are bad things */
         Delay (60*n);
 }
 Joe Porkka   porkka@frith.egr.msu.edu

hull@hao.ucar.edu (Howard Hull) (12/12/89)

In article <cZV0fcW00UkaMGMmR0@andrew.cmu.edu> bader+@andrew.cmu.edu (Miles Bader) writes:
>I was under the impression that "Delay" was a pretty nasty thing to use, that
>it stopped the whole system dead while it was delaying or something.
>Whatever the reason, I remember seeing posts to the effect of "Don't use
>Delay."  Anyone remember why?
>
>-Miles

Well, I don't think the Delay() function busywaits (as you've implied), but
it will totally cream the system if called with a zero argument.  Now it
might seem a simple thing to just check your args before the call.  But I
have a program which is unlikely to ever furnish a zero argument for a Delay()
call (the argument may be switched among four positive real integer values
by means of an Intuition Menu Item) and it nonetheless will eventually cream
things real good if run a very long time with a bunch of other stuff.  The
probability is quite low - but it's not nearly low enough!  Symptoms most
usually wailed about are that the DOS will fail to finish updating the root
directory of a floppy disk in a fashion that renders track 40 unreadable.
This is a disaster, since DiskDoctor cannot in any way deal with volumes that
have no root block (DOS cannot even "discover" that the bitmap is invalid,
either, since there's no pointer to the bitmap).  There is only one program
I've seen that seems to deal with this not totally infrequent condition.
The program was distibuted on the net, and is called FixDisk.  FixDisk is a
freeware contribution from Werner Gunther.  Everyone needs to have it handy,
because the day _will_ come...
						Howard Hull
						hull@ncar.ucar.edu

jms@tardis.Tymnet.COM (Joe Smith) (12/12/89)

In article <cZV0fcW00UkaMGMmR0@andrew.cmu.edu> bader+@andrew.cmu.edu (Miles Bader) writes:
>I was under the impression that "Delay" was a pretty nasty thing to use, that
>it stopped the whole system dead while it was delaying or something.
                ^^^^^^^^^^^^
		only your task.

The admonition was "Don't use Delay(0)" because at one time it destroyed
the root directory of floppies.  (Release 1.0 or something.)

sleep(n)
int n;
{
  long ticks = n * 50;
  if (ticks == 0) ticks = 1;	/* make sure it's not zero */
  Delay(ticks);
}

However, Delay does prevent your process from doing anything else until
the requested time elapses.  In the case of Kermit, the program needs to
wait until either a packet comes in on from the modem or until the
timeout period expires.

>gregg@cbnewsc.ATT.COM (gregg.g.wonderly) writes:
>> Which brings me to my favorite gripe.  Why do people insist on implementing
>> compatibility libraries from the lowest level?  It seems really rediculous
>> to try and invent new bugs in software.  My favorite is the sleep()
>> implementation in C-Kermit.  Instead of [see above]
>> the code was written with huge amounts of StartIO/WaitIO and other such
>> nonsense.

Miles is right; Gregg is mistaken.  Using sleep() as defined above would
mean that Kermit would transfer one packet every 30 seconds.  Sleep()
in Unix works because your process can be woken up in the middle of the
sleep.  Delay() in AmigaDOS won't wake up early.

-- 
Joe Smith (408)922-6220 | SMTP: JMS@F74.TYMNET.COM or jms@gemini.tymnet.com
BT Tymnet Tech Services | UUCP: ...!{ames,pyramid}!oliveb!tymix!tardis!jms
PO Box 49019, MS-D21    | PDP-10 support: My car's license plate is "POPJ P,"
San Jose, CA 95161-9019 | humorous dislaimer: "My Amiga speaks for me."

farren@well.UUCP (Mike Farren) (12/14/89)

In article <12147@cbnewsc.ATT.COM> gregg@cbnewsc.ATT.COM (gregg.g.wonderly) writes:
>From article <828@wet.UUCP>, by mcw@wet.UUCP (Martin Warnett):
>	Delay (60*n);
               ^^------- 50 * n, please - Delay works in 50ths of a second.

>the code was written with huge amounts of StartIO/WaitIO and other such
>nonsense.  This is really why the Exec functions were written.

Ahh - but Delay isn't an Exec function, it's a DOS function.  Personally,
I try to avoid using DOS functions whenever possible, on principle, mostly.
Huge amounts of StartIO/WaitIO implies usage of timer.device, which is
the approved, non-DOS, more accurate, and more flexible means of getting
a busyloop-free delay.  After all, you only have to design sleep() once...

-- 
Mike Farren 				     farren@well.sf.ca.usa