From 532784413143a00d29b14cc9a82aa058a0e680e9 Mon Sep 17 00:00:00 2001 From: howsohazard <143410553+howsohazard@users.noreply.github.com> Date: Wed, 28 Aug 2024 11:11:30 -0400 Subject: [PATCH] 21413: Fixes segfaults (#245) --- src/Amalgam/SeparableBoxFilterDataStore.h | 6 +- .../evaluablenode/EvaluableNodeManagement.h | 3 +- src/Amalgam/interpreter/Interpreter.cpp | 30 ++-- src/Amalgam/interpreter/Interpreter.h | 6 +- .../InterpreterOpcodesDataTypes.cpp | 6 +- .../InterpreterOpcodesEntityControl.cpp | 2 +- .../InterpreterOpcodesListManipulation.cpp | 2 +- .../InterpreterOpcodesTransformations.cpp | 8 +- src/Amalgam/out.txt | 139 +++--------------- 9 files changed, 56 insertions(+), 146 deletions(-) diff --git a/src/Amalgam/SeparableBoxFilterDataStore.h b/src/Amalgam/SeparableBoxFilterDataStore.h index 3d63ab0a..c9e3ec53 100644 --- a/src/Amalgam/SeparableBoxFilterDataStore.h +++ b/src/Amalgam/SeparableBoxFilterDataStore.h @@ -697,8 +697,8 @@ class SeparableBoxFilterDataStore //search a projection width in terms of bucket count or number of collected entities //accumulates partial sums - //searches until num_entities_to_populate are popluated or other heuristics have been reached - //will only consider indices in enabled_indiced + //searches until num_entities_to_populate are populated or other heuristics have been reached + //will only consider indices in enabled_indices // query_feature_index is the offset to access the feature relative to the particular query data parameters //returns the smallest partial sum for any value not yet computed double PopulatePartialSumsWithSimilarFeatureValue(RepeatedGeneralizedDistanceEvaluator &r_dist_eval, @@ -903,7 +903,7 @@ class SeparableBoxFilterDataStore auto [num_calculated_features, distance] = partial_sums.GetNumFilledAndSum(entity_index); //complete known sums with worst and best possibilities - //calculate the number of features for which the minkowski distance term has not yet been calculated + //calculate the number of features for which the Minkowski distance term has not yet been calculated size_t num_uncalculated_features = (num_features - num_calculated_features); //if have already calculated everything, then already have the distance if(num_uncalculated_features == 0) diff --git a/src/Amalgam/evaluablenode/EvaluableNodeManagement.h b/src/Amalgam/evaluablenode/EvaluableNodeManagement.h index 37f9fee8..693d5139 100644 --- a/src/Amalgam/evaluablenode/EvaluableNodeManagement.h +++ b/src/Amalgam/evaluablenode/EvaluableNodeManagement.h @@ -59,7 +59,8 @@ class EvaluableNodeReference //when attached a child node, make sure that this node reflects the same properties //if first_attachment_to_unique_node is true, then it will not call SetNeedCycleCheck(true) - //if the attached is not unique + //if the attached is not unique. Note that this parameter should not be set to true + //if the node can be accessed in any other way, such as the construction stack void UpdatePropertiesBasedOnAttachedNode(EvaluableNodeReference &attached, bool first_attachment_to_unique_node = false) { diff --git a/src/Amalgam/interpreter/Interpreter.cpp b/src/Amalgam/interpreter/Interpreter.cpp index 47745891..79b4237a 100644 --- a/src/Amalgam/interpreter/Interpreter.cpp +++ b/src/Amalgam/interpreter/Interpreter.cpp @@ -326,13 +326,13 @@ Interpreter::Interpreter(EvaluableNodeManager *enm, RandomStream rand_stream, #ifdef MULTITHREAD_SUPPORT EvaluableNodeReference Interpreter::ExecuteNode(EvaluableNode *en, - EvaluableNode *call_stack, EvaluableNode *interpreter_node_stack, + EvaluableNode *call_stack, EvaluableNode *opcode_stack, EvaluableNode *construction_stack, std::vector *construction_stack_indices, Concurrency::ReadWriteMutex *call_stack_write_mutex, bool immediate_result) #else EvaluableNodeReference Interpreter::ExecuteNode(EvaluableNode *en, - EvaluableNode *call_stack, EvaluableNode *interpreter_node_stack, + EvaluableNode *call_stack, EvaluableNode *opcode_stack, EvaluableNode *construction_stack, std::vector *construction_stack_indices, bool immediate_result) @@ -359,14 +359,22 @@ EvaluableNodeReference Interpreter::ExecuteNode(EvaluableNode *en, call_stack->AppendOrderedChildNode(new_context_entry); } - if(interpreter_node_stack == nullptr) - interpreter_node_stack = evaluableNodeManager->AllocNode(ENT_LIST); + bool free_opcode_stack = false; + if(opcode_stack == nullptr) + { + opcode_stack = evaluableNodeManager->AllocNode(ENT_LIST); + free_opcode_stack = true; + } + bool free_construction_stack = false; if(construction_stack == nullptr) + { construction_stack = evaluableNodeManager->AllocNode(ENT_LIST); + free_construction_stack = true; + } callStackNodes = &call_stack->GetOrderedChildNodes(); - opcodeStackNodes = &interpreter_node_stack->GetOrderedChildNodes(); + opcodeStackNodes = &opcode_stack->GetOrderedChildNodes(); constructionStackNodes = &construction_stack->GetOrderedChildNodes(); if(construction_stack_indices != nullptr) @@ -377,19 +385,21 @@ EvaluableNodeReference Interpreter::ExecuteNode(EvaluableNode *en, call_stack->SetNeedCycleCheck(true); for(auto &cn : call_stack->GetOrderedChildNodesReference()) cn->SetNeedCycleCheck(true); - interpreter_node_stack->SetNeedCycleCheck(true); + opcode_stack->SetNeedCycleCheck(true); construction_stack->SetNeedCycleCheck(true); //keep these references as long as the interpreter is around - evaluableNodeManager->KeepNodeReferences(call_stack, interpreter_node_stack, construction_stack); + evaluableNodeManager->KeepNodeReferences(call_stack, opcode_stack, construction_stack); auto retval = InterpretNode(en, immediate_result); - evaluableNodeManager->FreeNodeReferences(call_stack, interpreter_node_stack, construction_stack); + evaluableNodeManager->FreeNodeReferences(call_stack, opcode_stack, construction_stack); //remove these nodes - evaluableNodeManager->FreeNode(interpreter_node_stack); - evaluableNodeManager->FreeNode(construction_stack); + if(free_opcode_stack) + evaluableNodeManager->FreeNode(opcode_stack); + if(free_construction_stack) + evaluableNodeManager->FreeNode(construction_stack); return retval; } diff --git a/src/Amalgam/interpreter/Interpreter.h b/src/Amalgam/interpreter/Interpreter.h index fb3f16c4..8ac6cc8e 100644 --- a/src/Amalgam/interpreter/Interpreter.h +++ b/src/Amalgam/interpreter/Interpreter.h @@ -163,21 +163,21 @@ class Interpreter //Executes the current Entity that this Interpreter is contained by // sets up all of the stack and contextual structures, then calls InterpretNode on en - //if call_stack, interpreter_node_stack, or construction_stack are nullptr, it will start with a new one + //if call_stack, opcode_stack, or construction_stack are nullptr, it will start with a new one //note that construction_stack and construction_stack_indices should be specified together and should be the same length //if immediate_result is true, then the returned value may be immediate #ifdef MULTITHREAD_SUPPORT //if run multithreaded, then for performance reasons, it is optimal to have one of each stack per thread // and call_stack_write_mutex is the mutex needed to lock for writing EvaluableNodeReference ExecuteNode(EvaluableNode *en, - EvaluableNode *call_stack = nullptr, EvaluableNode *interpreter_node_stack = nullptr, + EvaluableNode *call_stack = nullptr, EvaluableNode *opcode_stack = nullptr, EvaluableNode *construction_stack = nullptr, std::vector *construction_stack_indices = nullptr, Concurrency::ReadWriteMutex *call_stack_write_mutex = nullptr, bool immediate_result = false); #else EvaluableNodeReference ExecuteNode(EvaluableNode *en, - EvaluableNode *call_stack = nullptr, EvaluableNode *interpreter_node_stack = nullptr, + EvaluableNode *call_stack = nullptr, EvaluableNode *opcode_stack = nullptr, EvaluableNode *construction_stack = nullptr, std::vector *construction_stack_indices = nullptr, bool immediate_result = false); diff --git a/src/Amalgam/interpreter/InterpreterOpcodesDataTypes.cpp b/src/Amalgam/interpreter/InterpreterOpcodesDataTypes.cpp index e1d233aa..e94d27ef 100644 --- a/src/Amalgam/interpreter/InterpreterOpcodesDataTypes.cpp +++ b/src/Amalgam/interpreter/InterpreterOpcodesDataTypes.cpp @@ -91,7 +91,7 @@ EvaluableNodeReference Interpreter::InterpretNode_ENT_LIST(EvaluableNode *en, bo auto value = InterpretNode(ocn[i]); //add it to the list new_list_ocn[i] = value; - new_list.UpdatePropertiesBasedOnAttachedNode(value, i == 0); + new_list.UpdatePropertiesBasedOnAttachedNode(value); } if(PopConstructionContextAndGetExecutionSideEffectFlag()) @@ -142,7 +142,6 @@ EvaluableNodeReference Interpreter::InterpretNode_ENT_ASSOC(EvaluableNode *en, b //construction stack has a reference, so no KeepNodeReference isn't needed for anything referenced PushNewConstructionContext(en, new_assoc, EvaluableNodeImmediateValueWithType(StringInternPool::NOT_A_STRING_ID), nullptr); - bool first_node = true; for(auto &[cn_id, cn] : new_mcn) { SetTopCurrentIndexInConstructionStack(cn_id); @@ -151,8 +150,7 @@ EvaluableNodeReference Interpreter::InterpretNode_ENT_ASSOC(EvaluableNode *en, b EvaluableNodeReference element_result = InterpretNode(cn); cn = element_result; - new_assoc.UpdatePropertiesBasedOnAttachedNode(element_result, first_node); - first_node = false; + new_assoc.UpdatePropertiesBasedOnAttachedNode(element_result); } if(PopConstructionContextAndGetExecutionSideEffectFlag()) diff --git a/src/Amalgam/interpreter/InterpreterOpcodesEntityControl.cpp b/src/Amalgam/interpreter/InterpreterOpcodesEntityControl.cpp index deeed1f7..ac9d21e9 100644 --- a/src/Amalgam/interpreter/InterpreterOpcodesEntityControl.cpp +++ b/src/Amalgam/interpreter/InterpreterOpcodesEntityControl.cpp @@ -804,7 +804,7 @@ EvaluableNodeReference Interpreter::InterpretNode_ENT_STORE_ENTITY(EvaluableNode evaluableNodeManager->FreeNodeTreeIfPossible(params); } - //get the id of the source entity to store. Don't need to keep the reference because it won't be used once the source entety pointer is looked up + //get the id of the source entity to store. Don't need to keep the reference because it won't be used once the source entity pointer is looked up //retrieve the entity after other parameters to minimize time in locks // and prevent deadlock if one of the params accessed the entity //StoreEntityToResourcePath will read lock all contained entities appropriately diff --git a/src/Amalgam/interpreter/InterpreterOpcodesListManipulation.cpp b/src/Amalgam/interpreter/InterpreterOpcodesListManipulation.cpp index 8dbdef3e..8fb8e9ab 100644 --- a/src/Amalgam/interpreter/InterpreterOpcodesListManipulation.cpp +++ b/src/Amalgam/interpreter/InterpreterOpcodesListManipulation.cpp @@ -648,7 +648,7 @@ EvaluableNodeReference Interpreter::InterpretNode_ENT_RANGE(EvaluableNode *en, b EvaluableNodeReference element_result = InterpretNode(function); result_ocn[i] = element_result; - result.UpdatePropertiesBasedOnAttachedNode(element_result, i == 0); + result.UpdatePropertiesBasedOnAttachedNode(element_result); } if(PopConstructionContextAndGetExecutionSideEffectFlag()) diff --git a/src/Amalgam/interpreter/InterpreterOpcodesTransformations.cpp b/src/Amalgam/interpreter/InterpreterOpcodesTransformations.cpp index c218e6b8..92399fea 100644 --- a/src/Amalgam/interpreter/InterpreterOpcodesTransformations.cpp +++ b/src/Amalgam/interpreter/InterpreterOpcodesTransformations.cpp @@ -115,7 +115,7 @@ EvaluableNodeReference Interpreter::InterpretNode_ENT_MAP(EvaluableNode *en, boo EvaluableNodeReference element_result = InterpretNode(function); result_ocn[i] = element_result; - result.UpdatePropertiesBasedOnAttachedNode(element_result, i == 0); + result.UpdatePropertiesBasedOnAttachedNode(element_result); } if(PopConstructionContextAndGetExecutionSideEffectFlag()) @@ -170,7 +170,6 @@ EvaluableNodeReference Interpreter::InterpretNode_ENT_MAP(EvaluableNode *en, boo PushNewConstructionContext(list, result, EvaluableNodeImmediateValueWithType(StringInternPool::NOT_A_STRING_ID), nullptr); - bool first_element = true; for(auto &[result_id, result_node] : result_mcn) { SetTopCurrentIndexInConstructionStack(result_id); @@ -184,8 +183,7 @@ EvaluableNodeReference Interpreter::InterpretNode_ENT_MAP(EvaluableNode *en, boo //in order to keep the node properties to be updated below EvaluableNodeReference element_result = InterpretNode(function); result_node = element_result; - result.UpdatePropertiesBasedOnAttachedNode(element_result, first_element); - first_element = false; + result.UpdatePropertiesBasedOnAttachedNode(element_result); } if(PopConstructionContextAndGetExecutionSideEffectFlag()) @@ -1567,7 +1565,7 @@ EvaluableNodeReference Interpreter::InterpretNode_ENT_ASSOCIATE(EvaluableNode *e //handoff the reference from index_value to the assoc new_assoc->SetMappedChildNodeWithReferenceHandoff(key_sid, value); - new_assoc.UpdatePropertiesBasedOnAttachedNode(value, i == 0); + new_assoc.UpdatePropertiesBasedOnAttachedNode(value); } if(PopConstructionContextAndGetExecutionSideEffectFlag()) diff --git a/src/Amalgam/out.txt b/src/Amalgam/out.txt index 27deeb53..22368dab 100644 --- a/src/Amalgam/out.txt +++ b/src/Amalgam/out.txt @@ -638,9 +638,9 @@ a {a 1 d 4} { a 1 - b 2 d 4 e 5 + f 6 } { a 1 @@ -689,9 +689,9 @@ c {a 1 d 4} { a 1 - b 2 d 4 e 5 + f 6 } { a 1 @@ -1042,7 +1042,7 @@ abcdef [1 3] [9 5] --indices-- -["c" "4" "b" "a"] +["c" "b" "4" "a"] [ 0 1 @@ -1288,7 +1288,7 @@ current_index: 2 interpreter "C:\\Users\\Chris Hazard\\Desktop\\Howso_repos\\amalgam\\x64\\MT_Release_EXE\\Amalgam.exe" raaa 2 rwww 1 - start_time 1724798627.460355 + start_time 1724856124.867852 www 1 x 12 zz 10 @@ -1331,7 +1331,7 @@ current_index: 2 interpreter "C:\\Users\\Chris Hazard\\Desktop\\Howso_repos\\amalgam\\x64\\MT_Release_EXE\\Amalgam.exe" raaa 2 rwww 1 - start_time 1724798627.460355 + start_time 1724856124.867852 www 1 x 12 zz 10 @@ -1373,7 +1373,7 @@ current_index: 2 interpreter "C:\\Users\\Chris Hazard\\Desktop\\Howso_repos\\amalgam\\x64\\MT_Release_EXE\\Amalgam.exe" raaa 2 rwww 1 - start_time 1724798627.460355 + start_time 1724856124.867852 www 1 x 12 zz 10 @@ -1521,7 +1521,7 @@ infinity test c or d: ["d" @(get (target 2) 0) "c" @(get (target 2) 2)] {a 24 b 52 c 24} -["3" "4" "2"] +["1" "3" "2"] --get_rand_seed-- N9“˸U¯OaVÆT zÿ @@ -1626,8 +1626,8 @@ e: - b - - .inf -d: 4 a: 1 +d: 4 24: a: 1 b: 2 @@ -1640,7 +1640,7 @@ e: - .inf 25: {a 1} -current date-time in epoch: 2024-08-27-18.43.47.6824470 +current date-time in epoch: 2024-08-28-10.42.05.5137340 2020-06-07 00:22:59 1391230800 1391230800 @@ -2767,18 +2767,16 @@ flatten restore with parallel 19.264241099357605 --intersect_entities-- (associate "b" 4) -MergeEntityChild1 -(associate "x" 3 "y" 4) MergeEntityChild2 (associate "p" 3 "q" 4) +MergeEntityChild1 +(associate "x" 3 "y" 4) _2280722175 (associate "E" 3 "F" 4) _2169689611 (associate "e" 3 "f" 4) --union_entities-- (associate "b" 4 "a" 3 "c" 3) -MergeEntityChild1 -(associate "x" 3 "y" 4 "z" 5) MergeEntityChild2 (associate "p" @@ -2792,6 +2790,8 @@ MergeEntityChild2 "w" 7 ) +MergeEntityChild1 +(associate "x" 3 "y" 4 "z" 5) _2280722175 (associate "E" @@ -3391,14 +3391,8 @@ difference between DiffEntity2 and new_entity: [] (lambda { - E (get - (current_value 1) - "E" - ) - F (get - (current_value 1) - "F" - ) + E 3 + F 4 G 5 H 6 } @@ -3423,16 +3417,7 @@ difference between DiffEntity2 and new_entity: _ [] (lambda - { - e (get - (current_value 1) - "e" - ) - f (get - (current_value 1) - "f" - ) - } + {e 3 f 4} ) ) ) @@ -3458,96 +3443,14 @@ contained_entities new_entity: ["OnlyIn2" "_3631850880" "_1938178219" "DiffEntit difference between DiffContainer and DiffEntity2: (declare {_ (null) new_entity (null)} - (assign - "new_entity" - (first - (create_entities - new_entity - (call - (lambda - (declare - {_ (null)} - (replace _) - ) - ) - { - _ (retrieve_entity_root _) - } - ) - ) - ) - ) - (create_entities - (append new_entity "_3631850880") - (call - (lambda - (declare - {_ (null)} - (replace - _ - [] - (lambda - { - E (null) - F (null) - G (get - (current_value 1) - "G" - ) - H (get - (current_value 1) - "H" - ) - } - ) - ) - ) - ) - { - _ (retrieve_entity_root - (append _ "_3631850880") - ) - } - ) - ) - (create_entities - (append new_entity "_1938178219") - (call - (lambda - (declare - {_ (null)} - (replace - _ - [] - (lambda - {e (null) f (null)} - ) - ) - ) - ) - { - _ (retrieve_entity_root - (append _ "_1938178219") - ) - } - ) - ) - (clone_entities - (append _ "OnlyIn2") - (append new_entity "OnlyIn2") - ) - (clone_entities - (append _ "DiffEntityChild1") - (append new_entity "DiffEntityChild1") - ) - new_entity + (clone_entities _ new_entity) ) --mix_entities-- (associate "b" 4 "c" 3) -MergeEntityChild1 -(associate "x" 3 "y" 4) MergeEntityChild2 (associate "p" 3 "q" 4 "w" 7) +MergeEntityChild1 +(associate "x" 3 "y" 4) _2280722175 (associate "E" 3 "F" 4 "H" 6) _2169689611 @@ -3629,7 +3532,7 @@ deep sets --set_entity_root_permission-- RootTest -1724798627.851431 +1724856125.751855 (true) RootTest @@ -4973,4 +4876,4 @@ concurrent entity writes successful: (true) --clean-up test files-- --total execution time-- -1.170624017715454 +1.6281991004943848