Recent

Author Topic: Likely incorrect code generation in FPC v3.0.4  (Read 1358 times)

440bx

  • Hero Member
  • *****
  • Posts: 826
Likely incorrect code generation in FPC v3.0.4
« on: March 04, 2019, 10:57:22 pm »
Hello,

the following simple procedure puts a hex dump of a memory block into a listbox:

Code: Pascal  [Select]
  1. procedure UpdateListbox(ListBoxHex  : HWND;
  2.                         BaseAddress : pointer;
  3.                         BlockSize   : DWORD);
  4.   { displays a memory block in hex                                            }
  5. const
  6.   SPACE          = $20;                { ASCII space character                }
  7.   HEX_DUMP_WIDTH = 16;                 { number of hex values per line        }
  8.  
  9.   HexDigits      : packed array[0..$F] of char = '0123456789ABCDEF';
  10.  
  11. var
  12.   { variables related to the formatting of hex strings to be displayed        }
  13.  
  14.   Buf            : packed array[0..127] of char;  { one output line           }
  15.  
  16.   HexPtr         : ^char; { pointer to hex area of line buffer                }
  17.   CharPtr        : ^char; { pointer to character area of line buffer          }
  18.   p              : PBYTE; { used to walk the memory block                     }
  19.  
  20.   I              : DWORD;
  21.  
  22.   b              : byte;
  23.  
  24. begin
  25.   { reset the contents of the hex view listbox. Minimize the flicker by       }
  26.   { turning listbox redrawing off while we change its settings.               }
  27.  
  28.   SendMessage(ListBoxHex, WM_SETREDRAW, ord(FALSE), 0);   { redraw off        }
  29.  
  30.   I := 0;
  31.   while I < BlockSize do
  32.   begin
  33.     FillMemory(@Buf, sizeof(Buf), SPACE);      { space out the buffer         }
  34.  
  35.     { place the offset at the beginning of the line                           }
  36.  
  37.     StrFmt(Buf, '  %p : ', [pointer(pchar(BaseAddress) + I)]);
  38.  
  39.     { calculate the pointer value to the start of the hex area in buf         }
  40.  
  41.     HexPtr := pointer(pchar(@Buf) + lstrlen(Buf));
  42.  
  43.     { calculate the pointer value to the start of the character area          }
  44.  
  45.     CharPtr := pointer(pchar(HexPtr) + (HEX_DUMP_WIDTH * 3) + 1);
  46.  
  47.     repeat
  48.       pchar(p) := pchar(BaseAddress) + I;          { current byte pointer     }
  49.  
  50.       HexPtr^ := HexDigits[p^ shr 4];              { first nibble             }
  51.       inc(HexPtr);
  52.  
  53.      { following lines to demonstrate the problem by comparison }
  54.  
  55.       if I = BlockSize - 1 then DebugBreak;
  56.  
  57.       b := p^;
  58.       b := b and $F;
  59.       HexPtr^ := HexDigits[b];
  60.  
  61.       { problematic statement - occasionally causes an access violation on the last byte of the memory block }
  62.       HexPtr^ := HexDigits[p^ and $F];             { second nibble            }
  63.  
  64.       inc(HexPtr);
  65.  
  66.       { increment HexPtr again to leave a space between bytes                 }
  67.  
  68.       inc(HexPtr);
  69.  
  70.       { if the byte is a printable character then just place it in the char   }
  71.       { area of the buffer, otherwise put a dot instead                       }
  72.  
  73.       if   p^ in [32..126]
  74.       then CharPtr^ := char(p^)              { the byte is printable          }
  75.       else CharPtr^ := '.';                  { not printable, put a dot       }
  76.  
  77.       inc(CharPtr);
  78.  
  79.       { put an extra space between the first and second half of the hex area  }
  80.  
  81.       if I mod HEX_DUMP_WIDTH = (HEX_DUMP_WIDTH div 2) - 1 then Inc(HexPtr);
  82.  
  83.       inc(I);
  84.     until (I >= BlockSize) or ((I mod HEX_DUMP_WIDTH) = 0);
  85.  
  86.     CharPtr^ := #0;                       { null terminate the buffer         }
  87.  
  88.     SendMessage(ListBoxHex, LB_ADDSTRING, 0, ptruint(@Buf)); { add to listbox }
  89.   end;
  90.  
  91.   SendMessage(ListBoxHex, WM_SETREDRAW, ord(TRUE), 0);   { redraw on          }
  92. end;
  93.  
Occasionally, it produces an access violation when accessing the last byte in the block.  Inspecting the generated code shows it is as follows:

