gcc 3.2.3 bug?

[muta...@gmail.com]

Does this code look wrong?

edx appears to be used to contain the cluster number:

andl $-16, %edx

From the AND here:

cluster = (buf[offset]
| ((unsigned long)buf[offset + 1] << 8)
| ((unsigned long)buf[offset + 2] << 16)
| ((unsigned long)buf[offset + 3] << 24))
& 0x0fffffff;

But this code wipes the value:

xorl %edx, %edx

And the value would have been corrupt anyway because of this:

movb _buf.8+3(%edx), %dl


This is my version of gcc 3.2.3, but as far as I know, I
didn't change the code generation logic.

C source code is here:

https://sourceforge.net/p/pdos/gitcode/ci/master/tree/src/fat.c

and it is being built with this:

https://sourceforge.net/p/pdos/gitcode/ci/master/tree/src/comp5w.bat

using gcc386.exe available from http://pdos.org/customb.zip


It is the fatNuke() function.


Thanks. Paul.




L665:
subl $8, %esp
pushl %ebx
pushl %edi
call _fatEndCluster
addl $16, %esp
testl %eax, %eax
jne L689
movl 92(%edi), %eax
cmpl $16, %eax
je L690
cmpl $12, %eax
je L691
L671:
cmpl $32, %eax
jne L665
xorl %edx, %edx
leal 0(,%ebx,4), %eax
movl 16(%edi), %ecx
divl %ecx
movl %eax, %esi
addl 48(%edi), %esi
cmpl %esi, -16(%ebp)
je L681
movl -16(%ebp), %eax
testl %eax, %eax
jne L692
L682:
pushl %eax
pushl $_buf.8
pushl %esi
pushl %edi
call _fatReadLogical
movl %esi, -16(%ebp)
addl $16, %esp
movl 16(%edi), %ecx
L681:
leal 0(,%ebx,4), %eax
xorl %edx, %edx
divl %ecx
movzbl _buf.8+1(%edx), %eax
movzbl _buf.8(%edx), %ebx
sall $8, %eax
orl %eax, %ebx
movzbl _buf.8+2(%edx), %eax
movl %edx, %ecx
sall $16, %eax
movb _buf.8+3(%edx), %dl
orl %eax, %ebx
movl %edx, %eax
sall $24, %eax
orl %eax, %ebx
andl $-16, %edx
andl $268435455, %ebx
movb $0, _buf.8(%ecx)
movb $0, _buf.8+1(%ecx)
movb $0, _buf.8+2(%ecx)
movb %dl, _buf.8+3(%ecx)
incl -24(%ebp)
jmp L665

[Dan Cross]

In article <341826d2-1682-415f...@googlegroups.com>,

muta...@gmail.com <muta...@gmail.com> wrote:
>Does this code look wrong?

No.

>edx appears to be used to contain the cluster number:
>
> andl $-16, %edx

`%ebx` contains your cluster number, not `%edx`.

>From the AND here:

That is not from the logical `and` in the next stanza.

> cluster = (buf[offset]
> | ((unsigned long)buf[offset + 1] << 8)
> | ((unsigned long)buf[offset + 2] << 16)
> | ((unsigned long)buf[offset + 3] << 24))
> & 0x0fffffff;
>
>But this code wipes the value:
>
> xorl %edx, %edx

I should hope so, since you're doing a division after this and
the numerator fits in %eax.

>And the value would have been corrupt anyway because of this:
>
> movb _buf.8+3(%edx), %dl

What do you mean, "corrupt?" `%edx` holds an offset into some
buffer at the beginning of this instruction; that offset is not
used after this instruction, so the compiler chose to load a
byte into the register. That byte is eventually OR'd into the
"cluster" value in `%ebx`.

>This is my version of gcc 3.2.3, but as far as I know, I
>didn't change the code generation logic.
>
>C source code is here:
>
>https://sourceforge.net/p/pdos/gitcode/ci/master/tree/src/fat.c
>
>and it is being built with this:
>
>https://sourceforge.net/p/pdos/gitcode/ci/master/tree/src/comp5w.bat
>
>using gcc386.exe available from http://pdos.org/customb.zip
>
>
>It is the fatNuke() function.

Let's go through this section-by-section.

>L665:
> subl $8, %esp

Allocate some space on the stack. Why? To keep it aligned for
the call to `_fatEndCluster`.

