Invalid IL code exception

[tenko]

The follow code generates an error:

module TestDate

type
    TDateTime* = longint

(* Convert # of Rata Die Days to proleptic Gregorian calendar Year, Month & Day. *)
proc InverseRataDie(days : integer; var year, month, day : integer)
var
    a, b, c, h, z : integer
begin
    z := days + 306 h := 100*z - 25 a := h div 3652425 b := a - a div 4
    year := (100*b + h) div 36525
    c := b + z - 365*year - year div 4
    month := (5*c + 456) div 153
    day := c - (153*month - 457) div 5
    if month > 12 then
        inc(year)
        dec(month, 12)
    end
end InverseRataDie

(* Decode DateTime to Year, Month & Day *)
proc DecodeDate* (datetime: TDateTime; var year, month, day: integer)
begin
    InverseRataDie(datetime div 86400000, year, month, day)
end DecodeDate

var
  year,month,day : integer
begin
  DecodeDate(86400000, year, month, day)
end TestDate

The code compiles fine and have been tested on other Oberon version. At runtime it fails with the following message:

System.TypeInitializationException: The type initializer for 'TestDate' threw an exception. ---> System.InvalidProgramException: Invalid IL code in TestDate:DecodeDate (long,int&,int&,int&): IL_000f: call      0x06000001

  at TestDate..cctor () [0x00012] in <76014164cb8c4a7daaa157469381efdb>:0 
   --- End of inner exception stack trace ---
  at OBX.Runtime.pcall (OBX.Command cmd, System.Boolean report) [0x00000] in <846778f9d7b048ae8cfecf933b9fa525>:0 

The application finished with code 0

[rochus-keller]

Will have a look at it; dont see the exact reason yet, but it seems again to be a longint vs integer issue; if I change TDateTime to integer the code works; adding a conv.i8 after ldc.i4 86400000 and call int64 [OBX.Runtime]OBX.Runtime::DIV(int64,int64) seems to solve the issue; apparently Mono is very delicate when mixing i4 and i8.

[rochus-keller]

Ok, here is a work-around which seems to solve the issue:

diff --git a/ObxCilGen.cpp b/ObxCilGen.cpp
index 5d89764..c05432a 100644
--- a/ObxCilGen.cpp
+++ b/ObxCilGen.cpp
@@ -2736,7 +2736,8 @@ struct ObxCilGenImp : public AstVisitor
         }else if( td->d_unsafe && tag == Thing::T_Record )
         {
             line(loc).ldobj_(formatType(tf));
-        }
+        }else if( tag == Thing::T_BaseType )
+            convertTo(td->getBaseType(), ta, ea->d_loc);
     }

     void visit( ArgExpr* me )

Will be part of the next commit.

[tenko]

Excelent. The issue is now fixed.

I found another small issue when mixing integer/longint:

(* Increment DateTime with Value according to Type.*)
proc Inc* (var datetime : TDateTime; typ : TType; value : integer)
begin
    case typ of
        Year : 
            datetime := IncYear(datetime, value)
      | Quarter :
            datetime := IncMonth(datetime, 4*value)
      | Month :
            datetime := IncMonth(datetime, value)
      | Week :
            inc(datetime, 7 * value * 86400000)
      | Day :
            inc(datetime, value * 86400000)
      | Hour :
            inc(datetime, value * 3600000)
      | Min :
            inc(datetime, value * 60000)
      | Sec :
            inc(datetime, value * 1000)
      | MSec :
           inc(datetime, value)
    else
        assert(false)
    end
end Inc

The value procedure argument must change to longint otherwise it seems the inc procedure will overflow it's second argument. The first argument for inc is longint (datetime). The second argument should then promote to longint? Note : This seems to happen when value is negative.

[rochus-keller]

I have a work around but it unfortunately causes other test cases to not work anymore; so I need some more time this evening. INC/DEC is rewritten as arithmetic expressions, but unfortunately I cannot simply converty to the known result type of the binary expressions because e.g. literals are all i4, i8, r4 or r8, deviating from the operand types in the AST; needs some tickering.

[rochus-keller]

Ok, I found a solution which works with all test cases so far; commit is up.

[tenko]

The inc problem still persist. I need to wait to be able to print a longint to create a small test case.

