Recent

Author Topic: The declaration of WriteFile, ReadFile win32 api may be incorrect.  (Read 1996 times)

marunguy

  • New Member
  • *
  • Posts: 23
The declaration of WriteFile, ReadFile win32 api may be incorrect.

Window 10 64bit, Lazarus 2.2.0 x86_64-win64-win32/win64, FPC 3.2.2

* original declaration
Code: Pascal  [Select][+][-]
  1. // lazarus\fpc\3.2.2\source\rtl\win\wininc\redef.inc
  2. function WriteFile(hFile: THandle; const Buffer; nNumberOfBytesToWrite: DWORD; var lpNumberOfBytesWritten: DWORD; lpOverlapped: POverlapped): BOOL; external 'kernel32' name 'WriteFile';
  3. function ReadFile(hFile: THandle; var Buffer; nNumberOfBytesToRead: DWORD; var lpNumberOfBytesRead: DWORD; lpOverlapped: POverlapped): BOOL; external 'kernel32' name 'ReadFile';
Code: Pascal  [Select][+][-]
  1. Windows.WriteFile(my_hdl, buf, buf_len, written_bytes, nil)
  2. Windows.ReadFile(my_hdl, buf, buf_len, read_bytes, nil);

Invalid Buffer value is passed in win64 build mode.
The access viloation exception occurs in win32 build mode when ReadFile is called.

* first fix - change type of Buffer.
Code: Pascal  [Select][+][-]
  1. // mykernel32.pas
  2. function WriteFile(hFile: THandle; const Buffer: Pointer; const nNumberOfBytesToWrite: DWORD; var lpNumberOfBytesWritten: DWORD; lpOverlapped: POverlapped): BOOL; external 'kernel32' name 'WriteFile';
  3. function ReadFile(hFile: THandle; Buffer: Pointer; const nNumberOfBytesToRead: DWORD; var lpNumberOfBytesRead: DWORD; lpOverlapped: POverlapped): BOOL; external 'kernel32' name 'ReadFile';
Code: Pascal  [Select][+][-]
  1. mykernel32.WriteFile(my_hdl, buf, buf_len, written_bytes, nil)
  2. mykernel32.ReadFile(my_hdl, buf, buf_len, read_bytes, nil);

It works well in win64 build mode.
Sometimes and exception occurs in win32 build mode

* second fix - change type of Buffer, lpNumberOfBytesWritten and lpNumberOfBytesRead, add stdcall
Code: Pascal  [Select][+][-]
  1. // mykernel32.pas
  2. function WriteFile(hFile: THandle; const Buffer: Pointer; const nNumberOfBytesToWrite: DWORD; lpNumberOfBytesWritten: PDWORD; lpOverlapped: POverlapped): BOOL; stdcall; external 'kernel32' name 'WriteFile';
  3. function ReadFile(hFile: THandle; Buffer: Pointer; const nNumberOfBytesToRead: DWORD; lpNumberOfBytesRead: PDWORD; lpOverlapped: POverlapped): BOOL; stdcall; external 'kernel32' name 'ReadFile';
Code: Pascal  [Select][+][-]
  1. mykernel32.WriteFile(my_hdl, buf, buf_len, @written_bytes, nil)
  2. mykernel32.ReadFile(my_hdl, buf, buf_len, @read_bytes, nil);

It works well in both win32 and win64 build mode.

marcov

  • Administrator
  • Hero Member
  • *
  • Posts: 10166
  • FPC developer.
Re: The declaration of WriteFile, ReadFile win32 api may be incorrect.
« Reply #1 on: May 19, 2022, 10:35:12 am »
A change that requires calls to be changed (like @) is not acceptable,
moreover the current declaration is Delphi compatible. (redef.inc was for delphi compatibility)