Code: Pascal  [Select]
  1. 32bit generated code (GDB assembler window)
  2.  
  3. MapViewOfFile.lpr:124             b := p^;
  4. 004019F7 8b8568ffffff             mov    -0x98(%ebp),%eax
  5.  
  6. // note the BYTE access in the following statement (as it should be)
  7.  
  8. 004019FD 8a00                     mov    (%eax),%al
  9.  
  10. 004019FF 888560ffffff             mov    %al,-0xa0(%ebp)
  11. MapViewOfFile.lpr:125             b := b and $F;
  12. 00401A05 668b8560ffffff           mov    -0xa0(%ebp),%ax
  13. 00401A0C 66250f00                 and    $0xf,%ax
  14. 00401A10 888560ffffff             mov    %al,-0xa0(%ebp)
  15. MapViewOfFile.lpr:126             HexPtr^ := HexDigits[b];
  16. 00401A16 8b9570ffffff             mov    -0x90(%ebp),%edx
  17. 00401A1C 0fb68560ffffff           movzbl -0xa0(%ebp),%eax
  18. 00401A23 8a80c0704200             mov    0x4270c0(%eax),%al
  19. 00401A29 8802                     mov    %al,(%edx)
  20. MapViewOfFile.lpr:128             HexPtr^ := HexDigits[p^ and $F];             { second nibble            }
  21. 00401A2B 8b8568ffffff             mov    -0x98(%ebp),%eax
  22.  
  23. // note the WORD access in the following statement (which is incorrect.)
  24.  
  25. 00401A31 668b00                   mov    (%eax),%ax
  26.  
  27. 00401A34 83e00f                   and    $0xf,%eax
  28. 00401A37 8b9570ffffff             mov    -0x90(%ebp),%edx
  29. 00401A3D 8a80c0704200             mov    0x4270c0(%eax),%al
  30. 00401A43 8802                     mov    %al,(%edx)
  31.  
Note that at address 00401A31 the instruction mov    (%eax),%ax attempts to retrieve a _word_ instead of a byte, which is what causes a violation when that last byte is also the last accessible byte in the memory block.

Note that when the "operation" is done a step at a time (the first part of the code snippet), the instruction at 004019FD mov    (%eax),%al accesses only a byte (as it should.)

The problem is also present in 64bit as the following shows:
Code: Pascal  [Select]
  1. 64bit generated code (visual studio debugger)
  2.  
  3.       if I = BlockSize - 1 then DebugBreak;
  4. 0000000100001A86 8B 45 E8             mov         eax,dword ptr [BLOCKSIZE]  
  5. 0000000100001A89 48 8D 50 FF          lea         rdx,[rax-1]  
  6. 0000000100001A8D 8B 85 48 FF FF FF    mov         eax,dword ptr [I]  
  7. 0000000100001A93 48 39 C2             cmp         rdx,rax  
  8. 0000000100001A96 75 05                jne         UPDATELISTBOX+13Dh (0100001A9Dh)  
  9. 0000000100001A98 E8 63 FA FF FF       call        public_all+500h (0100001500h)  
  10.  
  11.       b := p^;
  12. 0000000100001A9D 48 8B 85 50 FF FF FF mov         rax,qword ptr [P]  
  13.  
  14. // following statement accesses a byte, which is correct
  15.  
  16. 0000000100001AA4 8A 00                mov         al,byte ptr [rax]  
  17.  
  18. 0000000100001AA6 88 85 40 FF FF FF    mov         byte ptr [B],al  
  19.       b := b and $F;
  20. 0000000100001AAC 80 A5 40 FF FF FF 0F and         byte ptr [B],0Fh  
  21.       HexPtr^ := HexDigits[b];
  22. 0000000100001AB3 48 8B 8D 60 FF FF FF mov         rcx,qword ptr [HEXPTR]  
  23. 0000000100001ABA 0F B6 85 40 FF FF FF movzx       eax,byte ptr [B]  
  24. 0000000100001AC1 48 8D 15 58 A6 02 00 lea         rdx,[main+29C50h (010002C120h)]  
  25. 0000000100001AC8 8A 04 02             mov         al,byte ptr [rdx+rax]  
  26. 0000000100001ACB 88 01                mov         byte ptr [rcx],al  
  27.  
  28.       HexPtr^ := HexDigits[p^ and $F];             { second nibble            }
  29. 0000000100001ACD 48 8B 85 50 FF FF FF mov         rax,qword ptr [P]  
  30.  
  31. // the following statement accesses a word which is _incorrect_
  32.  
  33. 0000000100001AD4 66 8B 00             mov         ax,word ptr [rax]  
  34.  
  35. 0000000100001AD7 66 25 0F 00          and         ax,0Fh  
  36. 0000000100001ADB 0F B7 C0             movzx       eax,ax  
  37. 0000000100001ADE 48 8B 8D 60 FF FF FF mov         rcx,qword ptr [HEXPTR]  
  38. 0000000100001AE5 48 8D 15 34 A6 02 00 lea         rdx,[main+29C50h (010002C120h)]  
  39. 0000000100001AEC 8A 04 02             mov         al,byte ptr [rdx+rax]  
  40. 0000000100001AEF 88 01                mov         byte ptr [rcx],al  
  41.  
