Recent

Author Topic: Is this a type helper bug?  (Read 1727 times)

avra

  • Hero Member
  • *****
  • Posts: 2161
    • Additional info
Is this a type helper bug?
« on: March 26, 2021, 03:51:14 pm »
Type helpers do not seam to work for subrange types. For example, following type helper works only if I change type of TNewJ1939PGN from subrange to dword.
Code: Pascal  [Select][+][-]
  1. program forumtest;
  2.  
  3. {$mode delphi} {$H+}
  4. {$modeswitch typehelpers}
  5.  
  6. uses
  7.   cthreads;
  8.  
  9. type
  10.   TNewJ1939PGN = 0..(1 shl 18)-1; // does not work with subrange, but works if type is changed to dword
  11.  
  12.   TNewJ1939CanID = bitpacked record
  13.     SA:       byte;         // if PGN was a record, bit packing would not work and
  14.     PGN:      TNewJ1939PGN; // Priority bits would not be immediatelly after PGN bits
  15.     Priority: 0..7;         // since PGN record would then be aligned to byte by pascal
  16.   end;                      // and there is no solution for that, so let's try type helpers instead
  17.  
  18.   TNewJ1939PGNHelper = type helper for TNewJ1939PGN
  19.   private
  20.     function  GetPS: byte;
  21.     procedure SetPS(const ANewPS: byte);
  22.   public
  23.     property  PS: byte read GetPS write SetPS;
  24.   end;
  25.  
  26. function TNewJ1939PGNHelper.GetPS: byte;
  27. begin
  28.   Result := Self and $FF; //  and %00 00000000 11111111 (PS is in lowest 8 bits)
  29. end;
  30.  
  31. procedure TNewJ1939PGNHelper.SetPS(const ANewPS: byte);
  32. begin // debug says that Self was 100 and becomes 255, but PGN value is not changed
  33.   Self := (Self and $3FF00) or ANewPS; //  and %11 11111111 00000000
  34. end;  // If I change type TNewJ1939PGN to dword, Self becomes 255 as it should
  35.  
  36. var
  37.   NewJ1939CanID: TNewJ1939CanID;
  38. begin
  39.   NewJ1939CanID.PGN    := 100;
  40.   NewJ1939CanID.PGN.PS := 255;
  41.   WriteLn('BitSizeOf(NewJ1939CanID)           = ', BitSizeOf(NewJ1939CanID));
  42.   WriteLn('BitSizeOf(NewJ1939CanID.SA)        = ', BitSizeOf(NewJ1939CanID.SA));
  43.   WriteLn('BitSizeOf(NewJ1939CanID.PGN)       = ', BitSizeOf(NewJ1939CanID.PGN));
  44.   WriteLn('BitSizeOf(NewJ1939CanID.Priority)  = ', BitSizeOf(NewJ1939CanID.Priority));
  45.   WriteLn('NewJ1939CanID.PGN                  = ', NewJ1939CanID.PGN);
  46.   WriteLn('NewJ1939CanID.PGN.PS               = ', NewJ1939CanID.PGN.PS);
  47. end.

Checked on FPC 3.3.1 SVN64623 x64 Linux.
ct2laz - Conversion between Lazarus and CodeTyphon
bithelpers - Bit manipulation for standard types
pasettimino - Siemens S7 PLC lib

avk

  • Sr. Member
  • ****
  • Posts: 411
    • my self-education project
Re: Is this a type helper bug?
« Reply #1 on: March 26, 2021, 05:14:29 pm »
It looks like a helper bug, and only when writing.(Linux, FPC-3.3.1-r49028)
Maybe replace it with this one?
Code: Pascal  [Select][+][-]
  1.   //TNewJ1939PGNHelper = type helper for TNewJ1939PGN
  2.   TNewJ1939PGNHelper = type helper for TNewJ1939CanID
  3.   private
  4.     function  GetPS: byte;
  5.     procedure SetPS(const ANewPS: byte);
  6.   public
  7.     property  PS: byte read GetPS write SetPS;
  8.   end;
  9.  
  10. function TNewJ1939PGNHelper.GetPS: byte;
  11. begin
  12.   //Result := Self and $FF; //  and %00 00000000 11111111 (PS is in lowest 8 bits)
  13.   Result := Self.PGN and $FF;
  14. end;
  15.  
  16. procedure TNewJ1939PGNHelper.SetPS(const ANewPS: byte);
  17. begin // debug says that Self was 100 and becomes 255, but PGN value is not changed
  18.   //Self := (Self and $3FF00) or ANewPS; //  and %11 11111111 00000000
  19.   Self.PGN := (Self.PGN and $3FF00) or ANewPS;
  20. end;  
  21.  