Note that the windows unit  (rtl/win<xxx>/windows.pp contains {$calling stdcall} which changes the whole unit's calling convention to stdcall.

I'm also not sure what this would exactly improve. The declarations are the same, pascal call by var is equivalent to call with a pointer.

These calls have been declared like this since probably 1997, and are not exactly the rarest used calls.


« Last Edit: May 19, 2022, 10:45:08 am by marcov »

AlexTP

  • Hero Member
  • *****
  • Posts: 1843
    • UVviewsoft
Re: The declaration of WriteFile, ReadFile win32 api may be incorrect.
« Reply #2 on: May 19, 2022, 10:39:22 am »
Maybe post small example app, which shows access-violations for the current FPC code?

440bx

  • Hero Member
  • *****
  • Posts: 2931
Re: The declaration of WriteFile, ReadFile win32 api may be incorrect.
« Reply #3 on: May 19, 2022, 11:21:44 am »
The declaration of WriteFile, ReadFile win32 api may be incorrect.
It is incorrect but, you won't get it fixed.  The incorrect definition has been in use for years and correcting it would likely break a fair amount of code, it's not going to happen.

I reported that problem over a year ago and had a long and, totally fruitless, argument about it.  That's all you can get. <chuckle> if you care to read "the entertaining" part of the discussion, it's at : https://forum.lazarus.freepascal.org/index.php/topic,46185.msg329022.html#msg329022

Your consolation prize is, you're right.  Unfortunately, it doesn't matter.
FPC v3.0.4 and Lazarus 1.8.2 on Windows 7 64bit.

trev

  • Global Moderator
  • Hero Member
  • *****
  • Posts: 1983
  • Former Delphi 1-7, 10.2 user
Re: The declaration of WriteFile, ReadFile win32 api may be incorrect.
« Reply #4 on: May 19, 2022, 12:31:26 pm »
From the previous thread - PascalDragon:
Quote
What I do grant you however is that we don't have the equivalent declaration as an overload. Those should in my opinion definitely be there as those declarations with untyped parameters or typed var parameters are considered "convenience" declarations and thus an "add on".

If that were done would it not make everyone happy? I guess someone needs to submit a patch...

Lazarus 2.3, FPC 3.3.1 macOS 12.3.1 x86_64 Xcode 13.4
Lazarus 2.3, FPC 3.3.1 macOS 12.3.1 aarch64 Xcode 13.4

tetrastes

  • Full Member
  • ***
  • Posts: 221
Re: The declaration of WriteFile, ReadFile win32 api may be incorrect.
« Reply #5 on: May 19, 2022, 02:08:53 pm »
Code: Pascal  [Select][+][-]
  1. Windows.WriteFile(my_hdl, buf, buf_len, written_bytes, nil)
  2. Windows.ReadFile(my_hdl, buf, buf_len, read_bytes, nil);

Invalid Buffer value is passed in win64 build mode.
The access viloation exception occurs in win32 build mode when ReadFile is called.

You didn't write what were your parameters, and how you passed them exactly.
This is how it is done e.g. in synaser.pas:
Code: Pascal  [Select][+][-]
  1. var
  2. FHandle: THandle;
  3. Buffer: pointer;
  4. Length: longint;
  5. Result: longint;
  6. Overlapped: TOverlapped;
  7.  
  8. ...
  9.  
  10. WriteFile(FHandle, Buffer^, Length, DWord(Result), @Overlapped);
  11. ReadFile(FHandle, Buffer^, Length, Dword(Result), @Overlapped);

and it works for more than twenty years without any problem.

440bx

  • Hero Member
  • *****
  • Posts: 2931
Re: The declaration of WriteFile, ReadFile win32 api may be incorrect.
« Reply #6 on: May 19, 2022, 02:40:00 pm »
and it works for more than twenty years without any problem.
and, it could work for 20,000 years and it would still be wrong because it should NOT be necessary to dereference the Buffer variable.

Also, since the variable Buffer is just a "pointer", what is Buffer^ supposed to be pointing to ?... a byte ?.. a word ?... a qword ?... mother Theresa ?... the neighbor's dog ? ... semantically, dereferencing a generic pointer is meaningless since it doesn't point to a known type.

@Trev,

yes, an overload (using the correct definition) would definitely be a good thing in this case and, it seems its presence would not break any existing code.
FPC v3.0.4 and Lazarus 1.8.2 on Windows 7 64bit.

tetrastes

  • Full Member
  • ***
  • Posts: 221
Re: The declaration of WriteFile, ReadFile win32 api may be incorrect.
« Reply #7 on: May 19, 2022, 03:59:31 pm »
and it works for more than twenty years without any problem.
and, it could work for 20,000 years and it would still be wrong because it should NOT be necessary to dereference the Buffer variable.

Also, since the variable Buffer is just a "pointer", what is Buffer^ supposed to be pointing to ?... a byte ?.. a word ?... a qword ?... mother Theresa ?... the neighbor's dog ? ... semantically, dereferencing a generic pointer is meaningless since it doesn't point to a known type.
OP started this thread because he got errors, and supposed that these errors were due to incorrect declarations. I simply gave him example of code (from heavily used library) using these declarations without errors.
I can hardly suppose that you don't know, that you can use these declarations without explicit dereferencing. You think that such declaration is wrong because it leads to some overhead in compiled code? It depends on compiler, I think, and doesn't mean that it is wrong, at least in Pascal.

And of course Buffer points to something valid somewhere in the real code. This is not complete compilable example, as you may guess.  ;)
« Last Edit: May 19, 2022, 04:02:05 pm by tetrastes »