> pushl %ebx
> pushl %edi

Push the arguments to `_fatEndCluster` onto the stack.

> call _fatEndCluster

Execute that call....

> addl $16, %esp

Deallocate the stack space allocated above. Note: it would look
like both `%ebx` and `%edi` could be clobbered here, but I'm
guessing you're using the "cdecl" calling convention, where
`%ebi` is callee-save.

> testl %eax, %eax

Did the call to `fatEndCluster` return zero?

> jne L689

If not, go to 689 (terminates the loop).

> movl 92(%edi), %eax

Move the `fat->fat_type` member into `%eax`.

> cmpl $16, %eax
> je L690

If `fat_type == 16`, branch to the code that handles that.

> cmpl $12, %eax
> je L691

If `fat_type == 12`, branch to the code that handles that.

>L671:
> cmpl $32, %eax
> jne L665

If `fat_type` is not 32, branch to the top of the loop. Note
that there is no case for handling bad data here; even minor
corruption could lead to an infinite loop.

> xorl %edx, %edx

Clear `%edx` for the division.

> leal 0(,%ebx,4), %eax

Move `cluster*4` into `%eax`.

> movl 16(%edi), %ecx

Copy `fat->sector_size` into `%ecx`.

> divl %ecx

Divide `cluster*4` (`%eax`) by `fat->sector_size` (in `%ecx`)
the quotient is placed in `%eax`. Note that this is why `%edx`
was cleared above (it is part of the numerator). After this
`%edx` contains `(cluster*4)%fat->sector_size`.

> movl %eax, %esi

Copy the quotient to `%esi`; see below about the modulus part in
`%edx`.

> addl 48(%edi), %esi

Add `fat->fat_sector` to the quotient. `%esi` is now
`fatSector`.

> cmpl %esi, -16(%ebp)

Compare `fatSector` to `buffered`.

> je L681

If they're equal, skip the next few instructions.

> movl -16(%ebp), %eax
> testl %eax, %eax
> jne L692

If `buffered != 0`, jump to whatever code handles that.

>L682:
> pushl %eax
> pushl $_buf.8
> pushl %esi
> pushl %edi
> call _fatReadLogical

Now push the arguments to `_fatReadLogical` onto the stack and
call it. Note that this will clobber `%eax` and `%edx`, and the
latter of held the value `(cluster*4)%fat->sector_size`. The
rest of the arguments to `_fatReadLogical` are callee-safe.
`$_buf.8` is a static, so just a pointer.

> movl %esi, -16(%ebp)

Copy `fatSector` to `buffered`.

> addl $16, %esp

Clean up the stack after the call to `_fatReadLogical`.

> movl 16(%edi), %ecx

As above, copy `fat->sector_size` into `%ecx`.

>L681:
> leal 0(,%ebx,4), %eax

Load `cluster*4` into %eax. Note that this is where the program
lands if `buffered` was equal to what was calculated for
`fatSector` above.

> xorl %edx, %edx

Clear `%edx` for the division.

> divl %ecx

Execute the division. `%edx` is now `offset`.

One may note, at this point, that `%edx` already contained this
value, so why recompute it here? As mentioned above, it was
likely clobbered by the call to `_fatReadLogical`.

> movzbl _buf.8+1(%edx), %eax

Load `buf[offset+1]` into `%eax`.

> movzbl _buf.8(%edx), %ebx

Copy `buf[offset]` into `%ebx`.

> sall $8, %eax

Arithmetic shift left 8 bits `%eax` so that it is
`buf[offset+1]<<8`.

> orl %eax, %ebx

Set `%ebx` to `(buf[offset+1]<<8)|buf[offset]`;

> movzbl _buf.8+2(%edx), %eax

Load `buf[offset+2]` into `%eax`.

> movl %edx, %ecx

Save a copy of `offset` into `%ecx`.

> sall $16, %eax

Set `%eax` to `buf[offset+2]<<16`.

> movb _buf.8+3(%edx), %dl

Load `buf[offset+3]` into `dl`.

> orl %eax, %ebx

Or `buf[offset+2]<<16` into %ebx.

At this point, `%ebx` is set to the value of,
`buf[offset]|(buf[offset+1]<<8)|(buf[offset+2]<<16)`.

> movl %edx, %eax
> sall $24, %eax

This moves `buf[offset+3]<<24` into `%eax`.

