Page 1 of 2

issue-#204 SYSTEM.BIT bug

Posted: Thu Oct 31, 2019 10:42 am
by luowy
according to Intel manual, then BT instruction only accept 0..31 immediate offset and range -2^31 to 2^31 - 1 for a register offset,but SYSTEM.BIT seems output wrong code;

Code: Select all

MODULE TestSystemBit;

	IMPORT SYSTEM;
 
	PROCEDURE Do();
		VAR b:BOOLEAN; x,y:INTEGER;
	BEGIN
		b:=SYSTEM.BIT(x,31);
		b:=SYSTEM.BIT(x,200); (*wrong here *)
		b :=SYSTEM.BIT(x,y);    (* seem wrong here *)
	END Do;



END TestSystemBit.
decode

Code: Select all

PROCEDURE Do
00000001H:	55 	push  ebp
00000002H:	8B EC 	mov  ebp, esp
00000004H:	83 C4 F4 	add  esp, -12

00000007H:	8B 45 F8 	mov  eax, -8[ebp]       ;;b:=SYSTEM.BIT(x,31);
0000000AH:	0F BA 20 1F 	bt  0[eax], 31
0000000EH:	0F 92 C0 	setb/setc/setnae  al
00000011H:	88 45 FF 	mov  -1[ebp], al

00000014H:	8B 45 F8 	mov  eax, -8[ebp]        ;;b:=SYSTEM.BIT(x,200);
00000017H:	0F BA 20 C8 	bt  0[eax], -56     ;;wrong here,
                                              ;;correct: MOV EDX, 200;MOV EAX, [EBP-8];BT[EAX], EDX
0000001BH:	0F 92 C0 	setb/setc/setnae  al
0000001EH:	88 45 FF 	mov  -1[ebp], al

00000021H:	8B 45 F8 	mov  eax, -8[ebp]        ;;b :=SYSTEM.BIT(x,y);
00000024H:	8B 55 F4 	mov  edx, -12[ebp]
00000027H:	0F A3 10 	bt  edx, 0[eax]            ;;seem should be:  BT [EAX], EDX
0000002AH:	0F 92 C0 	setb/setc/setnae  al
0000002DH:	88 45 FF 	mov  -1[ebp], al

00000030H:	8B E5 	mov  esp, ebp
00000032H:	5D 	pop  ebp
00000033H:	C3 	ret  
comments welcome;

luowy

Re: SYSTEM.BIT bug

Posted: Fri Nov 01, 2019 8:53 am
by Josef Templ
SYSTEM.BIT is undocumented and unused in BB.
It is possible that it has been introduced for compatibility with some ETH Oberon version
but later the standard function BITS has been added and SYSTEM.BIT became unnecessary.

My tests show correct behavior of SYSTEM.BIT but you must be aware that
it expects a memory address as its first argument.

> b :=SYSTEM.BIT(x,y); (* seem wrong here *)

It looks very much like a problem in the decoder. The operands are swapped.

- Josef

Re: SYSTEM.BIT bug

Posted: Fri Nov 01, 2019 2:22 pm
by luowy
Josef Templ wrote:> b :=SYSTEM.BIT(x,y); (* seem wrong here *)

It looks very much like a problem in the decoder. The operands are swapped.
yes,this is a decode problem:

Code: Select all

0F A3 10 	bt  edx, 0[eax]
the above is wrong, it should be:

Code: Select all

  0F A3 10 	BT	[EAX], EDX  
luowy

Re: SYSTEM.BIT bug

Posted: Sat Nov 02, 2019 1:58 am
by luowy
a demo for using SYSTEM.BIT, actually, it is different from BITS;

Code: Select all

MODULE ObxSystemBit;

	IMPORT Log := StdLog, S := SYSTEM;
	
	(* memory manager,  *)
	CONST MaxPage = 512 - 1;    
	VAR bitmap: ARRAY (MaxPage + 1) DIV 32 OF SET; (* 16  *)
	
	PROCEDURE Do();
		VAR b: BOOLEAN; x, y: INTEGER;
	BEGIN
		b := S.BIT(x, 31);   (* ok *)
		b := S.BIT(x, 200); (*wrong here *)
		b := S.BIT(x, y);     (* ok *)
	END Do;
	
	
	PROCEDURE Init();
		VAR i: INTEGER;
	BEGIN
		FOR i := 0 TO LEN(bitmap) - 1 DO 
			bitmap[i] := {};
		END;
	END Init;
	
	PROCEDURE Dump();
		VAR i: INTEGER;
	BEGIN
		FOR i := 0 TO LEN(bitmap) - 1 DO 
			Log.Set(bitmap[i]); Log.Ln;
		END;
	END Dump;
	
		
	PROCEDURE DumpDirty();
		VAR i: INTEGER;
	BEGIN
		FOR i := 0 TO MaxPage DO 
			IF i MOD 32 IN bitmap[i DIV 32] THEN 
				Log.Int(i); Log.Ln;
			END;
		END;
	END DumpDirty;
	
	PROCEDURE DumpDirty2();
		VAR i: INTEGER;
	BEGIN
		FOR i := 0 TO MaxPage DO 
			IF S.BIT(S.ADR(bitmap), i) THEN 
				Log.Int(i); Log.Ln;
			END;
		END;
	END DumpDirty2;
	
	PROCEDURE SetDirty (num: INTEGER);
	BEGIN
		ASSERT((num >= 0)&(num <= MaxPage), 20);
		INCL(bitmap[num DIV 32], num MOD 32);
	END SetDirty;
	
	PROCEDURE Do2* (); (* ObxSystemBit.Do2 *)
		VAR i, x: INTEGER;
	BEGIN
		Init();
		SetDirty(31);
		SetDirty(200);
		SetDirty(511);
		Dump(); Log.Ln;
		DumpDirty();Log.Ln;
		DumpDirty2();
	END Do2;