marcov

  • Administrator
  • Hero Member
  • *
  • Posts: 10166
  • FPC developer.
Re: The declaration of WriteFile, ReadFile win32 api may be incorrect.
« Reply #8 on: May 19, 2022, 04:41:01 pm »
and it works for more than twenty years without any problem.
and, it could work for 20,000 years and it would still be wrong because it should NOT be necessary to dereference the Buffer variable.

"wrong" is a relative term here. Code that hits this is wrong, since it is incompatible with both FPC and Delphi declarations. So the original code is simply wrong, or from a very deviant Pascal dialect.

Quote
Also, since the variable Buffer is just a "pointer", what is Buffer^ supposed to be pointing to ?... a byte ?..
a word ?... a qword ?... mother Theresa ?... the neighbor's dog ? ... semantically, dereferencing a generic pointer is meaningless since it doesn't point to a known type.

The equivalent of void, which is easy to see as sizeof(p^)=0 if you print it in both FPC and Delphi. Though in this case it never really is dereferenced, but passed to a formal parameter (which is byref).

So please don't deref Mother Theressa and let her rest :-)

Quote
a word ?... a qword ?... mother Theresa ?... the neighbor's dog ? ... semantically, dereferencing a generic pointer is meaningless since it doesn't point to a known type.

Can you do it in C ?  * a void* ?     But I assume it is less useful there as C has no formal parameters. But keep in mind that pascal's  move() also has formal parameters, as do blockread/write etc.

Quote
yes, an overload (using the correct definition) would definitely be a good thing in this case and, it seems its presence would not break any existing code.

Overloading more than one difference is somewhat dangerous, because people get weird errormessages if they pass e.g. a pointer and don't @ the other etc. 

Normally I wouldn't mind, but by such a core procedure I get a tad conservative. It has the potential to cause problems without much gain.

marunguy

  • New Member
  • *
  • Posts: 23
Re: The declaration of WriteFile, ReadFile win32 api may be incorrect.
« Reply #9 on: May 20, 2022, 09:51:41 am »
Windows.WriteFile sample source

Windows 10 64bit, Lazarus 2.2.2 64bit, FPC 3.2.2

I'm a lazarus beginner and there may be mistakes in my sample code.

When use Windows.WriteFile, garbage values are written to the file.
When use MyWriteFile, correct values are written to the file.

* hexdump test.txt - Windows.WriteFile
Code: Pascal  [Select][+][-]
  1. 80 5E 03 00 01 00 00 00 2C 01

* hexdump test.txt - MyWriteFile
Code: Pascal  [Select][+][-]
  1. 30 31 32 33 34 35 36 37 38 39

* sample code
Code: Pascal  [Select][+][-]
  1. unit unitmain;
  2.  
  3. {$mode ObjFPC}{$H+}
  4.  
  5. interface
  6.  
  7. uses
  8.     Classes, SysUtils, Windows;
  9.  
  10. function MyWriteFile(hFile: THandle; const Buffer: Pointer; const nNumberOfBytesToWrite: DWORD; lpNumberOfBytesWritten: PDWORD; lpOverlapped: POverlapped): BOOL; stdcall; external 'kernel32' name 'WriteFile';
  11.  
  12. procedure test_writefile();
  13.  
  14. implementation
  15.  
  16. procedure test_writefile();
  17. var
  18.     file_hdl: HANDLE = 0;
  19.     buffer: PChar = '0123456789';
  20.     written: DWORD = 0;
  21. begin
  22.     file_hdl := Windows.CreateFileW(PWideChar(UnicodeString('test.txt')), GENERIC_WRITE, 0, nil, CREATE_ALWAYS, FILE_ATTRIBUTE_NORMAL, 0);
  23.     if file_hdl = INVALID_HANDLE_VALUE then
  24.     begin
  25.         WriteLn(Format('failed to CreateFileW, LE=%d', [GetLastError()]));
  26.         exit;
  27.     end;
  28.  
  29.     try
  30.         if not Windows.WriteFile(file_hdl, buffer, Length(buffer), written, nil) then
  31.         // if not MyWriteFile(file_hdl, buffer, Length(buffer), @written, nil) then
  32.         begin
  33.             WriteLn(Format('failed to WriteFile, LE=%d', [GetLastError()]));
  34.             exit;
  35.         end;
  36.     finally
  37.         Windows.CloseHandle(file_hdl);
  38.     end;
  39.  
  40. end;
  41.  
  42. initialization
  43.  
  44. test_writefile();
  45.  
  46. end.
  47.  