> orl %eax, %ebx

Or `buf[offset+3]<<24` into `%ebx`, which is now:

```
(unsigned)buf[offset] |
((unsigned)buf[offset + 1] << 8) |
((unsigned)buf[offset + 2] << 16) |
((unsigned)buf[offset + 3] << 24)
```

> andl $-16, %edx

Recall that `%edx` holds the value of `buf[offset + 3]`; this
clears the low 4-bits of that byte. That is, this sets %edx to
`buf[offset + 3] & 0xF0`. It is used a few instructions down.

> andl $268435455, %ebx

268435455 == 0x0FFFFFFF, so this is `%ebx & 0x0FFFFFFF`. And
now, `%ebx` is equal to:

```
((unsigned)buf[offset] |
((unsigned)buf[offset + 1] << 8) |
((unsigned)buf[offset + 2] << 16) |
((unsigned)buf[offset + 3] << 24)) &
0x0FFFFFFF
```

`%ebx` is now the newly computed value of `cluster`.

> movb $0, _buf.8(%ecx)

This sets `buf[offset]` to 0.

> movb $0, _buf.8+1(%ecx)

Similarly for `buf[offset + 1]`.

> movb $0, _buf.8+2(%ecx)

Similarly for `buf[offset + 2]`.

> movb %dl, _buf.8+3(%ecx)

Recall at this point that `%edx` contains the value of
`buf[offset + 3] & 0xF0`. This copies that byte to back into
`buf[offset + 3]` .

> incl -24(%ebp)

This increments `deletedclusters`.

> jmp L665

And finally jumps back up to the top of the loop.

The code appears to be correct given the source code.

- Dan C.

[muta...@gmail.com]

Thanks Dan.

I posted that at something like 1am hoping for an answer when
I woke up, but there wasn't one so I went ahead and did my own
version (below), which took so long, including interruptions, that
by the time I came to post it I found your message. :-)

I'll just answer some points you raised.

On Sunday, April 2, 2023 at 9:37:28 AM UTC+8, Dan Cross wrote:

> >And the value would have been corrupt anyway because of this:
> >
> > movb _buf.8+3(%edx), %dl

> What do you mean, "corrupt?" `%edx` holds an offset into some
> buffer at the beginning of this instruction; that offset is not
> used after this instruction, so the compiler chose to load a
> byte into the register. That byte is eventually OR'd into the
> "cluster" value in `%ebx`.

Yes, I can't remember why I was confused.

Note that adding printf (which means edx may be trashed)
causes different code to be generated (to avoid edx being
trashed), and I may have remembered that or something.

> Deallocate the stack space allocated above. Note: it would look
> like both `%ebx` and `%edi` could be clobbered here, but I'm
> guessing you're using the "cdecl" calling convention, where
> `%ebi` is callee-save.

Not sure if ebi means ebx or edi, but both are preserved by
the called function I think (edi apparently only in 32-bit code):

