Bug 133311 - Crash if correcting reference to non-existent scalar in an equation
Summary: Crash if correcting reference to non-existent scalar in an equation
Status: RESOLVED FIXED
Alias: None
Product: kst
Classification: Applications
Component: general (show other bugs)
Version: 1.x
Platform: unspecified Linux
: NOR crash
Target Milestone: ---
Assignee: kst
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2006-08-31 03:29 UTC by Netterfield
Modified: 2006-10-07 00:13 UTC (History)
0 users

See Also:
Latest Commit:
Version Fixed In:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Netterfield 2006-08-31 03:29:54 UTC
Version:           1.3.0_devel (using KDE 3.5.4, Kubuntu Package 4:3.5.4-0ubuntu2~dapper1 )
Compiler:          Target: i486-linux-gnu
OS:                Linux (i686) release 2.6.15-26-686

Start kst
Create an equation which refers to a non existent scalar (eg, [BLABLA])

The equation is evaluated, but is nonsense (maybe 0) because it refers to a non-existent scalar.

Edit the equation and change the equation so it refers to something that actually does exist (eg, [C_PI]).

kst: FATAL: Thread -1239136576 tried to unlock KstRWLock 0x818345c (unlocked) without holding the lock

Either the equation should should not allow the bad equation to be created in the first place, or it should allow itself to get fixed without puking....
Comment 1 Netterfield 2006-08-31 04:15:59 UTC
This crash happens if you increase the number of valid scalars in an equation.

So, change an equation from x to x + [C_PI] and it will crash.

It is not related to referring to an invalid scalar, except that fixing it increases the number of valid scalars, causing the crash.
Comment 2 George Staikos 2006-08-31 04:57:01 UTC
Also it seems that there are some cases where Kst can get really messed up such as:

kst --ye "2*[C_D]" foo.txt
Comment 3 Netterfield 2006-09-01 02:50:26 UTC
This bug can also generate a hang:
create an equation which uses [C_PI].
Edit the equation so that it no longer uses [C_PI].
Now create a new equation which uses [C_PI]: hang on [OK].

It looks like on apply equation edit, kst locks all scalars which were used by the equation, re-parses the equation, and then attempts to unlock all of the scalars which are now used.  So
  -adding a scalar causes an attempt to unlock a not-locked scalar.
  -removing a scalar leaves it locked.

Comment 4 Adam Treat 2006-10-07 00:13:25 UTC
SVN commit 593145 by treat:

* Do not allow equation parsing to proceed when
the equation references non-existent objects, whether
they be scalars, vectors or else.

BUG: 133311
BUG: 134499
BUG: 134996



 M  +18 -9     enodes.cpp  
 M  +5 -5      enodes.h  
 M  +10 -7     kstequation.cpp  


--- trunk/extragear/graphics/kst/src/libkstmath/enodes.cpp #593144:593145
@@ -93,7 +93,8 @@
 }
 
 
-void Node::collectObjects(KstVectorMap&, KstScalarMap&, KstStringMap&) {
+bool Node::collectObjects(KstVectorMap&, KstScalarMap&, KstStringMap&) {
+  return true;
 }
 
 
@@ -128,9 +129,11 @@
 }
 
 
-void BinaryNode::collectObjects(KstVectorMap& v, KstScalarMap& s, KstStringMap& t) {
-  _left->collectObjects(v, s, t);
-  _right->collectObjects(v, s, t);
+bool BinaryNode::collectObjects(KstVectorMap& v, KstScalarMap& s, KstStringMap& t) {
+  bool ok = true;
+  ok = _left->collectObjects(v, s, t) ? ok : false;
+  ok = _right->collectObjects(v, s, t) ? ok : false;
+  return ok;
 }
 
 
@@ -600,8 +603,8 @@
 }
 
 
-void Function::collectObjects(KstVectorMap& v, KstScalarMap& s, KstStringMap& t) {
-  _args->collectObjects(v, s, t);
+bool Function::collectObjects(KstVectorMap& v, KstScalarMap& s, KstStringMap& t) {
+  return _args->collectObjects(v, s, t);
 }
 
 
@@ -651,10 +654,12 @@
 }
 
 
-void ArgumentList::collectObjects(KstVectorMap& v, KstScalarMap& s, KstStringMap& t) {
+bool ArgumentList::collectObjects(KstVectorMap& v, KstScalarMap& s, KstStringMap& t) {
+  bool ok = true;
   for (Node *i = _args.first(); i; i = _args.next()) {
-    i->collectObjects(v, s, t);
+    ok = i->collectObjects(v, s, t) ? ok : false;
   }
+  return ok;
 }
 
 
@@ -853,7 +858,7 @@
 }
 
 
