[ruby-core:55550] [ruby-trunk - Bug #8542][Open] BigMath::exp modifies its first argument

[ruby-core:55550] [ruby-trunk - Bug #8542][Open] BigMath::exp modifies its first argument

[GSnyder (Garth Snyder)]

Issue #8542 has been reported by GSnyder (Garth Snyder).

----------------------------------------
Bug #8542: BigMath::exp modifies its first argument
https://bugs.ruby-lang.org/issues/8542

Author: GSnyder (Garth Snyder)
Status: Open
Priority: Normal
Assignee: 
Category: 
Target version: 
ruby -v: ruby 2.0.0p195 (2013-05-14 revision 40734) [i386-solaris2.11]
Backport: 1.9.3: UNKNOWN, 2.0.0: UNKNOWN


=begin
I noticed this when creating the patch at https://github.com/ruby/ruby/pull/332. I believe it affects everything from ruby 1.8 up.

BigMath::exp is implemented for negative numbers according to the identity E ** -x == 1 / (E ** x). At bigdecimal.c:2742 (in GitHub commit 258acf3, in the implementation of BigMath_s_exp), the code flips the sign on the input argument for negative numbers. Later, it returns the reciprocal of the result.

Problem: When the first argument is a BigDecimal (or, presumably, Bignum), the original argument is modified, so its sign bit remains flipped. Hence:

 $ irb
 2.0.0-p195 :001 > require 'bigdecimal/math'
 => true 
 2.0.0-p195 :002 > x = BigDecimal(-1)
 => #<BigDecimal:827d960,'-0.1E1',9(36)> 
 2.0.0-p195 :003 > BigMath.exp(x, 20)
 => #<BigDecimal:822dca8,'0.3678794411 714423216E0',27(54)> 
 2.0.0-p195 :004 > x
 => #<BigDecimal:827d960,'0.1E1',9(36)> 


Here, x is modified when used as an argument to BigMath.exp AND the answer is wrong. I have already submitted the patch mentioned above for the latter problem, but I'm not sure what the appropriate fix would be for the sign modification. Just resetting the sign bit on exit would be easy but not thread safe; BigMath.exp really shouldn't be modifying the argument at all. But copying the whole argument is potentially wasteful if the precision is high.

I suspect that the special calculation track for negative values is actually not needed at all. Without the patch I just submitted, BigMath.exp is reliably returning the reciprocal of the correct answer, which means that it's properly calculating the correct answer by using the (negative) VALUE x passed in as the original argument -- at least for immediate values. So perhaps the basic iteration loop is just as valid for negative exponents as it is already implemented.
=end


-- 
http://bugs.ruby-lang.org/

[ruby-core:55551] [ruby-trunk - Bug #8542] BigMath::exp modifies its first argument

[GSnyder (Garth Snyder)]

Issue #8542 has been updated by GSnyder (Garth Snyder).


It appears that the iteration loop in BigMath_s_exp is just calculating the series expansion (X ** N) / N! from N = 0 until the incremental term vanishes below the precision threshold. I don't see any reason this would not be equally valid for negative X. The code already checks the increment d with VpIsZero(), which accommodates either negative or positive zero.

----------------------------------------
Bug #8542: BigMath::exp modifies its first argument
https://bugs.ruby-lang.org/issues/8542#change-40049

Author: GSnyder (Garth Snyder)
Status: Open
Priority: Normal
Assignee: 
Category: 
Target version: 
ruby -v: ruby 2.0.0p195 (2013-05-14 revision 40734) [i386-solaris2.11]
Backport: 1.9.3: UNKNOWN, 2.0.0: UNKNOWN


=begin
I noticed this when creating the patch at https://github.com/ruby/ruby/pull/332. I believe it affects everything from ruby 1.8 up.

BigMath::exp is implemented for negative numbers according to the identity E ** -x == 1 / (E ** x). At bigdecimal.c:2742 (in GitHub commit 258acf3, in the implementation of BigMath_s_exp), the code flips the sign on the input argument for negative numbers. Later, it returns the reciprocal of the result.

Problem: When the first argument is a BigDecimal (or, presumably, Bignum), the original argument is modified, so its sign bit remains flipped. Hence:

 $ irb
 2.0.0-p195 :001 > require 'bigdecimal/math'
 => true 
 2.0.0-p195 :002 > x = BigDecimal(-1)
 => #<BigDecimal:827d960,'-0.1E1',9(36)> 
 2.0.0-p195 :003 > BigMath.exp(x, 20)
 => #<BigDecimal:822dca8,'0.3678794411 714423216E0',27(54)> 
 2.0.0-p195 :004 > x
 => #<BigDecimal:827d960,'0.1E1',9(36)> 


Here, x is modified when used as an argument to BigMath.exp AND the answer is wrong. I have already submitted the patch mentioned above for the latter problem, but I'm not sure what the appropriate fix would be for the sign modification. Just resetting the sign bit on exit would be easy but not thread safe; BigMath.exp really shouldn't be modifying the argument at all. But copying the whole argument is potentially wasteful if the precision is high.

I suspect that the special calculation track for negative values is actually not needed at all. Without the patch I just submitted, BigMath.exp is reliably returning the reciprocal of the correct answer, which means that it's properly calculating the correct answer by using the (negative) VALUE x passed in as the original argument -- at least for immediate values. So perhaps the basic iteration loop is just as valid for negative exponents as it is already implemented.
=end


-- 
http://bugs.ruby-lang.org/

[ruby-core:55555] [ruby-trunk - Bug #8542][Assigned] BigMath::exp modifies its first argument

[naruse (Yui NARUSE)]

Issue #8542 has been updated by naruse (Yui NARUSE).

Category set to ext
Status changed from Open to Assigned
Assignee set to mrkn (Kenta Murata)
Target version set to current: 2.1.0


----------------------------------------
Bug #8542: BigMath::exp modifies its first argument
https://bugs.ruby-lang.org/issues/8542#change-40053

Author: GSnyder (Garth Snyder)
Status: Assigned
Priority: Normal
Assignee: mrkn (Kenta Murata)
Category: ext
Target version: current: 2.1.0
ruby -v: ruby 2.0.0p195 (2013-05-14 revision 40734) [i386-solaris2.11]
Backport: 1.9.3: UNKNOWN, 2.0.0: UNKNOWN


=begin
I noticed this when creating the patch at https://github.com/ruby/ruby/pull/332. I believe it affects everything from ruby 1.8 up.

BigMath::exp is implemented for negative numbers according to the identity E ** -x == 1 / (E ** x). At bigdecimal.c:2742 (in GitHub commit 258acf3, in the implementation of BigMath_s_exp), the code flips the sign on the input argument for negative numbers. Later, it returns the reciprocal of the result.

Problem: When the first argument is a BigDecimal (or, presumably, Bignum), the original argument is modified, so its sign bit remains flipped. Hence:

 $ irb
 2.0.0-p195 :001 > require 'bigdecimal/math'
 => true 
 2.0.0-p195 :002 > x = BigDecimal(-1)
 => #<BigDecimal:827d960,'-0.1E1',9(36)> 
 2.0.0-p195 :003 > BigMath.exp(x, 20)
 => #<BigDecimal:822dca8,'0.3678794411 714423216E0',27(54)> 
 2.0.0-p195 :004 > x
 => #<BigDecimal:827d960,'0.1E1',9(36)> 


Here, x is modified when used as an argument to BigMath.exp AND the answer is wrong. I have already submitted the patch mentioned above for the latter problem, but I'm not sure what the appropriate fix would be for the sign modification. Just resetting the sign bit on exit would be easy but not thread safe; BigMath.exp really shouldn't be modifying the argument at all. But copying the whole argument is potentially wasteful if the precision is high.

I suspect that the special calculation track for negative values is actually not needed at all. Without the patch I just submitted, BigMath.exp is reliably returning the reciprocal of the correct answer, which means that it's properly calculating the correct answer by using the (negative) VALUE x passed in as the original argument -- at least for immediate values. So perhaps the basic iteration loop is just as valid for negative exponents as it is already implemented.
=end


-- 
http://bugs.ruby-lang.org/

[ruby-core:55567] [ruby-trunk - Bug #8542] BigMath::exp modifies its first argument

[GSnyder (Garth Snyder)]

Issue #8542 has been updated by GSnyder (Garth Snyder).


In the original report, I stated that the answer was wrong; that is incorrect. It's wrong (in the current trunk) if the first argument is a negative, immediate type, but BigDecimal arguments give correct answers. They just become modified in the process.

I experimented with just removing the special handling for negative arguments in BigMath.exp(), but there is in fact something odd about the calculation mechanism that gives incorrect results.
----------------------------------------
Bug #8542: BigMath::exp modifies its first argument
https://bugs.ruby-lang.org/issues/8542#change-40060

Author: GSnyder (Garth Snyder)
Status: Assigned
Priority: Normal
Assignee: mrkn (Kenta Murata)
Category: ext
Target version: current: 2.1.0
ruby -v: ruby 2.0.0p195 (2013-05-14 revision 40734) [i386-solaris2.11]
Backport: 1.9.3: UNKNOWN, 2.0.0: UNKNOWN


=begin
I noticed this when creating the patch at https://github.com/ruby/ruby/pull/332. I believe it affects everything from ruby 1.8 up.

BigMath::exp is implemented for negative numbers according to the identity E ** -x == 1 / (E ** x). At bigdecimal.c:2742 (in GitHub commit 258acf3, in the implementation of BigMath_s_exp), the code flips the sign on the input argument for negative numbers. Later, it returns the reciprocal of the result.

Problem: When the first argument is a BigDecimal (or, presumably, Bignum), the original argument is modified, so its sign bit remains flipped. Hence:

 $ irb
 2.0.0-p195 :001 > require 'bigdecimal/math'
 => true 
 2.0.0-p195 :002 > x = BigDecimal(-1)
 => #<BigDecimal:827d960,'-0.1E1',9(36)> 
 2.0.0-p195 :003 > BigMath.exp(x, 20)
 => #<BigDecimal:822dca8,'0.3678794411 714423216E0',27(54)> 
 2.0.0-p195 :004 > x
 => #<BigDecimal:827d960,'0.1E1',9(36)> 


Here, x is modified when used as an argument to BigMath.exp AND the answer is wrong. I have already submitted the patch mentioned above for the latter problem, but I'm not sure what the appropriate fix would be for the sign modification. Just resetting the sign bit on exit would be easy but not thread safe; BigMath.exp really shouldn't be modifying the argument at all. But copying the whole argument is potentially wasteful if the precision is high.

I suspect that the special calculation track for negative values is actually not needed at all. Without the patch I just submitted, BigMath.exp is reliably returning the reciprocal of the correct answer, which means that it's properly calculating the correct answer by using the (negative) VALUE x passed in as the original argument -- at least for immediate values. So perhaps the basic iteration loop is just as valid for negative exponents as it is already implemented.
=end


-- 
http://bugs.ruby-lang.org/

[ruby-core:60298] [ruby-trunk - Bug #8542] BigMath::exp modifies its first argument

[shibata.hiroshi]

Issue #8542 has been updated by Hiroshi SHIBATA.

Target version changed from 2.1.0 to current: 2.2.0

----------------------------------------
Bug #8542: BigMath::exp modifies its first argument
https://bugs.ruby-lang.org/issues/8542#change-44783

* Author: Garth Snyder
* Status: Assigned
* Priority: Normal
* Assignee: Kenta Murata
* Category: ext
* Target version: current: 2.2.0
* ruby -v: ruby 2.0.0p195 (2013-05-14 revision 40734) [i386-solaris2.11]
* Backport: 1.9.3: UNKNOWN, 2.0.0: UNKNOWN
----------------------------------------
=begin
I noticed this when creating the patch at https://github.com/ruby/ruby/pull/332. I believe it affects everything from ruby 1.8 up.

BigMath::exp is implemented for negative numbers according to the identity E ** -x == 1 / (E ** x). At bigdecimal.c:2742 (in GitHub commit 258acf3, in the implementation of BigMath_s_exp), the code flips the sign on the input argument for negative numbers. Later, it returns the reciprocal of the result.

Problem: When the first argument is a BigDecimal (or, presumably, Bignum), the original argument is modified, so its sign bit remains flipped. Hence:

 $ irb
 2.0.0-p195 :001 > require 'bigdecimal/math'
 => true 
 2.0.0-p195 :002 > x = BigDecimal(-1)
 => #<BigDecimal:827d960,'-0.1E1',9(36)> 
 2.0.0-p195 :003 > BigMath.exp(x, 20)
 => #<BigDecimal:822dca8,'0.3678794411 714423216E0',27(54)> 
 2.0.0-p195 :004 > x
 => #<BigDecimal:827d960,'0.1E1',9(36)> 


Here, x is modified when used as an argument to BigMath.exp AND the answer is wrong. I have already submitted the patch mentioned above for the latter problem, but I'm not sure what the appropriate fix would be for the sign modification. Just resetting the sign bit on exit would be easy but not thread safe; BigMath.exp really shouldn't be modifying the argument at all. But copying the whole argument is potentially wasteful if the precision is high.

I suspect that the special calculation track for negative values is actually not needed at all. Without the patch I just submitted, BigMath.exp is reliably returning the reciprocal of the correct answer, which means that it's properly calculating the correct answer by using the (negative) VALUE x passed in as the original argument -- at least for immediate values. So perhaps the basic iteration loop is just as valid for negative exponents as it is already implemented.
=end



-- 
http://bugs.ruby-lang.org/

[ruby-core:94245] [Ruby master Bug#8542] BigMath::exp modifies its first argument

[merch-redmine]

Issue #8542 has been updated by jeremyevans0 (Jeremy Evans).

Backport changed from 1.9.3: UNKNOWN, 2.0.0: UNKNOWN to 2.6: REQUIRED
File bigdecimal-exp-no-mutate-8542.patch added

This bug is still present in the master branch.  This is actually a fairly significant bug since Ruby 2.6, as starting in Ruby 2.6, BigDecimal instances are frozen and should never be mutated.  Attached is a patch that fixes this issue.  I think this fix should be backported to 2.6 because Ruby should not allow modification of frozen objects.

----------------------------------------
Bug #8542: BigMath::exp modifies its first argument
https://bugs.ruby-lang.org/issues/8542#change-80553

* Author: GSnyder (Garth Snyder)
* Status: Assigned
* Priority: Normal
* Assignee: mrkn (Kenta Murata)
* Target version: 
* ruby -v: ruby 2.0.0p195 (2013-05-14 revision 40734) [i386-solaris2.11]
* Backport: 2.6: REQUIRED
----------------------------------------
=begin
I noticed this when creating the patch at https://github.com/ruby/ruby/pull/332. I believe it affects everything from ruby 1.8 up.

BigMath::exp is implemented for negative numbers according to the identity E ** -x == 1 / (E ** x). At bigdecimal.c:2742 (in GitHub commit 258acf3, in the implementation of BigMath_s_exp), the code flips the sign on the input argument for negative numbers. Later, it returns the reciprocal of the result.

Problem: When the first argument is a BigDecimal (or, presumably, Bignum), the original argument is modified, so its sign bit remains flipped. Hence:

 $ irb
 2.0.0-p195 :001 > require 'bigdecimal/math'
 => true 
 2.0.0-p195 :002 > x = BigDecimal(-1)
 => #<BigDecimal:827d960,'-0.1E1',9(36)> 
 2.0.0-p195 :003 > BigMath.exp(x, 20)
 => #<BigDecimal:822dca8,'0.3678794411 714423216E0',27(54)> 
 2.0.0-p195 :004 > x
 => #<BigDecimal:827d960,'0.1E1',9(36)> 


Here, x is modified when used as an argument to BigMath.exp AND the answer is wrong. I have already submitted the patch mentioned above for the latter problem, but I'm not sure what the appropriate fix would be for the sign modification. Just resetting the sign bit on exit would be easy but not thread safe; BigMath.exp really shouldn't be modifying the argument at all. But copying the whole argument is potentially wasteful if the precision is high.

I suspect that the special calculation track for negative values is actually not needed at all. Without the patch I just submitted, BigMath.exp is reliably returning the reciprocal of the correct answer, which means that it's properly calculating the correct answer by using the (negative) VALUE x passed in as the original argument -- at least for immediate values. So perhaps the basic iteration loop is just as valid for negative exponents as it is already implemented.
=end

---Files--------------------------------
bigdecimal-exp-no-mutate-8542.patch (1.63 KB)


-- 
https://bugs.ruby-lang.org/

[ruby-core:94370] [Ruby master Bug#8542] BigMath::exp modifies its first argument

[merch-redmine]

Issue #8542 has been updated by jeremyevans0 (Jeremy Evans).


As bigdecimal upstream is in a separate repository, I submitted a pull request for this: https://github.com/ruby/bigdecimal/pull/150.

----------------------------------------
Bug #8542: BigMath::exp modifies its first argument
https://bugs.ruby-lang.org/issues/8542#change-80784

* Author: GSnyder (Garth Snyder)
* Status: Assigned
* Priority: Normal
* Assignee: mrkn (Kenta Murata)
* Target version: 
* ruby -v: ruby 2.0.0p195 (2013-05-14 revision 40734) [i386-solaris2.11]
* Backport: 2.6: REQUIRED
----------------------------------------
=begin
I noticed this when creating the patch at https://github.com/ruby/ruby/pull/332. I believe it affects everything from ruby 1.8 up.

BigMath::exp is implemented for negative numbers according to the identity E ** -x == 1 / (E ** x). At bigdecimal.c:2742 (in GitHub commit 258acf3, in the implementation of BigMath_s_exp), the code flips the sign on the input argument for negative numbers. Later, it returns the reciprocal of the result.

Problem: When the first argument is a BigDecimal (or, presumably, Bignum), the original argument is modified, so its sign bit remains flipped. Hence:

 $ irb
 2.0.0-p195 :001 > require 'bigdecimal/math'
 => true 
 2.0.0-p195 :002 > x = BigDecimal(-1)
 => #<BigDecimal:827d960,'-0.1E1',9(36)> 
 2.0.0-p195 :003 > BigMath.exp(x, 20)
 => #<BigDecimal:822dca8,'0.3678794411 714423216E0',27(54)> 
 2.0.0-p195 :004 > x
 => #<BigDecimal:827d960,'0.1E1',9(36)> 


Here, x is modified when used as an argument to BigMath.exp AND the answer is wrong. I have already submitted the patch mentioned above for the latter problem, but I'm not sure what the appropriate fix would be for the sign modification. Just resetting the sign bit on exit would be easy but not thread safe; BigMath.exp really shouldn't be modifying the argument at all. But copying the whole argument is potentially wasteful if the precision is high.

I suspect that the special calculation track for negative values is actually not needed at all. Without the patch I just submitted, BigMath.exp is reliably returning the reciprocal of the correct answer, which means that it's properly calculating the correct answer by using the (negative) VALUE x passed in as the original argument -- at least for immediate values. So perhaps the basic iteration loop is just as valid for negative exponents as it is already implemented.
=end

---Files--------------------------------
bigdecimal-exp-no-mutate-8542.patch (1.63 KB)


-- 
https://bugs.ruby-lang.org/

[ruby-core:95265] [Ruby master Bug#8542] BigMath::exp modifies its first argument

[merch-redmine]

Issue #8542 has been updated by jeremyevans0 (Jeremy Evans).

Status changed from Assigned to Closed

Pull request was merged: https://github.com/ruby/bigdecimal/pull/150 

----------------------------------------
Bug #8542: BigMath::exp modifies its first argument
https://bugs.ruby-lang.org/issues/8542#change-81942

* Author: GSnyder (Garth Snyder)
* Status: Closed
* Priority: Normal
* Assignee: mrkn (Kenta Murata)
* Target version: 
* ruby -v: ruby 2.0.0p195 (2013-05-14 revision 40734) [i386-solaris2.11]
* Backport: 2.6: REQUIRED
----------------------------------------
=begin
I noticed this when creating the patch at https://github.com/ruby/ruby/pull/332. I believe it affects everything from ruby 1.8 up.

BigMath::exp is implemented for negative numbers according to the identity E ** -x == 1 / (E ** x). At bigdecimal.c:2742 (in GitHub commit 258acf3, in the implementation of BigMath_s_exp), the code flips the sign on the input argument for negative numbers. Later, it returns the reciprocal of the result.

Problem: When the first argument is a BigDecimal (or, presumably, Bignum), the original argument is modified, so its sign bit remains flipped. Hence:

 $ irb
 2.0.0-p195 :001 > require 'bigdecimal/math'
 => true 
 2.0.0-p195 :002 > x = BigDecimal(-1)
 => #<BigDecimal:827d960,'-0.1E1',9(36)> 
 2.0.0-p195 :003 > BigMath.exp(x, 20)
 => #<BigDecimal:822dca8,'0.3678794411 714423216E0',27(54)> 
 2.0.0-p195 :004 > x
 => #<BigDecimal:827d960,'0.1E1',9(36)> 


Here, x is modified when used as an argument to BigMath.exp AND the answer is wrong. I have already submitted the patch mentioned above for the latter problem, but I'm not sure what the appropriate fix would be for the sign modification. Just resetting the sign bit on exit would be easy but not thread safe; BigMath.exp really shouldn't be modifying the argument at all. But copying the whole argument is potentially wasteful if the precision is high.

I suspect that the special calculation track for negative values is actually not needed at all. Without the patch I just submitted, BigMath.exp is reliably returning the reciprocal of the correct answer, which means that it's properly calculating the correct answer by using the (negative) VALUE x passed in as the original argument -- at least for immediate values. So perhaps the basic iteration loop is just as valid for negative exponents as it is already implemented.
=end

---Files--------------------------------
bigdecimal-exp-no-mutate-8542.patch (1.63 KB)


-- 
https://bugs.ruby-lang.org/