/* 1 for registers not available across function calls.
These must include the FIXED_REGISTERS and also any
registers that can be used without being saved.
The latter must include the registers where values are returned
and the register where structure-value addresses are passed.
Aside from that, you can include as many other registers as you like.

The value is an mask - bit 1 is set for call used
for 32bit target, while 2 is set for call used for 64bit.
Proper value is computed in the CONDITIONAL_REGISTER_USAGE.
*/
#define CALL_USED_REGISTERS \
/*ax,dx,cx,bx,si,di,bp,sp,st,st1,st2,st3,st4,st5,st6,st7*/ \
{ 3, 3, 3, 0, 2, 2, 0, 3, 3, 3, 3, 3, 3, 3, 3, 3, \

> If `fat_type` is not 32, branch to the top of the loop. Note
> that there is no case for handling bad data here; even minor
> corruption could lead to an infinite loop.

Sure. I have added minimal error checking. Mainly
just some divide by zero errors that caused me issue
at boot time when it was exposed to a corrupt USB stick.

> The code appears to be correct given the source code.

Yep, thanks.

Since I can't add printf at will without making the problem
go away I will insert a call to __brkpoint() into the generated
assembler to see if that enables me to track the problem down.

BFN. Paul.



_fatNuke:
pushl %ebp
movl %esp, %ebp
pushl %edi
pushl %esi
pushl %ebx
subl $28, %esp
movl 12(%ebp), %ebx ; second parameter is cluster number, now in ebx
testl %ebx, %ebx
movl 8(%ebp), %edi ; first parameter is FAT *, now in edi
movl $0, -16(%ebp)
movl $0, -24(%ebp)
je L663
.p2align 2

L665:
subl $8, %esp
pushl %ebx
pushl %edi
call _fatEndCluster
addl $16, %esp
testl %eax, %eax
jne L689
movl 92(%edi), %eax

cmpl $16, %eax
je L690

cmpl $12, %eax
je L691

L671:
cmpl $32, %eax
jne L665

; fatSector = fat->fatstart + (cluster * 4) / fat->sector_size;
xorl %edx, %edx
leal 0(,%ebx,4), %eax ; cluster number in ebx multiplied by 4
movl 16(%edi), %ecx ; sector_size is at offset 16 in FAT
divl %ecx ; i think edx contains remainder and is ignored, eax contains extra sectors
movl %eax, %esi
addl 48(%edi), %esi ; offset 48 is fatstart, now added to extra sectors, result in esi
cmpl %esi, -16(%ebp) ; stack variable at -16 is "buffered" variable
; if (buffered != fatSector)
je L681 ; if we're still doing the same sector number, then jump

movl -16(%ebp), %eax
testl %eax, %eax
jne L692

L682:
pushl %eax
pushl $_buf.8
pushl %esi
pushl %edi
call _fatReadLogical

movl %esi, -16(%ebp)
addl $16, %esp
movl 16(%edi), %ecx
L681:

leal 0(,%ebx,4), %eax ; cluster number in ebx multiplied by 4, and stored in eax
xorl %edx, %edx
divl %ecx ; repeat the division, and now using the remainder we have the offset in the sector buffer of interest in edx
movzbl _buf.8+1(%edx), %eax ; al has one value
movzbl _buf.8(%edx), %ebx ; bl has one value
sall $8, %eax
orl %eax, %ebx ; bx has combination of 2 values
movzbl _buf.8+2(%edx), %eax ; al has one value
movl %edx, %ecx ; copy of the offset for later use
sall $16, %eax ; get al into position
movb _buf.8+3(%edx), %dl ; dl has one value - rest of edx hasn't been zeroed
orl %eax, %ebx ; al above is now in right position of ebx
movl %edx, %eax ; al has the last (4th) value now (copied from dl)
sall $24, %eax ; al into right position
orl %eax, %ebx ; ebx now complete (cluster number)
; & 0x0fffffff;
andl $-16, %edx ; only want low 4 bits of dl
; buf[offset + 3] = buf[offset + 3] & 0xf0;
andl $268435455, %ebx ; cluster number now masked with 0f ff ff ff
; cluster number in ebx is now correct and undisturbed for use as original
; now using ecx as offset

movb $0, _buf.8(%ecx)
movb $0, _buf.8+1(%ecx)
movb $0, _buf.8+2(%ecx)

movb %dl, _buf.8+3(%ecx) ; those low 4 bits now available for insert
; deletedclusters++;
incl -24(%ebp)
jmp L665
L692:

[muta...@gmail.com]

I spent all of today on it, and it looks like the linker
(ld86), has the wrong address for the relocation,
and it is zapping instructions.

And my call to brkpoint() stopped working because
x'e8' was turned into x'e9', which I believe is a jmp
instead of call.

Spent a lot of time scratching my head (actually,
posting screenshots on discord), trying to find a
sensible return address on the stack.

BFN. Paul.

[Dan Cross]

In article <5ad13902-cf39-4e2c...@googlegroups.com>,

`%ebi` was a typo, but it doesn't matter, as the ABI (that you
appear to be using) says both will be preserved.

>> The code appears to be correct given the source code.
>
>Yep, thanks.
>
>Since I can't add printf at will without making the problem
>go away I will insert a call to __brkpoint() into the generated
>assembler to see if that enables me to track the problem down.

It's not clear what problem you are referring to. You asked
whether there was a compiler bug, but it appears there is not;
at least not in this case.

Generally, it would help if you stated what the actual problem
you need assistance with is.

Instead of adding a call to a builtin function, perhaps
consider inserting an `int3` instruction.

>[snip]

> andl $-16, %edx ; only want low 4 bits of dl
> ; buf[offset + 3] = buf[offset + 3] & 0xf0;

