[bcc patch] Fix variable offset macros for #asm (was: Re: ELKS memcpy_fromfs() failing. Wrong variable offsets)

[Eduardo Pereira Habkost <ehabkost@conectiva.com.br>]

[-- Attachment #1: Type: text/plain, Size: 4342 bytes --]

On Wed, Jun 02, 2004 at 09:47:25AM +0100, Robert de Bath wrote:
<snip>
> 
> You have highlighted a bug though, the definition some of the 'set'
> variables is actually wrong, they should have memcpy_fromfs.off added
> when it might be non-zero.

I am sending a fix to this bug. I hope that I didn't anything wrong,
as I am not familiar with bcc code.

I guess that creating an outoffset(variable) function that uses
<function>.off when needed would be better, but that was just a quick fix.

> 
> BTW: You should really be using those variables to access C arguments
> and locals, the '_memcpy_fromfs.ds' is for stack pointer relative the
> '.memcpy_fromfs.ds' is for 'bp' relative.

As we have a fix for bcc >= 0.16.8, I will send a fix for ELKS code,
for using .<function>.<variable>, now.  :)

> 
> I can fix the bug in one of two ways; either actually add the
> 'memcpy_fromfs.off' variable to the 'set' variables or simply
> have the compiler assume that any '#asm' will use si and di.
> (The asm("...") function will not however.)
> 
> Actually I think assuming that #asm uses si/di is best because as this
> example has shown it is quite likely to be true, plus, it's the failsafe
> option.

I prefer the code as it is now: if some #asm will use some register,
then it should save it. Or maybe a better one (IMO): having a way to
tell the compiler which registers will be used.

And even if assuming that all #asm will use SI and DI, it is a little
safer fixing the offsets, too. Just in case someone make more changes
to the code, try to optmize #asm, and break things because there are
implict things (like: dumplocs() only work because when using #asm,
<function>.off is zero) they didn't see.

Which fix do you plan to include on the next releases of bcc?

(BTW, I really missed a BCC_VERSION macro or equivalent. It would be
very useful in cases like this one)

--
Eduardo


--- dev86-0.16.15/bcc/codefrag.c	2002-08-03 13:38:27.000000000 -0300
+++ dev86-current/bcc/codefrag.c	2004-06-02 12:38:28.233060704 -0300
@@ -105,7 +105,6 @@
 # define outlswitch() (outload(), outstr(ireg0str), outncregname(DREG))
 # define outnc1() outnstr(",*1")
 # define outsbc() outop3str("sbb\t")
-# define outset() outstr ("\tset\t")
 # define outsl() outop2str("shl\t")
 # define outsr() outop2str("sar\t")
 # define outtransfer() outload()
@@ -134,6 +133,10 @@
     outstr(acclostr);
     outncregname(BREG);
 }
+PUBLIC void outset()
+{
+    outstr ("\tset\t");
+}
 PUBLIC void comment()
 {
     outstr("! ");
@@ -417,7 +420,6 @@
 # define outpil2switch() outnop2str("LDD\tD,X")
 # define outrolhi() outnop1str("ROLA");
 # define outsbc() outop2str("SBC")
-# define outset() outstr ("\tSET\t")
 # define outsl() outop1str("LSL")
 # define outtransfer() outop2str("TFR\t")
 # define reclaimfactor() outnstr ("++")	/* discard factor from stack */
@@ -434,6 +436,10 @@
 # define tfrhilo() outnop2str("TFR\tA,B")
 # define tfrlohi() outnop2str("TFR\tB,A")
 # define usr1() (outnop1str("LSRA"), outnop1str("RORB"))
+PUBLIC void outset()
+{
+    outstr ("\tSET\t");
+}
 PUBLIC void clrBreg()
 {
     outnop1str("CLRB");
--- dev86-0.16.15/bcc/table.c	2002-07-28 04:43:13.000000000 -0300
+++ dev86-current/bcc/table.c	2004-06-02 12:48:12.176287840 -0300
@@ -402,8 +402,38 @@
 
     for (i = 0; i < HASHTABSIZE; ++i)
 	for (symptr = hashtab[i]; symptr != NULL; symptr = symptr->next)
-	    if (symptr->storage == LOCAL)
-		set(symptr->name.namea, symptr->offset.offi - sp);
+	    if (symptr->storage == LOCAL) {
+		/* Variable offset relative to sp */
+		outccname(funcname);
+		outbyte(LOCALSTARTCHAR);
+		outstr(symptr->name.namea);
+		outset();
+		outshex(symptr->offset.offi - sp);
+		outnl();
+
+#ifdef FRAMEPOINTER
+		/* Variable offset relative to bp */
+		if (framep) 
+		{
+		    outbyte(LOCALSTARTCHAR);
+		    outstr(funcname);
+		    outbyte(LOCALSTARTCHAR);
+		    outstr(symptr->name.namea);
+		    outset();
+		    outshex(symptr->offset.offi - framep);
+#ifdef I8088
+#ifndef NO_DEL_PUSH
+		    if (optimise && !callersaves && symptr->offset.offi < framep) {
+			outbyte('+');
+			outstr(funcname);
+			outstr(".off");
+		    }
+#endif
+#endif
+		    outnl();
+		}
+#endif
+	    }
 }
 
 #ifdef HOLDSTRINGS

[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]