x64: Application exit abnormally when exception raised in event

[ccy]

Try run this script in Win64 build that raise exception in event handler:

procedure OnChange(Sender: TObject);
begin
   Abort;
end;

function Execute: string;
var s: TStringList;
begin
  s := TStringList.Create;
  try
    s.OnChange := @OnChange;
    s.Add('test');
  finally
    s.Free;
  end;
end;

The above code works in Win32 build that raise proper EAbort exception.

[martijnlaan]

@pult Any idea what could be wrong? Above code crashes for me as well with the improved x64.inc from #149.

[martijnlaan]

Or would could this be related to #107's fix @velter? Before #107 the code would not run at all, but does the fix include exception support?

[evgeny-k]

pay attention, that testcase is a bit incorrect.

event is declared as of object (i.e. Method Pointers):

  TStringList = class(TStrings)
..
    property OnChange: TNotifyEvent read FOnChange write FOnChange;

TNotifyEvent = procedure(Sender: TObject) of object;

but in testcase, it is used as usual method: procedure OnChange(Sender: TObject);.

check more about differences at https://docwiki.embarcadero.com/RADStudio/Sydney/en/Procedural_Types_%28Delphi%29

[ccy]

@evgeny-k , you are right about TNotifyEvent. However, the sample script works on Win32 build.

Would you suggest a way to assign write TNotifyEvent in the script and assign to TStringList.OnChange?

[evgeny-k]

in Delphi, this code works as expected:

type
  myclass = class
  public
    class procedure OnChange(Sender: TObject);
  end;

class procedure myclass.OnChange(Sender: TObject);
begin
  //ShowMessage('Abort was called');
  Abort;
end;

function Execute: string;
var
  s: TStringList;
begin
  s := TStringList.Create;
  try
    s.OnChange := myclass.OnChange;
    s.Add('test');
    Result := s.Text;
  finally
    s.Free;
  end;
end;

procedure TForm15.Button1Click(Sender: TObject);
begin
  ShowMessage(execute);
end;

so I would start "play" with it.

[martijnlaan]

@evgeny-k The problem appears to be the x64 code of MyAllMethodsHandler: it is not set up to allow exception handling.

This is what happens with the script above:

• MyAllMethodsHandler is called which then calls MyAllMethodsHandler3.
• MyAllMethodsHandler3 catches the EAbort and reraises it at line 12720 of UPSRuntime (raise TObject(FStack);).
• Control then simply returns to MyAllMethodsHandler, which is wrong.
  • It crashes at next //Restore stack, ADD RSP, 40h, much like x64.inc did before @pult's changes were added.
• Instead control should have returned to the try .. except handling of the TPSExternalProcRec(u).ProcPtr call at line 8432 of UPSRuntime

Updating MyAllMethodsHandler to be like x64.inc makes the script work in my test, regardless of your of object obversation.

Do you agree it needs this fix? According to Git you are the author of the x64 code of MyAllMethodsHandler.

[ccy]

@martijnlaan , what is your fix for MyAllMethodsHandler ?

[evgeny-k]

@martijnlaan

Updating MyAllMethodsHandler to be like x64.inc makes the script work in my test, regardless of your of object obversation.

Do you agree it needs this fix? According to Git you are the author of the x64 code of MyAllMethodsHandler.

if you can fix that code, it would be great.

[martijnlaan]

pay attention, that testcase is a bit incorrect.

event is declared as of object (i.e. Method Pointers):

but in testcase, it is used as usual method: procedure OnChange(Sender: TObject);.

Don't think I agree, it's in fact the opposite: the events must be of object, because ROPS (ab)uses Self/Method.Data to store the info it needs to run the event, as setup by GetMethodInfoRec. So the testcase is correct.

Am in the progress of finishing up the MyAllMethodsHandler fix, and also found another problem: in a 64-bit build SupportsRegister should return True for (U)Int64, but it doesn't, which breaks any (U)Int64 parameters used by events.

[martijnlaan]

if you can fix that code, it would be great.

Ok, thanks, please review my 2 commits: a118e3a and cd31a61

[martijnlaan]

By the way, I also checked @pult's fork but was unable to find if it had a solution for this problem. I did notice a comment saying '{$include x64.inc} // can implement "MyAllMethodsHandler"' but was unsure what that means.