marunguy

  • New Member
  • *
  • Posts: 23
Re: The declaration of WriteFile, ReadFile win32 api may be incorrect.
« Reply #10 on: May 20, 2022, 09:55:12 am »
Windows.ReadFile sample source

Windows 10 64bit, Lazarus 2.2.2 64bit, FPC 3.2.2

I'm a lazarus beginner and there may be mistakes in my sample code.

It is not possible to write a Windows.ReadFile sample source that reproduce the crash situation.
It seems to have been moved to the side by the size of the pointer.
However, garbage values are read into the buffer when Windows.ReadFile is called.

* test.txt contents
Code: Pascal  [Select][+][-]
  1. 01234567890123456789012345678901

* win64 build mode output - Windows.ReadFile
Code: Pascal  [Select][+][-]
  1. read_num=32
  2. buffer=38 39 30 31 32 33 34 35

* win32 build mode output - Windows.ReadFile
Code: Pascal  [Select][+][-]
  1. read_num=32
  2. buffer=34 35 36 37 38 39 30 31

* win32/win64 build mode output - MyReadFile

Code: Pascal  [Select][+][-]
  1. read_num=32
  2. buffer=30 31 32 33 34 35 36 37

* sample code
Code: Pascal  [Select][+][-]
  1. unit unitmain;
  2.  
  3. {$mode ObjFPC}{$H+}
  4.  
  5. interface
  6.  
  7. uses
  8.     Classes, SysUtils, Windows;
  9.  
  10. function MyReadFile(hFile: THandle; Buffer: Pointer; const nNumberOfBytesToRead: DWORD; lpNumberOfBytesRead: PDWORD; lpOverlapped: POverlapped): BOOL; stdcall; external 'kernel32' name 'ReadFile';
  11.  
  12. procedure test_readfile();
  13.  
  14. implementation
  15.  
  16. procedure test_readfile();
  17. var
  18.     file_hdl: HANDLE = 0;
  19.     buffer: array[0..31] of Byte;
  20.     ptr: pointer;
  21.     read_num: DWORD = 0;
  22. begin
  23.     file_hdl := Windows.CreateFileW(PWideChar(UnicodeString('test.txt')), GENERIC_READ, 0, nil, OPEN_EXISTING, FILE_ATTRIBUTE_NORMAL, 0);
  24.     if file_hdl = INVALID_HANDLE_VALUE then
  25.     begin
  26.         WriteLn(Format('failed to CreateFileW, LE=%d', [GetLastError()]));
  27.         exit;
  28.     end;
  29.  
  30.     try
  31.         ptr := @buffer[0];
  32.         if not Windows.ReadFile(file_hdl, ptr, 32, read_num, nil) then
  33.         //if not MyReadFile(file_hdl, ptr, 32, @read_num, nil) then
  34.         begin
  35.             WriteLn(Format('failed to ReadFile, LE=%d', [GetLastError()]));
  36.             exit;
  37.         end;
  38.  
  39.         WriteLn(Format('read_num=%d', [read_num]));
  40.         WriteLn(Format('buffer=%x %x %x %x %x %x %x %x',
  41.             [buffer[0], buffer[1], buffer[2], buffer[3], buffer[4], buffer[5], buffer[6], buffer[7]]));
  42.     finally
  43.         Windows.CloseHandle(file_hdl);
  44.     end;
  45.  
  46. end;
  47.  
  48. initialization
  49.  
  50. test_readfile();
  51.  
  52. end.
  53.  

marunguy

  • New Member
  • *
  • Posts: 23
Re: The declaration of WriteFile, ReadFile win32 api may be incorrect.
« Reply #11 on: May 20, 2022, 10:10:32 am »
Code: Pascal  [Select][+][-]
  1. Windows.WriteFile(my_hdl, buf, buf_len, written_bytes, nil)
  2. Windows.ReadFile(my_hdl, buf, buf_len, read_bytes, nil);

