Recent

Author Topic: Range Check Error in SynEditMarks.CompareSynEditMarks  (Read 4905 times)

Pascal

  • Hero Member
  • *****
  • Posts: 831
Range Check Error in SynEditMarks.CompareSynEditMarks
« on: June 26, 2017, 09:38:43 am »
In a 64bit executable with memory consumption above 4 GB a range check error is thrown at the last line of:
Code: Pascal  [Select]
  1. function CompareSynEditMarks(Mark1, Mark2: Pointer): Integer;
  2. var
  3.   m1: TSynEditMark absolute Mark1;
  4.   m2: TSynEditMark absolute Mark2;
  5. begin
  6.   .
  7.   .
  8.   .
  9.   Result := PtrInt(m2) - PtrInt(m1);  
  10. end;
  11.  

Values are:
Mark1: $0000000087406FD0
Mark2: $0000000138C98910

PtrInt is Int64. Isn't Integer also 64 bit? So why the range check error here?
laz trunk - fpc trunk 32bit - Windows 10 Pro x64 (1803)

molly

  • Hero Member
  • *****
  • Posts: 2345
Re: Range Check Error in SynEditMarks.CompareSynEditMarks
« Reply #1 on: June 26, 2017, 09:59:35 am »
PtrInt is Int64. Isn't Integer also 64 bit? So why the range check error here?
Not even by a longshot. For details See ordinal types:

Quote
The integer type maps to the smallint type in the default Free Pascal mode. It maps to either a longint in either Delphi or ObjFPC mode. The cardinal type is currently always mapped to the longword type.

Thaddy

  • Hero Member
  • *****
  • Posts: 8673
Re: Range Check Error in SynEditMarks.CompareSynEditMarks
« Reply #2 on: June 26, 2017, 10:12:36 am »
Furthermore pointers are always unsigned types. Treating them as integers is more or less legacy that refuses to go away..

Anyway, why such cumbersome code if you can do this:
Code: Pascal  [Select]
  1. program Project1;
  2. {$Pointermath On}
  3. function CompareSynEditMarks(Mark1, Mark2: Pointer): NativeInt;
  4. begin
  5.   Result := mark2 - mark1;  
  6. end;
  7.  
  8. begin
  9. end.
  10.  

This will give a proper result, without range problems on both 32 and 64 bit.
« Last Edit: June 26, 2017, 10:20:08 am by Thaddy »
Most people that want to use threading should learn to patch their jeans first: use a needle.

molly

  • Hero Member
  • *****
  • Posts: 2345
Re: Range Check Error in SynEditMarks.CompareSynEditMarks
« Reply #3 on: June 26, 2017, 10:18:48 am »
To further complement Thaddy's remark, see ptrInt documentation:
Quote
Ptrint is a signed integer type which has always the same size as a pointer. Ptrint is considered harmfull and should almost never be used in actual code, because pointers are normally unsigned.

also note:
Quote
PtrInt - Signed integer type with same size as Pointer.
That does not necessarily means 64 bit. Therefor the size depends on the target the code was compiled for.

Thaddy

  • Hero Member
  • *****
  • Posts: 8673
Re: Range Check Error in SynEditMarks.CompareSynEditMarks
« Reply #4 on: June 26, 2017, 10:20:59 am »
molly, sorry, posts crossed again. I have added example to handle it correctly.
Most people that want to use threading should learn to patch their jeans first: use a needle.

molly

  • Hero Member
  • *****
  • Posts: 2345
Re: Range Check Error in SynEditMarks.CompareSynEditMarks
« Reply #5 on: June 26, 2017, 10:22:20 am »
No problem, also sorry from my end.

Pascal

  • Hero Member
  • *****
  • Posts: 831
Re: Range Check Error in SynEditMarks.CompareSynEditMarks
« Reply #6 on: June 26, 2017, 10:30:33 am »
For details See ordinal types:
I did and it says:
Quote
Every platform has a ”native” integer size, depending on whether the platform is 8-bit, 16-bit, 32-bit or 64-bit. e.g. On AVR this is 8-bit.

So function Result should be SizeInt here and not Integer, right?

And also: Test for 64bit should be done with 4GB of used memory to detect things like this.
laz trunk - fpc trunk 32bit - Windows 10 Pro x64 (1803)

Thaddy

  • Hero Member
  • *****
  • Posts: 8673
Re: Range Check Error in SynEditMarks.CompareSynEditMarks
« Reply #7 on: June 26, 2017, 10:33:32 am »
For details See ordinal types:
I did and it says:
Quote
Every platform has a ”native” integer size, depending on whether the platform is 8-bit, 16-bit, 32-bit or 64-bit. e.g. On AVR this is 8-bit.

So function Result should be SizeInt here and not Integer, right?

And also: Test for 64bit should be done with 4GB of used memory to detect things like this.
The result should be NativeInt. As per my example.

No you don't need to test this with 4 GB of actual memory.
Just use my code and it will work on any (well, actually, most, there are some exotic exceptions) platform.

Explanation of my code:
Pointermath allows you to do calculations directly on pointers. This is also Delphi compatible, btw.
That means that the calculation is always correct for the pointer size of any platform.
Since on most platforms a pointer maps to a NativeUINT, but we want to allow for negative difference, we use a NativeINT.
This is safe because this particular calculation can never exceed range, So that code will work automatically correctly on most platforms.
That means that at least for 32 bit and 64 bit platforms the function will always return a valid value within range, regardless of the platform.
« Last Edit: June 26, 2017, 10:43:57 am by Thaddy »
Most people that want to use threading should learn to patch their jeans first: use a needle.

