Bug fix: deeply nested expressions threw error in SBC mode

2 files changed, 27 insertions, 26 deletions

diff --git a/src/GCodes/GCodeBuffer/ExpressionParser.cpp b/src/GCodes/GCodeBuffer/ExpressionParser.cpp
index 4a121e91..c4231e4e 100644
--- a/src/GCodes/GCodeBuffer/ExpressionParser.cpp
+++ b/src/GCodes/GCodeBuffer/ExpressionParser.cpp
@@ -24,10 +24,11 @@ constexpr size_t MaxStringExpressionLength = StringLength100;
 
 namespace StackUsage
 {
-	// The following values are obtained from file ExpressionParser.su generated by the compiler
-	constexpr uint32_t ParseInternal = 72;
-	constexpr uint32_t ParseIdentifierExpression = 256;
-	constexpr uint32_t GetObjectValue_withTable = 48;
+	// The following values are the number of bytes of stack space needed by the corresponding functions and functions they call,
+	// not counting other called functions that call CheckStack. They are obtained from file ExpressionParser.su generated by the compiler.
+	constexpr uint32_t ParseInternal = 80;
+	constexpr uint32_t ParseIdentifierExpression = 240;
+	constexpr uint32_t GetObjectValueUsingTableNumber = 48;
 }
 
 // These can't be declared locally inside ParseIdentifierExpression because NamedEnum includes static data
@@ -884,6 +885,7 @@ void ExpressionParser::ParseNumber(ExpressionValue& rslt) noexcept
 // Parse an identifier expression
 // If 'evaluate' is false then the object model path may not exist, in which case we must ignore error that and parse it all anyway
 // This means we can use expressions such as: if {a.b == null || a.b.c == 1}
+// *** This function is recursive, so keep its stack usage low!
 void ExpressionParser::ParseIdentifierExpression(ExpressionValue& rslt, bool evaluate, bool applyLengthOperator, bool applyExists) THROWS(GCodeException)
 {
 	if (!isalpha(CurrentCharacter()))
@@ -915,7 +917,7 @@ void ExpressionParser::ParseIdentifierExpression(ExpressionValue& rslt, bool eva
 			}
 			AdvancePointer();										// skip the ']'
 			context.ProvideIndex(index.iVal);
-			c = '^';									// add the marker
+			c = '^';												// add the marker
 		}
 		if (id.cat(c))
 		{
@@ -1254,25 +1256,11 @@ void ExpressionParser::ParseIdentifierExpression(ExpressionValue& rslt, bool eva
 						break;
 
 					case TypeCode::CString:
-						{
-							tm timeInfo;
-							if (SafeStrptime(rslt.sVal, "%Y-%m-%dT%H:%M:%S", &timeInfo) == nullptr)
-							{
-								ThrowParseException("string is not a valid date and time");
-							}
-							val = mktime(&timeInfo);
-						}
+						val = ParseDateTime(rslt.sVal);
 						break;
 
 					case TypeCode::HeapString:
-						{
-							tm timeInfo;
-							if (SafeStrptime(rslt.shVal.Get().Ptr(), "%Y-%m-%dT%H:%M:%S", &timeInfo) == nullptr)
-							{
-								ThrowParseException("string is not a valid date and time");
-							}
-							val = mktime(&timeInfo);
-						}
+						val = ParseDateTime(rslt.shVal.Get().Ptr());
 						break;
 
 					default:
@@ -1329,8 +1317,8 @@ void ExpressionParser::ParseIdentifierExpression(ExpressionValue& rslt, bool eva
 		}
 
 		// Else assume an object model value
-		CheckStack(StackUsage::GetObjectValue_withTable);
-		rslt = reprap.GetObjectValue(context, nullptr, id.c_str(), 0);
+		CheckStack(StackUsage::GetObjectValueUsingTableNumber);
+		rslt = reprap.GetObjectValueUsingTableNumber(context, nullptr, id.c_str(), 0);
 		if (context.ObsoleteFieldQueried() && obsoleteField.IsEmpty())
 		{
 			obsoleteField.copy(id.c_str());
@@ -1340,6 +1328,17 @@ void ExpressionParser::ParseIdentifierExpression(ExpressionValue& rslt, bool eva
 	rslt.Set(nullptr);
 }
 
+// Parse a string to a DateTime
+time_t ExpressionParser::ParseDateTime(const char *s) const THROWS(GCodeException)
+{
+	tm timeInfo;
+	if (SafeStrptime(s, "%Y-%m-%dT%H:%M:%S", &timeInfo) == nullptr)
+	{
+		ThrowParseException("string is not a valid date and time");
+	}
+	return mktime(&timeInfo);
+}
+
 // Get the value of a variable
 void ExpressionParser::GetVariableValue(ExpressionValue& rslt, const VariableSet *vars, const char *name, bool parameter, bool wantExists) THROWS(GCodeException)
 {
@@ -1433,12 +1432,13 @@ void ExpressionParser::ThrowParseException(const char *str, uint32_t param) cons
 // Call this before making a recursive call, or before calling a function that needs a lot of stack from a recursive function
 void ExpressionParser::CheckStack(uint32_t calledFunctionStackUsage) const THROWS(GCodeException)
 {
-	register const char * stackPtr asm ("sp");
-	const char *stackLimit = (const char*)TaskBase::GetCallerTaskHandle() + sizeof(TaskBase);
+	register const char * stackPtr asm ("sp");									// get the current stack pointer
+	const char *stackLimit = (const char*)TaskBase::GetCurrentTaskStackBase();	// get the base (lowest available address) of the stack for this task
+
 	//debugPrintf("Margin: %u\n", stackPtr - stackLimit);
 	if (stackLimit + calledFunctionStackUsage + (StackUsage::Throw + StackUsage::Margin) <= stackPtr)
 	{
-		return;
+		return;				// we have enough stack
 	}
 
 	// The stack is in danger of overflowing. Throw an exception if we have enough stack to do so (ideally, this should always be the case)
diff --git a/src/GCodes/GCodeBuffer/ExpressionParser.h b/src/GCodes/GCodeBuffer/ExpressionParser.h
index 011b3eab..2fbee391 100644
--- a/src/GCodes/GCodeBuffer/ExpressionParser.h
+++ b/src/GCodes/GCodeBuffer/ExpressionParser.h
@@ -50,6 +50,7 @@ private:
 	void __attribute__((noinline)) ParseQuotedString(ExpressionValue& rslt) THROWS(GCodeException);
 
 	void ParseArray(size_t& length, function_ref<void(size_t index) THROWS(GCodeException)> processElement) THROWS(GCodeException);
+	time_t ParseDateTime(const char *s) const THROWS(GCodeException);
 
 	void GetVariableValue(ExpressionValue& rslt, const VariableSet *vars, const char *name, bool parameter, bool wantExists) THROWS(GCodeException);