-void Data::collectObjects(KstVectorMap& v, KstScalarMap& s, KstStringMap& t) {
+bool Data::collectObjects(KstVectorMap& v, KstScalarMap& s, KstStringMap& t) {
   if (_isEquation) {
     if (_equation) {
       _equation->collectObjects(v, s, t);
@@ -862,7 +867,11 @@
     v.insert(_tagName, _vector);
   } else if (_scalar && !s.contains(_tagName)) {
     s.insert(_tagName, _scalar);
+  } else {
+    KstDebug::self()->log(i18n("Equation has unknown object [%1].").arg(_tagName), KstDebug::Error);
+    return false;
   }
+  return true;
 }
 
 
--- trunk/extragear/graphics/kst/src/libkstmath/enodes.h #593144:593145
@@ -57,7 +57,7 @@
       virtual ~Node();
 
       virtual bool isConst() = 0; // can't be const
-      virtual void collectObjects(KstVectorMap& v, KstScalarMap& s, KstStringMap& t);
+      virtual bool collectObjects(KstVectorMap& v, KstScalarMap& s, KstStringMap& t);
       virtual bool takeVectors(const KstVectorMap& c);
       virtual double value(Context*) = 0;
       virtual void visit(NodeVisitor*);
@@ -75,7 +75,7 @@
       BinaryNode(Node *left, Node *right);
       virtual ~BinaryNode();
 
-      virtual void collectObjects(KstVectorMap& v, KstScalarMap& s, KstStringMap& t);
+      virtual bool collectObjects(KstVectorMap& v, KstScalarMap& s, KstStringMap& t);
       virtual bool takeVectors(const KstVectorMap& c);
       virtual void visit(NodeVisitor*);
       virtual KstObject::UpdateType update(int counter, Context *ctx);
@@ -102,7 +102,7 @@
       double value(Context*) { return 0.0; }
 
       bool isConst();
-      void collectObjects(KstVectorMap& v, KstScalarMap& s, KstStringMap& t);
+      bool collectObjects(KstVectorMap& v, KstScalarMap& s, KstStringMap& t);
       bool takeVectors(const KstVectorMap& c);
       double at(int, Context*);
       Node *node(int idx);
@@ -124,7 +124,7 @@
       bool isPlugin() const;
       bool isConst();
       double value(Context*);
-      void collectObjects(KstVectorMap& v, KstScalarMap& s, KstStringMap& t);
+      bool collectObjects(KstVectorMap& v, KstScalarMap& s, KstStringMap& t);
       bool takeVectors(const KstVectorMap& c);
       KstObject::UpdateType update(int counter, Context *ctx);
       QString text() const;
@@ -186,7 +186,7 @@
 
       bool isConst();
       double value(Context*);
-      void collectObjects(KstVectorMap& v, KstScalarMap& s, KstStringMap& t);
+      bool collectObjects(KstVectorMap& v, KstScalarMap& s, KstStringMap& t);
       bool takeVectors(const KstVectorMap& c);
       KstObject::UpdateType update(int counter, Context *ctx);
       QString text() const;
--- trunk/extragear/graphics/kst/src/libkstmath/kstequation.cpp #593144:593145
@@ -268,9 +268,6 @@
   setDirty();
   _equation = in_fn;
   VectorsUsed.clear();
-  for (KstScalarMap::Iterator i = _inputScalars.begin(); i != _inputScalars.end(); ++i) {
-    (*i)->unlock();
-  }
   _inputScalars.clear();
   _ns = 2; // reset the updating
   delete _pe;
@@ -288,17 +285,23 @@
       ctx.xVector = *_xInVector;
       Equation::FoldVisitor vis(&ctx, &_pe);
       KstStringMap sm;
-      _pe->collectObjects(VectorsUsed, _inputScalars, sm);
-      for (KstScalarMap::Iterator i = _inputScalars.begin(); i != _inputScalars.end(); ++i) {
-        (*i)->readLock();
+      if (_pe->collectObjects(VectorsUsed, _inputScalars, sm)) {
+        _pe->update(-1, &ctx);
+      } else {
+        //we have bad objects...
+        KstDebug::self()->log(i18n("Equation [%1] references non-existent objects.").arg(_equation), KstDebug::Error);
+        _inputScalars.clear();
+        delete (Equation::Node*)ParsedEquation;
+        ParsedEquation = 0L;
+        Equation::mutex().unlock();
       }
-      _pe->update(-1, &ctx);
     } else {
       // Parse error
       KstDebug::self()->log(i18n("Equation [%1] failed to parse.  Errors follow.").arg(_equation), KstDebug::Warning);
       for (QStringList::ConstIterator i = Equation::errorStack.begin(); i != Equation::errorStack.end(); ++i) {
         KstDebug::self()->log(i18n("Parse Error: %1").arg(*i), KstDebug::Warning);
       }
+      _inputScalars.clear();
       delete (Equation::Node*)ParsedEquation;
       ParsedEquation = 0L;
       Equation::mutex().unlock();