the step-at-a-time instruction
Code: Pascal  [Select]
  1. 0000000100001AA4 8A 00                mov         al,byte ptr [rax]  
is correct whereas the one generated for the array indexing expression is not
Code: Pascal  [Select]
  1. 0000000100001AD4 66 8B 00             mov         ax,word ptr [rax]  

Note: I have not tested this using any of the trunk versions of FPC.  Maybe this bug has been corrected (which would be great) but, just in case it has gone unnoticed, I thought it was worth mentioning.

using FPC v3.0.4 and Lazarus 1.8.2 on Windows 7 64bit.

marcov

  • Global Moderator
  • Hero Member
  • *****
  • Posts: 7073
Re: Likely incorrect code generation in FPC v3.0.4
« Reply #1 on: March 04, 2019, 11:02:36 pm »
Please try to transform it into a full program (doesn't need to be runnable, just compiling, e.g. a console program calling the function), and then post it to the bugtracker together with params.

One of the devs will then try it on trunk.

440bx

  • Hero Member
  • *****
  • Posts: 826
Re: Likely incorrect code generation in FPC v3.0.4
« Reply #2 on: March 04, 2019, 11:13:41 pm »
Please try to transform it into a full program (doesn't need to be runnable, just compiling, e.g. a console program calling the function), and then post it to the bugtracker together with params.

One of the devs will then try it on trunk.
I understand the reason for your request but, the real problem is that even with a complete program the problem isn't easy to reproduce because it only happens when the last byte of the memory block happens to also be the last accessible byte in the block.

As I was typing the above, what came to mind is to allocate a 1 page block in the middle of reserved pages.  That would ensure that the last byte is also the last accessible byte.  I'll post a test program soon (I have something else to take care of right now).

All that said, the expression
Code: Pascal  [Select]
  1. HexPtr^ := HexDigits[p^ and $F];
should NOT generate a word access, if the generated code accesses a word then, it is wrong. p is a pbyte, p^ cannot access a word.
using FPC v3.0.4 and Lazarus 1.8.2 on Windows 7 64bit.

marcov

  • Global Moderator
  • Hero Member
  • *****
  • Posts: 7073
Re: Likely incorrect code generation in FPC v3.0.4
« Reply #3 on: March 04, 2019, 11:26:01 pm »
I understand the reason for your request but, the real problem is that even with a complete program the problem isn't easy to reproduce because it only happens when the last byte of the memory block happens to also be the last accessible byte in the block.

As said it doesn't have to run or reproduce the crash. Just the 16-bits mov must be visible where it shouldn't in the asm output.

440bx

  • Hero Member
  • *****
  • Posts: 826
Re: Likely incorrect code generation in FPC v3.0.4
« Reply #4 on: March 05, 2019, 12:46:30 am »
As said it doesn't have to run or reproduce the crash. Just the 16-bits mov must be visible where it shouldn't in the asm output.
The attached program reliably reproduces the crash and the assembly code clearly shows that a word, instead of a byte, is being accessed.

ETA

reported on Mantis, ticket 0035187
« Last Edit: March 05, 2019, 01:30:16 am by 440bx »
using FPC v3.0.4 and Lazarus 1.8.2 on Windows 7 64bit.

Cyrax

  • Hero Member
  • *****
  • Posts: 727
Re: Likely incorrect code generation in FPC v3.0.4
« Reply #5 on: March 05, 2019, 11:39:30 am »

440bx

  • Hero Member
  • *****
  • Posts: 826
Re: Likely incorrect code generation in FPC v3.0.4
« Reply #6 on: March 05, 2019, 11:50:51 am »
Link to the bug report : https://bugs.freepascal.org/view.php?id=35187
I should have probably included that ;)  Thank you for the additions you made to the bug report.
using FPC v3.0.4 and Lazarus 1.8.2 on Windows 7 64bit.