NetBSD Problem Report #59266

From www@netbsd.org  Mon Apr  7 01:10:58 2025
Return-Path: <www@netbsd.org>
Received: from mail.netbsd.org (mail.netbsd.org [199.233.217.200])
	(using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits)
	 key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256
	 client-signature RSA-PSS (2048 bits) client-digest SHA256)
	(Client CN "mail.NetBSD.org", Issuer "mail.NetBSD.org CA" (not verified))
	by mollari.NetBSD.org (Postfix) with ESMTPS id C382D1A9239
	for <gnats-bugs@gnats.NetBSD.org>; Mon,  7 Apr 2025 01:10:57 +0000 (UTC)
Message-Id: <20250407011056.5469B1A923C@mollari.NetBSD.org>
Date: Mon,  7 Apr 2025 01:10:56 +0000 (UTC)
From: campbell+netbsd@mumble.net
Reply-To: campbell+netbsd@mumble.net
To: gnats-bugs@NetBSD.org
Subject: t_isnan:isinf_basic test is bogus on some ports
X-Send-Pr-Version: www-1.0

>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
>Last-Modified:  Mon Apr 07 01:55:00 +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.

>Audit-Trail:
From: "Taylor R Campbell" <riastradh@netbsd.org>
To: gnats-bugs@gnats.NetBSD.org
Cc: 
Subject: PR/59266 CVS commit: src/tests/lib/libc/gen
Date: Mon, 7 Apr 2025 01:54:22 +0000

 Module Name:	src
 Committed By:	riastradh
 Date:		Mon Apr  7 01:54:22 UTC 2025

 Modified Files:
 	src/tests/lib/libc/gen: t_isnan.c

 Log Message:
 tests/lib/libc/gen/t_isnan: Fix architecture conditionals.

 1. Skip isinf_basic on VAX.  We have no other way to query `does this
    architecture support infinities?' and other tests already use
    isinf(HUGE_VAL) to determine that, so let's just make sure it
    works correctly on VAX.

 2. Don't skip these tests on m68k.  This was originally conditional
    on m68000 (which is to say, NetBSD's MACHINE_ARCH for Motorola
    68010 CPUs, i.e., the sun2 port), and was later mistakenly changed
    to m68k.  But there's no reason any m68k, 68010 or later, should
    skip this -- the floating-point format of the ABI does have NaN
    and infinity, whether it's a softfloat ABI or hardfloat ABI.

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


 To generate a diff of this commit:
 cvs rdiff -u -r1.5 -r1.6 src/tests/lib/libc/gen/t_isnan.c

 Please note that diffs are not public domain; they are subject to the
 copyright notices on the relevant files.

Home	PR Database Search