mORMot Open Source

danielkuettner

Member

From: Germany

Registered: 2014-08-06

Posts: 357

Zeos+Batch issue under 64bit

I've found an issue in ZDbcInterbase6Statement.pas line 202:

procedure TZInterbase6PreparedStatement.BindInParameters;
begin
  if (ArrayCount > 0) then --> under 32bit is 4; under 64bit is 0

That raises an Exceptionin ZDbcInterbaseUtils.pas on line 1030:

      stLongWord, stLong, stULong:
        ParamSqlData.UpdateLong(I,
          ClientVarManager.GetAsInteger(InParamValues[i]));

@EgonHugeist
Perhaps you can help us?

ab

Administrator

From: France

Registered: 2010-06-21

Posts: 14,661

Website

Re: Zeos+Batch issue under 64bit

A dynamic array in SynCommons.pas is defined as such:

function TDynArray.GetCount: integer;
begin
  if Value<>nil then
    if fCountP=nil then
      if PtrInt(Value^)<>0 then
        result := PInteger(PtrUInt(Value^)-sizeof(PtrInt))^ else
        result := 0 else
      result := fCountP^ else
    result := 0; // avoid GPF if void
end;

So array length in Win64 is stored as a PtrInt, i.e. Int64.
Whereas Zeos uses:

PLengthInt(NativeUInt(ZArray) - SizeOf(LengthInt))^

And I guess that LengthInt is a longint.

All those low-level access to the dynamic array are IMHO a wrong optimization, which leads to potential GPF issues.
A simple function or method (like in our TDynArray wrapper) is always better, and less error prone.
And with newer versions of Delphi or FPC, declaring them as "inline" is as fast as manual writing.

danielkuettner

Member

From: Germany

Registered: 2014-08-06

Posts: 357

Re: Zeos+Batch issue under 64bit

In ZDbcStatment after line 2126

        AssertLength({%H-}PLengthInt(NativeUInt(ZArray) - SizeOf(LengthInt))^{$IFDEF FPC}+1{$ENDIF}); //FPC returns High() for this pointer location

under 32bit ArrayCount=1
under 64bit ArrayCount=0

LengthInt is LongInt.

Last edited by danielkuettner (2014-09-12 16:21:39)

ab

Administrator

From: France

Registered: 2010-06-21

Posts: 14,661

Website

Re: Zeos+Batch issue under 64bit

LengthInt is LongInt.

... and it should be a Int64, i.e. NativeInt/PtrInt, under Win64.

This is indeed the cause of the issue.

danielkuettner

Member

From: Germany

Registered: 2014-08-06

Posts: 357

Re: Zeos+Batch issue under 64bit

Ok, I understand nothing from that things and feel me a little bit stupid.

But I've tested with Int64 and than Len is 1 (like under 32 bit). But there comes an exception later (something like: arraycount differs from initial count).

I must give before I kill my brain with that.

ab

Administrator

From: France

Registered: 2010-06-21

Posts: 14,661

Website

Re: Zeos+Batch issue under 64bit

Yes, the current whole ZDBC implementation of low-level array access is broken.

But I've notified Michael (aka EgonHugeist), and he is aware of the problem.
I'm quite confident it will be fixed soon!

danielkuettner

Member

From: Germany

Registered: 2014-08-06

Posts: 357

Re: Zeos+Batch issue under 64bit

Why you need PPointer for TDynArray? I've never seen this before.

EgonHugeist

Member

From: Germany

Registered: 2013-02-15

Posts: 190

Re: Zeos+Batch issue under 64bit

public:

Hello Michael

What do you think of http://synopse.info/forum/viewtopic.php … 461#p12461 ?

Hi Arnaud,

Writing from Smartphone now.. So i can't login on your board. It seems to me my Length determination for the DynArrays doesn't fit.
Somewhere in
ZDbcStatement.pas procedure BindArray();

Two possible issues:
1. The SizeOf the Length-Integer did increase to a Int64 an i read in second wrong bitmask.
2. There is another field i don't know about on Position where i read the value.

Now you're the DynArray hero, where is the position of DynArray-Length() located and how is this integer type defined?

IIRC do i read the length by doing something like this:

FInitalArray?Size? := PLengthInt(NativeUInt(Data)-LengthIntOffset)^;
These types are defined in ZCompatibility.pas. Not a problem to add a new type and offset for DynArrays too.

