Bug in GiNaC 1.7.0
Hi, I found a regression in GiNaC 1.7.0. In basic.cpp (function subs_one_level) the lines 588 auto it = m.find(*this); 589 if (it != m.end()) 590 return it->second; 591 return *this; are causing problems. In GiNaC 1.6.7 we have 599 ex thisex = *this; 600 it = m.find(thisex); 601 if (it != m.end()) 602 return it->second; 603 return thisex; which work properly. The problem with the 1.7.0 version is, that the this-pointer is dynamically allocated in line 618 (when called from line 629). *this is implicitly casted into an ex, both in line 588 and in line 591. As the dynallocated-flag is set, the first cast destroys the this-pointer, and the second cast crashes. Mario
Hi, On 07/19/2016 08:30 PM, Mario Prausa wrote:
I found a regression in GiNaC 1.7.0.
In basic.cpp (function subs_one_level) the lines
588 auto it = m.find(*this); 589 if (it != m.end()) 590 return it->second; 591 return *this;
are causing problems. In GiNaC 1.6.7 we have
599 ex thisex = *this; 600 it = m.find(thisex); 601 if (it != m.end()) 602 return it->second; 603 return thisex;
which work properly.
The problem with the 1.7.0 version is, that the this-pointer is dynamically allocated in line 618 (when called from line 629).
*this is implicitly casted into an ex, both in line 588 and in line 591. As the dynallocated-flag is set, the first cast destroys the this-pointer, and the second cast crashes.
Thank you very much for carefully debugging this regression and explaining it. Such attention and enduringness is highly appreciated! It was wrong to remove the temporary variable thisex in 1b8bcb06. The obvious fix is to reinstate the temporary ex. This section definitely should have a comment explaining what's going on so it doesn't happen again. (@Ralf: I see that you've independently introduced the same bug in pynac.) @Mario, since you found the regression, could you, please, send a small test case in order to put it into the regression tests? All my best, -richy. -- Richard B. Kreckel <http://in.terlu.de/~kreckel/>
Hi, it wasn't that easy to come up with a minimal code, as we need to force basic::subs to be called (which seems to be overridden in most classes provided by ginac). The attached example is based on the scalar product example in the tutorial. It segfaults in GiNaC 1.7.0, but works in older versions as well as in my own patched version, where I restored the temporary variable. Mario On 20.07.2016 09:10, Richard B. Kreckel wrote:
Hi,
On 07/19/2016 08:30 PM, Mario Prausa wrote:
I found a regression in GiNaC 1.7.0.
In basic.cpp (function subs_one_level) the lines
588 auto it = m.find(*this); 589 if (it != m.end()) 590 return it->second; 591 return *this;
are causing problems. In GiNaC 1.6.7 we have
599 ex thisex = *this; 600 it = m.find(thisex); 601 if (it != m.end()) 602 return it->second; 603 return thisex;
which work properly.
The problem with the 1.7.0 version is, that the this-pointer is dynamically allocated in line 618 (when called from line 629).
*this is implicitly casted into an ex, both in line 588 and in line 591. As the dynallocated-flag is set, the first cast destroys the this-pointer, and the second cast crashes. Thank you very much for carefully debugging this regression and explaining it. Such attention and enduringness is highly appreciated!
It was wrong to remove the temporary variable thisex in 1b8bcb06. The obvious fix is to reinstate the temporary ex. This section definitely should have a comment explaining what's going on so it doesn't happen again. (@Ralf: I see that you've independently introduced the same bug in pynac.)
@Mario, since you found the regression, could you, please, send a small test case in order to put it into the regression tests?
All my best, -richy.
participants (2)
-
Mario Prausa
-
Richard B. Kreckel