Free Basic Compilers Version 1.06.0 (08-26-2018) seem broken

General FreeBASIC programming questions.
Post Reply
deltarho[1859]
Posts: 4292
Joined: Jan 02, 2017 0:34
Location: UK
Contact:

Re: Free Basic Compilers Version 1.06.0 (08-26-2018) seem broken

Post by deltarho[1859] »

PaulSquires wrote:Well then, just for you
Gosh, I feel ruined. <smile>
Josep Roca
Posts: 564
Joined: Sep 27, 2016 18:20
Location: Valencia, Spain

Re: Free Basic Compilers Version 1.06.0 (08-26-2018) seem broken

Post by Josep Roca »

For the second warning

warning 41(0): Return type mismatch
bi.lpfn = cast(BFFCALLBACK, @AfxBrowseForFolderProc)

that was being reported by the 64 bit compiler only, the cause was that I was wrongly using LRESULT instead of LONG as the return type of the callback function. Changing LRESULT to LONG works fine.

Code: Select all

' ========================================================================================
' Displays a dialog box that enables the user to select a folder.
' - pwszTitle       = A string value that represents the title displayed inside the Browse dialog box.
' - pwszStartFolder = The initial folder that the dialog will show.
' - nFlags          = Optional. A LONG value that contains the options for the method. This can be a
'                     combination of the values listed under the ulFlags member of the BROWSEINFO structure.
'                     See: http://msdn.microsoft.com/en-us/library/windows/desktop/bb773205%28v=vs.85%29.aspx
'                     Default value = BIF_RETURNONLYFSDIRS OR BIF_DONTGOBELOWDOMAIN OR BIF_USENEWUI OR BIF_RETURNFSANCESTORS
' Note: To display the old style dialog, pass -1 in the dwFlags parameter. (Crashes in Windows 10).
' ========================================================================================
' // Browse for folder dialog procedure
PRIVATE FUNCTION AfxBrowseForFolderProc (BYVAL hwnd AS HWND, BYVAL uMsg AS UINT, BYVAL wParam AS WPARAM, BYVAL lParam AS LPARAM) AS LONG

   DIM wszBuffer AS WSTRING * MAX_PATH

   IF uMsg = BFFM_INITIALIZED THEN
      SendMessageW hwnd, BFFM_SETSELECTIONW, CTRUE, cast(LPARAM, lParam)
   ELSEIF uMsg = BFFM_SELCHANGED THEN
      SHGetPathFromIDListW(cast(ITEMIDLIST PTR, wParam), @wszBuffer)
      IF wParam = 0 OR _ ' // No id number
         LEN(wszBuffer) = 0 OR _ ' // No name
         (GetFileAttributesW(wszBuffer) AND FILE_ATTRIBUTE_DIRECTORY) <> FILE_ATTRIBUTE_DIRECTORY OR _ ' // Not a real directory
         MID(wszBuffer, 2, 1) <> ":" THEN ' // Not a local or mapped drive
            SendMessageW hwnd, BFFM_ENABLEOK, FALSE, FALSE
      ELSEIF ((GetFileAttributesW(wszBuffer) AND FILE_ATTRIBUTE_SYSTEM) = FILE_ATTRIBUTE_SYSTEM) AND _
            RIGHT(wszBuffer, 2) <> ":\" THEN   ' // Exclude system folders, allow root directories
         SendMessageW hwnd, BFFM_ENABLEOK, FALSE, FALSE
      END IF
   END IF
   RETURN 0

END FUNCTION

PRIVATE FUNCTION AfxBrowseForFolder (BYVAL hwnd AS HWND, BYVAL pwszTitle AS WSTRING PTR = NULL, _
   BYVAL pwszStartFolder AS WSTRING PTR = NULL, BYVAL nFlags AS LONG = 0) AS CWSTR

   ' // Initialize the COM library
   CoInitialize NULL

   DIM wszBuffer AS WSTRING * MAX_PATH, bi AS BROWSEINFOW, pidl AS ITEMIDLIST PTR
   IF nFlags = 0 THEN nFlags = BIF_RETURNONLYFSDIRS OR BIF_DONTGOBELOWDOMAIN OR BIF_USENEWUI OR BIF_RETURNFSANCESTORS
   ' // Crashes in Windows 10
