Bug 272539 - cospi, sinpi and tanpi incorrectly test for NaN and Infinity
Summary: cospi, sinpi and tanpi incorrectly test for NaN and Infinity
Status: Closed FIXED
Alias: None
Product: Base System
Classification: Unclassified
Component: bin (show other bugs)
Version: CURRENT
Hardware: Any Any
: --- Affects Many People
Assignee: freebsd-numerics (Nobody)
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2023-07-17 02:24 UTC by Paul Green
Modified: 2023-07-31 15:06 UTC (History)
4 users (show)

See Also:


Attachments
patch (944 bytes, patch)
2023-07-17 04:13 UTC, Steve Kargl
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Paul Green 2023-07-17 02:24:43 UTC
The current versions of lib/msun/src/s_cospi.c, s_sinpi.c and s_tanpi.c all exhibit the same defect. After checking for various numeric ranges, they check to see whether the input argument is a NaN or an Infinity. However, the code uses a value of 0x7f80000 instead of the correct value of 0x7ff00000. See line 141 of s_cospi.c, line 158 of s_sinpi.c, and line 166 of s_tanpi.c. 

I strongly recommend that s_cospi.c and s_sinpi.c have a comment added just before this line to say that the code is testing for infinity or nan. Such a comment already exists in s_tanpi.c.

If you review s_cospif.c, s_sinpif.c, and s_tanpif.c, you will see that the equivalent statements in these functions are accurate and have appropriate source comments.

Cross-ref: PR 218514, which added these functions and is still open for reasons that are unclear.

I have run this issue past the author of these functions and he agrees with my analysis. 

The impact of these defects is to flag some valid input values as invalid and raise a pole error (divide by zero).
Comment 1 Steve Kargl freebsd_committer freebsd_triage 2023-07-17 04:11:31 UTC
All, as the author of the code in question, Paul contacted me directly (which is fine!).  I concur with his analysis.  Here's a diff with the suggested changes.

diff --git a/lib/msun/src/s_cospi.c b/lib/msun/src/s_cospi.c
index 860219efd3e4..10e34e0d3e58 100644
--- a/lib/msun/src/s_cospi.c
+++ b/lib/msun/src/s_cospi.c
@@ -138,7 +138,7 @@ cospi(double x)
 		return (j0 & 1 ? -c : c);
 	}
 
-	if (ix >= 0x7f800000)
+	if (ix >= 0x7ff00000)
 		return (vzero / vzero);
 
 	/*
diff --git a/lib/msun/src/s_sinpi.c b/lib/msun/src/s_sinpi.c
index 858459a5fcb4..a114229d6df7 100644
--- a/lib/msun/src/s_sinpi.c
+++ b/lib/msun/src/s_sinpi.c
@@ -155,7 +155,7 @@ sinpi(double x)
 		return ((hx & 0x80000000) ? -s : s);
 	}
 
-	if (ix >= 0x7f800000)
+	if (ix >= 0x7ff00000)
 		return (vzero / vzero);
 
 	/*
diff --git a/lib/msun/src/s_tanpi.c b/lib/msun/src/s_tanpi.c
index 01d4c74367fd..f911d56156b3 100644
--- a/lib/msun/src/s_tanpi.c
+++ b/lib/msun/src/s_tanpi.c
@@ -163,7 +163,7 @@ tanpi(double x)
 	}
 
 	/* x = +-inf or nan. */
-	if (ix >= 0x7f800000)
+	if (ix >= 0x7ff00000)
 		return (vzero / vzero);
 
 	/*
Comment 2 Steve Kargl freebsd_committer freebsd_triage 2023-07-17 04:13:06 UTC
Created attachment 243428 [details]
patch

I've attached the diff as I don't remember if it is preferred.
Comment 3 commit-hook freebsd_committer freebsd_triage 2023-07-17 05:26:38 UTC
A commit in branch main references this bug:

URL: https://cgit.FreeBSD.org/src/commit/?id=be4c7f273508994638b68d2fae742be37d3cb117

commit be4c7f273508994638b68d2fae742be37d3cb117
Author:     Steve Kargl <kargl@FreeBSD.org>
AuthorDate: 2023-07-17 05:19:28 +0000
Commit:     Konstantin Belousov <kib@FreeBSD.org>
CommitDate: 2023-07-17 05:23:27 +0000

    libm: correctly test for for NaN and Infinity in sinpi(), cospi(), and tanpi()

    The current versions of lib/msun/src/s_cospi.c, s_sinpi.c and s_tanpi.c
    all exhibit the same defect. After checking for various numeric ranges,
    they check to see whether the input argument is a NaN or an Infinity.
    However, the code uses a value of 0x7f80000 instead of the correct value
    of 0x7ff00000.

    If you review s_cospif.c, s_sinpif.c, and s_tanpif.c, you will see that
    the equivalent statements in these functions are accurate and have
    appropriate source comments.

    The impact of these defects is to flag some valid input values as
    invalid and raise a pole error (divide by zero).

    Reported by:    Paul Green <Paul.Green@stratus.com>
    PR:     272539
    MFC after:      1 week

 lib/msun/src/s_cospi.c | 3 ++-
 lib/msun/src/s_sinpi.c | 3 ++-
 lib/msun/src/s_tanpi.c | 2 +-
 3 files changed, 5 insertions(+), 3 deletions(-)
Comment 4 commit-hook freebsd_committer freebsd_triage 2023-07-23 11:46:01 UTC
A commit in branch stable/13 references this bug:

URL: https://cgit.FreeBSD.org/src/commit/?id=b14d94c27ce2d5ca47193d99db51f516e74c2f74

commit b14d94c27ce2d5ca47193d99db51f516e74c2f74
Author:     Steve Kargl <kargl@FreeBSD.org>
AuthorDate: 2023-07-17 05:19:28 +0000
Commit:     Konstantin Belousov <kib@FreeBSD.org>
CommitDate: 2023-07-23 11:44:24 +0000

    libm: correctly test for for NaN and Infinity in sinpi(), cospi(), and tanpi()

    PR:     272539

    (cherry picked from commit be4c7f273508994638b68d2fae742be37d3cb117)

 lib/msun/src/s_cospi.c | 3 ++-
 lib/msun/src/s_sinpi.c | 3 ++-
 lib/msun/src/s_tanpi.c | 2 +-
 3 files changed, 5 insertions(+), 3 deletions(-)