No. This _clears_ the low bits of `%edx`: -16 in two's
complement is 0xFFFFFF00. The assignment is done later.
What this code says is that you only want the _high_ bits
of `%edx`.

>[snip]

> movb %dl, _buf.8+3(%ecx) ; those low 4 bits now available for insert

_high_ bits.

- Dan C.

[muta...@gmail.com]

On Sunday, April 2, 2023 at 9:50:51 PM UTC+8, Dan Cross wrote:

> >Since I can't add printf at will without making the problem
> >go away I will insert a call to __brkpoint() into the generated
> >assembler to see if that enables me to track the problem down.

> It's not clear what problem you are referring to. You asked

Sorry - yeah, the original problem was that I could printfs
into this code, and it wasn't reaching the printfs. ie, it hung.

But adding printfs to try to isolate the problem sometimes
made the problem go away, so I needed to switch to
assembler.

It just occurred to me that I should include a __dummy
function so that when tracking down a problem, I can
replace brkpoint with dummy if that's what is required
to preserve an environment.

> whether there was a compiler bug, but it appears there is not;
> at least not in this case.
>
> Generally, it would help if you stated what the actual problem
> you need assistance with is.

The actual problem was when I did:

copy d:pdos.exe pdos.sys

on my old computer with a BIOS, PDOS hung.

I can't report that problem here. I need to debug it myself.

I had isolated the rough area where the freeze was
occurring. It was beyond the ability to debug with
printf. And I misunderstood the assembler. That
was the help I needed and received, thanks.

Robert has been working on ld86 for hours, trying
to determine what the issue is, and hasn't yet agreed
that the problem isn't with as386 (from the binutils
I provide).

> Instead of adding a call to a builtin function, perhaps
> consider inserting an `int3` instruction.

Good point. I am normally working from C so I put in
a call. It never occurred to me that now that I was
using assembler I could directly code int3.

Note that brkpoint isn't really a builtin function - it's
something I added to pdpclib.

> >[snip]
> > andl $-16, %edx ; only want low 4 bits of dl
> > ; buf[offset + 3] = buf[offset + 3] & 0xf0;

> No. This _clears_ the low bits of `%edx`: -16 in two's
> complement is 0xFFFFFF00. The assignment is done later.
> What this code says is that you only want the _high_ bits
> of `%edx`.
>
> >[snip]
> > movb %dl, _buf.8+3(%ecx) ; those low 4 bits now available for insert
> _high_ bits.

Thanks, fixed.

BFN. Paul.

[muta...@gmail.com]

Note that the actual problem while running this code
(as opposed to "in this code") is that it doesn't hit the
bits where it says "doesn't get here".

And I have determined that the problem is an incorrect
relocation - bbdd (bbfd in the executable after the 32
bytes of header) is using an offset that goes over the
x'e8' call.

Just by chance, the load point of the executable (x'010700')
converted that x'e8' into x'e9', so I believe I got a jmp instead
of a call.

comp5w.bat in sourceforge will reproduce an equivalent
broken executable - just that call won't be there at all, so
something else gets clobbered.

BFN. Paul.


C:\devel\pdos\src\xxx2>doit

C:\devel\pdos\src\xxx2>del pdos.exe

C:\devel\pdos\src\xxx2>ld86 -Map map.txt -N -s -e start -o pdos.exe strt32.o pdos.o os.a pdos.a

C:\devel\pdos\src\xxx2>hexdump pdos.exe 0xbbf0 50
00BBF0 F10FB682 D1100200 0FB69A00 000D21E8 ..............!.
00BC00 F8500000 C1E00809 C30FB682 D2100200 .P..............
00BC10 89D1C1E0 108A92D3 10020009 C389D0C1 ................
00BC20 E018 ..

C:\devel\pdos\src\xxx2>od386 -x pdos.exe | grep bbd
0000bbd4 32 *ABS*
0000bbdd 32 *ABS*

_fatNuke:
pushl %ebp
movl %esp, %ebp
pushl %edi
pushl %esi
pushl %ebx
subl $28, %esp

movl 12(%ebp), %ebx # second parameter is cluster number, now in ebx
testl %ebx, %ebx
movl 8(%ebp), %edi # first parameter is FAT *, now in edi

