Wrong code when combining anonymous and nested procedures

Question

I've got some unexpected access violations for Delphi code that I think is correct, but seems to be miscompiled. I can reduce it to

procedure Run(Proc: TProc);
begin
  Proc;
end;

procedure Test;
begin
  Run(
    procedure
    var
      S: PChar;

      procedure Nested;
      begin
        Run(
          procedure
          begin
          end);
        S := 'Hello, world!';
      end;

    begin
      Run(
        procedure
        begin
          S := 'Hello';
        end);
      Nested;
      ShowMessage(S);
    end);
end;

What happens for me is that S := 'Hello, world!' is storing in the wrong location. Because of that, either an access violation is raised, or ShowMessage(S) shows "Hello" (and sometimes, an access violation is raised when freeing the objects used to implement anonymous procedures).

I'm using Delphi XE, all updates installed.

How can I know where this is going to cause problems? I know how to rewrite my code to avoid anonymous procedures, but I have trouble figuring out in precisely which situations they lead to wrong code, so I don't know where to avoid them.

It would be interesting to me to know if this is fixed in later versions of Delphi, but nothing more than interesting, upgrading is not an option at this point.

On QC, the most recent report I can find the similar #91876, but that is resolved in Delphi XE.

Update:

Based on AlexSC's comments, with a slight modification:

...

      procedure Nested;
      begin
        Run(
          procedure
          begin
            S := S;
          end);
        S := 'Hello, world!';
      end;

...

does work.

The generated machine code for

S := 'Hello, world!';

in the failing program is

ScratchForm.pas.44: S := 'Hello, world!';
004BD971 B89CD94B00       mov eax,$004bd99c
004BD976 894524           mov [ebp+$24],eax

whereas the correct version is

ScratchForm.pas.45: S := 'Hello, world!';
004BD981 B8B0D94B00       mov eax,$004bd9b0
004BD986 8B5508           mov edx,[ebp+$08]
004BD989 8B52FC           mov edx,[edx-$04]
004BD98C 89420C           mov [edx+$0c],eax

The generated code in the failing program is not seeing that S has been moved to a compiler-generated class, [ebp+$24] is how outer local variables of nested methods are accessed how local variables are accessed.

Answer 1

Without seeing the whole Assembler Code for the whole (procedure Test) and only assuming on the Snippet you posted, it's probably that on the failing Snippet only a Pointer has been moved where on the correct version there is some Data moved too.

So it seems that S:=S or S:='' causes the Compiler to create a reference by it's own and could even allocate some Memory, which would explain why it works then.

I also assume that's why a Access Violation occurs without S:=S or S:='', because if there is no Memory allocated for the String (remember you only declared S: PChar) then a Access Violation is raised because non-allocated Memory was accessed.

If you simply declare S: String instead, this probably won't happen.

Additions after extended Commenting:

A PChar is only a Pointer to Data Structure, that must exist. Also another common Issue with PChar is to declare local Variables and then passing a PChar to that Variable to other Procs, because what happens is that the local Variable is freed once the routine ends, but the PChar will still point to it, which then raise Access Violations once accessed.

The only possibility that exists per Documentation is declaring something like that const S: PChar = 'Hello, world!' this works because the Compiler can resolve a relative Adresse to it. But this only works for Constants and not for Variables like on the Example above. Doing it like in the Example above needs Storage to be allocated for the string literal to which the PChar then points to like S:String; P:PChar; S:='Hello, world!'; P:=PChar(S); or similar.

If it still fails with declaring String or Integer then perhaps the Variable disappears somewhere along or suddenly isn't visible anymore in a proc, but that would be another Issue that has nothing to do with the existing PChar Issue explained already.

Final Conclusion:

It's possible to do S:PChar; S:='Hello, world!' but the Compiler then simply allocates it as a local or global Constant like const S: PChar = 'Hello, world!' does which is saved into Executable, a second S := 'Hello' then creates another one which is also saved into Executable and so on - but S then only points to the last one allocated, all others are still in the Executable but not accessible any more without knowing the exact Location, because S only points to the last one allocated.

So depending which one was the last S points either to Hello, world! or Hello. On the Example above i can only guess which one was the last and who knows perhaps the Compiler can only guess too and depending on optimizations and other unpredictable Factors S could suddenly point to unallocated Mem instead of the last one by the Time Showmessage(S) is executed which then raises a Access Violation.

Answer 2

How can I know where this is going to cause problems?

It's hard to tell at this point in time.
If we knew the nature of the fix in Delphi XE2 we'd be in a better position.
All you can do is refrain from using anonymous functions.
Delphi has had procedural variables, so the need for anonymous functions ready is not that dire.
See http://www.deltics.co.nz/blog/posts/48.

It would be interesting to me to know if this is fixed in later versions of Delphi

According to @Sertac Akyuz this has been fixed in XE2.

Personally I dislike anonymous methods and have had to ban people from using them in my Java projects because a sizable proportion of our code base was going anonymous (event handlers).
Used in extreme moderation I can see the use case.
But in Delphi where we have procedural variables and nested procedures... Not so much.