Prevent conflicts with GLIBC style BIT_* macros

[se]

Summary

The BIT_SET et.al. macros used in the FreeBSD kernel (and a few utilities that access low level data) differ from same-name macros used by third party programs.
The conflict is avoided by hiding these kernel specific macro definitions from user-land sources, and user-land programs can then define BIT_* macros with different parameters and semantics.

This patch is a pre-condition for adding BIT_* macros and some function signatures that conform with calling conventions expected in 3rd party code to sched.h.

Test Plan

Apply patch and build world, kernel and all kernel modules.
Verify that the build succeeds and the kernel is functional.

[se]

The diff has been updated to include the changes to pfctl_altq.c which apparently had been lost ...

[kib]

sys/sys/bitset.h

314–358

Shouldn't these symbols (BITSET_XXX) hidden as well?

360

You may do #if defined(_KERNEL) || defined(_WANT_FREEBSD_BIT) there, and then #define _WANT_FREEBSD_BIT in consumers allows to avoid manual redefinition of BIT_ for src/.

[se]

sys/sys/bitset.h

314–358

I was not sure whether those are conflicting, but did expect only the BIT_* to be an issue.
If the BITSET_* macros should be hidden too, I'll update the patch accordingly.

360

Yes, good idea - I'll wait for further comments and then upload another patch-set.

[kib]

sys/sys/bitset.h

314–358

The problem there is any user' namespace pollution. It is somewhat strange to fix some, but leave other.

[se]

Update according to kib@'s review comments.

Userland programs and kernel modules may define _WANT_FREEBSD_BITSET to make the BIT_* macros visible.
Header files use the __ prefix to directly access the private definitions.
The visibility of the BITSET_DEFINE* macros could have been controlled by the condition in bitset.h, as well, but I think having the conditional definition within the file that defines the __BITSET_DEFINE macros is clearer.
The _WITH_CPU_SET_T condition can be removed from sched.h, since cpuset.h no longer depends on the BIT_* macros and name space pollution is avoided.

A MFC to 13-STABLE and 12-STABLE is planned 1 month after the commit to -CURRENT.

This will allow to remove _WITH_CPU_SET_T from ports, but only after all supported FreeBSD versions have been updated to use the non-polluting form of the BIT_* macros.
A test-build of graphics/cairo has succeeded on a system with this patch-set applied and without _WITH_CPU_SET_T set by the port.

[kib]

sys/sys/_bitset.h

47–48

I think we should use implementation namespace for tag as well, i.e. struct __t instead of struct t.

sys/sys/bitset.h

317

Wouldn't it be easier for compiler to check the correctness if BIT_ macros were functional, i.e. not just redefine symbols, but define them as argument-passing macros?

#define BIT_AND(_s, d, s) __BIT_AND(_s, d, s)

is what I mean.

[markj]

sbin/pfctl/pfctl_altq.c

25

bitset.9 should be updated to document this flag.

sys/dev/ena/ena.h

185 ↗

(On Diff #99386)

Is this file included by userspace?

sys/sys/bitset.h

344

sys/vm/vm_dumpset.h

53 ↗

(On Diff #99386)

I think this file doesn't need to use the prefix.

[se]

sys/dev/ena/ena.h

185 ↗

(On Diff #99386)

I had added this patch due to compiler errors, but apparently it is not required (anymore).

sys/sys/_bitset.h

47–48

OK, I'll upload another patch ...

sys/sys/bitset.h

317

Wouldn't it be easier for compiler to check the correctness if BIT_ macros were functional, i.e. not just redefine symbols, but define them as argument-passing macros?

#define BIT_AND(_s, d, s) __BIT_AND(_s, d, s)

is what I mean.

Sure, I'd been hoping to get away with the non-functional definitions - next patch will have arguments added.

[se]

This updated patch-set should address all review comments received to date.

[arrowd]

I've got a failure for print/miktex port, which seems related to this commit: http://beefy17.nyi.freebsd.org/data/main-i386-default/p7539e33f88ff_s169b368a62/logs/miktex-21.8.log

/wrkdirs/usr/ports/print/miktex/work/miktex-21.8/Libraries/MiKTeX/Util/Tokenizer.cpp:51:3: error: reference to 'bitset' is ambiguous
  bitset<256> delims;
  ^
/usr/include/sys/_bitset.h:64:17: note: candidate found by name lookup is 'bitset'
__BITSET_DEFINE(bitset, 1);
                ^
/usr/include/c++/v1/bitset:667:28: note: candidate found by name lookup is 'std::bitset'
class _LIBCPP_TEMPLATE_VIS bitset
                           ^
1 error generated.

[kib]

In D33235#753327, @arrowd wrote:

I've got a failure for print/miktex port, which seems related to this commit: http://beefy17.nyi.freebsd.org/data/main-i386-default/p7539e33f88ff_s169b368a62/logs/miktex-21.8.log

/wrkdirs/usr/ports/print/miktex/work/miktex-21.8/Libraries/MiKTeX/Util/Tokenizer.cpp:51:3: error: reference to 'bitset' is ambiguous
  bitset<256> delims;
  ^
/usr/include/sys/_bitset.h:64:17: note: candidate found by name lookup is 'bitset'
__BITSET_DEFINE(bitset, 1);
                ^
/usr/include/c++/v1/bitset:667:28: note: candidate found by name lookup is 'std::bitset'
class _LIBCPP_TEMPLATE_VIS bitset
                           ^
1 error generated.

Could you try this please?

diff --git a/sys/sys/_bitset.h b/sys/sys/_bitset.h
index 1c167daf3f09..86dfa0f545f0 100644
--- a/sys/sys/_bitset.h
+++ b/sys/sys/_bitset.h
@@ -61,7 +61,7 @@ struct _t {								\
  * Define a default type that can be used while manually specifying size
  * to every call.
  */
-__BITSET_DEFINE(bitset, 1);
+__BITSET_DEFINE(__bitset, 1);
 
 #if defined(_KERNEL) || defined(_WANT_FREEBSD_BITSET)
 #define	BITSET_DEFINE(_t, _s)	__BITSET_DEFINE(_t, _s)

[arrowd]

I don't have that recent CURRENT at hand, unfortunately. This error comes from the FreeBSD package cluster.

[se]

In D33235#753331, @kib wrote:

Could you try this please?

diff --git a/sys/sys/_bitset.h b/sys/sys/_bitset.h
index 1c167daf3f09..86dfa0f545f0 100644
--- a/sys/sys/_bitset.h
+++ b/sys/sys/_bitset.h
@@ -61,7 +61,7 @@ struct _t {								\
  * Define a default type that can be used while manually specifying size
  * to every call.
  */
-__BITSET_DEFINE(bitset, 1);
+__BITSET_DEFINE(__bitset, 1);
 
 #if defined(_KERNEL) || defined(_WANT_FREEBSD_BITSET)
 #define	BITSET_DEFINE(_t, _s)	__BITSET_DEFINE(_t, _s)

I'll give this a try - but __bitset will obviously also need to be mapped to bitset together with the other private names.

If I can build both the port and the world that way, I'll commit the patch suggested by kib with the addition of a conditional "#define bitset __bitset" within one hour.

[se]

I have committed two more patches to fix issues caused by 5e04571cf3cf:

• lang/gcc11 failed to build due to the #define __BITSET_ALLOC referencing malloc() which is "poisoned" in the GCC build
• print/miktex defined a variable bitset which collided with the default bitset defined in sys/bitset.h and now unconditionally included implicitly when sched.h is used.

The two commits that should fix those issues are 74e014dbfab5 and 22c4ab6cb015.