movl $0, -16(%ebp)
movl $0, -24(%ebp)
je L663
.p2align 2
L665:
subl $8, %esp
pushl %ebx
pushl %edi
call _fatEndCluster
addl $16, %esp
testl %eax, %eax
jne L689
movl 92(%edi), %eax
cmpl $16, %eax
je L690
cmpl $12, %eax
je L691
L671:
cmpl $32, %eax
jne L665

# fatSector = fat->fatstart + (cluster * 4) / fat->sector_size#
xorl %edx, %edx
leal 0(,%ebx,4), %eax # cluster number in ebx multiplied by 4
movl 16(%edi), %ecx # sector_size is at offset 16 in FAT
divl %ecx # i think edx contains remainder and is ignored, eax contains extra sectors
movl %eax, %esi
addl 48(%edi), %esi # offset 48 is fatstart, now added to extra sectors, result in esi
cmpl %esi, -16(%ebp) # stack variable at -16 is "buffered" variable
# if (buffered != fatSector)
je L681 # if we're still doing the same sector number, then jump

movl -16(%ebp), %eax
testl %eax, %eax
jne L692
L682:
pushl %eax
pushl $_buf.8
pushl %esi
pushl %edi
call _fatReadLogical
movl %esi, -16(%ebp)
addl $16, %esp
movl 16(%edi), %ecx
L681:

# gets here
leal 0(,%ebx,4), %eax # cluster number in ebx multiplied by 4, and stored in eax
xorl %edx, %edx
divl %ecx # repeat the division, and now using the remainder we have the offset in the sector buffer of interest in edx
movzbl _buf.8+1(%edx), %eax # al has one value
movzbl _buf.8(%edx), %ebx # bl has one value
# gets here
call ___brkpoint
sall $8, %eax
# doesn't get here
orl %eax, %ebx # bx has combination of 2 values
# doesn't get here
movzbl _buf.8+2(%edx), %eax # al has one value
movl %edx, %ecx # copy of the offset for later use
sall $16, %eax # get al into position
# doesn't get here
movb _buf.8+3(%edx), %dl # dl has one value - rest of edx hasn't been zeroed
orl %eax, %ebx # al above is now in right position of ebx
movl %edx, %eax # al has the last (4th) value now (copied from dl)
sall $24, %eax # al into right position
# doesn't get here
orl %eax, %ebx # ebx now complete (cluster number)
# & 0x0fffffff#
andl $-16, %edx # only want high 4 bits of dl
# buf[offset + 3] = buf[offset + 3] & 0xf0#
andl $268435455, %ebx # cluster number now masked with 0f ff ff ff
# cluster number in ebx is now correct and undisturbed for use as original
# now using ecx as offset
movb $0, _buf.8(%ecx)
# doesn't get here

movb $0, _buf.8+1(%ecx)
movb $0, _buf.8+2(%ecx)

movb %dl, _buf.8+3(%ecx) # those high 4 bits now available for insert
# deletedclusters++#
incl -24(%ebp)
# doesn't get here
jmp L665
L692:

[muta...@gmail.com]

Here is the fix to the linker (ld86):

C:\devel\ld86>git show a53d705d941cf7ef2fb5e5178989715c1226162c
commit a53d705d941cf7ef2fb5e5178989715c1226162c (HEAD -> master, origin/master, origin/HEAD)
Author: Robert Pengelly <roberta...@hotmail.com>
Date: Mon Apr 3 16:48:25 2023 +0100

Also check for if text symbol

diff --git a/aout.c b/aout.c
index c464f6c..b3b088f 100644
--- a/aout.c
+++ b/aout.c
@@ -671,7 +671,7 @@ static int relocate (struct aout_object *object, struct relocation_info *r, int

}

- if (opcode == 0x9A) {
+ if (opcode == 0x9A && symbolnum == 4) {

uint32_t temp = *(int32_t *) ((char *) output + header_size + GET_INT32 (r->r_address));

@@ -730,7 +730,7 @@ static int relocate (struct aout_object *object, struct relocation_info *r, int

}

- if (opcode == 0x9A) {
+ if (opcode == 0x9A && symbolnum == 4) {

int32_t i;


It's not the solution I suggested (provide memory model as
a parameter, don't guess based on opcode).

But my own suggestion hasn't necessarily been thought
through properly, and I'm not sure if I can (and not willing
to spend the effort currently) to try to construct a test
case that demonstrates a problem (and there may not
be a problem), so this is the fix that will remain until I
discover a real problem, rather than a possible problem.

BFN. Paul.