'   IF nFlags = -1 THEN nFlags = BIF_RETURNONLYFSDIRS OR BIF_DONTGOBELOWDOMAIN OR BIF_RETURNFSANCESTORS

   bi.hWndOwner = hwnd
   bi.lpszTitle = pwszTitle
   bi.ulFlags   = nFlags
   bi.lpfn      = cast(BFFCALLBACK, @AfxBrowseForFolderProc)
   bi.lParam    = cast(LPARAM, pwszStartFolder)
   pidl         = SHBrowseForFolderW(@bi)

   IF pidl THEN
      SHGetPathFromIDListW(pidl, @wszBuffer)
      CoTaskMemFree pidl
   END IF
   RETURN wszBuffer

   ' // Uninitialize the COM library
   CoUninitialize

END FUNCTION
' ========================================================================================
So the cause was a small mistake that was being tolerated by the previous versions of the compiler, but now it has become more strict.

As I'm writing reusable code, I always compile with the pedantic option to avoid that users will get unwanted warnings.
Josep Roca
Posts: 564
Joined: Sep 27, 2016 18:20
Location: Valencia, Spain

Re: Free Basic Compilers Version 1.06.0 (08-26-2018) seem broken

Post by Josep Roca »

For the first warning, I was using WNDPROC in the method prototype:

Code: Select all


PRIVATE FUNCTION CWindow.AddControl ( _
   BYREF wszClassName AS WSTRING, _                       ' // Class name
   BYVAL hParent AS HWND = NULL, _                        ' // Parent window handle
   BYVAL cID AS LONG_PTR = 0, _                           ' // Control identifier
   BYREF wszTitle AS WSTRING = "", _                      ' // Control caption
   BYVAL x AS LONG = 0, _                                 ' // Horizontal position
   BYVAL y AS LONG = 0, _                                 ' // Vertical position
   BYVAL nWidth AS LONG = 0, _                            ' // Control width
   BYVAL nHeight AS LONG = 0, _                           ' // Control height
   BYVAL dwStyle AS LONG = -1, _                          ' // Control style
   BYVAL dwExStyle AS LONG = -1, _                        ' // Extended style
   BYVAL lpParam AS LONG_PTR = 0, _                       ' // Pointer to custom data
   BYVAL pWndProc AS WNDPROC = NULL, _                    ' // Address of the window callback procedure
   BYVAL uIdSubclass AS UINT_PTR = &HFFFFFFFF, _          ' // The subclass ID
   BYVAL dwRefData as DWORD_PTR = NULL _                  ' // Pointer to reference data
   ) AS HWND                                              ' // Control handle
and inside the method, I was trying to cast it to SUBCLASSPROC, to comply with the definition for the SetWindowSubclass function:

Code: Select all

SetWindowSubclass hCtl, CAST(SUBCLASSPROC, pWndProc), uIdSubclass, dwRefData
Changing the parameter to BYVAL pWndProc AS SUBCLASSPROC and the call to SetWindowSubclass hCtl, pWndProc, uIdSubclass, dwRefData solves the issue.

This behavior makes sense if the purpose is to make the compiler yet more strict, but it caught me by surprise. I understand that it will issue a warning if I pass a wrong type, but I think that CAST(SUBCLASSPROC, pWndProc) should work.
coderJeff
Site Admin
Posts: 4313
Joined: Nov 04, 2005 14:23
Location: Ontario, Canada
Contact:

Re: Free Basic Compilers Version 1.06.0 (08-26-2018) seem broken

Post by coderJeff »

Josep Roca wrote:This behavior makes sense if the purpose is to make the compiler yet more strict, but it caught me by surprise. I understand that it will issue a warning if I pass a wrong type, but I think that CAST(SUBCLASSPROC, pWndProc) should work.
Thanks again for the report. The purpose is provide better type checking, but not necessarily more strict. There were several related bugs on sf.net where fbc was not checking anything, resulting in bad code.

I will have an update soon as possible so CAST won't complain about incompatible function pointers. There's 4 things checked:
- return data type
- return method (byref/byval)
- calling convention (cdecl/stdcal/etc)
- number of arguments

