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

[Josef Templ]

> It is clear that is was done intentionally.

Ivan, do you really mean that the TRAP has been built in intentionally?
To me it looks more like a bug if there is a Trap after emitting an error message.

- Josef

[Ivan Denisov]

Ivan, do you really mean that the TRAP has been built in intentionally?

The TRAP never happen in DevCPC486.Convert.

You maybe forgot about one of the main feature of BlackBox. It shows many errors in the code simultaneously.
The fix proposed by you is killing this feature. It stops compilation and BlackBox is not able to show further errors.

At the same time LouWy's fix gives errors to flow up and show them all after single compilation.

So I think that in DevCPC486.Convert this RETURN absence is the element of the software design. And this error DevCPM.err(220) shown only if there is no adjustment in the front-end.

To me it looks more like a bug if there is a Trap after emitting an error message.

Like a bug in the error propagation handling.

[Josef Templ]

The TRAP never happen in DevCPC486.Convert.

And that is why it was so difficult to find the fix.
It took me almost 2 days of hard work to find it.
I will try to explain it in detail.

A TRAP happens for example when continuing code generation with a LONGINT register pair
that uses an undefined second register and if that undefined register happens to be
a large value (the trap is an "illegal memory read"). If it happens to be a value within 0..31
the code generator may generate code that delivers random results
or it may fail at a different place in the compiler.
The TRAP occurs when testing the undefined register for membership in the SET of registers.
Such a test takes the set as a bit stream and does not perform a range check by default
(see docu of range checks).

The reason why there is an undefined second register for a LONGINT register pair
is that DevCPC486.Convert is called even after detecting that a conversion to LONGINT is not possible.
When a conversion is not possible due to the architecture of the machine (i486), an error message is generated
right before calling DevCPC486.Convert by means of DevCPM.err(...).
Any error message, be it from the parser or the code generator, stops the code generator
at the next statement. The current statement, however, is continued to be compiled and
that is where the TRAPs occur.

Conversion to LONGINT is not the only source of a TRAP as I have shown in earlier posts.
All traps that I have observed so far are avoided by the fix.

It is easy to proof that there cannot be any unintended side-effect from this fix:
1. the fix changes only the error behavior from trapping to non-trapping.
2. It does not introduce any new error detection logic.
3. It does not change the code generator for successful SYSTEM.VALs.
4. Whenever there is an invalid SYSTEM.VAL detected (with existing unmodified error detection logic),
code generation skips the invalid conversion and continues with the unconverted operand.
The unconverted operand is always defined because it is the source of a conversion.

- Josef

[Ivan Denisov]

Thanks for you comment. I have read it five times. I am understanding most of it. I will try to understand the all message.

This code of DevCPC486.Convert is intentionally not stopping compilation if error found. It seems that it is general design of DevCPM.
If you look all the module DevCPC486 you will not find a place where DevCPM.err is leading to stop of code generation.
That is why you solution introducing another error reporting logic. There are should be serious reasons to adopt it.
But in fact that we have another solution which is not breaking this logic, it will be strange to adopt your one.
It does not give information about error position. But there possible many conversions in one string. Oberon is not a Python!
Also BlackBox reports many errors after single compilation cycle.

Any error message, be it from the parser or the code generator, stops the code generator at the next statement.

How BlackBox can show many errors at the same time if this proposition is true?

[Robert]

Guys, guys, guys ... I can't follow this in detail (I can't currently afford the time.)

Can we have a top level summary for outside observers to maybe comment on?

I shall attempt that summary here, but this will be incomplete, and possibly very wrong.

1 - There are thee implementations on the table:
-- The existing compiler code
-- The L or "Luowy" suggestion
-- The T or "Temple" suggestion.
2 - People agree that the existing code is unsatisfactory; it TRAPs.
3 - Ivan thinks the L code is fully satisfactory, but the T code is not.
4 - Josef thinks the T code is fully satisfactory, but the L code is not.
5 - The differences concern behaviour and aesthetics.
6 - To help outsiders comment on the behaviour can we have:
-- A simple example of why each of L & T is unsatisfactory, and a reply as to why it is not.

Over to you guys.

(I am sure that these examples are already here, but please repeat them simply to help us disentangle the "wood from the trees".)

[Josef Templ]

Robert, excellent summary from your side.
I will try to summarize the differences and pros and cons of the approaches from my point of view.
Here is again a link to the issue: http://redmine.blackboxframework.org/issues/126, which describes a test case
that is very likely to produce a TRAP.

My approach:
Remove the observed TRAPs when compiling SYSTEM.VAL in the Code Generator (back-end of the compiler).
See the diff at http://redmine.blackboxframework.org/pr ... 3e1d01b11f.