Invalid Buffer value is passed in win64 build mode.
The access viloation exception occurs in win32 build mode when ReadFile is called.

You didn't write what were your parameters, and how you passed them exactly.
This is how it is done e.g. in synaser.pas:
Code: Pascal  [Select][+][-]
  1. var
  2. FHandle: THandle;
  3. Buffer: pointer;
  4. Length: longint;
  5. Result: longint;
  6. Overlapped: TOverlapped;
  7.  
  8. ...
  9.  
  10. WriteFile(FHandle, Buffer^, Length, DWord(Result), @Overlapped);
  11. ReadFile(FHandle, Buffer^, Length, Dword(Result), @Overlapped);

and it works for more than twenty years without any problem.

buf type is pointer in my code;
Code: Pascal  [Select][+][-]
  1. buf: pointer;

When modify my code(buf  -> buf^), Windows.WriteFile, ReadFile works correctly.
Code: Pascal  [Select][+][-]
  1. Windows.WriteFile(my_hdl, buf^, buf_len, written_bytes, nil)
  2. Windows.ReadFile(my_hdl, buf^, buf_len, read_bytes, nil);

But I don't understand the difference between 'buf' and 'buf^'.

440bx

  • Hero Member
  • *****
  • Posts: 2931
Re: The declaration of WriteFile, ReadFile win32 api may be incorrect.
« Reply #12 on: May 20, 2022, 10:13:39 am »
the problem is in this line of code:
Code: Pascal  [Select][+][-]
  1. if not Windows.WriteFile(file_hdl, buffer, Length(buffer), written, nil) then

what that line of code does is pass @buffer to Windows.WriteFile because the parameter is untyped.  That's what causes the address to be passed instead of the value.

if you change that line of code to read:
Code: Pascal  [Select][+][-]
  1. if not Windows.WriteFile(file_hdl, buffer^, Length(buffer), written, nil) then
(note that buffer is dereferenced, i.e, buffer^, that should work.

The reason it works with "MyWriteFile" is because your definition is _correct_, i.e, the parameter "buffer" is of type pointer (as it should be) instead of untyped.  That causes the value of buffer (which is a pointer) to be passed to MyWriteFile instead of a pointer to the buffer pointer which is what's passed with Windows.WriteFile.




ETA:

But I don't understand the difference between 'buf' and 'buf^'.
in the call to Windows.WriteFile, buf^ dereferences buf but since the parameter is untyped what ends up being passed is buf, i.e, @buf^ = buf, which is what should have been passed without all these binary contortions.

In the call to MyWriteFile you can simply pass buf because buf is already a pointer to the buffer and, since your definition is correct (buf is a pointer, not an untyped var), it works. 

In the call to Windows.WriteFile, because the definition automatically takes the address of the variable you are passing, you need to dereference the variable you're passing so that when the address is taken, you end up with the value of the variable, in this case, buf.
« Last Edit: May 20, 2022, 10:24:27 am by 440bx »
FPC v3.0.4 and Lazarus 1.8.2 on Windows 7 64bit.

marcov

  • Administrator
  • Hero Member
  • *
  • Posts: 10166
  • FPC developer.
Re: The declaration of WriteFile, ReadFile win32 api may be incorrect.
« Reply #13 on: May 20, 2022, 10:28:29 am »
Note that WriteFileEX does have an overload.

440bx

  • Hero Member
  • *****
  • Posts: 2931
Re: The declaration of WriteFile, ReadFile win32 api may be incorrect.
« Reply #14 on: May 20, 2022, 10:43:43 am »
Note that WriteFileEX does have an overload.
WriteFileEx is defined correctly, that is, the buffer  parameter is of type pointer as it should be.  That's one more reason there should be an overload for WriteFile with "Buffer : pointer" because those two functions are supposed to differ _only_ in the fact that WriteFileEx takes an additional parameter but, all the other parameters they have in common should be identical and, currently, they aren't.

OTH, I understand why the FPC definition is the way it is, WriteFile is incorrectly defined in Delphi's Windows.pas (it's an untyped var just as it is in FPC) and, that "deficiency" was replicated in FPC but, definitions like that make porting C code to Pascal quite a headache when they are used (the code is a perfect translation but, it doesn't work in Pascal because the definition is "creative".)
FPC v3.0.4 and Lazarus 1.8.2 on Windows 7 64bit.

 

TinyPortal © 2005-2018