Like in deltarho's example with GetProcAddress() returning FARPROC:
- on win-32, returns function() as long
- on win-64, returns function() as longint
Almost guaranteed caller will need to cast the result to some other function type.

Anywhere else (not a CAST), then should still do the checks for compatibility, where function pointer would be implicitly converted.
fxm
Moderator
Posts: 12083
Joined: Apr 22, 2009 12:46
Location: Paris suburbs, FRANCE

Re: Free Basic Compilers Version 1.06.0 (08-26-2018) seem broken

Post by fxm »

Let's be reasonable

1)
The "Argument count mismatch" remaining warning, even after full casting, corresponds generally to cases inducing crashing (crash from stack). Such warnings are justified (they could have been reported, even as errors).
Example:

Code: Select all

Sub test (Byref s As String, Byval I As Integer)
  Print s, I
End Sub

Dim As Sub (Byref As String) p
p = Cast(Sub (Byref As String), @test)

Do
  p("FreeBASIC")
Loop

Sleep  
- Without the casting at line 6, 2 warnings:
warning 4(1): Suspicious pointer assignment
warning 43(0): Argument count mismatch
- With the casting at line 6, 1 warning remains (the one corresponding to the potential runtime error:
warning 43(0): Argument count mismatch

Output:

Code: Select all

FreeBASIC      16600
FreeBASIC      0
FreeBASIC      84
FreeBASIC      9901441
FreeBASIC      9906784
ÿ■↓

Aborting due to runtime error 12 ("segmentation violation" signal) in C:\Users\fxmam\Documents\Mes Outils Personnels\FBIde0.4.6r4_fbc1.06.0\FBIDETEMP.bas::()
2)
At opposite, the "Return type mismatch" remaining warning seems not to induce crashing (perhaps because the possibility to ignore a function return is already taken into account).
It could be reported, as pedantic warning only.
coderJeff
Site Admin
Posts: 4313
Joined: Nov 04, 2005 14:23
Location: Ontario, Canada
Contact:

Re: Free Basic Compilers Version 1.06.0 (08-26-2018) seem broken

Post by coderJeff »

Hm. yea, the checks are generally unsafe, but not always. There's some choices. '-w pedantic' or '-w funcptr' for example because I think it would still be helpful to have checks even inside a CAST, and don't want to get rid of that. The type checks are useful, but not always, so how to express to fbc. There was a short discussion on sf.net #642 about have CAST be the safe/strict conversion giving warnings/errors, and having CPTR be more relaxed.
Josep Roca
Posts: 564
Joined: Sep 27, 2016 18:20
Location: Valencia, Spain

Re: Free Basic Compilers Version 1.06.0 (08-26-2018) seem broken

Post by Josep Roca »

But the "Argument count mismatch" warning confused because I was passing the right number of arguments.
coderJeff
Site Admin
Posts: 4313
Joined: Nov 04, 2005 14:23
Location: Ontario, Canada
Contact:

Re: Free Basic Compilers Version 1.06.0 (08-26-2018) seem broken

Post by coderJeff »

I'm not sure which example: SUBCLASSPROC does have a different number of arguments than WNDPROC. Argument count mismatch would be expected.

In this example:

Code: Select all

Dim pProc As BCryptGenRandomProc = Cast(BCryptGenRandomProc, GetProcAddress(hLib, "BCryptGenRandom")) ' OFFENDING LINE
- on win-32, GetProcAddress() returns function() as long
- on win-64, GetProcAddress() returns function() as longint

NTSTATUS is long on both 32/64 bit. So when CASTing to a function returning NTSTATUS it depends on if fbc is compiling 32 or 64 bit, because the function pointer type check that fbc is doing, only reports the first incompatibility. So when I tried deltarho's example, I would get
In 32-bit, I get "Argument count mismatch"
In 64-bit, I get "Return type mismatch"

EDIT:
The messages will get updated to be:
Return type mismatch in function pointer
Calling convention mismatch in function pointer
Argument count mismatch in function pointer
jj2007
Posts: 2326
Joined: Oct 23, 2016 15:28
Location: Roma, Italia
Contact:

