View Issue Details

IDProjectCategoryView StatusLast Update
0035745FPCCompilerpublic2020-01-14 00:44
ReporternanobitAssigned To 
PrioritynormalSeverityminorReproducibilityalways
Status newResolutionopen 
Platformwin32OSWindowsOS Version10
Product Version3.2.0Product Build 
Target VersionFixed in Version 
Summary0035745: pa[index] wrong if item is array
DescriptionArrayPointer (pa) to array of items fails for indexed access,
if the arrayPointer is a typed itemPointer (^Titem with {$PointerMath on}),
and Titem is arrayType (subarray).
(pa + index) works, pa[index] fails, but they should access the same item.
Steps To Reproduceprocedure test;
type Tatom = single;
  type Titem = packed array[0..7] of Tatom; // buggy (if Titem is array)
  //type Titem = record d1, d2: Tatom; end; // ok (if Titem is record)
  type PItem = ^Titem;
  {$PointerMath on}
    type PItemArray = ^Titem;
  {$PointerMath off}
var
  memoryItems: array of Titem;
  pa: PItemArray;
  pItemOk, pItemWrong: PItem;
  stride: integer;
begin
setLength( memoryItems, 100);
pa := @memoryItems[0];
pItemOk := pa + 1;
pItemWrong := @pa[1];

stride := PtrUInt(pItemOk) - PtrUInt(pa);
assert( stride = sizeOf( Titem)); // correct

stride := PtrUInt(pItemWrong) - PtrUInt(pa);
// assert( stride = sizeOf( Titem)); // true would be correct
assert( stride = sizeOf( Tatom)); // true means bug:
// pa[0], pa[1], ... are atoms, but should be items (according to PItemArray = ^Titem)
end;
TagsNo tags attached.
Fixed in Revision
FPCOldBugId
FPCTarget
Attached Files

Activities

Jonas Maebe

2019-06-22 22:22

manager   ~0116859

I cannot reproduce this on trunk nor on 3.0.4, so it seems like a regression that was already fixed. We'll have to find the revision so it can be merged to the 3.2 fixes branch

Jonas Maebe

2019-06-23 16:48

manager   ~0116873

Ah, it only happens in Delphi mode (also in FPC 3.0.4). Please always mention all necessary mode switches and/or used command line options.

Jonas Maebe

2019-06-23 17:04

manager   ~0116874

Last edited: 2019-06-23 17:07

View 3 revisions

This is actually correct due to the Delphi quirk of auto-dereferencing pointers:

type
  Tatom = single;
  Titem = packed array[0..7] of Tatom;
  Pitem = ^Titem;
var
  a: Pitem;
begin
  new(a);
  a[4]:=5;
  writeln(a[4]);
end.

This compiles in Delphi and in FPC's Delphi mode, because Delphi automatically dereferences pointers if you don't. This means that in your code above, pa[1] is actually equivalent to pa^[1], and the type of that expression is indeed Tatom.

I agree this is confusing, but I'm not sure whether inverting the priority of this auto-dereferencing and of pointermath won't cause other confusing side-effect

Edit: actually, we can't invert it because the auto-dereferencing is handled in the parser while pointermath is handled later. We could possibly disable auto-dereferencing for pointermath types or if pointermath is switched on, but that too may cause unexpected evaluations.

nanobit

2019-06-24 12:04

reporter   ~0116885

Last edited: 2019-06-24 12:25

View 3 revisions

Now I see, only {$modeswitch autoderef} or {$mode delphi} produce smaller stride.

Solution: autoDerefMode should be ignored on itemPointer[index],
because the intention is unambiguous: index refers to item (not to atom).

Because under {$PointerMath on}, itemPointer[index] should mean itemsPointer[index]:
implies dereferencing in the sense of (itemPointer + index)^,
which means, the intention of explicit autoDerefMode is already fulfilled:
The expression contains a pointer and we want dereferencing once.
Another (additional) dereferencing is not wanted.

Jonas Maebe

2019-06-24 19:18

manager   ~0116897

