Silence warnings in win32/win32.c #21925

sisyphus · 2024-02-04T13:04:02Z

In win32/win32.c, this PR silences numerous [-Wunused-parameter] and [-Wunused-variable] warnings; also a [-Wsign-compare], a [-Wdangling-else] and a [-Wparentheses] warning.

The following warnings remain:

win32.c: In function 'set_w32_module_name':
win32.c:243:9: warning: cast between incompatible function types from 'FARPROC' {aka 'long long int (*)()'} to 'DWORD (*)(const WCHAR *, WCHAR *, DWORD)' {aka 'long unsigned int (*)(const short unsigned int *, short unsigned int *, long unsigned int)'} [-Wcast-function-type]
  243 |         (DWORD (__stdcall *)(LPCWSTR, LPWSTR, DWORD))
      |         ^
win32.c: In function 'win32_stat_low':
win32.c:1688:25: warning: cast between incompatible function types from 'FARPROC' {aka 'long long int (*)()'} to 'DWORD (*)(void *, CHAR *, DWORD,  DWORD)' {aka 'long unsigned int (*)(void *, char *, long unsigned int,  long unsigned int)'} [-Wcast-function-type]
 1688 |                         (pGetFinalPathNameByHandleA_t)GetProcAddress(GetModuleHandle("kernel32.dll"), "GetFinalPathNameByHandleA");
      |                         ^
