Thread: msys inet_pton strangeness
A week or so ago I upgraded the msys2 animal fairywren to the latest msys2, and ever since then the build has been failing for Release 15. It's complaining like this:
ccache gcc -Wall -Wmissing-prototypes -Wpointer-arith -Wdeclaration-after-statement -Werror=vla -Wendif-labels -Wmissing-format-attribute -Wimplicit-fallthrough=3 -Wcast-function-type -Wformat-security -fno-strict-aliasing -fwrapv -fexcess-precision=standard -Wno-format-truncation -Wno-stringop-truncation -g -O2 -DFRONTEND -DUNSAFE_STAT_OK -I/home/pgrunner/bf/root/REL_15_STABLE/pgsql.build/../pgsql/src/interfaces/libpq -I../../../src/include -I/home/pgrunner/bf/root/REL_15_STABLE/pgsql.build/../pgsql/src/include -I../pgsql/src/include/port/win32 -I/c/progra~1/openssl-win64/include "-I/home/pgrunner/bf/root/REL_15_STABLE/pgsql.build/../pgsql/src/include/port/win32" -DWIN32_STACK_RLIMIT=4194304 -I../../../src/port -I/home/pgrunner/bf/root/REL_15_STABLE/pgsql.build/../pgsql/src/port -DSO_MAJOR_VERSION=5 -c -o fe-secure-common.o /home/pgrunner/bf/root/REL_15_STABLE/pgsql.build/../pgsql/src/interfaces/libpq/fe-secure-common.c C:/tools/xmsys64/home/pgrunner/bf/root/REL_15_STABLE/pgsql/src/interfaces/libpq/fe-secure-common.c: In function 'pq_verify_peer_name_matches_certificate_ip': C:/tools/xmsys64/home/pgrunner/bf/root/REL_15_STABLE/pgsql/src/interfaces/libpq/fe-secure-common.c:219:21: error: implicit declaration of function 'inet_pton'; did you mean 'inet_aton'? [-Wimplicit-function-declaration] 219 | if (inet_pton(AF_INET6, host, &addr) == 1) | ^~~~~~~~~ | inet_aton make[3]: *** [<builtin>: fe-secure-common.o] Error 1
configure has determined that we have inet_pton, and I have repeated the test manually. It's not a ccache issue - I have cleared the cache and the problem persists. The test run by meson on the same animal reports not finding the function.
So I'm a bit flummoxed about how to fix this, and would appreciate any suggestions.
cheers
andrew
-- Andrew Dunstan EDB: https://www.enterprisedb.com
Andrew Dunstan <andrew@dunslane.net> writes:
> It's complaining like this:
> C:/tools/xmsys64/home/pgrunner/bf/root/REL_15_STABLE/pgsql/src/interfaces/libpq/fe-secure-common.c:219:21: error:
implicitdeclaration of function 'inet_pton'; did you mean 'inet_aton'? [-Wimplicit-function-declaration]
> 219 | if (inet_pton(AF_INET6, host, &addr) == 1)
> | ^~~~~~~~~
> configure has determined that we have inet_pton, and I have repeated the
> test manually.
configure's test is purely a linker test. It does not check to see
where/whether the function is declared. Meanwhile, the compiler is
complaining that it doesn't see a declaration. So the problem
probably can be fixed by adding an #include, but you'll need to
figure out what.
I see that our other user of inet_pton, fe-secure-openssl.c,
has a rather different #include setup than fe-secure-common.c;
does it compile OK?
regards, tom lane
On 2024-09-28 Sa 11:49 AM, Tom Lane wrote:
Andrew Dunstan <andrew@dunslane.net> writes:It's complaining like this:C:/tools/xmsys64/home/pgrunner/bf/root/REL_15_STABLE/pgsql/src/interfaces/libpq/fe-secure-common.c:219:21: error: implicit declaration of function 'inet_pton'; did you mean 'inet_aton'? [-Wimplicit-function-declaration] 219 | if (inet_pton(AF_INET6, host, &addr) == 1) | ^~~~~~~~~configure has determined that we have inet_pton, and I have repeated the test manually.configure's test is purely a linker test. It does not check to see where/whether the function is declared. Meanwhile, the compiler is complaining that it doesn't see a declaration. So the problem probably can be fixed by adding an #include, but you'll need to figure out what. I see that our other user of inet_pton, fe-secure-openssl.c, has a rather different #include setup than fe-secure-common.c; does it compile OK?
I'll try, but this error occurs before we get that far.
We should have included ws2tcpip.h, which includes this:
#define InetPtonA inet_pton WINSOCK_API_LINKAGE INT WSAAPI InetPtonA(INT Family, LPCSTR pStringBuf, PVOID pAddr);
It's conditioned on (_WIN32_WINNT >= 0x0600), but that should be true.
So I'm still very confused ;-(
cheers
andrew
-- Andrew Dunstan EDB: https://www.enterprisedb.com
Andrew Dunstan <andrew@dunslane.net> writes:
> We should have included ws2tcpip.h, which includes this:
> #define InetPtonA inet_pton
> WINSOCK_API_LINKAGE INT WSAAPI InetPtonA(INT Family, LPCSTR pStringBuf, PVOID pAddr);
> It's conditioned on (_WIN32_WINNT >= 0x0600), but that should be true.
> So I'm still very confused ;-(
Me too. Does this compiler support the equivalent of -E, so
that you can verify that the InetPtonA declaration is being
read?
regards, tom lane
On Sun, Sep 29, 2024 at 6:26 AM Andrew Dunstan <andrew@dunslane.net> wrote: > We should have included ws2tcpip.h, which includes this: > > #define InetPtonA inet_pton > WINSOCK_API_LINKAGE INT WSAAPI InetPtonA(INT Family, LPCSTR pStringBuf, PVOID pAddr); > > It's conditioned on (_WIN32_WINNT >= 0x0600), but that should be true. Can you print out the value to be sure? I can't imagine they'd set it lower themselves or make it go backwards in an upgrade, but perhaps it's somehow not being set at all, and then we do: #if defined(_MSC_VER) && _MSC_VER >= 1900 #define MIN_WINNT 0x0600 #else #define MIN_WINNT 0x0501 #endif In 16 we don't do that anymore, we just always set it to 0x0A00 (commit 495ed0ef2d72). And before 15, we didn't want that function yet (commit c1932e542863).
Hello Thomas and Andrew, 28.09.2024 23:52, Thomas Munro wrote: > On Sun, Sep 29, 2024 at 6:26 AM Andrew Dunstan <andrew@dunslane.net> wrote: >> We should have included ws2tcpip.h, which includes this: >> >> #define InetPtonA inet_pton >> WINSOCK_API_LINKAGE INT WSAAPI InetPtonA(INT Family, LPCSTR pStringBuf, PVOID pAddr); >> >> It's conditioned on (_WIN32_WINNT >= 0x0600), but that should be true. > Can you print out the value to be sure? I can't imagine they'd set it > lower themselves or make it go backwards in an upgrade, but perhaps > it's somehow not being set at all, and then we do: > > #if defined(_MSC_VER) && _MSC_VER >= 1900 > #define MIN_WINNT 0x0600 > #else > #define MIN_WINNT 0x0501 > #endif > > In 16 we don't do that anymore, we just always set it to 0x0A00 > (commit 495ed0ef2d72). And before 15, we didn't want that function > yet (commit c1932e542863). FWIW, I'm observing the same here. For a trivial test.c (compiled with the same command line as fe-secure-common.c) like: "===_WIN32" _WIN32; "===_WIN32_WINNT"; _WIN32_WINNT; with gcc -E (from mingw-w64-ucrt-x86_64-gcc 14.2.0-1), I get: "===_WIN32" 1; "===_WIN32_WINNT"; _WIN32_WINNT; That is, _WIN32_WINNT is not defined, but with #include <windows.h> above, I see: "===_WIN32_WINNT"; 0x603 With #include "postgres_fe.h" (as in fe-secure-common.c) I get: "===_WIN32_WINNT"; 0x0501; Best regards, Alexander
On 2024-09-29 Su 1:00 AM, Alexander Lakhin wrote:
Hello Thomas and Andrew,
28.09.2024 23:52, Thomas Munro wrote:On Sun, Sep 29, 2024 at 6:26 AM Andrew Dunstan <andrew@dunslane.net> wrote:We should have included ws2tcpip.h, which includes this:Can you print out the value to be sure? I can't imagine they'd set it
#define InetPtonA inet_pton
WINSOCK_API_LINKAGE INT WSAAPI InetPtonA(INT Family, LPCSTR pStringBuf, PVOID pAddr);
It's conditioned on (_WIN32_WINNT >= 0x0600), but that should be true.
lower themselves or make it go backwards in an upgrade, but perhaps
it's somehow not being set at all, and then we do:
#if defined(_MSC_VER) && _MSC_VER >= 1900
#define MIN_WINNT 0x0600
#else
#define MIN_WINNT 0x0501
#endif
In 16 we don't do that anymore, we just always set it to 0x0A00
(commit 495ed0ef2d72). And before 15, we didn't want that function
yet (commit c1932e542863).
FWIW, I'm observing the same here.
For a trivial test.c (compiled with the same command line as
fe-secure-common.c) like:
"===_WIN32"
_WIN32;
"===_WIN32_WINNT";
_WIN32_WINNT;
with gcc -E (from mingw-w64-ucrt-x86_64-gcc 14.2.0-1), I get:
"===_WIN32"
1;
"===_WIN32_WINNT";
_WIN32_WINNT;
That is, _WIN32_WINNT is not defined, but with #include <windows.h> above,
I see:
"===_WIN32_WINNT";
0x603
With #include "postgres_fe.h" (as in fe-secure-common.c) I get:
"===_WIN32_WINNT";
0x0501;
Yeah, src/include/port/win32/sys/socket.h has:
#include <winsock2.h> #include <ws2tcpip.h> #include <windows.h>
I'm inclined to think we might need to reverse the order of the last two. TBH I don't really understand how this has worked up to now.
cheers
andrew
-- Andrew Dunstan EDB: https://www.enterprisedb.com
Andrew Dunstan <andrew@dunslane.net> writes:
> Yeah, src/include/port/win32/sys/socket.h has:
> #include <winsock2.h>
> #include <ws2tcpip.h>
> #include <windows.h>
> I'm inclined to think we might need to reverse the order of the last
> two. TBH I don't really understand how this has worked up to now.
I see the same in src/include/port/win32_port.h ... wouldn't that
get included first?
regards, tom lane
29.09.2024 18:47, Andrew Dunstan wrote:
Yeah, src/include/port/win32/sys/socket.h has:#include <winsock2.h> #include <ws2tcpip.h> #include <windows.h>I'm inclined to think we might need to reverse the order of the last two. TBH I don't really understand how this has worked up to now.
As far as I can see, in my environment _WIN32_WINNT defined with
#ifndef _WIN32_WINNT
#define _WIN32_WINNT 0x603
#endif
inside C:/msys64/ucrt64/include/_mingw.h, which can be included
from C:/msys64/ucrt64/include/corecrt.h:10,
from C:/msys64/ucrt64/include/crtdefs.h:10,
from ../../../src/include/pg_config_os.h:40,
from ../../../src/include/c.h:56,
from ../../../src/include/postgres_fe.h:25,
from fe-secure-common.c:20
or (if HAVE_CRTDEFS_H is not defined):
from C:/msys64/ucrt64/include/corecrt.h:10,
from C:/msys64/ucrt64/include/corecrt_stdio_config.h:10,
from C:/msys64/ucrt64/include/stdio.h:9,
from ../../../src/include/c.h:59,
from ../../../src/include/postgres_fe.h:25,
from fe-secure-common.c:20
or (if winsock2.h included directly):
from C:/msys64/ucrt64/include/windows.h:9,
from C:/msys64/ucrt64/include/winsock2.h:23
so including winsock2.h is sufficient to include _mingw.h, but it doesn't
redefine _WIN32_WINNT, unfortunately.
Best regards,
Alexander
Just an idea... --- a/src/include/port/win32.h +++ b/src/include/port/win32.h @@ -16,7 +16,7 @@ * get support for GetLocaleInfoEx() with locales. For everything else * the minimum version is Windows XP (0x0501). */ -#if defined(_MSC_VER) && _MSC_VER >= 1900 +#if !defined(_MSC_VER) || _MSC_VER >= 1900 #define MIN_WINNT 0x0600 #else #define MIN_WINNT 0x0501 That was done to reveal the Vista locale stuff, which MingGW certainly has: we're calling it unconditionally in the master branch in at least one place (and we should do more of that, to make MSVC and MinGW code paths the same wherever possible). In 15 the users of GetLocaleInfoEx() are guarded with checks that you're on MSVC so it still wouldn't actually call them anyway. Obviously it's not good to change the target in the back branches. But apparently it already changed by accident due to some header order nonsense (could it be related to MinGW's recent switch to the UCRT by default?), so changing it again so that it compiles seems OK? We don't seem to have a documented MinGW support range, and I've always sort of assumed that it's just 'recent versions only' because it's effectively only for developers (cross builds and suchlike). And it certainly didn't really intend to be runnable on Windows XP (PostgreSQL 11 was the last to claim to run on Windows XP (0x0501)). I doubt anyone's actually going to test this on Vista or other ancient SDKs either, which is why I was looking for a change that *only* affects MinGW and doesn't risk changining anything for MSVC users on the retro-computers and toolchains we claim to support. For example, header order dependencies and side effects are a little chaotic on that OS, so you could easily break something else... I guess the objection would be that (apparently) some translation units are being compiled with 0x0603 from system headers, and this one would use 0x0600, which might be confusing.
Hello Andrew and Thomas, 29.09.2024 18:47, Andrew Dunstan пишет: > > I'm inclined to think we might need to reverse the order of the last two. TBH I don't really understand how this has > worked up to now. > I've looked at the last successful run [1] and discovered that fe-secure-common.c didn't compile cleanly too: ccache gcc ... /home/pgrunner/bf/root/REL_15_STABLE/pgsql.build/../pgsql/src/interfaces/libpq/fe-secure-common.c C:/tools/nmsys64/home/pgrunner/bf/root/REL_15_STABLE/pgsql/src/interfaces/libpq/fe-secure-common.c: In function 'pq_verify_peer_name_matches_certificate_ip': C:/tools/nmsys64/home/pgrunner/bf/root/REL_15_STABLE/pgsql/src/interfaces/libpq/fe-secure-common.c:219:21: warning: implicit declaration of function 'inet_pton'; did you mean 'inet_aton'? [-Wimplicit-function-declaration] 219 | if (inet_pton(AF_INET6, host, &addr) == 1) | ^~~~~~~~~ | inet_aton So it worked just because that missing declaration generated just a warning, not an error. 30.09.2024 01:28, Thomas Munro wrote: > Just an idea... > > --- a/src/include/port/win32.h > +++ b/src/include/port/win32.h > @@ -16,7 +16,7 @@ > * get support for GetLocaleInfoEx() with locales. For everything else > * the minimum version is Windows XP (0x0501). > */ > -#if defined(_MSC_VER) && _MSC_VER >= 1900 > +#if !defined(_MSC_VER) || _MSC_VER >= 1900 > #define MIN_WINNT 0x0600 > #else > #define MIN_WINNT 0x0501 This change works for me in the msys case. I have no VS 2013 on hand to test the other branch, but it looks like HAVE_INET_PTON set to 1 unconditionally in src/tools/msvc/Solution.pm, so we probably will stumble upon the same issue with _MSC_VER = 1800. What if we just set MIN_WINNT 0x0600 for REL_15_STABLE? Or may be it would make sense to get that old Visual Studio and recheck? The other question that I still have is: where we expect to get system _WIN32_WINNT from? As far as I can see, in the fe-secure-common.c case we have the following include chain: #include "postgres_fe.h" #include "c.h" // no other includes above #include "postgres_ext.h" #include "pg_config_ext.h" ... #include "pg_config.h" #include "pg_config_manual.h" /* must be after pg_config.h */ #include "pg_config_os.h" /* must be before any system header files */ // checks _WIN32_WINNT: #if defined(_WIN32_WINNT) && _WIN32_WINNT < MIN_WINNT So if pg_config_os.h is really included before any system headers, checking _WIN32_WINNT makes sense only when that define passed with -D_WIN32_WINNT, no? [1] https://buildfarm.postgresql.org/cgi-bin/show_stage_log.pl?nm=fairywren&dt=2024-09-19%2023%3A10%3A10&stg=build Best regards, Alexander
On 2024-09-30 Mo 7:00 AM, Alexander Lakhin wrote: > Hello Andrew and Thomas, > > 29.09.2024 18:47, Andrew Dunstan пишет: >> >> I'm inclined to think we might need to reverse the order of the last >> two. TBH I don't really understand how this has worked up to now. >> > > I've looked at the last successful run [1] and discovered that > fe-secure-common.c didn't compile cleanly too: > ccache gcc ... > /home/pgrunner/bf/root/REL_15_STABLE/pgsql.build/../pgsql/src/interfaces/libpq/fe-secure-common.c > C:/tools/nmsys64/home/pgrunner/bf/root/REL_15_STABLE/pgsql/src/interfaces/libpq/fe-secure-common.c: > In function 'pq_verify_peer_name_matches_certificate_ip': > C:/tools/nmsys64/home/pgrunner/bf/root/REL_15_STABLE/pgsql/src/interfaces/libpq/fe-secure-common.c:219:21: > warning: implicit declaration of function 'inet_pton'; did you mean > 'inet_aton'? [-Wimplicit-function-declaration] > 219 | if (inet_pton(AF_INET6, host, &addr) == 1) > | ^~~~~~~~~ > | inet_aton > > So it worked just because that missing declaration generated just a > warning, not an error. Ah, so this is because gcc 14.1.0 treats this as an error but gcc 12.2.0 treats it as a warning. Now it makes sense. cheers andrew -- Andrew Dunstan EDB: https://www.enterprisedb.com
On 2024-09-29 Su 6:28 PM, Thomas Munro wrote: > Just an idea... > > --- a/src/include/port/win32.h > +++ b/src/include/port/win32.h > @@ -16,7 +16,7 @@ > * get support for GetLocaleInfoEx() with locales. For everything else > * the minimum version is Windows XP (0x0501). > */ > -#if defined(_MSC_VER) && _MSC_VER >= 1900 > +#if !defined(_MSC_VER) || _MSC_VER >= 1900 > #define MIN_WINNT 0x0600 > #else > #define MIN_WINNT 0x0501 This seems reasonable as just about the most minimal change we can make work. cheers andrew -- Andrew Dunstan EDB: https://www.enterprisedb.com
Andrew Dunstan <andrew@dunslane.net> writes:
> Ah, so this is because gcc 14.1.0 treats this as an error but gcc 12.2.0
> treats it as a warning. Now it makes sense.
Not entirely ... if fairywren had been generating that warning all
along, I would have noticed it long ago, because I periodically
scrape the BF database for compiler warnings. There has to have
been some recent change in the system include files.
regards, tom lane
On 2024-09-30 Mo 10:08 AM, Tom Lane wrote:
> Andrew Dunstan <andrew@dunslane.net> writes:
>> Ah, so this is because gcc 14.1.0 treats this as an error but gcc 12.2.0
>> treats it as a warning. Now it makes sense.
> Not entirely ... if fairywren had been generating that warning all
> along, I would have noticed it long ago, because I periodically
> scrape the BF database for compiler warnings. There has to have
> been some recent change in the system include files.
here's what I see on vendikar:
pgbfprod=> select min(snapshot) from build_status_log where log_stage in
('build.log', 'make.log') and branch = 'REL_15_STABLE' and sysname =
'fairywren' and snapshot > now() - interval '1500 days' and log_text ~
'inet_pton';
min
---------------------
2022-06-30 18:04:08
(1 row)
cheers
andrew
--
Andrew Dunstan
EDB: https://www.enterprisedb.com
Andrew Dunstan <andrew@dunslane.net> writes:
> On 2024-09-30 Mo 10:08 AM, Tom Lane wrote:
>> Not entirely ... if fairywren had been generating that warning all
>> along, I would have noticed it long ago, because I periodically
>> scrape the BF database for compiler warnings. There has to have
>> been some recent change in the system include files.
> here's what I see on vendikar:
Oh, wait, I forgot this is only about the v15 branch. I seldom
search for warnings except on HEAD. Still, I'd have expected to
notice it while v15 was development tip. Maybe we changed something
since then?
Anyway, it's pretty moot, I see no reason not to push forward
with the proposed fix.
regards, tom lane
On 2024-09-30 Mo 11:11 AM, Tom Lane wrote:
Andrew Dunstan <andrew@dunslane.net> writes:On 2024-09-30 Mo 10:08 AM, Tom Lane wrote:Not entirely ... if fairywren had been generating that warning all along, I would have noticed it long ago, because I periodically scrape the BF database for compiler warnings. There has to have been some recent change in the system include files.here's what I see on vendikar:Oh, wait, I forgot this is only about the v15 branch. I seldom search for warnings except on HEAD. Still, I'd have expected to notice it while v15 was development tip. Maybe we changed something since then? Anyway, it's pretty moot, I see no reason not to push forward with the proposed fix.
Thanks, done.
cheers
andrew
-- Andrew Dunstan EDB: https://www.enterprisedb.com