« Last Edit: March 26, 2021, 06:21:51 pm by avk »

jamie

  • Hero Member
  • *****
  • Posts: 4468
Re: Is this a type helper bug?
« Reply #2 on: March 26, 2021, 11:00:18 pm »
I would wager a little to say its the old auto type level conversion taking place...

Code: Pascal  [Select][+][-]
  1.  Self := WORD((Self and $3FF00) or ANewPS);
  2.  

something like that...

Just a guess but doing things like that with FPC usually works and btw.

 It seems the Enumerator type isn't honored after the nominator/denominator has don't its job in the equation.

 But who knows, this could be a simple OOPS, when some one spilt their coffee on the keyboard!



The only true wisdom is knowing you know nothing

avra

  • Hero Member
  • *****
  • Posts: 2161
    • Additional info
Re: Is this a type helper bug?
« Reply #3 on: March 29, 2021, 08:32:16 am »
It looks like a helper bug, and only when writing.(Linux, FPC-3.3.1-r49028)
Thanks for confirmation.

Maybe replace it with this one?
Moving PS from TNewJ1939PGN to TNewJ1939CanID is not desirable. I wanted to keep PS in TNewJ1939PGN which is inside of TNewJ1939CanID. Otherwise, I could simply avoid record in a record which would then allow full bitpacking without non-desirable byte alignment effect triggering, and type helpers would not be needed at all. I like record in a record ease of use, so I hoped that type helpers could come to the rescue. Unfortunately they do not seam to work properly for subranges (which I do need in this case).

I would wager a little to say its the old auto type level conversion taking place...

Code: Pascal  [Select][+][-]
  1.  Self := WORD((Self and $3FF00) or ANewPS);
  2.  
Besides word() type casting loosing higher bits, and dword() dealing with more bits then needing, both do not work and result is still 100 instead of 255.

I will file this as a bug and see what devs think about it...
ct2laz - Conversion between Lazarus and CodeTyphon
bithelpers - Bit manipulation for standard types
pasettimino - Siemens S7 PLC lib

avra

  • Hero Member
  • *****
  • Posts: 2161
    • Additional info
Re: Is this a type helper bug?
« Reply #4 on: March 29, 2021, 08:46:50 am »
ct2laz - Conversion between Lazarus and CodeTyphon
bithelpers - Bit manipulation for standard types
pasettimino - Siemens S7 PLC lib

avra

  • Hero Member
  • *****
  • Posts: 2161
    • Additional info
Re: Is this a type helper bug?
« Reply #5 on: April 07, 2021, 11:18:07 am »
Reported helper bug for subrange types is still there, but I have found a work around for my use case. Initial goal was to have bitpacked PGN record inside of another record to achieve dotted field access like this:
Code: Pascal  [Select][+][-]
  1.   J1939CanID.PGN.Value    := 100;
  2.   J1939CanID.PGN.PS.Value := 255;
  3.   J1939CanID.PGN.PF := 122;
  4.   J1939CanID.PGN.DP := true;
Unfortunately PGN is 18 bits in length so when put inside of another record it expanded it self to 24 bits, which was bad since I needed 3 Priority bits right after PGN record structure (with side effect that whole record was now 1 byte bigger then original C structure). At first I was able to do it only after I gave up on PGN record (to avoid byte alignment of bitpacked sub-record structure) which looked really ugly:
Code: Pascal  [Select][+][-]
  1.   J1939CanID.PGN    := 100;
  2.   J1939CanID.PGN_PS.Value := 255;
  3.   J1939CanID.PGN_PF := 122;
  4.   J1939CanID.PGN_DP := true;

