That sounds like pretty serious compiler bug. What is the compiler and what is the machine arch?

In D58360#1338657, @glebius wrote:

That sounds like pretty serious compiler bug. What is the compiler and what is the machine arch?

Nothing fancy. all defaults with x86_64 - clang 21.1.8

I didn't check with gcc, but this is how atomic_load_int appears to resolve:

#define	__atomic_load_int_relaxed(p)	(*(const volatile u_int *)(p))

/*
 * When _Generic is available, try to provide some type checking.
 */
#if (defined(__STDC_VERSION__) && __STDC_VERSION__ >= 201112L) || \
    __has_extension(c_generic_selections)

#define	__atomic_load_generic(p, t, ut, n)	\
	_Generic(*(p),				\
	    t: __atomic_load_ ## n ## _relaxed(p), \
	    ut: __atomic_load_ ## n ## _relaxed(p))
#else
#define	__atomic_load_generic(p, t, ut, n)	\
	__atomic_load_ ## n ## _relaxed(p)
#endif

#define	atomic_load_int(p)	__atomic_load_generic(p, int, u_int, int)

So, the fallback appears to be uint by default, and only when the extentions are available, then maybe its using the int type? But I don't fully parse that precompile directives...

But reading c language generic extention shouldn't the signed and unsigned type ultimately resolve to *different* macros? After being separated for the type, they both go back to poin to (*(const volatile u_int *)(p)) - for the compiler to actually do the right thing, shouldn't there be a (*(const volatile int *)(p))

Adding Marc as he initially put in the typechecking - which doesn't appear to be doing all that much though.

There are apparently is only one other instances with a comparison for lower than zero, and in this other instance, a typecast is also used; everything else compares to non-zero values, exactly zero, etc...

sys/kern/vfs_default.c: return ((int)atomic_load_int(&ap->a_vp->v_writecount) < 0);
sys/netinet/tcp_hostcache.c:                        (int)atomic_load_int(&hc_entry->hc_expire) <= 0) {

Although with the name implying signed-type, and the compiler trying to do the right thing, maybe there should be a macro in sys/sys/atomic_common.h to (*(const volatile int *)(p))...

In D58360#1338871, @rscheff wrote:

Adding Marc as he initially put in the typechecking - which doesn't appear to be doing all that much though.

That type-checking is really meant to catch cases where you're doing something more egregious, like atomic_load_int() of a pointer to long, for example.

I agree that always returning an unsigned value here is somewhat surprising, though it is documented.

We could make the atomic_common.h macros always return the underlying type, i.e., atomic_load_int() could return a signed or unsigned int depending on the underlying type. The problem with this is that it'd be inconsistent with the macros defined in machdep/atomic.h, i.e., we'd want to make, e.g., atomic_load_acq_int() behave the same as atomic_load_int(), and that's not a small amount of work.

Since there appear only two instances of comparisons where negative values are relevant in the full kernel source tree, I think the most progressive way would then be to simply also typecast the atomic_load_int here.