Re: Free Basic Compilers Version 1.06.0 (08-26-2018) seem broken

Post by jj2007 »

deltarho[1859] wrote:I want to see the file size
You are giving me ideas for my IDE: File size in the statusbar; maybe even a warning if it's much different from the expected one calculated as fsize=(lines of code)*x+overhead. Thanks, I got a project for today ;-)
St_W
Posts: 1619
Joined: Feb 11, 2009 14:24
Location: Austria
Contact:

Re: Free Basic Compilers Version 1.06.0 (08-26-2018) seem broken

Post by St_W »

I expected that the FB CAST behaves similar to a cast in C (or at least not more strict than in C). So if you'd implement my previous example (see https://freebasic.net/forum/viewtopic.p ... 52#p251252) in C it could look like the one below. The thing is that C doesn't complain about the cast because it's ... a cast. It only complains (with a warning) when trying to use the incompatible function pointers directly without casting.

Code: Select all

typedef int delegate1 (int);

int dummy(delegate1 a, int b) { }

void oneparam (int first) { }

int main() {
    dummy((delegate1*)&dummy, 0);
    dummy((delegate1*)&oneparam, 0);

    dummy(&dummy, 0);
    dummy(&oneparam, 0);

    return 0;
}
Also note the nice type info we get from GCC, it would be nice if FB would emit similar additional error/warning infos.

Code: Select all

$ gcc test.c -Wall
test.c: In function 'main':
test.c:12:11: warning: passing argument 1 of 'dummy' from incompatible pointer type [-Wincompatible-pointer-types]
     dummy(&dummy, 0);
           ^
test.c:4:5: note: expected 'int (*)(int)' but argument is of type 'int (*)(int (*)(int), int)'
 int dummy(delegate1 a, int b) { }
     ^~~~~
test.c:13:11: warning: passing argument 1 of 'dummy' from incompatible pointer type [-Wincompatible-pointer-types]
     dummy(&oneparam, 0);
           ^
test.c:4:5: note: expected 'int (*)(int)' but argument is of type 'void (*)(int)'
 int dummy(delegate1 a, int b) { }
     ^~~~~
In FB with the recent change one can only avoid the warning with an additional cast e.g. like below, which is really not BASIC-like. (Of course I've to admit that the cast in the example doesn't make sense and easily could make the application crash, but in practice there probably are valid reasons for doing things like that)

Code: Select all

type delegate1 as function( a as integer ) as integer

function dummy ( a as delegate1, another as integer ) as integer
  return 1
end function

dummy(cast(delegate1, cast(any ptr, @dummy)), 0)     ' no warning
deltarho[1859]
Posts: 4292
Joined: Jan 02, 2017 0:34
Location: UK
Contact:

Re: Free Basic Compilers Version 1.06.0 (08-26-2018) seem broken

Post by deltarho[1859] »

Thanks St_W

Code: Select all

Dim pProc As BCryptGenRandomProc = Cast(BCryptGenRandomProc, Cast(Any Ptr, GetProcAddress(hLib, "BCryptGenRandom")))
works in both 32 bit and 64 bit.

On my Start menu I now have:

Code: Select all

WinFBE32 - as per Paul's download without alteration
WinFBE32LB - WinFBE32 using latest fbc 1.06 32 & 64 bit (27-Aug-2018)
WinFBE3281 - WinFBE32 using gcc 8.1 32 & 64 bit (28-Aug-2018)
I wasn't even tempted to build WinFBE32LB81 figuring that shortly after I'd get a knock on the door by two guys in white coats. <smile>

Working on the principle that "If you don't ask, you don't get" I have noticed that gcc 8.2 was released on 26-Jul-2018 <cough>
deltarho[1859]
Posts: 4292
Joined: Jan 02, 2017 0:34
Location: UK
Contact:

Re: Free Basic Compilers Version 1.06.0 (08-26-2018) seem broken

Post by deltarho[1859] »

On second thoughts.

From Help we have "A pure Any Ptr has no type checking by the compiler."

So we have a new stricter CAST regime and get around that by eliminating type checking by using Any Ptr. Gotta laugh, haven't we?

Code: Select all