Finally, I have now managed to solve it by multiple overlays and dummy bit inserts, Maybe it will be of use to someone else, so here it is:
Code: Pascal  [Select][+][-]
  1. program testforum;
  2.  
  3. {$mode delphi} {$H+}
  4.  
  5. uses
  6.   baseunix;
  7.  
  8. type
  9.   T1939SimplePGN = 0..(1 shl 18)-1;
  10.  
  11.   TJ1939PS = record case byte of
  12.     1: (Value: cuint8);
  13.     2: (DA:    cuint8);
  14.     3: (GE:    cuint8);
  15.   end;
  16.  
  17.   TNewJ1939PGN = bitpacked record case byte of // J1939 PGN (Parameter Group Number), as used in J1939 29 bit CAN ID
  18.     1: (Value: T1939SimplePGN; );              // full access helper
  19.     2: (PS: TJ1939PS; // 8 bit: PS in PDU2 , else 0 (PDU specific)
  20.         PF: cuint8;   // 8 bit: PF (PDU format). PDU1 (0..239) indicates a destination address in PS, PDU2 (250-255) indicates extension to PDU Format (PF).
  21.         DP: boolean;  // 1 bit: DP (Data Page), page selector for PDU (Protocol Data Unit) Format (PF) field. Currently at 0, pointing to Page 0. Page 1 is for future purposes)
  22.         R:  boolean); // 1 bit: R  (Reserved), should always be set to 0 when transmitting messages
  23.   end;
  24.  
  25.   TNewJ1939CanID = bitpacked record case byte of
  26.     1: ( SA:   byte;
  27.          case byte of
  28.            1: ( PGN:        TNewJ1939PGN; ); // PGN's 18 bits expand to 24 bits when used in record
  29.            2: ( __DummyPGN: T1939SimplePGN;  // so we need __DummyPGN just to skip 18 bits
  30.                 Priority:   0..7; ); );      // and then put these Priority 3 bits immediately after PGN's 18 bits
  31.     2: (Bit:   bitpacked array[0..31] of boolean); // bit access helper
  32.     3: (Full:  cuint32);                           // full access helper
  33.   end;
  34.  
  35. var
  36.   NewJ1939CanID: TNewJ1939CanID;
  37. begin
  38.   NewJ1939CanID.PGN.Value    := 100;
  39.   NewJ1939CanID.PGN.PS.Value := 255;
  40.   //NewJ1939CanID.PGN.Value := (1 shl 18)-1;
  41.   //NewJ1939CanID.PGN.DP := true;
  42.   //NewJ1939CanID.Priority := 7;
  43.   WriteLn('BitSizeOf(NewJ1939CanID)           = ', BitSizeOf(NewJ1939CanID));
  44.   WriteLn('BitSizeOf(NewJ1939CanID.SA)        = ', BitSizeOf(NewJ1939CanID.SA));
  45.   WriteLn('BitSizeOf(NewJ1939CanID.PGN)       = ', BitSizeOf(NewJ1939CanID.PGN));
  46.   WriteLn('BitSizeOf(NewJ1939CanID.Priority)  = ', BitSizeOf(NewJ1939CanID.Priority));
  47.   WriteLn('NewJ1939CanID.PGN.Value            = ', NewJ1939CanID.PGN.Value);
  48.   WriteLn('NewJ1939CanID.PGN.PS.Value         = ', NewJ1939CanID.PGN.PS.Value);
  49.   WriteLn('BinStr(NewJ1939CanID.Full, 32)     = ', BinStr(NewJ1939CanID.Full, 32));
  50. end.

You can explore comments for more explanation.
« Last Edit: April 07, 2021, 01:28:26 pm by avra »
ct2laz - Conversion between Lazarus and CodeTyphon
bithelpers - Bit manipulation for standard types
pasettimino - Siemens S7 PLC lib

avra

  • Hero Member
  • *****
  • Posts: 2161
    • Additional info
Re: Is this a type helper bug?
« Reply #6 on: April 28, 2021, 10:10:54 am »
UPDATE: Here is a little better example. It shows that plain subrange types do not have this type helper bug, plain records holding the same subrange type do not have this bug, but bitpacked records holding this subrange type do have this type helper bug!. There is still no activity in bugtracker regarding this bug report.

