issue-#126 Compiler TRAP for SYSTEM.VAL(LONGINT, intreg)

[Josef Templ]

There is another compiler TRAP for a rather exotic case.
Nevertheless it should be fixed.

See the issue at http://redmine.blackboxframework.org/issues/126.

Since it is not possible to re-interpret a single register as a register pair, as required for a LONGINT,
it should lead to a compiler error. Since it did not work at all in the past, it cannot produce
any compatibility problems.

luowy has prepared a proposal for a fix and posted it on the community forum.
See the posting at http://community.blackboxframework.org/ ... rt=10#p710.

I will look at it shortly.

- Josef

[Josef Templ]

Also these tests produce a TRAP, but a different one.

Code: Select all

MODULE  Test2;
  IMPORT SYSTEM;
  VAR x: BYTE; y: REAL;
BEGIN
  x := SYSTEM.VAL(BYTE, y * 2);
END  Test2.

Code: Select all

MODULE  Test3;
  IMPORT SYSTEM;
  VAR x: INTEGER; y: REAL;
BEGIN
  x := SYSTEM.VAL(INTEGER, y * 2);
END  Test3.

It seems that in reality SYSTEM.VAL(T, x) assumes that the argument x is either in memory
or the same register can be used for x and the result.
But this is not checked.

@luowy: the correct place to look at is DevCPV486.expr (Nmop:val).

Interestingly, the related line of code already had a comment (* ??? *).
This indicates that the code has some problems and the author knew about that.

- Josef

[luowy]

it do work;

do you patch it? DevCPB.StPar1

Code: Select all

				IF (x.class = Nconst) & (x.typ = p.typ) THEN	(* ok *)
				ELSIF (x.class >= Nconst) OR ((f IN realSet) # (p.typ.form IN realSet))
						OR (DevCPM.options * {DevCPM.java, DevCPM.allSysVal} # {}) THEN
					IF (p.typ.comp=Basic)&(x.typ.comp=Basic)&(p.typ.size # x.typ.size) & ((p.typ.size > 4) OR (x.typ.size > 4))THEN 
						DevCPM.err(220) (* 220 illegal value of parameter*);
					ELSE
						t := DevCPT.NewNode(Nmop); t.subcl := val; t.left := x; x := t
					END;
				ELSE x.readonly := FALSE; 
					(**)
				END ;
				x.typ := p.typ; p := x
		| movefn: (*SYSTEM.MOVE*)

[Josef Templ]

I will work on a patch.

But not in DevCPB.StPar1 and not in this way.
Sorry, but your proposal is completely wrong.

This is a non-trivial issue because it touches the register management of the compiler.
It will take some time but I am making progress.

- Josef

[Josef Templ]

After a long analysis I found the surprisingly simple solution:

DevCPC486.Convert is called in DevCPV486.expr for a SYSTEM.VAL.
It includes extra checks for SYSTEM.VAL, however, it continues
code generation even in case of finding an error.
This is what leads to the TRAPs.

The correction would be in DevCPC486.Convert:

Code: Select all

				OR (f  = Int64) & (x.mode = Stk)) THEN DevCPM.err(220) END;
		y.mode := Undef; y.form := f; ConvMove(y, x, size >= 0, hint, stop)
	END Convert;

changed to

Code: Select all

				OR (f  = Int64) & (x.mode = Stk)) THEN DevCPM.err(111); RETURN END;
		y.mode := Undef; y.form := f; ConvMove(y, x, size >= 0, hint, stop)
	END Convert;

This stops code generation after an error. It also changes the error number to 111 (operand inapplicable to (this) function)
which I find more appropriate than 220 (illegal value of parameter) because it has nothing to do with the value.
The change is trivial and and it is obvious that it does not introduce any incompatibilities.

See the diffs at http://redmine.blackboxframework.org/pr ... 3e1d01b11f.

- Josef

[Robert]

After a long analysis I found the surprisingly simple solution:

Where is the "(* ?? *)" you mentioned earlier, and should that be removed since this situation has now been thought about?

[Josef Templ]

This comment is in another module (DevCPV486, at the caller). I didn't touch that module.
There are more places with this kind of comment in this other module.

- Josef

[Robert]

I think the comment http://community.blackboxframework.org/ ... 1&start=14 requires a reply.

[Ivan Denisov]

I think the comment http://community.blackboxframework.org/ ... 1&start=14 requires a reply.

I agree with X512. The TRAP should be replaced to warning.

[DGDanforth]

This comment is in another module (DevCPV486, at the caller). I didn't touch that module.
There are more places with this kind of comment in this other module.

- Josef

The comment appears in these framework files

Location
Dev/Mod/Analyzer.odc
?? [2]
Dev/Mod/CPC486.odc
?? [20]
Dev/Mod/CPP.odc
?? [2]
Dev/Mod/CPV486.odc
?? [12]
Dev/Mod/Debug.odc
?? [2]
Dev/Mod/HeapSpy.odc
?? [2]
Dev/Mod/Linker.odc
?? [14]
Dev/Mod/TypeLibs.odc
?? [2]
Host/Mod/Pictures.odc
?? [44]
Host/Mod/Ports.odc
?? [2]
Std/Mod/Debug.odc
?? [2]
Text/Mod/Setters.odc
?? [2]

12 Files found
263 Files searched
14 Dirs searched
0.453 Time (sec)