[rochus-keller]

use println() which is a built-in utility for test purposes.

here is my test case:

  module Test2
  var
      a : longint
      v : integer
  begin
      a := 12345678
      v := 10
      inc(a, v * 86400000)
      println(a)
  end Test2

[rochus-keller]

The Oakwood Out.Int procedure now accepts LONGINT and there is a new Out.LongReal procedure. Commit is up.

[rochus-keller]

Do you still see the problem?

[tenko]

I condensed it down to a small test:

module Test
var
    a : integer
    b : longint
    c : longint
begin 
    c := 63457286195000
    a := -60
    inc(c, a * 86400000)
    println(c)
    c := 63457286195000
    b := -60
    inc(c, b * 86400000)
    println(c)
end Test

this gives the output:

Starting application...

-1744636104
-6039603400

The application finished with code 0

This should give the same result and the last is correct.

[rochus-keller]

Great, thanks again for your support; I will analyze the generated IL and C code and try to fix it.

[rochus-keller]

Ok, I think I found the issue. Here is a patch which from my point of view solves the issue:

  diff --git a/ObxCilGen.cpp b/ObxCilGen.cpp
  index 95e6be6..4778c86 100644
  @@ -1471,7 +1471,7 @@ struct ObxCilGenImp : public AstVisitor
               line(loc).ldc_i4(val.toInt());
               break;
           case Type::LONGINT:
  -            line(loc).ldc_i8(val.toInt());
  +            line(loc).ldc_i8(val.toLongLong());
               break;
           case Type::BYTE:
               line(loc).ldc_i4(val.toInt());

Note that in inc(c, a * 86400000) the product a * 86400000 is done with integer precision, because both 86400000 and a are integers. You can force longint precision by e.g. by using the L suffix like 86400000L; but this seem also not to work in the present version; will check that and come back.

Here is the second patch for the lexer so the L suffix works:

diff --git a/ObLexer.cpp b/ObLexer.cpp
index aec23a1..5a44a89 100644
@@ -337,9 +337,9 @@ static inline bool checkHexNumber( QByteArray str )
         return true;
 }

-static inline bool checkDecNumber( QByteArray str )
+static inline bool checkDecNumber( QByteArray str, bool oneOff = false )
 {
-    for( int i = 0; i < str.size(); i++ )
+    for( int i = 0; i < (str.size() - (oneOff ? 1 : 0)); i++ )
     {
         if( !::isdigit(str[i]) )
             return false;
@@ -494,7 +494,7 @@ Token Lexer::number()
             return token( Tok_Invalid, off, "literal too large for LONGREAL" );
 #endif
         return tok;
-    }else if( !isHex && !checkDecNumber(str) )
+    }else if( !isHex && !checkDecNumber(str, is32bit || is64bit) )
         return token( Tok_Invalid, off, "invalid decimal integer" );

With these changes I get the following output, which I verified with a calculator (both CilGen and Cgen same result):

Starting application...

63452102195000
63452102195000

The application finished with code 0

Fixes will be included in the next commit.

[rochus-keller]

Can we close this?

[tenko]

I get the result :

Starting application...

63456397162296
63452102195000

The application finished with code 0

This is Release: 0.9.54 Date: 2022-02-06 Platform Windows 10 Pro Build 19044.1466 IDE built with MSYS2 GCC 11.2.0 Mono JIT compiler version 6.12.0 (Visual Studio built mono)

There are some differences between platforms?

[rochus-keller]

I only get this result if I leave out the L suffix and let "a * 86400000" be a 32 bit multiplication.

Have you tried this:

    module Test
    var
        a : integer
        b : longint
        c : longint
    begin 
        c := 63457286195000
        a := -60
        inc(c, a * 86400000L)
        println(c)
        c := 63457286195000
        b := -60
        inc(c, b * 86400000)
        println(c)
    end Test

[tenko]

My error. I was thinking it was supposed to work with the original code unmodified. There will be some kind of -debug mode later which will catch these overflows? Yes. This can be closed.

[rochus-keller]

For the CIL gen I could in principle generate the mul.ovf instead of only mul opcode which issues an exeption in case of overflow. There are also a couple of other overflow checked opcodes which I could use for debugging.