It turns out that the most recent Delphi versions also support the pointermath directive, and behave the same as FPC in Delphi mode in that particular case. As a result, we cannot change the behaviour, since that could break existing Delphi code making use of this construct.

However, it behaves differently than FPC for " pItemOk := pa + 1;": it seems to interpret that as " pItemOk := pa^ + 1;" and therefore gives a compilation error on that line. That's something we'll have to fix for Delphi mode.

nanobit

2019-06-24 21:31

reporter   ~0116901

Last edited: 2019-06-27 07:22

View 3 revisions

You want to import this buggy design? I completely disagree with this change.
This makes the gap between fpc mode and Delphi mode (in fpc) larger,
and no one can use this, neither in Delphi nor in fpc delphi mode.
But all who want an atoms pointer, naturally use Patom = ^Tatom, but not Pitem = ^Titem.

Pointermath is supposed to support unbounded arrays, also with negative indices.
If you remove this option for arrays of fixed subarrays, then no one in Delphi mode will use it.
In my case, I will resort to array of records, because my subarray has only 2 atoms.
Others will need to resort to staticArrayPointers based on array[0..max] of subarray,
if they don't need negative index. All Delphi code already looks like this (workarounds),
example: https://github.com/graphics32/graphics32/blob/master/Source/GR32_VPR.pas
TLineSegment = array [0..1] of TFloatPoint;
TLineSegmentArray = array [0..0] of TLineSegment;
PLineSegmentArray = ^TLineSegmentArray;

Added (2019-06-27) hint for users:
Due to the bug (as reported, under autoderefMode):
{$PointerMath on} PLineSegmentArray = ^TLineSegment;
is not a possible replacement. And no warning, but crashes occur.

Sven Barth

2019-06-25 07:37

manager   ~0116911

The point of Delphi mode is to be Delphi compatible. So if Delphi behaves in a certain way (that can be determined) then this should be copied in FPC's Delphi mode as well.

You can always disable autodereferencing in FPC by putting {$modeswitch autoderef-} after the {$mode delphi} directive. Then you'll only work with the pointer types.

nanobit

2019-06-25 11:45

reporter   ~0116915

Last edited: 2019-06-25 12:45

View 3 revisions

But in Delphi, this is not a feature, but the default handling of unsupported pointermath.
The default handling is so unreasonable that no one uses it (source code nonexistent).
And fpc Delphi mode allows many features which are not in Delphi.

I don't see which problem will be solved now.
The outcome will be a mess for a pointer (pitems) to array of fixedSubarrays:
1) delphiMode: (pitems[index] = atom); (pitems+1) unsupported, is part of this bugreport
2) fpcMode with autoDerefOn: (pitems[index] = atom); (pitems+1) with (stride = itemSize), is part of this bugreport
3) fpcMode with autoDerefOff: (pitems[index] = item); (pitems+1) with (stride = itemSize)
Reasonable is only variant (3), but the user cannot use it if he wants the benefits of autoDerefOn (for records).

Therefore, people will use workarounds. The last resort which works in all modes:
manual pointermath (index * sizeof( item))

And the real solution is in my earlier comment:
We can ignore special treatment of autoDerefOn at pItems[index],
because this user-wish is already fulfilled implicitly.

Jonas Maebe

2019-06-25 19:05

manager   ~0116930

> But in Delphi, this is not a feature, but the default handling of unsupported pointermath.

The point is that current Delphi versions do support pointermath, and behave the same as FPC when it's enabled.

nanobit

2019-06-25 19:52

reporter   ~0116935

Last edited: 2019-06-28 11:51

View 2 revisions

Delphi mode supports pointermath, but not for the reported case with autoDerefOn.
The reason, an oversight in the past, led to the default handling:
The formal algorithm treats autoderefOn and pointermath isolatedly
(each adds a dereference operator), thus ignores the original intention:

Generally, autoDerefOn should not! formally add a dereference operator,
but autoDerefOn should make the sourcecode notation more convenient:
AutoDerefOn should add ^ only in those expressions,
which require this "cumbersome" notation in AutoDerefOff.

1) recordPtr.field means recordPtr^.field.
autoDerefOn needed

