misc/59266: t_isnan:isinf_basic test is bogus on some ports

[campbell+netbsd%mumble.net@localhost]

>Number:         59266
>Category:       misc
>Synopsis:       t_isnan:isinf_basic test is bogus on some ports
>Confidential:   no
>Severity:       serious
>Priority:       medium
>Responsible:    misc-bug-people
>State:          open
>Class:          sw-bug
>Submitter-Id:   net
>Arrival-Date:   Mon Apr 07 01:15:01 +0000 2025
>Originator:     Taylor R Campbell
>Release:        current
>Organization:
The NanBSD Predication
>Environment:
>Description:
*** Check failed: /tmp/build/2025.04.04.21.52.19-vax/src/tests/lib/libc/gen/t_isnan.c:51: isinf(HUGE_VAL) != 0 not met
*** Check failed: /tmp/build/2025.04.04.21.52.19-vax/src/tests/lib/libc/gen/t_isnan.c:54: isinf(HUGE_VALF) != 0 not met
*** Check failed: /tmp/build/2025.04.04.21.52.19-vax/src/tests/lib/libc/gen/t_isnan.c:57: isinf(HUGE_VALL) != 0 not met

https://releng.netbsd.org/b5reports/vax/2025/2025.04.04.21.52.19/test.html#lib_libc_gen_t_isnan_isinf_basic

Obviously, these will not work because VAX doesn't have infinities, so isinf(HUGE_VAL) is always false.

But the test is also broken because it (as well as its sibling isnan_basic) is skipped on m68k, not on VAX, which is surely wrong:

     23 ATF_TC_BODY(isnan_basic, tc)
     24 {
     25 #if defined(__m68k__)
     26 	atf_tc_skip("Test not applicable on " MACHINE_ARCH);
     27 #endif
...
     44 ATF_TC_BODY(isinf_basic, tc)
     45 {
     46 #if defined(__m68k__)
     47 	atf_tc_skip("Test not applicable on " MACHINE_ARCH);
     48 #endif

https://nxr.netbsd.org/xref/src/tests/lib/libc/gen/t_isnan.c?r=1.5

This conditional was added in rev. 1.4 with the following explanation for PR bin/48582:

    Use compiler builtins instead of atf_arch and atf_machine.
    
    The atf_arch and atf_machine configuration variables were removed from
    atf-0.19 without me realizing that some tests were querying them directly.
    
    Instead of reintroducing those variables, just rely on compiler builtins
    as many other tests already do.
    
    Should fix PR bin/48582.

But what it changed was:

+#if defined(__m68k__)
+       atf_tc_skip("Test not applicable on " MACHINE_ARCH);
+#endif
...
-       arch = atf_config_get("atf_arch");
-
-       if (strcmp("m68000", arch) == 0)
-               atf_tc_skip("Test not applicable on %s", arch);


Now that in turn was from rev. 1.2 with the following message:

    Make it compile on archs where NAN is not defined - previously it only
    compiled by chance (and details of the __isnan macro) on vax.

which changed:

-       if (strcmp("vax", arch) == 0 || strcmp("m68000", arch) == 0)
+       if (strcmp("m68000", arch) == 0)
                atf_tc_skip("Test not applicable on %s", arch);
        else {
+#ifdef NAN
                ATF_TP_ADD_TC(tp, isnan_basic);
+#endif
                ATF_TP_ADD_TC(tp, isinf_basic);

The original test, strcmp("vax",arch) == 0 || strcmp("m68000", arch) == 0, came from:

      9 .if (${MACHINE_ARCH} != "vax" && ${MACHINE_ARCH} != "m68000")
     10 SUBDIR+= ieeefp
     11 .endif

https://nxr.netbsd.org/xref/src/regress/lib/libc/Makefile?r=1.69#9

Now, `m68000' is not an alias for `m68k'.  `m68000' is _only_ used by sun2 (which, strictly speaking, for machines that use Motorola 68010 CPUs, if I understand correctly; the Motorola 68000 lacked support for virtual memory, which NetBSD requires) -- the others use `m68k':

$ ./build.sh list-arch | grep m68
MACHINE=amiga           MACHINE_ARCH=m68k
MACHINE=atari           MACHINE_ARCH=m68k
MACHINE=cesfic          MACHINE_ARCH=m68k
MACHINE=hp300           MACHINE_ARCH=m68k
MACHINE=luna68k         MACHINE_ARCH=m68k
MACHINE=mac68k          MACHINE_ARCH=m68k
MACHINE=mvme68k         MACHINE_ARCH=m68k
MACHINE=news68k         MACHINE_ARCH=m68k
MACHINE=next68k         MACHINE_ARCH=m68k
MACHINE=sun2            MACHINE_ARCH=m68000
MACHINE=sun3            MACHINE_ARCH=m68k
MACHINE=virt68k         MACHINE_ARCH=m68k
MACHINE=x68k            MACHINE_ARCH=m68k

If I surmise correctly, MACHINE_ARCH=m68000 is only for machines that lack a floating-point coprocessor, support for which was new in the Motorola 68020.  But the actual conditional for m68020 was added in regress/lib/libc/Makefile rev. 1.26 in 2002 without explanation:

    On m68000, don't descend into ieeefp.

-.if (${MACHINE_ARCH} != "arm32" && ${MACHINE_ARCH} != "vax")
+.if (${MACHINE_ARCH} != "arm32" && ${MACHINE_ARCH} != "vax" && ${MACHINE_ARCH} != "m68000")

In any case, I think it is wrong to skip these tests on m68k -- and I'm not even sure they should be skipped on m68000, because surely it supports softfloat (whether a softfloat ABI or kernel softfloat emulation of hardfloat instructions), no?

But these tests _should_ be skipped on VAX because there is no NaN or infinity on VAX.
>How-To-Repeat:
cd /usr/tests/lib/libc/stdlib
atf-run t_strtod | atf-report
>Fix:
1. Don't skip these tests on m68k.
2. _Maybe_ skip these tests on m68000.  I think we can distinguish these by:

#if defined __m68k__ && !defined __mc68020__    // m68000

3. Do skip isinf_basic on VAX just like we skip isnan_basic.  Probably best to do this with #ifdef __vax__, because other tests already use isinf(HUGE_VAL) as a proxy for whether the architecture supports infinities at all so they _don't_ have to splatter #ifdef __vax__ all over the code.