Skip to content

Fix several code generation and optimizer bugs#21

Open
sheumann wants to merge 16 commits into
byteworksinc:masterfrom
sheumann:bad-code-fixes
Open

Fix several code generation and optimizer bugs#21
sheumann wants to merge 16 commits into
byteworksinc:masterfrom
sheumann:bad-code-fixes

Conversation

@sheumann

@sheumann sheumann commented Mar 30, 2025

Copy link
Copy Markdown

This fixes several bugs in the code generator and optimizer that could cause incorrect code to be generated in various situations. Most of these correspond to bugs that were already fixed in ORCA/C. The details of each bug and the correspondence to ORCA/C bug fixes are described in the individual commits.

sheumann added 16 commits March 12, 2025 19:12
@sheumann
Fix bug causing wrong code generation for single-byte indirect local …
a4973f4 
…stores beyond the first 256 bytes of the stack frame.

This corresponds to byteworksinc/ORCA-C@6bd0bef.

Here is an example affected by this:

{$optimize -1}
program compca19simp(output);

procedure testproc(idx: integer);
var
        str1, str2: packed array[0..300] of char;
begin
        str1[idx] := 'x';
        writeln(str1[idx]);
end;

begin
testproc(145);
end.
@sheumann
Fix a problem where a conversion to a smaller type could be improperl…
17a7a80 
…y omitted when followed by a conversion to a larger type.

This also fixes a logic error that may have permitted other conversions to be improperly omitted in some cases.

This corresponds to byteworksinc/ORCA-C@cc75a9b.

Here is an example affected by this (using ord on an out-of-range value might be considered undefined behavior, but in practice I think this should work when range checking is off):

{$optimize 1}
program convbug(output);

var
        l: longint;

begin
        l := 123456;
        l := ord4(ord(l+1));
        writeln(l);
end.
@sheumann
Fix bug where comparing 32-bit values in global arrays or structs aga…
2ae5e7f 
…inst 0 may give wrong results with large memory model.

The issue was that 16-bit absolute addressing (in the data bank) was being used to access the data to compare, but with the large memory model the global arrays or structs are not necessarily in the same bank, so absolute long addressing should be used.

This corresponds to byteworksinc/ORCA-C@709f9b3.

The following program often demonstrates the problem (depending on memory layout and contents):

{$memorymodel 1}
{$optimize 1}
program largemmbug(output);

var
        ch1: packed array[1..32000] of char;
        L1: array[0..1] of longint;

begin
L1[0] := 0;
if L1[0] <> 0 then
        writeln('bug detected'); {depends on memory layout and contents}
end.
@sheumann
Fix bug causing functions with 254 bytes of locals and parameters to …
4849214 
…crash on return.

This bug occurred because the generated code tried to store part of the return address to a direct page offset of 256, but instead an offset of 0 was used, resulting in an invalid return address and (typically) a crash. It could occur if the function took one or more parameters, and the total size of parameters and local variables (including compiler-generated ones) was 254 bytes.

This corresponds to byteworksinc/ORCA-C@fb612e1.

The following program demonstrates the problem:

program biglocals1;

var
        x: integer;

function testfn(a: integer; b: longint): integer;
var
        s: packed array[0..245] of char;
begin
testfn := a;
end;

begin
x := testfn(1,2);
end.
@sheumann
Fix optimizer bug that could limit certain address calculations to a …
760596c 
…32k or 64k range even when using the large memory model.

This optimization could apply when indexing into an array whose elements are a power-of-2 size using a 16-bit index value. It is now only used when addressing arrays on the stack (which are necessarily smaller than 64k).

This corresponds to byteworksinc/ORCA-C@730544a.

The following program demonstrates the problem:

{$optimize 1}
{$memorymodel 1}
program longindex(output);

var
        c: array[0..40000] of longint;
        i: integer;

begin
        i := 30000;
        c[30000] := 3;
        writeln(c[i]); {should print 3}
end.
@sheumann
Fix issue where certain address computations could be improperly rest…
28fad8d 
…ricted to a 32K or 64K range (even when using the large memory model).

This could occur with computations where multiple variables were added to a pointer.

This corresponds to byteworksinc/ORCA-C@df42ce2.

The following program is an example that was miscompiled:

{$optimize 1}
{$memorymodel 1}
program addrarith1(output);

var
        c: array[0..80000] of byte;

function testfn: integer;
var
        i,j,k: 0..maxint;
        p: ^byte;
begin
        i := 25000;
        j := 25000;
        k := 25000;
        c[75000] := 3;
        p := pointer(ord4(@c[0]) + i + j + k);
        testfn := p^;
end;

begin
writeln(testfn);
end.
@sheumann
Generate function exit cop with profiling / {$debug 2}. Expected by p…
35daee2 
…rizm.

This corresponds to byteworksinc/ORCA-C@dbfa542.

Here is an example program affected by this (the prizm profiler would misattribute the bulk of the run time to testproc):

{$debug 2}
program profile;

var
        i,j,k: integer;

procedure testproc;
begin
end;

begin
testproc;
for i := 0 to maxint do begin
        for j := 1 to 100 do begin
                k := j - i;
        end;
end;
end.
@sheumann
Fix mis-evaluation of certain equality comparisons with intermediate …
95d6f04 
…code optimization.