END ObxSystemBit.

Re: SYSTEM.BIT bug

Posted: Sat Nov 02, 2019 2:11 pm
by Josef Templ
in the decoder in PROCEDURE Bit I thnk it is the wrong d value, which sets the output order.
When I use d = 0 it works as expected, with d = 1 it is swapped.
The question is now if this applies to all of the instructions in that group.
Probably, but should be checked.

- Josef

Re: SYSTEM.BIT bug

Posted: Thu Nov 21, 2019 2:12 pm
by luowy
I have checked all the group, a patch here;

Code: Select all

		PROCEDURE Bit (op: INTEGER);
			VAR reg, base, inx, d: INTEGER; scale, mode: SHORTINT; disp: INTEGER;
		BEGIN
			w := 1;
			ModRm(mode, reg, base, inx, scale, disp);
			
			d := 0;
			IF op = 0A3H THEN
				WriteOp("bt"); IF mode = Reg THEN d := 1; END;
			ELSIF op = 0ABH THEN
				WriteOp("bts"); IF mode = Reg THEN d := 1; END;
			ELSIF op = 0B3H THEN
				WriteOp("btr"); IF mode = Reg THEN d := 1; END;
			ELSIF op = 0BBH THEN
				WriteOp("btc"); IF mode = Reg THEN d := 1; END;
			ELSIF op = 0BCH THEN
				WriteOp("bsf");  IF mode # Reg THEN d := 1; END;
			ELSE
				WriteOp("bsr");  IF mode # Reg THEN d := 1; END;
			END;
			IF mode = Reg THEN WriteRM(Reg, d, base, reg, none, 0, 0, 0, FALSE)
			ELSE WriteRM(mode, d, reg, base, inx, scale, disp, 0, FALSE)
			END
		END Bit; 
luo

Re: SYSTEM.BIT bug

Posted: Thu Nov 21, 2019 7:36 pm
by Josef Templ
Thanks for checking this out.
I do hope that I will find some time to include this fix soon.

- Josef

Re: SYSTEM.BIT bug

Posted: Thu Nov 21, 2019 9:26 pm
by Robert
In simple terms, what does SYSTEM.BIT do, and why is this different to BITS?

Re: SYSTEM.BIT bug

Posted: Fri Nov 22, 2019 4:00 am
by luowy
BITS is type converter procedure,
SYSTEM.BIT is a bit test procedure:

Code: Select all

PROCEDURE BIT(adr:INTEGER,offset:INTEGER):BOOLEAN

BIT is most similar to the IN operator:

Code: Select all

VAR b:BOOLEAN; set:SET; 
  b := 2 IN set;
  (*same as*)
  b := SYSTEM.BIT(SYSTEM.ADR(set),2);  
BIT is useful to embeded program, You can find it in some oberon-07 code;

luowy

Re: SYSTEM.BIT bug

Posted: Sat Nov 23, 2019 10:53 am
by Josef Templ
@luowy:
in your proposal the order of operands (d) is changed if mode=Reg.
After the IF ... END (in the old code) the order of operands is swapped again when writing output because there is another
test for mode = Reg.

My question is if it would be possible to remove the second test and to always use
WriteRM(mode, d, reg, base, inx, scale, disp, 0, FALSE)
with the appropriate d = 0 or d = 1 for defining the order of operands.

The resulting procudure would look like this

Code: Select all

		PROCEDURE Bit (op: INTEGER);
			VAR reg, base, inx, d: INTEGER; scale, mode: SHORTINT; disp: INTEGER;
		BEGIN
			w := 1; d := 0;
			ModRm(mode, reg, base, inx, scale, disp);
			IF op = 0A3H THEN WriteOp("bt")
			ELSIF op = 0ABH THEN WriteOp("bts")
			ELSIF op = 0B3H THEN WriteOp("btr")
			ELSIF op = 0BBH THEN WriteOp("btc")
			ELSIF op = 0BCH THEN WriteOp("bsf"); d := 1
			ELSIF op = 0BDH THEN WriteOp("bsr"); d := 1
			ELSE Error
			END ;
			WriteRM(mode, d, reg, base, inx, scale, disp, 0, FALSE)
		END Bit;
- Josef