molly

  • Hero Member
  • *****
  • Posts: 2345
Re: Range Check Error in SynEditMarks.CompareSynEditMarks
« Reply #8 on: June 26, 2017, 10:40:00 am »
For details See ordinal types:
I did and it says:
Quote
Every platform has a ”native” integer size, depending on whether the platform is 8-bit, 16-bit, 32-bit or 64-bit. e.g. On AVR this is 8-bit.
You can check the actual size of the type integer with a simple check:
Code: [Select]
  WriteLn(SizeOf(integer));
It will write out the actual number of bytes that the integer type occupies.

Quote
So function Result should be SizeInt here and not Integer, right?
Just like Thaddy i would also prefer/opt for NativeInt, although in theory you could perhaps also use SizeInt. Tbh i do not know all targets from heart, so that might pose a problem for some of them.

Pascal

  • Hero Member
  • *****
  • Posts: 831
Re: Range Check Error in SynEditMarks.CompareSynEditMarks
« Reply #9 on: June 26, 2017, 10:41:17 am »
Sorry, didn't see new posts while replying.

This code is not mine it's SynEdit's code.

By the way: I haven't seen NativeInt before. But makes sense if Integer has not the same size as pointers.

The code could also be changed to:
Code: Pascal  [Select]
  1.   if Mark2 > Mark1 then
  2.     Result := 1
  3.   else if Mark1 > Mark2 then
  4.     Result := -1
  5.   else
  6.     Result := 0;
  7.  

... as this is a compare method.
laz trunk - fpc trunk 32bit - Windows 10 Pro x64 (1803)

Pascal

  • Hero Member
  • *****
  • Posts: 831
Re: Range Check Error in SynEditMarks.CompareSynEditMarks
« Reply #10 on: June 26, 2017, 10:46:34 am »
No you don't need to test this with 4 GB of actual memory.
Just use my code and it will work on any (well, actually, most, there are some exotic exceptions) platform.

For sure this works, but to find errors like these memory above 4GB would have helped here, as this is SynEdit code  :D
laz trunk - fpc trunk 32bit - Windows 10 Pro x64 (1803)

Thaddy

  • Hero Member
  • *****
  • Posts: 8673
Re: Range Check Error in SynEditMarks.CompareSynEditMarks
« Reply #11 on: June 26, 2017, 10:51:00 am »
Yes.
Code: Pascal  [Select]
  1. program Project1;
  2. uses math;
  3. {$Pointermath On}
  4. function CompareSynEditMarks(Mark1, Mark2: Pointer): TValueSign;
  5. begin
  6.   Result := Sign(mark2 - mark1);  
  7. end;
  8. begin
  9. end.
Is another option.
Most people that want to use threading should learn to patch their jeans first: use a needle.

Pascal

  • Hero Member
  • *****
  • Posts: 831
Re: Range Check Error in SynEditMarks.CompareSynEditMarks
« Reply #12 on: June 26, 2017, 10:51:51 am »
Quote
So function Result should be SizeInt here and not Integer, right?
Just like Thaddy i would also prefer/opt for NativeInt, although in theory you could perhaps also use SizeInt. Tbh i do not know all targets from heart, so that might pose a problem for some of them.
Maybe PtrInt would be best (see systemh.inc):
Code: Pascal  [Select]
  1.   { NativeInt and NativeUInt are Delphi compatibility types. Even though Delphi
  2.     has IntPtr and UIntPtr, the Delphi documentation for NativeInt states that
  3.     'The size of NativeInt is equivalent to the size of the pointer on the
  4.     current platform'. Because of the misleading names, these types shouldn't be
  5.     used in the FPC RTL. Note that on i8086 their size changes between 16-bit
  6.     and 32-bit according to the memory model, so they're not really a 'native
  7.     int' type there at all. }
  8.   NativeInt  = Type PtrInt;
  9.   NativeUInt = Type PtrUInt;  
laz trunk - fpc trunk 32bit - Windows 10 Pro x64 (1803)

Thaddy

  • Hero Member
  • *****
  • Posts: 8673
Re: Range Check Error in SynEditMarks.CompareSynEditMarks
« Reply #13 on: June 26, 2017, 10:59:38 am »
Well what you try to measure is distance. Normally it is much clearer when you use a readable type. That's why molly and I suggest to use NativeInt.
Note that type NativeXXX= type PtrXXX; makes NativeXXX a distinct type! It has a different meaning. So please use NativeInt. That makes syntactically more sense.
Most people that want to use threading should learn to patch their jeans first: use a needle.

molly

  • Hero Member
  • *****
  • Posts: 2345
Re: Range Check Error in SynEditMarks.CompareSynEditMarks
« Reply #14 on: June 26, 2017, 11:04:11 am »
Sorry, didn't see new posts while replying.
As can been seen we're in each other hairs all along, so please feel free to join the club :)

Quote
This code is not mine it's SynEdit's code.
In that case do realize that even though the code is perhaps wrong for your shown example code, that it might have been declared this way on purpose. Might be for legacy/compatibility  reasons, might even be because rest of the code does not know how to handle the result (correctly) if larger then integer.

So, be careful in case you are preparing a patch.