A advice would be welcome and you can have your patch in 3 hours if i'm back on computer.

Btw. All trunk merges synopse need are done!

Michael

.....

@EgonHugeist
Perhaps you can help us?

done: R3303 /testing-7.2 should resolve this problem. Sorry have no 64Bit Delphi to play with by my selves...
Sorry Arnaud friends did visit us so now it are 10 hours

All those low-level access to the dynamic array are IMHO a wrong optimization, which leads to potential GPF issues.
A simple function or method (like in our TDynArray wrapper) is always better, and less error prone.
And with newer versions of Delphi or FPC, declaring them as "inline" is as fast as manual writing.

Than you don't understand my intention for the code in SetDataArry/SetNullArray completely wrong. That's NO low level optimization, Arnaud. We don't have a DynArray helper class like you need for your ORM. On this place i'm just checking the incomming lengths of the arrays to avod overflows later on processing the data! All i have is a POINTER, NO PTypeInfo(where the FPC did bother me on debugging again). So the simplest and minimal code todo that was this idea!

AFAICS do you use such low-level stuff too. DynArray -> FPC returns allways HIGH() not Length for this address. Check this on you side too. 
String-Length determination seems to differ too. Delphi uses a LongInt as RefCount & Length for 32&64Bit, FPC uses the SizeInt instead. The reason why i introduced the PLengthInt-thing.  Check this too for your framework, just a hint..

Cheers, Michael

ab

Administrator

From: France

Registered: 2010-06-21

Posts: 14,661

Website

Re: Zeos+Batch issue under 64bit

done: R3303 /testing-7.2 should resolve this problem. Sorry have no 64Bit Delphi to play with by my selves...
Sorry Arnaud friends did visit us so now it are 10 hours

No problem, you have been very reactive, as usual!

Than you don't understand my intention for the code in SetDataArry/SetNullArray completely wrong. That's NO low level optimization, Arnaud. We don't have a DynArray helper class like you need for your ORM. On this place i'm just checking the incomming lengths of the arrays to avod overflows later on processing the data! All i have is a POINTER, NO PTypeInfo(where the FPC did bother me on debugging again). So the simplest and minimal code todo that was this idea!
AFAICS do you use such low-level stuff too. DynArray -> FPC returns allways HIGH() not Length for this address. Check this on you side too. 
String-Length determination seems to differ too. Delphi uses a LongInt as RefCount & Length for 32&64Bit, FPC uses the SizeInt instead. The reason why i introduced the PLengthInt-thing.  Check this too for your framework, just a hint..

If that is not low-level implementation, why not just one function (taking the pointer as input?), without inlining the complex expression (including the +1 for FPC) in every place of the code?
This was my remark. It sounds like if this range check was "polluting" the code logic.

Thanks for fixing the issue, in all cases!

EgonHugeist

Member

From: Germany

Registered: 2013-02-15

Posts: 190

Re: Zeos+Batch issue under 64bit

If that is not low-level implementation, why not just one function (taking the pointer as input?), without inlining the complex expression (including the +1 for FPC) in every place of the code?

True. But only persits 3x in SetDataArray(). The String-length things are different from that. As you've pointed me in the past: PChar(aString) calls the RTL (since FPC2.7 too) so a check if Pointer(aString)=nil the Result := myEmptyStringPChar else Result := Pointer(aString). So it's not only the missing inline D7 function Length() which was reason for this kind of code...

Edit: simplification of LowLevel code is done R3304 \testing-7.2 (SVN)

Last edited by EgonHugeist (2014-09-12 22:04:42)

ab

Administrator

From: France

Registered: 2010-06-21

Posts: 14,661

Website

Re: Zeos+Batch issue under 64bit

Thanks for the quick fix.

For strings it sounds a bit like premature optimization to use it everywhere, doesn't it?
The bottlenecks are elsewhere, in a rdbms library, IMHO.

In mORMot I first used such patterns then refrain myself to places only where it makes a difference.

danielkuettner

Member

From: Germany

Registered: 2014-08-06

Posts: 357

Re: Zeos+Batch issue under 64bit

I've tested under Win64 and it seems to work.
Thanks a lot @EgonHugeist!

To the discussion between you and ab I've sent a little post under the other topic:
http://synopse.info/forum/viewtopic.php?id=2006