Code: Pascal  [Select][+][-]
  1. program testtypehelperbug;
  2. {$mode delphi} {$H+}
  3. {$modeswitch typehelpers}
  4.  
  5. type
  6.   TNewJ1939PGN = 0..(1 shl 18)-1;   // Both PGN and ID.PGN work well if we replace 18 bits subrange type with dword type
  7.  
  8.   TNewJ1939CanID = bitpacked record // if TNewJ1939PGN is 18bits then ID.PGN works well only when record is not bitpacked
  9.     SA: byte;
  10.     PGN: TNewJ1939PGN;
  11.     Priority: 0..7;
  12.   end;
  13.  
  14.   TNewJ1939PGNHelper = type helper for TNewJ1939PGN
  15.   private
  16.     function GetPS: byte;
  17.     procedure SetPS(const ANewPS: byte);
  18.   public
  19.     property PS: byte read GetPS write SetPS;
  20.   end;
  21.  
  22. function TNewJ1939PGNHelper.GetPS: byte;
  23. begin
  24.   Result := Self and $FF; // and %00 00000000 11111111 (PS is in lowest 8 bits)
  25. end;
  26.  
  27. procedure TNewJ1939PGNHelper.SetPS(const ANewPS: byte); // works well only when type is not used in a record
  28. begin // debug says that Self was 100 and becomes 255, but PGN value is not changed
  29.   Self := (Self and $3FF00) or ANewPS; // and %11 11111111 00000000
  30. end; // If I change type TNewJ1939PGN to dword, although debug says that Self becomes 255 it does not propagate further
  31.  
  32. var
  33.   PGN: TNewJ1939PGN;
  34.   ID: TNewJ1939CanID;
  35. begin
  36.   PGN    := 100;
  37.   PGN.PS := 255;
  38.   WriteLn('PGN    = ', PGN);          // PGN is 255 as it should be
  39.   WriteLn('PGN.PS = ', PGN.PS);       // PGN.PS is 255 as it should be
  40.  
  41.   ID.PGN    := 100;
  42.   ID.PGN.PS := 255;
  43.   WriteLn('ID.PGN    = ', ID.PGN);    // PGN is 100 (WRONG!!!)
  44.   WriteLn('ID.PGN.PS = ', ID.PGN.PS); // PGN.PS is 100 (WRONG!!!)
  45. end.
ct2laz - Conversion between Lazarus and CodeTyphon
bithelpers - Bit manipulation for standard types
pasettimino - Siemens S7 PLC lib

jamie

  • Hero Member
  • *****
  • Posts: 4468
Re: Is this a type helper bug?
« Reply #7 on: April 29, 2021, 03:25:35 am »
Interesting problem.

This is one of those things that is kind of hard for the compiler to detect.
Technically, the compiler should complain about this because the access field for the 18 bits is actually a 32 bit type and you only have a total of 3 bytes after the SA field.

 So the helper of course does not know it just got placed in a bitpacked field and there isn't enough room left at the end of the record to read and write that field as a 32 bit.

 The total size of that record is 4 bytes but you have the SA field at the start which is a byte in itself so that leaves you with 3 bytes left .

 Very interesting..

 
I would say the compiler will simply load and write that field as it was a stand alone because I can't see how the helper knows it is living in a bitpacked field.
The only true wisdom is knowing you know nothing

avra

  • Hero Member
  • *****
  • Posts: 2161
    • Additional info
Re: Is this a type helper bug?
« Reply #8 on: April 29, 2021, 08:31:40 am »
Interesting problem.
Unfortunately  :D

This is one of those things that is kind of hard for the compiler to detect.
...
I would say the compiler will simply load and write that field as it was a stand alone because I can't see how the helper knows it is living in a bitpacked field.
Well, when I directly assign value to 18 bit type in a bitpacked record (ID.PGN := 100;), compiler knows how to handle it correctly and does all needed bit shifting and masking (even with such a hard example as leading 2 bits before that 18 bit type and 3 trailing bits after that 18 bit type, which puts types totally out of record byte boundaries). I would expect the same behavior when helper on such 18 bit type in a bitpacked helper is used (ID.PGN.PS := 255;). That is a special case which needs additional handling, but it should be done for completeness. Unfortunately, compiler does not treat it like that and we do have a bug. How it should be handled may be debated, but it is a bug.
« Last Edit: April 29, 2021, 08:38:47 am by avra »
ct2laz - Conversion between Lazarus and CodeTyphon
bithelpers - Bit manipulation for standard types
pasettimino - Siemens S7 PLC lib

jamie

  • Hero Member
  • *****
  • Posts: 4468
Re: Is this a type helper bug?
« Reply #9 on: April 29, 2021, 11:03:39 pm »
when you directly read/write to the field as a stand alone item the compiler is treating as a 32 bit because that is the next logical upswing for the byte orders and since there is no bitpacking indicating there the helper simply acceses as a 32 bit.

 When you place that same type in a bitpacked record now that field is sharing the priority field of the bits that exist.

 being a helper at work I can't how the compiler will know the difference here..

