21456: Improves AFMI at detecting memory issues (#253)

src/Amalgam/Parser.cpp

@@ -456,9 +456,12 @@ EvaluableNode *Parser::GetNextToken(EvaluableNode *parent_node, EvaluableNode *n
 		{
 			std::string token = GetNextIdentifier();
 			EvaluableNodeType token_type = GetEvaluableNodeTypeFromString(token);
-			new_token->SetType(token_type, evaluableNodeManager, false);
 
-			if(!IsEvaluableNodeTypeValid(token_type) || IsEvaluableNodeTypeImmediate(token_type))
+			if(IsEvaluableNodeTypeValid(token_type) && !IsEvaluableNodeTypeImmediate(token_type))
+			{
+				new_token->SetType(token_type, evaluableNodeManager, false);
+			}
+			else
 			{
 				//invalid opcode, warn if possible and store the identifier as a string
 				if(!originalSource.empty())

src/Amalgam/evaluablenode/EvaluableNode.cpp

@@ -335,6 +335,42 @@ size_t EvaluableNode::GetEstimatedNodeSizeInBytes(EvaluableNode *n)
 	return total_size;
 }
 
+bool EvaluableNode::IsNodeValid()
+{
+	if(!IsEvaluableNodeTypeValid(type))
+		return false;
+
+	//set a maximum number of valid elements of 100 million
+	//this is not a hard limit, but a heuristic to detect issues
+	size_t max_size = 100000000;
+	if(DoesEvaluableNodeTypeUseAssocData(type))
+	{
+		auto &mcn = GetMappedChildNodesReference();
+		return (mcn.size() < max_size);
+	}
+	else if(DoesEvaluableNodeTypeUseNumberData(type))
+	{
+		double number = GetNumberValueReference();
+		return !FastIsNaN(number);
+	}
+	else if(DoesEvaluableNodeTypeUseStringData(type))
+	{
+		auto sid = GetStringIDReference();
+		if(sid == string_intern_pool.NOT_A_STRING_ID)
+			return true;
+
+		return (sid->string.size() < max_size);
+	}
+	else //ordered
+	{
+		auto &ocn = GetOrderedChildNodesReference();
+		return (ocn.size() < max_size);
+	}
+
+	//shouldn't make it here
+	return false;
+}
+
 void EvaluableNode::InitializeType(EvaluableNode *n, bool copy_labels, bool copy_comments_and_concurrency)
 {
 	attributes.allAttributes = 0;
@@ -347,6 +383,10 @@ void EvaluableNode::InitializeType(EvaluableNode *n, bool copy_labels, bool copy
 
 	type = n->GetType();
 
+#ifdef AMALGAM_FAST_MEMORY_INTEGRITY
+	assert(IsEvaluableNodeTypeValid(type));
+#endif
+
 	if(DoesEvaluableNodeTypeUseAssocData(type))
 	{
 		value.ConstructMappedChildNodes();
@@ -442,6 +482,11 @@ void EvaluableNode::CopyValueFrom(EvaluableNode *n)
 	}
 
 	auto cur_type = n->GetType();
+
+#ifdef AMALGAM_FAST_MEMORY_INTEGRITY
+	assert(IsEvaluableNodeTypeValid(cur_type));
+#endif
+
 	//doesn't need an EvaluableNodeManager because not converting child nodes from one type to another
 	SetType(cur_type, nullptr, false);
 
@@ -510,6 +555,10 @@ void EvaluableNode::CopyMetadataFrom(EvaluableNode *n)
 void EvaluableNode::SetType(EvaluableNodeType new_type, EvaluableNodeManager *enm,
 	bool attempt_to_preserve_immediate_value)
 {
+#ifdef AMALGAM_FAST_MEMORY_INTEGRITY
+	assert(IsEvaluableNodeTypeValid(new_type));
+#endif
+
 	EvaluableNodeType cur_type = GetType();
 	if(new_type == cur_type)
 		return;

src/Amalgam/evaluablenode/EvaluableNode.h

@@ -72,6 +72,10 @@ class EvaluableNode
 	//Each InitializeType* sets up a given type with appropriate data
 	inline void InitializeType(EvaluableNodeType _type, const std::string &string_value)
 	{
+	#ifdef AMALGAM_FAST_MEMORY_INTEGRITY
+		assert(IsEvaluableNodeTypeValid(_type));
+	#endif
+
 		type = _type;
 		attributes.allAttributes = 0;
 		attributes.individualAttribs.isIdempotent = true;
@@ -81,6 +85,10 @@ class EvaluableNode
 
 	inline void InitializeType(EvaluableNodeType _type, StringInternPool::StringID string_id)
 	{
+	#ifdef AMALGAM_FAST_MEMORY_INTEGRITY
+		assert(IsEvaluableNodeTypeValid(_type));
+	#endif
+
 		attributes.allAttributes = 0;
 		if(string_id == StringInternPool::NOT_A_STRING_ID)
 		{
@@ -98,6 +106,10 @@ class EvaluableNode
 	//like InitializeType, but hands off the string reference to string_id
 	inline void InitializeTypeWithReferenceHandoff(EvaluableNodeType _type, StringInternPool::StringID string_id)
 	{
+	#ifdef AMALGAM_FAST_MEMORY_INTEGRITY
+		assert(IsEvaluableNodeTypeValid(_type));
+	#endif
+
 		attributes.allAttributes = 0;
 		if(string_id == StringInternPool::NOT_A_STRING_ID)
 		{
@@ -139,6 +151,10 @@ class EvaluableNode
 
 	inline void InitializeType(EvaluableNodeType _type)
 	{
+	#ifdef AMALGAM_FAST_MEMORY_INTEGRITY
+		assert(IsEvaluableNodeTypeValid(_type));
+	#endif
+
 		type = _type;
 		attributes.allAttributes = 0;
 		attributes.individualAttribs.isIdempotent = IsEvaluableNodeTypePotentiallyIdempotent(_type);
@@ -433,6 +449,9 @@ class EvaluableNode
 		return (type == ENT_DEALLOCATED);
 	}
 
+	//returns true if the node is a valid type and has valid data structures
+	bool IsNodeValid();
+
 	//transforms node to new_type, converting data if types are different
 	// enm is used if it needs to allocate nodes when changing types
 	// if enm is nullptr, then it will not necessarily keep child nodes

src/Amalgam/evaluablenode/EvaluableNodeManagement.cpp

@@ -316,7 +316,7 @@ EvaluableNode *EvaluableNodeManager::AllocUninitializedNode()
 	#ifdef MULTITHREAD_SUPPORT
 		//before releasing the lock, make sure the EvaluableNode is initialized, otherwise it could get grabbed by another thread
 		nodes[firstUnusedNodeIndex]->InitializeUnallocated();
-	#endif	
+	#endif
 	}
 	else //allocate if nullptr
 	{
@@ -438,7 +438,7 @@ void EvaluableNodeManager::FreeAllNodesExceptReferencedNodes(size_t cur_first_un
 void EvaluableNodeManager::FreeNodeTreeRecurse(EvaluableNode *tree)
 {
 #ifdef AMALGAM_FAST_MEMORY_INTEGRITY
-	assert(!tree->IsNodeDeallocated());
+	assert(tree->IsNodeValid());
 	assert(!tree->GetNeedCycleCheck());
 #endif
 
@@ -465,7 +465,7 @@ void EvaluableNodeManager::FreeNodeTreeRecurse(EvaluableNode *tree)
 void EvaluableNodeManager::FreeNodeTreeWithCyclesRecurse(EvaluableNode *tree)
 {
 #ifdef AMALGAM_FAST_MEMORY_INTEGRITY
-	assert(!tree->IsNodeDeallocated());
+	assert(tree->IsNodeValid());
 #endif
 
 	if(tree->IsAssociativeArray())
@@ -1005,7 +1005,7 @@ std::pair<bool, bool> EvaluableNodeManager::UpdateFlagsForNodeTreeRecurse(Evalua
 void EvaluableNodeManager::MarkAllReferencedNodesInUseRecurse(EvaluableNode *tree)
 {
 #ifdef AMALGAM_FAST_MEMORY_INTEGRITY
-	assert(!tree->IsNodeDeallocated());
+	assert(tree->IsNodeValid());
 #endif
 
 	//if entering this function, then the node hasn't been marked yet
@@ -1034,7 +1034,7 @@ void EvaluableNodeManager::MarkAllReferencedNodesInUseRecurse(EvaluableNode *tre
 void EvaluableNodeManager::MarkAllReferencedNodesInUseRecurseConcurrent(EvaluableNode* tree)
 {
 #ifdef AMALGAM_FAST_MEMORY_INTEGRITY
-	assert(!tree->IsNodeDeallocated());
+	assert(tree->IsNodeValid());
 #endif
 
 	//if entering this function, then the node hasn't been marked yet
@@ -1070,7 +1070,7 @@ std::pair<bool, bool> EvaluableNodeManager::ValidateEvaluableNodeTreeMemoryInteg
 	if(!inserted)
 		return std::make_pair(true, en->GetIsIdempotent());
 
-	if(en->IsNodeDeallocated() || en->GetKnownToBeInUse())
+	if(!en->IsNodeValid() || en->GetKnownToBeInUse())
 		assert(false);
 
 	if(existing_nodes != nullptr)

src/Amalgam/evaluablenode/EvaluableNodeManagement.h

@@ -255,7 +255,7 @@ class EvaluableNodeStackStateSaver
 		originalStackSize = stack->size();
 
 	#ifdef AMALGAM_FAST_MEMORY_INTEGRITY
-		assert(initial_element == nullptr || !initial_element->IsNodeDeallocated());
+		assert(initial_element == nullptr || initial_element->IsNodeValid());
 	#endif
 
 		stack->push_back(initial_element);
@@ -275,7 +275,7 @@ class EvaluableNodeStackStateSaver
 	__forceinline void PushEvaluableNode(EvaluableNode *n)
 	{
 	#ifdef AMALGAM_FAST_MEMORY_INTEGRITY
-		assert(n == nullptr || !n->IsNodeDeallocated());
+		assert(n == nullptr || n->IsNodeValid());
 	#endif
 		stack->push_back(n);
 	}
@@ -295,7 +295,7 @@ class EvaluableNodeStackStateSaver
 	__forceinline void SetStackLocation(size_t location, EvaluableNode *new_value)
 	{
 	#ifdef AMALGAM_FAST_MEMORY_INTEGRITY
-		assert(new_value == nullptr || !new_value->IsNodeDeallocated());
+		assert(new_value == nullptr || new_value->IsNodeValid());
 	#endif
 		(*stack)[location] = new_value;
 	}
@@ -681,7 +681,7 @@ class EvaluableNodeManager
 			return;
 
 	#ifdef AMALGAM_FAST_MEMORY_INTEGRITY
-		assert(!en->IsNodeDeallocated());
+		assert(en->IsNodeValid());
 	#endif
 
 		en->Invalidate();
@@ -695,10 +695,7 @@ class EvaluableNodeManager
 			enr.FreeImmediateResources();
 
 	#ifdef AMALGAM_FAST_MEMORY_INTEGRITY
-		if(enr != nullptr)
-		{
-			assert(!enr->IsNodeDeallocated());
-		}
+		assert(enr == nullptr || enr->IsNodeValid());
 	#endif
 
 		if(enr.unique && enr != nullptr && !enr->GetNeedCycleCheck())
@@ -718,7 +715,7 @@ class EvaluableNodeManager
 			return;
 
 	#ifdef AMALGAM_FAST_MEMORY_INTEGRITY
-		assert(!en->IsNodeDeallocated());
+		assert(en->IsNodeValid());
 	#endif
 
 		if(IsEvaluableNodeTypeImmediate(en->GetType()))