2) type PStaticArray = ^TStaticArray; var psa: PStaticArray;
psa[index] means psa^[index];
autoDerefOn needed

3) pointermath expression:
basePointer[index] means (basePointer+index)^
The convenient notation is already in AutoDerefOff,
So autoDerefOn is redundant here, no second dereference operator should be added.

Edited (2019-06-28): Changed first word (from Delphi to Delphi mode), to reduce further confusion.

Sven Barth

2019-06-27 07:22

manager   ~0116957

Of course Delphi supports pointermath with automatic dereferentiation, because Delphi *only* knows automatic dereferentiation. And the result of these are the same as in FPC currently. So, no, this won't be changed in mode Delphi.

nanobit

2019-06-27 12:31

reporter   ~0116963

Delphi mode (like Delphi) includes autoderefOn; I never questioned that.
The whole thread is about one specific case of typed pointer ( ^TFixedSizeArray).
I don't understand what you intend to say, therefore a simple question:
Will your next fpc version support the following in Delphi mode / autoderefOn ?

TLineSegmentArray = array [0..0] of TLineSegment;
PLineSegmentArray = ^TLineSegmentArray;"

can be replaced by:

{$PointerMath on} PLineSegmentArray = ^TLineSegment;

Yes or No?

nanobit

2019-06-28 08:22

reporter   ~0116982

Last edited: 2019-06-28 09:50

View 3 revisions

@Jonas,
I have a question about your test program under Delphi:
Have you also turned on pointermath there? The default is {$Pointermath off}.
Your first source code excerpt in this thread is not array of array, but only array.

A wrong assumption about Delphi would be very good news for us:
The solution (as written in my earlier notes) would be simple
(and the wrong Delphi mode inherits the corrected autoderefOn).

Jonas Maebe

2019-06-28 17:39

manager   ~0116995

It's @Sven Barth who tested it under Delphi (I only have Kylix, which corresponds to Delphi 6.5). This is the program Sven tested:

***
{$apptype console}

uses
  SysUtils;

procedure test;
type Tatom = single;
  type Titem = packed array[0..7] of Tatom; // buggy (if Titem is array)
  //type Titem = record d1, d2: Tatom; end; // ok (if Titem is record)
  type PItem = ^Titem;
  {$PointerMath on}
    type PItemArray = ^Titem;
  {$PointerMath off}
var
  memoryItems: array of Titem;
  pa: PItemArray;
  pItemOk, pItemWrong: PItem;
  stride: integer;
begin
setLength( memoryItems, 100);
pa := @memoryItems[0];
//pItemOk := pa + 1;
pItemWrong := @pa[1];

//stride := NativeUInt(pItemOk) - NativeUInt(pa);
//assert( stride = sizeOf( Titem)); // correct

stride := NativeUInt(pItemWrong) - NativeUInt(pa);
Writeln(stride);
// assert( stride = sizeOf( Titem)); // true would be correct
assert( stride = sizeOf( Tatom)); // true means bug:
// pa[0], pa[1], ... are atoms, but should be items (according to PItemArray = ^Titem)
end;

begin
  try
  Test;
  except
    on e: Exception do
      Writeln(e.ClassName, ': ', e.Message);
  end;
  Readln;
end.
***
At least in FPC, if you define a type inside a {$pointermath on} block, the pointermath directive "sticks" to the type.

nanobit

2019-06-28 21:55

reporter   ~0116999

Last edited: 2019-06-29 15:15

View 2 revisions

Hm, I have no modern Delphi at hand. Delphi help says clearly:
"Use the {POINTERMATH <ON|OFF>} directive to turn pointer arithmetic on or off
for all typed pointers, so that increment/decrement is by element size."
"... incrementing the index of an array of a type is equivalent to incrementing
a pointer to that type. An increment of one bumps the pointer by the size
an array element in bytes."

The above definition is under autoderefOn (always in Delphi), and also under autoderefOff in FPC.
I consider everything else to be a bug or unsupported feature (friendly speaking).