This affected comparisons of the form "ord(logical operation or comparison) = constant other than 0 or 1". These should always evaluate to false, but could mis-evaluate to true due to the bad optimization.

This corresponds to byteworksinc/ORCA-C@db98f78.

The following program gives an example showing the problem:

{$optimize 1}
program logicalcmp(output);

var
        i,j: boolean;

begin
i := false;
j := true;
writeln(ord(i or j) = 123);
end.
@sheumann
Fix bug causing functions with 254 bytes of locals to crash on return.
fd14754 
This was a bug with the code for moving the return address. It would generate a "LDA 0" instruction when it was trying to load the value at DP+256.

This corresponds to byteworksinc/ORCA-C@29de867.

The following program demonstrates the crash:

program local254;

procedure testproc(x: integer);
var
        a: array[0..253] of byte;
begin
end;

begin
testproc(1);
end.
@sheumann
Fix code generation for certain cases where addresses are stored to l…
517dddc 
…ocal variables that don't fit in the direct page.

There was a bug when storing addresses generated by expressions like @A[i], where a is a global array and i is a variable. In certain cases where the destination location was a local variable that didn't fit in the direct page, the result of the address calculation would be stored to the wrong location on the stack. This failed to give the correct result, and could also sometimes cause crashes or other problems due to stack corruption.

This corresponds to byteworksinc/ORCA-C@fd0ff03, but the code change is simplified from what was used there. (The ORCA-C patch includes code to handle pc_cop intermediate codes, but that should never actually be needed.)

The following program illustrates the issues:

{$optimize 1}
program storeaddr(output);

var
        a: array [0..4] of longint;
        idx: integer;

function testfn: longint;
var
        b: array [0..70] of ^longint;
begin
        b[70] := @A[idx];
        testfn := b[70]^;
end;

begin
idx := 4;
a[4] := 123;
writeln(testfn); {should print 123}
end.
@sheumann
Do not eliminate expressions with side effects in "exp | (-1)" or "ex…
e39c973 
…p & 0".

This was previously happening in intermediate code peephole optimization.

This corresponds to byteworksinc/ORCA-C@8078675.

The following example program demonstrates the problem:

{$optimize 1}
program fnbitconst(output);

var
        x: integer;
        y: longint;

function f: integer;
begin
        writeln('in f');
        f := 123;
end;

function g: longint;
begin
        writeln('in g');
        g := 123;
end;

begin
x := f & 0;
x := f | (-1);
y := g & 0;
y := g | (-1);
end.
@sheumann
Fix code generated by real negation optimization.
ebc8082 
When negating a comp variable and storing the result back to the same location, this optimization could generate code this did not work and would trash memory.

When negating a real/double/extended variable and storing the result back to the same location, the generated code would normally work, but it would read and write a byte beyond the value being modified. This could theoretically cause problems in some cases involving concurrency.

With regard to real/double/extended, this corresponds to byteworksinc/ORCA-C@cf9add4, but ORCA/C did not generate the incorrect code for negating comp variables.

Here is an example that shows the problem with negating a comp variable:

{$optimize 1}
program negcomp(output);

var
        x: comp;

begin
x := 123456789.0;
x := -x;
writeln(x);
end.
@sheumann
Fix bug in some cases where a byte value is loaded and then stored as…
f7fa1f0 
… a word.

It could wind up storing garbage in the upper 8 bits of the destination, because it was not doing a proper 8-bit to 16-bit conversion.

This corresponds to byteworksinc/ORCA-C@d72c0fb (with a logic error from the ORCA/C patch fixed).

Here is an example that could exhibit the bug:

{$optimize 1}
program bytetoword(output);

procedure testproc;
var
        k: array[0..1] of integer;
        i: integer;
        b,c: byte;
begin
        i := 0;
        b := 123;
        c := 42;
        k[i] := c;
        writeln(k[0]);
end;

begin
testproc;
end.
@sheumann
Stop capitalizing file names in debug information.
8b7ae89 
This does not seem to be necessary for any of the debuggers (at least in their latest versions), and it obviously causes problems with case-sensitive filesystems.

This corresponds to byteworksinc/ORCA-C@5ac79ff.
@sheumann
Fix evaluation of conditionals involving shifts.
48bc937 
The code for 16-bit shifts was assumed to set the z flag based on the result value, but did not actually do so. This could result in mis-evaluation of conditionals.

This corresponds to byteworksinc/ORCA-C@7898c61 with regard to shifts. (The ORCA/C patch also fixed cases involving bitfields or the ?: operator, which do not apply to Pascal.)

Here is an example showing the problem:

program shiftcond(output);

var
        i,j: integer;

begin
i := 1;
j := 0;
if (i >> j) <> 0 then
        writeln('OK');
end.
@sheumann
Fix bug with 4-byte comparisons against globals in large memory model.
38519a6 
Long addressing was not being used to access the values, which could lead to mis-evaluation of comparisons against values in global records or arrays, depending on the memory layout.

This corresponds to byteworksinc/ORCA-C@2550081.

Here is an example that can be affected by this (depending on memory layout):

{$memorymodel 1}
program globalcmp(output);

type
        recPtr = ^theRec;
        theRec = record
                p: recPtr;
        end;

var
        s: theRec;

begin
        s.p := @s;
        writeln(s.p <> @s); {should be false}
end.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

1 participant

@sheumann