Bug 135354

Summary:	crash when equation contains [scalar][scalar]
Product:	[Applications] kst	Reporter:	Nicolas Brisset <nicolas.brisset>
Component:	general	Assignee:	kst
Status:	RESOLVED FIXED
Severity:	crash
Priority:	NOR
Version:	1.x
Target Milestone:	---
Platform:	unspecified
OS:	Solaris
Latest Commit:		Version Fixed In:
Sentry Crash Report:

Description Nicolas Brisset 2006-10-09 18:06:18 UTC

Version:           1.3.1_devel (using KDE 3.4.0, compiled sources)
Compiler:          gcc version 3.4.3
OS:                SunOS (sun4u) release 5.8

There is a problem in the equation parser, which leads to a crash. To reproduce:
1) load column 1 vs INDEX from gyrodata.dat
2) create an equation with [1]+[C_PI][C_PI] and "INDEX" as X vector
3) Hit "OK": crash!

It should either recognize [C_PI][C_PI] as [C_PI]*[C_PI] and do it correctly, or invalidate the syntax.
The way [1][1] is handled leaves me quite puzzled :-( But it does not seem to be very clean, as kst crashes upon exit (or even before) even though it is less obvious than the previous case.

Comment 1 Adam Treat 2006-10-09 18:28:19 UTC

Couple of comments...

1. An equation which references multiple scalars is indeed giving a bogus error that it can't find the scalar the second time.  That is easy to fix.

2. [1] is not an existing scalar.  Kst will interpret this as a reference to a scalar object found in the global list of scalars.  If it doesn't currently exist, it will throw an error.  You can check if it is listed in the 'View Scalars' dialog to be sure.

3.  Is the implicit multiply operator supposed to work?  It definitely shouldn't crash, but throw a parse error I'd imagine.

Comment 2 Nicolas Brisset 2006-10-09 18:38:53 UTC

In that case, [1] is the name of a vector (the first column from gyrodata.dat, the example file in misc/tutorial).

Comment 3 Adam Treat 2006-10-09 19:02:49 UTC

Ok, then #3 is the only relevant question and I've found a EParseErrorNoImplicitMultiply in the parser.  So, this bug is really why the parser isn't throwing this error instead of crashing.

Comment 4 George Staikos 2006-10-09 20:42:17 UTC

  The error isn't used because the implementation of it caused a regression 
elsewhere.

Comment 5 George Staikos 2006-10-09 20:42:26 UTC

We don't support implicit multiplication and don't have plans to at this time.

Comment 6 Adam Treat 2006-10-16 18:10:20 UTC

This is not a problem as implicit multiplication is not allowed.  The testcases for the equation parser already note this.

Comment 7 Nicolas Brisset 2006-10-16 18:42:59 UTC

Not a problem functionality-wise, but I still find it bad that users may lose work because kst crashes before they get a chance to save ! Especially as the way scalars need to be edited makes this very likely to happen !
Is it complicated to make sure that:
a) the syntax is rejected 
b) if it is accepted (as seems to be the case right now) returned values are NaN (indicating a problem) and kst does not crash ?

Comment 8 George Staikos 2006-10-16 18:55:56 UTC

Right, and I see why it crashes.

Comment 9 George Staikos 2006-10-16 18:57:42 UTC

SVN commit 596053 by staikos:

don't crash when an equation fails to parse
BUG: 135354


 M  +1 -1      kstequation.cpp  


--- trunk/extragear/graphics/kst/src/libkstmath/kstequation.cpp #596052:596053
@@ -211,7 +211,7 @@
   Equation::Context ctx;
   ctx.sampleCount = _ns;
   ctx.xVector = v;
-  usedUpdated = KstObject::UPDATE == _pe->update(update_counter, &ctx);
+  usedUpdated = _pe && KstObject::UPDATE == _pe->update(update_counter, &ctx);
 
   KstObject::UpdateType rc = NO_CHANGE; // if force, rc = UPDATE anyway.
   if (force || xUpdated || usedUpdated) {