Dim pProc As BCryptGenRandomProc = Cast(BCryptGenRandomProc, Cast(Any Ptr, GetProcAddress(hLib, "BCryptGenRandom")))
If we replace 'Any' with "Integer" then we get no warnings either in 32 or 64 bit and maintain type checking.
St_W wrote:type delegate1 as function( a as integer ) as integer

function dummy ( a as delegate1, another as integer ) as integer
return 1
end function

sub oneparam ( first as integer )
end sub

dummy(cast(delegate1, @dummy), 0) ' warning 43(0): Argument count mismatch
dummy(cptr(delegate1, @dummy), 0) ' warning 43(0): Argument count mismatch
dummy(cast(delegate1, @oneparam), 0) ' warning 41(0): Return type mismatch
dummy(cptr(delegate1, @oneparam), 0) ' warning 41(0): Return type mismatch

dummy(cast(delegate1, 0), 0) ' no warning
dummy(0, 0) ' no warning
and then
St_W wrote:type delegate1 as function( a as integer ) as integer

function dummy ( a as delegate1, another as integer ) as integer
return 1
end function

dummy(cast(delegate1, cast(any ptr, @dummy)), 0) ' no warning
On the other hand, replacing 'Any' with 'Integer'

Code: Select all

type delegate1 as function( a as integer ) as integer
 
function dummy ( a as delegate1, another as integer ) as integer
  return 1
end function
 
sub oneparam ( first as integer )
end sub
 
dummy(cast(delegate1, cast(Integer Ptr, @dummy)), 0)     ' no warning
dummy(cptr(delegate1, cast(Integer Ptr, @dummy)), 0)     ' no warning
dummy(cast(delegate1, cast(Integer Ptr, @oneparam)), 0)  ' no warning
dummy(cptr(delegate1, cast(Integer Ptr, @oneparam)), 0)  ' no warning
 
dummy(cast(delegate1, 0), 0)          ' no warning
dummy(0, 0)                           ' no warning
... or do I need locking up? <smile>
deltarho[1859]
Posts: 4292
Joined: Jan 02, 2017 0:34
Location: UK
Contact:

Re: Free Basic Compilers Version 1.06.0 (08-26-2018) seem broken

Post by deltarho[1859] »

Code: Select all

SetWindowSubclass hCtl, CAST(SUBCLASSPROC, pWndProc), uIdSubclass, dwRefData
Change to

Code: Select all

SetWindowSubclass hCtl, CAST(SUBCLASSPROC, Cast(Integer Ptr, pWndProc)), uIdSubclass, dwRefData
 
coderJeff
Site Admin
Posts: 4313
Joined: Nov 04, 2005 14:23
Location: Ontario, Canada
Contact:

Re: Free Basic Compilers Version 1.06.0 (08-26-2018) seem broken

Post by coderJeff »

deltarho[1859] wrote:Gotta laugh, haven't we?
Heh, we have. :)

To disable warnings completely, compile with '-w 3'. Currently, there are no warning messages having level 3 or higher, so '-w 3' effectively quiets all warnings. See Compiler Option -w.
coderJeff
Site Admin
Posts: 4313
Joined: Nov 04, 2005 14:23
Location: Ontario, Canada
Contact:

Re: Free Basic Compilers Version 1.06.0 (08-26-2018) seem broken

Post by coderJeff »

St_W wrote:I expected that the FB CAST behaves similar to a cast in C (or at least not more strict than in C)....
I made a PR https://github.com/freebasic/fbc/pull/94 to address the immediate complaints. So,
- only '-w funcptr' will enable the function pointer warnings
- "in function pointer" appended to new warning messages

The more serious issue is the lack of const qualifier checking so it's easy to write code that drops constness and modifies data that is supposed to be const. And to reliably check const, need a somewhat reliable way to check function pointer types. Function pointers was to be the starting point.

Need to go a different route here, If CAST()/CPTR() causes a warning, even if optionally (for example if with '-w pedantic') then there needs to be a sensible way to formally express it another way. Otherwise, there's no way to not get the warning. Herein lies the problem. The only way we have to express these explicit function pointer conversions is CAST()/CPTR().

It would be helpful to have one of CAST() or CPTR() type checked with '-w pedantic'.
Post Reply