type TLineSegment = array [0..1] of TFloatPoint;
{$PointerMath on} type PLineSegmentArray = ^TLineSegment;
var LineSegments: PLineSegmentArray;

In our example, regardless of autoderef mode, it should be:
Array element is LineSegment (with sizeof( pointsArray)) and (LineSegments[i] = LineSegment).
Wrong is the curiosity (LineSegments[i] = Point), which creates only troubles.

If we define TLineSegment as record, then
all compilers produce (LineSegments[i] = LineSegment).
Tested with FPC, for all autoderef modes.

Sven Barth

2019-06-29 22:45

manager   ~0117011

Last edited: 2019-06-29 22:46

View 2 revisions

The problem is that Delphi's documentation does not document how the PointerMath directive is interacting with the automatic dereferentiation, especially when arrays are involved - as are in your example. As such we're dealing with undocumented behavior and that is a notoriously hard part when reverse engineering, cause it could be considered a bug in the next release and be fixed or they could decide to document the status quo. Someone could raise the issue with the Delphi developers to see what their stance is, but until then we should stick to the Delphi compatible behavior.

I've also done a bit investigation into the failing line (the "pItemOK := pa + 1"). The problem doesn't appear to be that the compiler interprets it as "pItemOK := pa^ + 1", but that it considers PItem and PItemArray to be incompatible types. Take the following example:

=== code begin ===

procedure Test;
type
  PTest = ^LongInt;
  PTest2 = ^LongInt;
{$PointerMath On}
  PTestArray = ^LongInt;
  PTestArray2 = ^LongInt;
{$PointerMath Off}
var
  pa, pa2: PTestArray;
  p: PTest;
  pa3: PTestArray2;
  p2: PTest2;
begin
  //p := pa + 1;
  pa2 := pa + 1;
  //p := pa2;
  //p2 := p;
  //pa2 := pa3;
end;

=== code end ===

All the commented lines fail in Delphi with "Incompatible Types" *unless* I specify "{$TypedAddress On}"... Don't know whether that has always been the case or not (I only have Delphi 10.2 to test), but it's definitely confusing...

Edit:
Jonas wrote:
> At least in FPC, if you define a type inside a {$pointermath on} block, the pointermath directive "sticks" to the type.

Same in Delphi.

nanobit

2019-06-30 08:43

reporter   ~0117013

Last edited: 2019-06-30 14:09

View 2 revisions

I just hope, all understand:
AutoDeref mode should be irrelevant (ignored) in pointermath pItems[i],
because the pointermath-index-expression has no cumbersome ^notation.
AutoDerefOn is a style mode, almost like changing to a narrow fontstyle.

The normal style of this index-expression is narrow (hidden ^) already.
One cannot make this narrowest style even narrower (by inserting another hidden ^):
This style change is not possible; but it would change the logic:
A different element type is returned.

The Delphi help is complete: The stride is element size for ALL element types.
I see no confusion about what the correct behavior should be...
But I see endless problems with the current behavior.
(Don't get me started, what the problems are ...)

Added: Why FPC/autoDerefOn should be irrelevant in pItems[i]:
Here a simpler formulation based on a FPC definition:
FPC has defined the notation under autoDerefOff: pItems[i].
This is the shortest notation (because without ^operator).
Shorter (more convenient) is not possible,
thus any notation-shortener (autoDerefOn) is redundant.
Notation and stride comply with Delphi help.

nanobit

2020-01-13 12:36

reporter   ~0120399

Delphi news are, their bug is or will be fixed.
IMO, a good time to proceed here too:

All fits together in FPC, despite the fact that FPC (unlike Delphi)
also supports the limiting autoderefOff mode.

1.0) Requirements for autoderef mode:
AutoderefOn (stylistic option) is a preprocessing step to allow
more pointer-expressions, which are illegal under autoderefOff.
The preprocessor appends a caret to some container-pointer
(recordPtr, staticArrayPtr) to allow short notation for element access.

The most important rule is:
Legal pointer-expressions have a fixed (unchanging) meaning,
regardless of autoderefMode. That is, all legal pointer-expressions
under autoderefOff have exactly the same meaning under autoderefOn.
Therefore, sourcecode under autoderefOff is fully compatible
under autoderefOn (and Delphi mode).

Examples working under autoderefOn:
1) pRecord.field resolves to pRecord^.field
2) pRecord^.field remains pRecord^.field, regardless of autoderefMode
3) pStaticArray[i] resolves to pStaticArray^[i]
4) pStaticArray^[i] remains pStaticArray^[i], regardless of autoderefMode
5) pItemTyped[i] is a legal pointermath-expression under autoderefOff,
and should mean the same under autoderefOn. This will be fulfilled
by the pointermath bugfix for (itemType = staticArray), see next:

2.0) Requirements for pointermath mode:
pItemTyped[i], (pItemTyped + i)^, @pItemTyped[i] support ALL itemtypes.
Bugfix: If item is static array, the pointermath-index has precedence over the default-index.
The default-index is used only under special AND-condition (pointermathOff and autoderefOn):
pStaticArray[i] resolves to pStaticArray^[i].

My other notes are under the same presumption (pointermath-index should have highest precedence),
and deduce the same consequence (autoderefMode should be ignored in pa[i]).

Sven Barth

2020-01-13 21:43

manager   ~0120412

> Delphi news are, their bug is or will be fixed.

Do you have a link for this?

nanobit

2020-01-14 00:44

reporter   ~0120418

They have a related bug (RSP-27196) in the bugtracker.
(pa + i)^ is correct in Delphi and FPC.
Only pa[i] needs the fix. Tiny change, but important:)

Issue History

Date Modified Username Field Change
2019-06-21 20:29 nanobit New Issue
2019-06-22 22:22 Jonas Maebe Note Added: 0116859
2019-06-23 16:48 Jonas Maebe Note Added: 0116873
2019-06-23 17:04 Jonas Maebe Note Added: 0116874
2019-06-23 17:07 Jonas Maebe Note Edited: 0116874 View Revisions
2019-06-23 17:07 Jonas Maebe Note Edited: 0116874 View Revisions
2019-06-24 12:04 nanobit Note Added: 0116885
2019-06-24 12:15 nanobit Note Edited: 0116885 View Revisions
2019-06-24 12:25 nanobit Note Edited: 0116885 View Revisions
2019-06-24 19:18 Jonas Maebe Note Added: 0116897
2019-06-24 21:31 nanobit Note Added: 0116901
2019-06-24 21:37 nanobit Note Edited: 0116901 View Revisions
2019-06-25 07:37 Sven Barth Note Added: 0116911
2019-06-25 11:45 nanobit Note Added: 0116915
2019-06-25 12:20 nanobit Note Edited: 0116915 View Revisions
2019-06-25 12:45 nanobit Note Edited: 0116915 View Revisions
2019-06-25 19:05 Jonas Maebe Note Added: 0116930
2019-06-25 19:52 nanobit Note Added: 0116935
2019-06-27 07:22 Sven Barth Note Added: 0116957
2019-06-27 07:22 nanobit Note Edited: 0116901 View Revisions
2019-06-27 12:31 nanobit Note Added: 0116963
2019-06-28 08:22 nanobit Note Added: 0116982
2019-06-28 08:54 nanobit Note Edited: 0116982 View Revisions
2019-06-28 09:50 nanobit Note Edited: 0116982 View Revisions
2019-06-28 11:51 nanobit Note Edited: 0116935 View Revisions
2019-06-28 17:39 Jonas Maebe Note Added: 0116995
2019-06-28 21:55 nanobit Note Added: 0116999
2019-06-29 15:15 nanobit Note Edited: 0116999 View Revisions
2019-06-29 22:45 Sven Barth Note Added: 0117011
2019-06-29 22:46 Sven Barth Note Edited: 0117011 View Revisions
2019-06-30 08:43 nanobit Note Added: 0117013
2019-06-30 14:09 nanobit Note Edited: 0117013 View Revisions
2020-01-13 12:36 nanobit Note Added: 0120399
2020-01-13 21:43 Sven Barth Note Added: 0120412
2020-01-14 00:44 nanobit Note Added: 0120418