Hello,

It looks like my experiments with returning strings in parameters introduced (or, more likely, triggered an existing) bug.

First, attached is the patch itself. It is pretty trivial, and it works fine with smaller programs. Compiling the compiler itself also works, but only if -O2 is not specified. With -O2 it starts crashing.

Digging in shows that the faulting function is cfileutl.PathExists(), and its assembler code looks this:

// no optimization, code is correct
CFILEUTL_PATHEXISTS$ANSISTRING$BOOLEAN$$BOOLEAN:
push ebp
mov ebp,esp
sub esp,24
mov dword ptr [ebp-4],eax
mov byte ptr [ebp-8],dl
; Initializing two temps
mov dword ptr [ebp-20],0
mov dword ptr [ebp-24],0
cmp dword ptr [ebp-4],0
je @@j954
jmp @@j955

// with -O2
CFILEUTL_PATHEXISTS$ANSISTRING$BOOLEAN$$BOOLEAN:
push ebp
mov ebp,esp
sub esp,20
mov dword ptr [ebp-20],ebx
mov dword ptr [ebp-4],eax
mov bl,dl
; Bug here. The following two instructions should NOT have the same offsets! One of the temps is left uninitialized.
mov dword ptr [ebp-12],0
mov dword ptr [ebp-12],0
test eax,eax
jne @@j955

With unpatched compiler, there is only one temp in this function, so the described behaviour cannot be observed.
I think it's an existing bug because unoptimized code is correct, and the optimizer should never disrupt its correctness. Anyway, I'm a bit lost at this point and need at least a hint where to look.

Best regards,
Sergei

Sergei Gorelkin wrote:

I had isolated the issue, it is maybechangetemp() in ncgld.pas. If I modify this function to always return false, the issue is gone. Actually the failure is caused by incomplete deallocation of the AnsiString temp variable - its initialization and finalization code is not deleted.

...But what the hell is happening? First, at pass1 compiler does a whole load of checks, decides that the assignment of function result cannot be optimized, and allocates an intermediate temp. Then, at pass2 (and with -O2 specified), it optimizes this assignment and tries to delete the temp, based solely on the fact that the right side of assignment is LOC_REFERENCE. All escape analysis from pass1 is void then?

How should this be fixed? Guess I can add a check for Ansi/WideString resultdef before call to maybechangetemp(), but it looks very like spaghetti to me...

Sergei

maybechangetemp() predates the existence of the escape analysis. It's orthogonal to the function result optimization. It's simply that if you have

memloc1:=memloc2

then you can, under certain circumstances (and there's probably an incompatibility between the checks here and your changes -- keep in mind that checks/optimizations are often written with what the compiler can generate in mind, and not based on all possible code sequences that exist; that's also why the assembler optimizer is not run on user-written assembler), actually do something like

@

(i.e., any further reference to memloc1 from then on will actually use memloc2 instead, regardless of whether memloc2 comes from a function result or from something else).

How should this be fixed?

I don't know by heart, and I'll soon leave on holidays until January, 1st, so I won't have time to check during the coming days (but it's possible that.

Jonas

I think I've fixed this in r9626, could you check again? At least on darwin/ppc, cycle and testsuite seem to work fine with your patch (but I didn't check it without my changes as well).

Jonas

I got the testsuite perform fine with TEST_OPT=-O2.

However, I could not build a working optimized compiler, neither could I get cycle working. It builds ppc1 and ppc2, but after the 'make FPC=.../ppc2 OLDFPC=.../ppc1 next' command, on Windows it hangs for several minutes and then presumably gets killed by timeout ('Interrupt/exception 0xc00000fd', I observe such behavior for the first time), and on Linux it reports Internal Error 2004121202.

But it seems to me that this crap is not related to the issue, but is caused by my environment. I'll revert everything, restart in a clean way, and then report more.

Sergei

Sergei Gorelkin wrote

I'll revert everything, restart in a clean way, and then report more.

Looks like everything is fine now, I got cycle working and an optimized compiler builds, too.
So, my patch for returning strings in parameters (Delphi-compatible) is also complete and probably may be applied at your will. I attach its final version.

Sergei

Thanks, committed.

Jonas