* Re: ELKS memcpy_fromfs() failing. Wrong variable offsets [not found] <20040601222140.GV21172@duckman.distro.conectiva> @ 2004-06-02 8:47 ` Robert de Bath 2004-06-02 12:52 ` Eduardo Pereira Habkost ` (2 more replies) 0 siblings, 3 replies; 5+ messages in thread From: Robert de Bath @ 2004-06-02 8:47 UTC (permalink / raw) To: Eduardo Pereira Habkost; +Cc: Linux-8086 [-- Attachment #1: Type: TEXT/PLAIN, Size: 1828 bytes --] On Tue, 1 Jun 2004, Eduardo Pereira Habkost wrote: > I am forwarding these messages to you, to check if you have any clue on > why this happened, and why the function worked on older versions. > > I've changed the function to use [bp-2] for the 'ds' variable, and now > the kernel works flawlessly. > > Do you have an explanation on how this function.off stuff works, and if > this have changed on older versions? Oh my, confusion reigns. Okay first the si and di variables are usually defined as callee saves unless you're compiling with the '-Mc' option. The "memcpy_fromfs.off" stuff is generated for optimising the push/pop of the si and di variables. It's a reasonably recent addition so all accesses of si and si can be eliminated if they are unused. However, it's only turned on if you use '-O'. Ie: this is working as it should. 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. 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. 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. (PS: It's also very easy to do :-) see attached. ) So do you want me to make it like it was before? :-) -- Rob. (Robert de Bath <robert$ @ debath.co.uk>) <http://www.cix.co.uk/~mayday> [-- Attachment #2: Type: TEXT/PLAIN, Size: 545 bytes --] diff -Nurd linux86.old/bcc/table.c linux86/bcc/table.c --- linux86.old/bcc/table.c Sun Jul 28 08:43:13 2002 +++ linux86/bcc/table.c Wed Jun 2 09:37:56 2004 @@ -400,6 +400,11 @@ register struct symstruct *symptr; int i; + if (framep && optimise && !callersaves) { + regfuse |= callee1mask; + outnstr("! Assuming #asm uses all callee saves registers"); + } + for (i = 0; i < HASHTABSIZE; ++i) for (symptr = hashtab[i]; symptr != NULL; symptr = symptr->next) if (symptr->storage == LOCAL) ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: ELKS memcpy_fromfs() failing. Wrong variable offsets 2004-06-02 8:47 ` ELKS memcpy_fromfs() failing. Wrong variable offsets Robert de Bath @ 2004-06-02 12:52 ` Eduardo Pereira Habkost 2004-06-02 16:10 ` [bcc patch] Fix variable offset macros for #asm (was: Re: ELKS memcpy_fromfs() failing. Wrong variable offsets) Eduardo Pereira Habkost 2004-06-03 14:19 ` ELKS memcpy_fromfs() failing. Wrong variable offsets Miguel Bolanos 2 siblings, 0 replies; 5+ messages in thread From: Eduardo Pereira Habkost @ 2004-06-02 12:52 UTC (permalink / raw) To: Robert de Bath; +Cc: Linux-8086 [-- Attachment #1: Type: text/plain, Size: 3158 bytes --] On Wed, Jun 02, 2004 at 09:47:25AM +0100, Robert de Bath wrote: <snip> > > Oh my, confusion reigns. I guess so. 8) > > Okay first the si and di variables are usually defined as callee saves > unless you're compiling with the '-Mc' option. > > The "memcpy_fromfs.off" stuff is generated for optimising the push/pop > of the si and di variables. It's a reasonably recent addition so all > accesses of si and si can be eliminated if they are unused. However, > it's only turned on if you use '-O'. I've seen that this .off stuff was added between 0.16.0 and 0.16.15. That is why the (wrong) ELKS code used to work. > > Ie: this is working as it should. > > 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. > > 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. Hey, nice! I've never looked at that code before, and I didn't know what the author have assumed about the offsets of the variables, but these [._]<function>.<variable> macros are what I was looking for fixing the code. Thanks. :) BTW: these [._]<function>.<variable>macros are available since old versions, right? I will change ELKS code to use them. > > 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. > > (PS: It's also very easy to do :-) see attached. ) > > So do you want me to make it like it was before? :-) No, I just wanted to understand what happened. Now I understand the Right Way to access variables and parameters inside #asm. :) I think that forcing the user to save DI and SI if he wants to use the registers is ok, unless it would break some existing software. It's up to you. Adding a way to make #asm tell the compiler which registers it will use (and maybe assume by default that it will change all registers) would be interesting, too. > > -- > Rob. (Robert de Bath <robert$ @ debath.co.uk>) > <http://www.cix.co.uk/~mayday> > > diff -Nurd linux86.old/bcc/table.c linux86/bcc/table.c > --- linux86.old/bcc/table.c Sun Jul 28 08:43:13 2002 > +++ linux86/bcc/table.c Wed Jun 2 09:37:56 2004 > @@ -400,6 +400,11 @@ > register struct symstruct *symptr; > int i; > > + if (framep && optimise && !callersaves) { > + regfuse |= callee1mask; > + outnstr("! Assuming #asm uses all callee saves registers"); > + } > + > for (i = 0; i < HASHTABSIZE; ++i) > for (symptr = hashtab[i]; symptr != NULL; symptr = symptr->next) > if (symptr->storage == LOCAL) -- Eduardo [-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --] ^ permalink raw reply [flat|nested] 5+ messages in thread
* [bcc patch] Fix variable offset macros for #asm (was: Re: ELKS memcpy_fromfs() failing. Wrong variable offsets) 2004-06-02 8:47 ` ELKS memcpy_fromfs() failing. Wrong variable offsets Robert de Bath 2004-06-02 12:52 ` Eduardo Pereira Habkost @ 2004-06-02 16:10 ` Eduardo Pereira Habkost 2004-06-03 14:20 ` Miguel Bolanos 2004-06-03 14:19 ` ELKS memcpy_fromfs() failing. Wrong variable offsets Miguel Bolanos 2 siblings, 1 reply; 5+ messages in thread From: Eduardo Pereira Habkost @ 2004-06-02 16:10 UTC (permalink / raw) To: Robert de Bath; +Cc: Linux-8086 [-- 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 --] ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [bcc patch] Fix variable offset macros for #asm (was: Re: ELKS memcpy_fromfs() failing. Wrong variable offsets) 2004-06-02 16:10 ` [bcc patch] Fix variable offset macros for #asm (was: Re: ELKS memcpy_fromfs() failing. Wrong variable offsets) Eduardo Pereira Habkost @ 2004-06-03 14:20 ` Miguel Bolanos 0 siblings, 0 replies; 5+ messages in thread From: Miguel Bolanos @ 2004-06-03 14:20 UTC (permalink / raw) To: Eduardo Pereira Habkost; +Cc: Robert de Bath, Linux-8086 I can't apply this patch either because is for dev86, so will forward it to Bruce Evans. thanks Mike On Wed, 2004-06-02 at 10:10, Eduardo Pereira Habkost wrote: > 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 -- --------------------miguel bolanos, systems administrator, linuxlabs ... ........ ..... .... 230 peachtree st nw ste 2701 the original linux labs atlanta.ga.us 30303 -since 1995 http://www.linuxlabs.com office 404.577.7747 fax 404.577.7743 -------------------------------------------------------------------- ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: ELKS memcpy_fromfs() failing. Wrong variable offsets 2004-06-02 8:47 ` ELKS memcpy_fromfs() failing. Wrong variable offsets Robert de Bath 2004-06-02 12:52 ` Eduardo Pereira Habkost 2004-06-02 16:10 ` [bcc patch] Fix variable offset macros for #asm (was: Re: ELKS memcpy_fromfs() failing. Wrong variable offsets) Eduardo Pereira Habkost @ 2004-06-03 14:19 ` Miguel Bolanos 2 siblings, 0 replies; 5+ messages in thread From: Miguel Bolanos @ 2004-06-03 14:19 UTC (permalink / raw) To: Robert de Bath; +Cc: Eduardo Pereira Habkost, Linux-8086 I can't apply this patch as this is for dev86.. this should go to Bruce Evans... I will forward it to him. Thanks Mike On Wed, 2004-06-02 at 02:47, Robert de Bath wrote: > On Tue, 1 Jun 2004, Eduardo Pereira Habkost wrote: > > > I am forwarding these messages to you, to check if you have any clue on > > why this happened, and why the function worked on older versions. > > > > I've changed the function to use [bp-2] for the 'ds' variable, and now > > the kernel works flawlessly. > > > > Do you have an explanation on how this function.off stuff works, and if > > this have changed on older versions? > > Oh my, confusion reigns. > > Okay first the si and di variables are usually defined as callee saves > unless you're compiling with the '-Mc' option. > > The "memcpy_fromfs.off" stuff is generated for optimising the push/pop > of the si and di variables. It's a reasonably recent addition so all > accesses of si and si can be eliminated if they are unused. However, > it's only turned on if you use '-O'. > > Ie: this is working as it should. > > 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. > > 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. > > 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. > > (PS: It's also very easy to do :-) see attached. ) > > So do you want me to make it like it was before? :-) -- --------------------miguel bolanos, systems administrator, linuxlabs ... ........ ..... .... 230 peachtree st nw ste 2701 the original linux labs atlanta.ga.us 30303 -since 1995 http://www.linuxlabs.com office 404.577.7747 fax 404.577.7743 -------------------------------------------------------------------- ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2004-06-03 14:20 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <20040601222140.GV21172@duckman.distro.conectiva>
2004-06-02 8:47 ` ELKS memcpy_fromfs() failing. Wrong variable offsets Robert de Bath
2004-06-02 12:52 ` Eduardo Pereira Habkost
2004-06-02 16:10 ` [bcc patch] Fix variable offset macros for #asm (was: Re: ELKS memcpy_fromfs() failing. Wrong variable offsets) Eduardo Pereira Habkost
2004-06-03 14:20 ` Miguel Bolanos
2004-06-03 14:19 ` ELKS memcpy_fromfs() failing. Wrong variable offsets Miguel Bolanos
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox