depends, NetBSD: Fix `bdb` package compilation with GCC-14

hebasto commented at 3:55 pm on January 6, 2025: member

On NetBSD 10, compilation of the bdb package with GCC-14 fails with the following error:

 0$ gmake -C depends bdb CC=/usr/pkg/gcc14/bin/gcc
 1<snip>
 2./libtool --mode=compile /usr/pkg/gcc14/bin/gcc -c -I. -I../dist/./..   -I/home/hebasto/dev/bitcoin/depends/x86_64-unknown-netbsd10.0/include -D_XOPEN_SOURCE=600  -pipe -std=c11 -O2 -Wno-error=implicit-function-declaration -Wno-error=format-security -Wno-error=implicit-int  ../dist/./../env/env_config.c
 3libtool: compile:  /usr/pkg/gcc14/bin/gcc -c -I. -I../dist/./.. -I/home/hebasto/dev/bitcoin/depends/x86_64-unknown-netbsd10.0/include -D_XOPEN_SOURCE=600 -pipe -std=c11 -O2 -Wno-error=implicit-function-declaration -Wno-error=format-security -Wno-error=implicit-int ../dist/./../env/env_config.c  -fPIC -DPIC -o env_config.o
 4../dist/./../env/env_config.c: In function '__config_parse':
 5../dist/./../env/env_config.c:98:13: warning: implicit declaration of function 'strcasecmp'; did you mean 'strncmp'? [-Wimplicit-function-declaration]
 6   98 |         if (strcasecmp(s, argv[0]) == 0) {                              \
 7      |             ^~~~~~~~~~
 8../dist/./../env/env_config.c:135:9: note: in expansion of macro 'CONFIG_UINT32'
 9  135 |         CONFIG_UINT32("mutex_set_align", __mutex_set_align);
10      |         ^~~~~~~~~~~~~
11../dist/./../env/env_config.c: In function '__config_split':
12../dist/./../env/env_config.c:537:43: warning: implicit declaration of function 'strsep'; did you mean 'strdup'? [-Wimplicit-function-declaration]
13  537 |         for (count = 0, ap = argv; (*ap = strsep(&input, " \t\n")) != NULL;)
14      |                                           ^~~~~~
15      |                                           strdup
16../dist/./../env/env_config.c:537:41: error: assignment to 'char *' from 'int' makes pointer from integer without a cast [-Wint-conversion]
17  537 |         for (count = 0, ap = argv; (*ap = strsep(&input, " \t\n")) != NULL;)
18      |                                         ^
19gmake[1]: *** [Makefile:1664: env_config.o] Error 1
20gmake[1]: Leaving directory '/home/hebasto/dev/bitcoin/depends/work/build/x86_64-unknown-netbsd10.0/bdb/4.8.30-cac45822ba8/build_unix'
21gmake: *** [funcs.mk:302: /home/hebasto/dev/bitcoin/depends/work/build/x86_64-unknown-netbsd10.0/bdb/4.8.30-cac45822ba8/build_unix/.stamp_built] Error 2
22gmake: Leaving directory '/home/hebasto/dev/bitcoin/depends'

This PR resolves the compilation issue.

depends, NetBSD: Fix `bdb` package compilation with GCC-14 9bcf135c16

hebasto added the label Build system on Jan 6, 2025

DrahtBot commented at 3:55 pm on January 6, 2025: contributor

The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

Code Coverage & Benchmarks

For details see: https://corecheck.dev/bitcoin/bitcoin/pulls/31613.

Reviews

See the guideline for information on the review process. A summary of reviews will appear here.

Conflicts

No conflicts as of last run.

luke-jr commented at 4:17 am on January 8, 2025: member

Why would this be NetBSD-specific? Shouldn’t GCC 14 have the same warning/error on every platform?

maflcko commented at 7:07 am on January 8, 2025: member

Wouldn’t it be better to add the missing includes? Next to just removing bdb, of course.

theuni commented at 5:17 pm on January 8, 2025: member

Wouldn’t it be better to add the missing includes? Next to just removing bdb, of course.

Yeah, it seems that it doesn’t know strsep’s function signature, so it assumes it returns an int. Not sure why this is a new failure though, I guess it’s a new warning with gcc14?

It looks like string.h includes strings.h which defines these functions, but only if _NETBSD_SOURCE is defined. But I’m not sure who’s supposed to define that? I suspect that adding it to our cppflags might be the correct thing to do, but who knows what side-effects it would have.

So agreed with @maflcko, let’s just add a patch to include strings.h for netbsd.

theuni commented at 5:30 pm on January 8, 2025: member

Also, it seems these were masked by the fact that we use -Wno-error=implicit-function-declaration -Wno-error=implicit-int for depends builds.

This is because exit(0) which is called a bunch during configure (and there are probably a bunch of others too, but exit is the first that causes problems) is implicitly defined because stdlib.h isn’t included. Blah.

If this were an important library I’d say we should patch up their configure and remove these warning-off-switches. But since we’re going to kill the lib soon anyway, it’s not worth the effort.

hebasto commented at 12:04 pm on January 14, 2025: member

It looks like string.h includes strings.h which defines these functions, but only if _NETBSD_SOURCE is defined. But I’m not sure who’s supposed to define that? I suspect that adding it to our cppflags might be the correct thing to do, but who knows what side-effects it would have.

Defining _NETBSD_SOURCE works, but I agree that it is not an optimal solution, especially when the _XOPEN_SOURCE is already defined.

Also, it seems these were masked by the fact that we use -Wno-error=implicit-function-declaration -Wno-error=implicit-int for depends builds.

This is because exit(0) which is called a bunch during configure (and there are probably a bunch of others too, but exit is the first that causes problems) is implicitly defined because stdlib.h isn’t included. Blah.

If this were an important library I’d say we should patch up their configure…

Like here: https://github.com/NetBSD/pkgsrc/blob/pkgsrc-2024Q4/databases/db4/patches/patch-ab.

hebasto commented at 7:06 pm on January 14, 2025: member

It looks like string.h includes strings.h which defines these functions, but only if _NETBSD_SOURCE is defined….

So agreed with @maflcko, let’s just add a patch to include strings.h for netbsd.

The strsep function declaration is guarded by _NETBSD_SOURCE as well::

 0#if defined(_NETBSD_SOURCE)
 1#include <strings.h>		/* for backwards-compatibility */
 2__BEGIN_DECLS
 3void	*memmem(const void *, size_t, const void *, size_t);
 4char	*strcasestr(const char *, const char *);
 5char	*strchrnul(const char *, int);
 6size_t	 strlcat(char *, const char *, size_t);
 7size_t	 strlcpy(char *, const char *, size_t);
 8char	*strsep(char **, const char *);
 9char	*stresep(char **, const char *, int);
10char	*strnstr(const char *, const char *, size_t);
11void	*memrchr(const void *, int, size_t);
12void	*explicit_memset(void *, int, size_t);
13int	consttime_memequal(const void *, const void *, size_t);
14__END_DECLS
15#endif

Therefore, including strings.h won’t resolve the issue.

But I’m not sure who’s supposed to define that?

The https://github.com/NetBSD/pkgsrc repository contains multiple examples demonstrating that this is the responsibility of a package.

In https://github.com/hebasto/bitcoin-core-nightly/pull/31 -D_NETBSD_SOURCE replaces -D_XOPEN_SOURCE=600. Notably, the later was introduced in #26073 without mentioning any tested NetBSD releases. This solution works for both supported NetBSD releases: 10.1 and 9.4.

depends, NetBSD: Fix bdb package compilation with GCC-14 #31613

Code Coverage & Benchmarks

Reviews

Conflicts

depends, NetBSD: Fix `bdb` package compilation with GCC-14 #31613