Also on top of all that, if the helper is handling that as it was out side the bitpacked record then this means there is a memory overflow at the end of the record by 1 byte. That would be a hard memory leak to find.
The only true wisdom is knowing you know nothing

avra

  • Hero Member
  • *****
  • Posts: 2161
    • Additional info
Re: Is this a type helper bug?
« Reply #10 on: May 01, 2021, 10:06:13 pm »
when you directly read/write to the field as a stand alone item the compiler is treating as a 32 bit because that is the next logical upswing for the byte orders and since there is no bitpacking indicating there the helper simply acceses as a 32 bit.

 When you place that same type in a bitpacked record now that field is sharing the priority field of the bits that exist.

 being a helper at work I can't how the compiler will know the difference here..

Take a look at the little modified example below. ID.PGN and ID.PGN.PS both point to the same memory location in a bitpacked record not aligned to a byte. ID.PGN is a field in the record and it can modify it self properly. ID.PGN.PS is a helper and it can not modify it's location properly. I still think that if a field in a bitpacked record can work well (probably by masking and bit shifting), then helper should be able to do the same.

Code: Pascal  [Select][+][-]
  1. program testtypehelperbug;
  2. {$mode delphi} {$H+}
  3. {$modeswitch typehelpers}
  4.  
  5. type
  6.   T18bits = 0..(1 shl 18)-1;   // Both PGN and ID.PGN work well if we replace 18 bits subrange type with dword type
  7.  
  8.   TBitPackedRec = bitpacked record // if T18bits is 18bits then ID.PGN works well only when record is not bitpacked
  9.     SA:  0..(1 shl 7)-1;
  10.     PGN: T18bits;
  11.     PR:  0..7;
  12.   end;
  13.  
  14.   T18bitsHelper = type helper for T18bits
  15.   private
  16.     function GetPS: byte;
  17.     procedure SetPS(const ANewPS: byte);
  18.   public
  19.     property PS: byte read GetPS write SetPS;
  20.   end;
  21.  
  22. function T18bitsHelper.GetPS: byte;
  23. begin
  24.   Result := Self and $FF; // and %00 00000000 11111111 (PS is in lowest 8 bits)
  25. end;
  26.  
  27. procedure T18bitsHelper.SetPS(const ANewPS: byte);
  28. begin
  29.   Self := (Self and $3FF00) or ANewPS; // and %11 11111111 00000000
  30. end;
  31.  
  32. var
  33.   PGN: T18bits;
  34.   ID:  TBitPackedRec;
  35. begin
  36.   PGN    := 100;
  37.   WriteLn('1. PGN    = ', PGN);          // PGN is 100 as it should be
  38.   WriteLn('1. PGN.PS = ', PGN.PS);       // PGN.PS is 100 as it should be
  39.   PGN.PS := 255;
  40.   WriteLn('2. PGN    = ', PGN);          // PGN is 255 as it should be
  41.   WriteLn('2. PGN.PS = ', PGN.PS);       // PGN.PS is 255 as it should be
  42.  
  43.   ID.SA  := $55;
  44.   ID.PR  := 3;
  45.   ID.PGN := 100;
  46.   WriteLn('1. ID.PGN    = ', ID.PGN);    // PGN is 100 as it should be
  47.   WriteLn('1. ID.PGN.PS = ', ID.PGN.PS); // PGN.PS is 100 as it should be
  48.   WriteLn('1. ID.SA     = ', ID.SA);     // PGN.SA is 85 as it should be
  49.   WriteLn('1. ID.PR     = ', ID.PR);     // PGN.PR is 3 as it should be
  50.   ID.PGN.PS   := 255;
  51.   WriteLn('2. ID.PGN    = ', ID.PGN);    // PGN is 100 (WRONG!!! IT SHOULD BE 255)
  52.   WriteLn('2. ID.PGN.PS = ', ID.PGN.PS); // PGN.PS is 100 (WRONG!!! IT SHOULD BE 255)
  53.   WriteLn('2. ID.SA     = ', ID.SA);     // PGN.SA is 85 as it should be
  54.   WriteLn('2. ID.PR     = ', ID.PR);     // PGN.PR is 3 as it should be
  55. end.
ct2laz - Conversion between Lazarus and CodeTyphon
bithelpers - Bit manipulation for standard types
pasettimino - Siemens S7 PLC lib

 

TinyPortal © 2005-2018