win32.c: In function 'win32_link':
win32.c:3708:40: warning: passing argument 1 to 'restrict'-qualified parameter aliases with argument 2 [-Wrestrict]
 3708 |         ((aTHXa(PERL_GET_THX)), wcscpy(wOldName, PerlDir_mapW(wOldName)),
      |                                        ^~~~~~~~
win32.c: In function 'win32_symlink':
win32.c:3733:9: warning: cast between incompatible function types from 'FARPROC' {aka 'long long int (*)()'} to 'BOOLEAN (*)(const CHAR *, const CHAR *, DWORD)' {aka 'unsigned char (*)(const char *, const char *, long unsigned int)'} [-Wcast-function-type]
 3733 |         (pCreateSymbolicLinkA_t)GetProcAddress(GetModuleHandle("kernel32.dll"), "CreateSymbolicLinkA");
      |         ^
win32.c: In function 'win32_hook_closehandle_in_crt':
win32.c:5370:24: warning: cast between incompatible function types from 'FARPROC' {aka 'long long int (*)()'} to 'BOOL (*)(void *)' {aka 'int (*)(void *)'} [-Wcast-function-type]
 5370 |     CloseHandle_orig = (pCloseHandle)GetProcAddress(
      |                        ^
win32.c:5377:36: warning: cast between incompatible function types from 'BOOL (*)(void *)' {aka 'int (*)(void *)'} to 'INT_PTR (*)()' {aka 'long long int (*)()'} [-Wcast-function-type]
 5377 |         crt_handle, "CloseHandle", (FARPROC)my_CloseHandle
      |                                    ^
win32.c:5380:31: warning: cast between incompatible function types from 'FARPROC' {aka 'long long int (*)()'} to 'NTSTATUS (*)(void *, struct _IO_STATUS_BLOCK *, void *, ULONG,  ULONG)' {aka 'long int (*)(void *, struct _IO_STATUS_BLOCK *, void *, long unsigned int,  long unsigned int)'} [-Wcast-function-type]
 5380 |     pNtQueryInformationFile = (pNtQueryInformationFile_t)GetProcAddress(
      |                               ^
win32.c: In function 'win32_unhook_closehandle_in_crt':
win32.c:5404:36: warning: cast between incompatible function types from 'pCloseHandle' {aka 'int (*)(void *)'} to 'INT_PTR (*)()' {aka 'long long int (*)()'} [-Wcast-function-type]
 5404 |         crt_handle, "CloseHandle", (FARPROC)CloseHandle_orig
      |                                    ^
win32.c: At top level:
win32.c:2250:1: warning: 'wstr_to_str' defined but not used [-Wunused-function]
 2250 | wstr_to_str(const wchar_t* wstr)
      | ^~~~~~~~~~~
....

win32.c: In function 'win32_gets':
win32.c:4742:12: warning: call to 'gets' declared with attribute warning: Using gets() is always unsafe - use fgets() instead [-Wattribute-warning]
 4742 |     return gets(s);
      |            ^~~~~~~

I have queries about the best way to deal with those remaining warnings, and I'm thinking if we can just pick the low hanging fruit for now, it will make it easier to deal with those remaining issues later on.

My perl -V output is:

Summary of my perl5 (revision 5 version 39 subversion 8) configuration:
  Derived from:
  Platform:
    osname=MSWin32
    osvers=10.0.22621.3007
    archname=MSWin32-x64-multi-thread
    uname=''
    config_args='undef'
    hint=recommended
    useposix=true
    d_sigaction=undef
    useithreads=define
    usemultiplicity=define
    use64bitint=define
    use64bitall=undef
    uselongdouble=undef
    usemymalloc=n
    default_inc_excludes_dot=define
  Compiler:
    cc='gcc'
    ccflags ='-Wall -Wextra -Werror=pointer-arith -Wno-format -Wno-long-long -Werror=vla -std=c99 -DWIN32 -DWIN64 -fdiagnostics-color=never -DPERL_TEXTMODE_SCRIPTS -DMULTIPLICITY -DPERL_IMPLICIT_SYS -DUSE_PERLIO -D__USE_MINGW_ANSI_STDIO -fwrapv -fno-strict-aliasing -mms-bitfields'
    optimize='-Os'
    cppflags='-DWIN32'
    ccversion=''
    gccversion='13.2.0'
    gccosandvers=''
    intsize=4
    longsize=4
    ptrsize=8
    doublesize=8
    byteorder=12345678
    doublekind=3
    d_longlong=define
    longlongsize=8
    d_longdbl=define
    longdblsize=16
    longdblkind=3
    ivtype='long long'
    ivsize=8
    nvtype='double'
    nvsize=8
    Off_t='long long'
    lseeksize=8
    alignbytes=8
    prototype=define
  Linker and Libraries:
    ld='g++'
    ldflags ='-s -L"D:\perl-5.39.71-1320-ucrt-mcf\lib\MSWin32-x64-multi-thread\CORE" -L"C:\winlibs-ucrt-1320\mingw64\lib" -L"C:\winlibs-ucrt-1320\mingw64\x86_64-w64-mingw32\lib" -L"C:\winlibs-ucrt-1320\mingw64\lib\gcc\x86_64-w64-mingw32\13.2.0"'
    libpth=C:\winlibs-ucrt-1320\mingw64\lib C:\winlibs-ucrt-1320\mingw64\x86_64-w64-mingw32\lib C:\winlibs-ucrt-1320\mingw64\lib\gcc\x86_64-w64-mingw32\13.2.0 D:\_64\msys_1320\1.0\local\lib
    libs= -lmoldname -lkernel32 -luser32 -lgdi32 -lwinspool -lcomdlg32 -ladvapi32 -lshell32 -lole32 -loleaut32 -lnetapi32 -luuid -lws2_32 -lmpr -lwinmm -lversion -lodbc32 -lodbccp32 -lcomctl32
    perllibs= -lmoldname -lkernel32 -luser32 -lgdi32 -lwinspool -lcomdlg32 -ladvapi32 -lshell32 -lole32 -loleaut32 -lnetapi32 -luuid -lws2_32 -lmpr -lwinmm -lversion -lodbc32 -lodbccp32 -lcomctl32
    libc=-lucrt
    so=dll
    useshrplib=true
    libperl=libperl539.a
    gnulibc_version=''
  Dynamic Linking:
    dlsrc=dl_win32.xs
    dlext=dll
    d_dlsymun=undef
    ccdlflags=' '
    cccdlflags=' '
    lddlflags='-shared -s -L"D:\perl-5.39.71-1320-ucrt-mcf\lib\MSWin32-x64-multi-thread\CORE" -L"C:\winlibs-ucrt-1320\mingw64\lib" -L"C:\winlibs-ucrt-1320\mingw64\x86_64-w64-mingw32\lib" -L"C:\winlibs-ucrt-1320\mingw64\lib\gcc\x86_64-w64-mingw32\13.2.0"'


Characteristics of this binary (from libperl):
  Compile-time options:
    HAS_LONG_DOUBLE
    HAS_TIMES
    HAVE_INTERP_INTERN
    MULTIPLICITY
    PERLIO_LAYERS
    PERL_COPY_ON_WRITE
    PERL_DONT_CREATE_GVSV
    PERL_HASH_FUNC_SIPHASH13
    PERL_HASH_USE_SBOX32
    PERL_IMPLICIT_SYS
    PERL_MALLOC_WRAP
    PERL_OP_PARENT
    PERL_PRESERVE_IVUV
    PERL_USE_SAFE_PUTENV
    USE_64_BIT_INT
    USE_ITHREADS
    USE_LARGE_FILES
    USE_LOCALE
    USE_LOCALE_COLLATE
    USE_LOCALE_CTYPE
    USE_LOCALE_NUMERIC
    USE_LOCALE_TIME
    USE_PERLIO
    USE_PERL_ATOF
    USE_THREAD_SAFE_LOCALE
  Locally applied patches:
    uncommitted-changes
  Built under MSWin32
  Compiled at Feb  4 2024 22:51:26
  @INC:
    D:/perl-5.39.71-1320-ucrt-mcf/site/lib/MSWin32-x64-multi-thread
    D:/perl-5.39.71-1320-ucrt-mcf/site/lib
    D:/perl-5.39.71-1320-ucrt-mcf/lib/MSWin32-x64-multi-thread
    D:/perl-5.39.71-1320-ucrt-mcf/lib

win32/win32.c

tonycoz · 2024-02-04T23:44:39Z

win32/win32.c

+    PERL_UNUSED_ARG(hwnd);
+    PERL_UNUSED_ARG(lParam);


These arguments are used and I don't see a warning for them.

win32/win32.c

tonycoz · 2024-02-05T00:07:41Z

win32.c: In function 'set_w32_module_name':
win32.c:243:9: warning: cast between incompatible function types from 'FARPROC' {aka 'long long int (*)()'} to 'DWORD (*)(const WCHAR *, WCHAR *, DWORD)' {aka 'long unsigned int (*)(const short unsigned int *, short unsigned int *, long unsigned int)'} [-Wcast-function-type]
  243 |         (DWORD (__stdcall *)(LPCWSTR, LPWSTR, DWORD))
      |         ^

We probably need to add -Wno-cast-function-type.

win32.c: At top level:
win32.c:2250:1: warning: 'wstr_to_str' defined but not used [-Wunused-function]
 2250 | wstr_to_str(const wchar_t* wstr)
      | ^~~~~~~~~~~

The code that uses this is conditional on _DEBUG, which this could be too.

I'm not sure about gets().

sisyphus · 2024-02-05T03:28:00Z

I've added -Wno-cast-function-type to the flags (in my local build of perl), and made the alterations suggested by @tonycoz.
That reduces the remaining warnings to just 3:

win32.c: In function 'win32_link':
win32.c:3708:40: warning: passing argument 1 to 'restrict'-qualified parameter aliases with argument 2 [-Wrestrict]
 3708 |         ((aTHXa(PERL_GET_THX)), wcscpy(wOldName, PerlDir_mapW(wOldName)),

win32.c: At top level:
win32.c:2250:1: warning: 'wstr_to_str' defined but not used [-Wunused-function]
 2250 | wstr_to_str(const wchar_t* wstr)
      | ^~~~~~~~~~~

win32.c: In function 'win32_gets':
win32.c:4742:12: warning: call to 'gets' declared with attribute warning: Using gets() is always unsafe - use fgets() instead [-Wattribute-warning]
 4742 |     return gets(s);
      |            ^~~~~~~

Is there anything I should try at this stage to deal with any of those remaining warnings ?
We haven't touched upon the [-Wrestrict] warning, which is included in -Wall in gcc-13.2.0.

tonycoz · 2024-02-05T22:05:04Z

The -Wrestrict warning may be a real issue, violating the rules on restrict can result in undefined behaviour.

The definition of wstr_to_str() and probably the declaration at the top should be in #ifdef _DEBUG.

The use of gets() could be surrounded by GCC_DIAG_IGNORE_STMT():

DllExport char*
win32_gets(char *s)
{
    GCC_DIAG_IGNORE_STMT(-Wattribute-warning); /* gets() only for compat */
    return gets(s);
    GCC_DIAG_RESTORE_STMT;
}

sisyphus · 2024-02-06T06:08:55Z

Having built perl with -Wno-cast-function-type added to the compilation flags, I now see only the [-Wrestrict] warning issued by win32_link().
The mingw-w64 CI build, which doesn't have that extra flag, still emits the [-Wcast-function-type] warnings.

sisyphus · 2024-02-10T04:06:02Z

I have prepared other files to avoid issuing of pointless warnings - including the removal of 650 lines of such warnings from win32/perlhost.h.
I was thinking it might be better to first get this current PR merged.
But I could add these other files to this PR if that's deemed preferable.
How should I proceed ?

tonycoz · 2024-02-11T22:54:01Z

Please squash the 3 commits here into one commit.

Please add the perlhost fixes to a new PR.

Thanks

sisyphus · 2024-02-20T03:06:31Z

I think I've succeeded in squashing the 3 commits into 1.
Anything else needed ?

jkeenan added build-time-warnings Replaces [META] Build-time warnings RT #133556 distro-mswin32 labels Feb 4, 2024

tonycoz reviewed Feb 4, 2024

View reviewed changes

win32/win32.c Show resolved Hide resolved

tonycoz reviewed Feb 4, 2024

View reviewed changes

win32/win32.c Outdated Show resolved Hide resolved

Silence compilation warnings (PR 21925).

331d832

sisyphus force-pushed the silence_win32_c_warnings branch from 44d4b29 to 331d832 Compare February 12, 2024 20:53

tonycoz approved these changes Feb 20, 2024

View reviewed changes

tonycoz merged commit 9213b22 into Perl:blead Mar 6, 2024

tonycoz mentioned this pull request Mar 7, 2024

win32/win32.c: silence the rest of the gcc warnings #22066

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Silence warnings in win32/win32.c #21925

Silence warnings in win32/win32.c #21925

Uh oh!

sisyphus commented Feb 4, 2024

Uh oh!

Uh oh!

tonycoz Feb 4, 2024

Uh oh!

Uh oh!

tonycoz commented Feb 5, 2024

Uh oh!

sisyphus commented Feb 5, 2024

Uh oh!

tonycoz commented Feb 5, 2024

Uh oh!

sisyphus commented Feb 6, 2024

Uh oh!

sisyphus commented Feb 10, 2024

Uh oh!

tonycoz commented Feb 11, 2024

Uh oh!

sisyphus commented Feb 20, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Silence warnings in win32/win32.c #21925

Silence warnings in win32/win32.c #21925

Uh oh!

Conversation

sisyphus commented Feb 4, 2024

Uh oh!

Uh oh!

tonycoz Feb 4, 2024

Choose a reason for hiding this comment

Uh oh!

Uh oh!

tonycoz commented Feb 5, 2024

Uh oh!

sisyphus commented Feb 5, 2024

Uh oh!

tonycoz commented Feb 5, 2024

Uh oh!

sisyphus commented Feb 6, 2024

Uh oh!

sisyphus commented Feb 10, 2024

Uh oh!

tonycoz commented Feb 11, 2024

Uh oh!

sisyphus commented Feb 20, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants