Skip to content

Commit

Permalink
22020: Fixes bug where inconsistent results can be returned from sand…
Browse files Browse the repository at this point in the history
…boxed calls if resources are exhausted (#299)
  • Loading branch information
howsohazard authored Oct 28, 2024
1 parent ddebf19 commit 6094d6d
Show file tree
Hide file tree
Showing 5 changed files with 202 additions and 70 deletions.
15 changes: 15 additions & 0 deletions src/Amalgam/interpreter/Interpreter.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -893,6 +893,8 @@ void Interpreter::PopulatePerformanceCounters(PerformanceConstraints *perf_const
if(perf_constraints == nullptr)
return;

perf_constraints->constraintsExceeded = false;

//handle execution steps
if(performanceConstraints != nullptr && performanceConstraints->ConstrainedExecutionSteps())
{
Expand All @@ -909,6 +911,7 @@ void Interpreter::PopulatePerformanceCounters(PerformanceConstraints *perf_const
{
perf_constraints->maxNumExecutionSteps = 1;
perf_constraints->curExecutionStep = 1;
perf_constraints->constraintsExceeded = true;
}
}

Expand All @@ -928,6 +931,7 @@ void Interpreter::PopulatePerformanceCounters(PerformanceConstraints *perf_const
else //out of resources, ensure nothing will run (can't use 0 for maxNumAllocatedNodes)
{
perf_constraints->maxNumAllocatedNodes = 1;
perf_constraints->constraintsExceeded = true;
}
}

Expand Down Expand Up @@ -958,6 +962,7 @@ void Interpreter::PopulatePerformanceCounters(PerformanceConstraints *perf_const
else //out of resources, ensure nothing will run (can't use 0 for maxOpcodeExecutionDepth)
{
perf_constraints->maxOpcodeExecutionDepth = 1;
perf_constraints->constraintsExceeded = true;
}
}

Expand All @@ -983,9 +988,14 @@ void Interpreter::PopulatePerformanceCounters(PerformanceConstraints *perf_const
erbr.Clear();

if(container_total_entities >= performanceConstraints->maxContainedEntities)
{
max_entities = 0;
perf_constraints->constraintsExceeded = true;
}
else
{
max_entities = performanceConstraints->maxContainedEntities - (container_total_entities - contained_total_entities);
}
}

perf_constraints->maxContainedEntities = std::min(perf_constraints->maxContainedEntities, max_entities);
Expand All @@ -1007,10 +1017,15 @@ void Interpreter::PopulatePerformanceCounters(PerformanceConstraints *perf_const
}

if(cur_depth >= max_depth)
{
perf_constraints->maxContainedEntityDepth = 0;
perf_constraints->constraintsExceeded = true;
}
else
{
perf_constraints->maxContainedEntityDepth = std::min(perf_constraints->maxContainedEntityDepth,
max_depth - cur_depth);
}
}

if(performanceConstraints != nullptr && performanceConstraints->maxEntityIdLength > 0)
Expand Down
15 changes: 14 additions & 1 deletion src/Amalgam/interpreter/Interpreter.h
Original file line number Diff line number Diff line change
Expand Up @@ -129,6 +129,9 @@ class PerformanceConstraints
//constrains the maximum length of an entity id (primarily to make sure it doesn't cause problems for file systems)
//If 0, then unlimited
size_t maxEntityIdLength;

//flag set to true if constraints have been exceeded
bool constraintsExceeded;
};

class Interpreter
Expand Down Expand Up @@ -1021,23 +1024,33 @@ class Interpreter
performanceConstraints->curExecutionStep++;

if(performanceConstraints->curExecutionStep > performanceConstraints->maxNumExecutionSteps)
{
performanceConstraints->constraintsExceeded = true;
return true;
}
}

if(performanceConstraints->ConstrainedAllocatedNodes())
{
size_t cur_allocated_nodes = performanceConstraints->curNumAllocatedNodesAllocatedToEntities + evaluableNodeManager->GetNumberOfUsedNodes();
if(cur_allocated_nodes > performanceConstraints->maxNumAllocatedNodes)
{
performanceConstraints->constraintsExceeded = true;
return true;
}
}

