Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Assignment of -0 in arithmetic expansion doesn't remove negative sign #781

Closed
laurenthuberdeau opened this issue Aug 31, 2024 · 5 comments
Closed
Labels
bug Something is not working wontfix This will not be worked on

Comments

@laurenthuberdeau
Copy link

laurenthuberdeau commented Aug 31, 2024

Assignment of -0 in arithmetic expansion assigns -0 to variables on ARM. ksh on x86 and other shells remove the negative sign, which makes me think it may be due to some undefined behavior like in #770.

This short example shows the bug.

> ksh
$ : $(( a = 0 ))
$ : $(( b = -0 ))
$ : $(( c = +0 ))
$ echo $a # print 0
0
$ echo $b # print -0 instead of 0
-0
$ echo $c # print 0 as expected
0
@ko1nksm
Copy link

ko1nksm commented Sep 5, 2024

Here is another example on Linux on x86_64. I don't think this includes undefined behavior.

$ ksh -c 'v=0; (( v = -1 * v )); echo $v'
-0

This issue may have been introduced between ksh93-2007-06-28 and ksh93-2007-11-05.

root@6eb222d646da:/# ksh93-2007-06-28 -c 'v=0; (( v = -1 * v )); echo $v'
0

root@6eb222d646da:/# ksh93-2007-11-05 -c 'v=0; (( v = -1 * v )); echo $v'
-0

@McDutchie
Copy link

My stash of historic versions (compiled on Debian arm64) says it was introduced between 2007-09-26 and 2007-10-31:

$ ksh_2007-09-26 -c 'v=0; (( v = -1 * v )); echo $v'
0
$ ksh_2007-10-31 -c 'v=0; (( v = -1 * v )); echo $v'
-0

@McDutchie
Copy link

There was only one arithmetic-related change between 2007-09-26 and 2007-10-31, and it's this one:

    Version: 2007-10-31 ksh93s+
    
    Sourced using the following ksh93-integration diff:
    3721fc2fa6fe51d5060dae7054b5c2692c811da692925ce15942c4d083a5ba18  ksh93_20070922_to_20071031.diff.txt

diff --git a/src/cmd/ksh93/sh/arith.c b/src/cmd/ksh93/sh/arith.c
index cb661177..f2e3773f 100644
--- a/src/cmd/ksh93/sh/arith.c
+++ b/src/cmd/ksh93/sh/arith.c
@@ -343,6 +343,8 @@ Sfdouble_t sh_strnum(register const char *str, char** ptr, int mode)
 		if(!ptr && *last && mode>0)
 			errormsg(SH_DICT,ERROR_exit(1),e_lexbadchar,*last,str);
 	}
+	else if (!d && *str=='-')
+		d = -0.0;
 	if(ptr)
 		*ptr = last;
 	return(d);

@McDutchie
Copy link

McDutchie commented Nov 2, 2024

Here's a patch to revert that change on ksh 93u+m:

diff --git a/src/cmd/ksh93/sh/arith.c b/src/cmd/ksh93/sh/arith.c
index b53b0f36c..e32b5f08e 100644
--- a/src/cmd/ksh93/sh/arith.c
+++ b/src/cmd/ksh93/sh/arith.c
@@ -594,8 +594,6 @@ Sfdouble_t sh_strnum(const char *str, char** ptr, int mode)
 				}
 			}
 		}
-		else if(!d && *str=='-')
-			d = -0.0;
 	}
 	if(ptr)
 		*ptr = last;

Removing those lines fixes this issue (as expected), but introduces two regression test failures:

	arith.sh[504]: FAIL: %g of x $x $((x)) for x=-0 should all be -0
	arith.sh[505]: FAIL: %g of y $y $((y)) for y=-0.0 should all be -0

Those tests are (see lines 499, 504, 505):

float x=-0 y=-0.0
r=-0
[[ $((-0)) == 0 ]] || err_exit '$((-0)) should be 0'
[[ $(( -1*0)) == 0 ]] || err_exit '$(( -1*0)) should be 0'
[[ $(( -1.0*0)) == -0 ]] || err_exit '$(( -1.0*0)) should be -0'
[[ $(printf "%g %g %g\n" x $x $((x)) ) == '-0 -0 -0' ]] || err_exit '%g of x $x $((x)) for x=-0 should all be -0'
[[ $(printf "%g %g %g\n" y $x $((y)) ) == '-0 -0 -0' ]] || err_exit '%g of y $y $((y)) for y=-0.0 should all be -0'

@McDutchie
Copy link

McDutchie commented Nov 2, 2024

I'm now leaning towards thinking that this is not a bug.

Zero is signed in floating point arithmetic; 0.0 and -0.0 are different. On ksh, floating point arithmetic is (currently) the default.

If you declare the variable as integer, the problem goes away:

$ typeset -i v; ((v = -0)); echo $v
0

Perhaps if a solution to #771 is ever figured out, this may change.

@McDutchie McDutchie added the wontfix This will not be worked on label Nov 21, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something is not working wontfix This will not be worked on
Projects
None yet
Development

No branches or pull requests

3 participants