1. + No additional error detection code. Reuses the existing sophisticated error detection logic.
2. + No replication of error detection logic.
3. + Fixes the error where it occurs. It occurs by producing uninitialized variables after detecting an error because code generation for SYSTEM.VAL erroneously continues even in case of detecting that this is not possible.
4. + No break of abstraction layers. This kind of error detection depends on the Intel i486 architecture, which is only known to the back-end (all the modules named DevXXX486). The error checks depend on properties of the available addressing modes and register sets.
5. + No changes in the code generator in case of successful compilation, i.e. existing code is not affected. Trivial to proof even without compiler knowledge. All you need is a basic understanding of a statement sequence, an if statement, and a RETURN (the RETURN could of course be changed to an ELSE as well).
6. + Minimal and trivial change in case of detecting an error. Code generation for SYSTEM.VAL is skipped in order not to produce uninitialized variables. Code generation for the current statement continues with the unconverted operand, which is always initialized. This operand (named x) is only used until terminating the code generation at the next statement.
7. - Uses the default error reporting mechanism of the back-end, leading to an error position at the beginning of the statement and stopping the code generation at the next statement.
   Note: this is not part of my fix but existing code. It is used throughout the back-end at dozens of places.

Luowy/Ivan approach:
Remove the observed TRAPs when compiling SYSTEM.VAL in the Parser (front-end of the compiler).
See http://community.blackboxframework.org/ ... rt=10#p710.

• - Introduces new and non-trivial code for error detection.
• - Replicates error detection logic because the error detection in the back-end still exists.
• - Breaks abstraction layers because it tries in the front-end, which is i486 independent, to avoid i486 specific follow-up problems in the back-end.
• - Does not fix the TRAPs where they occur but works around a bug in the back-end. The bug still exists in the back-end.
• - It is not possible to proof that correct programs are not affected.
• - It is not possible to proof that all related TRAPs are removed.
• + Uses the default error reporting mechanism of the front-end, leading to an error position near the faulty operand and continues parsing.

The list may seem biased, but, honestly, I did my best.
The claimed disadvantage regarding the error position resulting from my fix is certainly true but I don't see any relevance because we are using this error reporting behavior since 30 years without major problems. Such errors are rare in practice. If this error reporting strategy is considered a main problem, it must be changed throughout the whole back-end and this is a change that also affects the front-end. It would be out-of-scope of 1.7 but might be considered for a later release. The only reason for not having error positions inside statements is to keep the size of compiler data structures small. An argument that does not really count on today's computers.

- Josef

[Ivan Denisov]

I did my best.

Good job! I am understanding much better now the benefits. I thought that you are blocking the flow up of the error for further analysis. Now see that I be mistaken about your solution. Front-end have two sides: 1 - parser, 2 - error reporting. So I was thinking that LuoWy fix is about error reporting front-end. Now it is clear, that it is about parser.

Now the question is. "Should we pass to the compiler such wrong sources or we should block this in the parser stage?" Why do you think that it is i486 architecture dependent bug?

[Josef Templ]

Now the question is. "Should we pass to the compiler such wrong sources or we should block this in the parser stage?" Why do you think that it is i486 architecture dependent bug?

Code: Select all

		IF (size >= 0)
			& ((size # x.typ.size) & ((size > 4) OR (x.typ.size > 4))
				OR (f IN {Comp, Real64}) & (x.mode IN {Reg, Stk})
				OR (f = Int64) & (x.mode = Stk)) THEN DevCPM.err(111); RETURN END;

Because the existing error detection logic refers to i486 addressing modes such as Reg and Stk.
It is not possible to simply move the error detection logic into the parser without changing it.
Note that there are no so-called "items" (VAR x: DevCPL486.Item) available in the parser.
The items are intermediate results (describing the location and addressing of operands) that
appear only in the code generator.

- Josef

[Josef Templ]

@Ivan, luowy: why is the error reporting behavior in the context of this issue so important for you?

Searching for "DevCPM.err(" in the back-end yields:
Dev/Mod/CPC486.odc 30
Dev/Mod/CPV486.odc 15

In total, there are 45 places that use the same error reporting behavior as used for SYSTEM.VAL
and the error reporting has not been changed by my fix.

- Josef

[Ivan Denisov]

Josef, I mean if this code is not correct, why we allowing to send it to the compiler?
It will not be correct for any back-end isn't it?

@Ivan, luowy: why is the error reporting behavior in the context of this issue so important for you?
Searching for "DevCPM.

Code: Select all

err(" in the back-end yields:
Dev/Mod/CPC486.odc 30
Dev/Mod/CPV486.odc 15

And never RETURN after DevCPM.err().

For me it is important to understand why you want to choose the fix version with most uncomfortable behavior for user. You gave us some arguments and now I am trying to understand are this arguments correct. You are saying that this bug is dependent on architecture. But does the code it self correct? Is it allowed such construction in the Language Report?

Code: Select all

SYSTEM.VAL(LONGINT, x * 2)

What we see in the PSI

Code: Select all

VAL(T, x)	T, x: any type	T	x interpreted as of type T

Is it allow x to be not a variable or constant but the expression?