if(performanceConstraints->ConstrainedOpcodeExecutionDepth())
{
if(opcodeStackNodes->size() > performanceConstraints->maxOpcodeExecutionDepth)
{
performanceConstraints->constraintsExceeded = true;
return true;
}
}

return false;
//return whether they have ever been exceeded
return performanceConstraints->constraintsExceeded;
}

//opcodes
Expand Down
9 changes: 6 additions & 3 deletions src/Amalgam/interpreter/InterpreterOpcodesBase.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -569,9 +569,6 @@ EvaluableNodeReference Interpreter::InterpretNode_ENT_CALL_SANDBOXED(EvaluableNo
std::swap(memoryModificationLock, sandbox.memoryModificationLock);
#endif

if(performanceConstraints != nullptr)
performanceConstraints->AccruePerformanceCounters(perf_constraints_ptr);

evaluableNodeManager->FreeNode(call_stack->GetOrderedChildNodesReference()[0]);
evaluableNodeManager->FreeNode(call_stack);

Expand All @@ -582,6 +579,12 @@ EvaluableNodeReference Interpreter::InterpretNode_ENT_CALL_SANDBOXED(EvaluableNo
if(_label_profiling_enabled && function->GetNumLabels() > 0)
PerformanceProfiler::EndOperation(evaluableNodeManager->GetNumberOfUsedNodes());

if(performanceConstraints != nullptr)
performanceConstraints->AccruePerformanceCounters(perf_constraints_ptr);

if(perf_constraints_ptr != nullptr && perf_constraints_ptr->constraintsExceeded)
return EvaluableNodeReference::Null();

return result;
}

Expand Down
18 changes: 12 additions & 6 deletions src/Amalgam/interpreter/InterpreterOpcodesEntityAccess.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -503,9 +503,6 @@ EvaluableNodeReference Interpreter::InterpretNode_ENT_CALL_ENTITY_and_CALL_ENTIT
#endif
);

if(performanceConstraints != nullptr)
performanceConstraints->AccruePerformanceCounters(perf_constraints_ptr);

ce_enm.FreeNode(call_stack->GetOrderedChildNodesReference()[0]);
ce_enm.FreeNode(call_stack);

Expand Down Expand Up @@ -546,6 +543,12 @@ EvaluableNodeReference Interpreter::InterpretNode_ENT_CALL_ENTITY_and_CALL_ENTIT
if(_label_profiling_enabled)
PerformanceProfiler::EndOperation(evaluableNodeManager->GetNumberOfUsedNodes());

if(performanceConstraints != nullptr)
performanceConstraints->AccruePerformanceCounters(perf_constraints_ptr);

if(perf_constraints_ptr != nullptr && perf_constraints_ptr->constraintsExceeded)
return EvaluableNodeReference::Null();

return result;
}

Expand Down Expand Up @@ -619,9 +622,6 @@ EvaluableNodeReference Interpreter::InterpretNode_ENT_CALL_CONTAINER(EvaluableNo
#endif
);

if(performanceConstraints != nullptr)
performanceConstraints->AccruePerformanceCounters(perf_constraints_ptr);

container->evaluableNodeManager.FreeNode(call_stack->GetOrderedChildNodesReference()[0]);
container->evaluableNodeManager.FreeNode(call_stack);

Expand All @@ -640,5 +640,11 @@ EvaluableNodeReference Interpreter::InterpretNode_ENT_CALL_CONTAINER(EvaluableNo
if(_label_profiling_enabled)
PerformanceProfiler::EndOperation(evaluableNodeManager->GetNumberOfUsedNodes());

if(performanceConstraints != nullptr)
performanceConstraints->AccruePerformanceCounters(perf_constraints_ptr);

if(perf_constraints_ptr != nullptr && perf_constraints_ptr->constraintsExceeded)
return EvaluableNodeReference::Null();

return copied_result;
}
Loading

0 comments on commit 6094d6d

Please sign in to comment.