Skip to content

Conversation

@SuperFola
Copy link
Member

@SuperFola SuperFola commented Jul 9, 2025

Description

It is possible for ArkScript to create too many variables (more than 8192 at once), which results in a segfault. This aims to fix this bug, but I am aware that it will likely hinder performance.

Checklist

  • I have read the Contributor guide
  • My code follows the style guidelines of this project
  • I have commented my code, particularly in hard-to-understand areas
  • I have updated the documentation if needed
  • I have added tests that prove my fix/feature is working
  • New and existing tests pass locally with my changes

@github-actions
Copy link

github-actions bot commented Jul 9, 2025

Static analysis report

Lizard report

Listing only functions with cyclomatic complexity >= 15 or NLOC >= 100 or parameters >= 6.

Report about files you didn't modify in this PR
Filename Start line:end line Function name Parameters NLOC CCN
src/arkreactor/VM/VM.cpp 524:2181 Ark::VM::safeRun 3 1386 275
src/arkreactor/Compiler/Macros/Processor.cpp 253:637 Ark::internal::MacroProcessor::evaluate 3 354 122
src/arkreactor/Compiler/BytecodeReader.cpp 297:716 Ark::BytecodeReader::display 4 370 113
src/arkreactor/Error/Diagnostics.cpp 45:192 Ark::Diagnostics::makeContext 4 109 44
src/arkreactor/Compiler/Lowerer/ASTLowerer.cpp 568:743 Ark::internal::ASTLowerer::handleCalls 4 133 43
src/arkscript/JsonCompiler.cpp 27:276 JsonCompiler::_compile 1 214 38
src/arkreactor/Compiler/NameResolution/NameResolutionPass.cpp 161:266 Ark::internal::NameResolutionPass::visitKeyword 3 85 35
src/arkreactor/Compiler/AST/Parser.cpp 840:931 Ark::internal::Parser::string 1 88 32
src/arkreactor/Compiler/AST/Node.cpp 179:285 Ark::internal::Node::repr 0 90 30
src/arkscript/main.cpp 23:323 main 2 255 29
src/arkreactor/Compiler/Lowerer/ASTLowerer.cpp 138:241 Ark::internal::ASTLowerer::compileExpression 4 85 27
src/arkreactor/Compiler/Macros/Processor.cpp 102:185 Ark::internal::MacroProcessor::processNode 3 61 27
src/arkreactor/Compiler/AST/Node.cpp 287:368 Ark::internal::Node::debugPrint 1 70 26
src/arkreactor/TypeChecker.cpp 149:236 Ark::types::generateError 6 75 25
src/arkreactor/TypeChecker.cpp 32:147 Ark::types::displayContract 6 102 24
src/arkreactor/Compiler/NameResolution/NameResolutionPass.cpp 55:159 Ark::internal::NameResolutionPass::visit 2 83 23
src/arkreactor/Compiler/AST/Parser.cpp 288:416 Ark::internal::Parser::import_ 1 98 23
src/arkreactor/Compiler/Lowerer/ASTLowerer.cpp 270:329 Ark::internal::ASTLowerer::compileListInstruction 3 49 22
src/arkreactor/VM/VM.cpp 2301:2405 Ark::VM::backtrace 3 89 22
include/utf8.hpp 138:184 utf8::isValid 1 44 21
src/arkscript/REPL/Utils.cpp 52:187 Ark::internal::getColorPerKeyword 0 113 20
src/arkreactor/Compiler/AST/Optimizer.cpp 33:83 Ark::internal::Optimizer::countAndPruneDeadCode 1 42 20
src/arkscript/Formatter.cpp 503:562 Formatter::formatCall 2 51 19
src/arkreactor/Compiler/NameResolution/StaticScope.cpp 68:109 Ark::internal::NamespaceScope::get 3 32 19
src/arkreactor/VM/Value.cpp 77:140 Ark::Value::toString 1 50 19
src/arkscript/Formatter.cpp 188:250 Formatter::format 3 59 18
src/arkreactor/Compiler/Macros/Processor.cpp 719:760 Ark::internal::MacroProcessor::isConstEval 1 37 18
src/arkreactor/Compiler/Macros/Executors/Function.cpp 16:91 Ark::internal::FunctionExecutor::applyMacro 2 55 17
include/Ark/Compiler/AST/Predicates.hpp 132:156 Ark::internal::IsSymbol::operator ( ) 1 24 16
src/arkscript/Formatter.cpp 300:341 Formatter::formatFunction 2 35 16
src/arkreactor/Compiler/Lowerer/ASTLowerer.cpp 370:449 Ark::internal::ASTLowerer::compileFunction 3 57 16
src/arkreactor/Compiler/Lowerer/ASTLowerer.cpp 78:94 Ark::internal::ASTLowerer::nodeProducesOutput 1 13 15
src/arkreactor/Compiler/Macros/Executors/Function.cpp 101:158 Ark::internal::FunctionExecutor::unify 5 50 15
src/arkreactor/Compiler/IntermediateRepresentation/IROptimizer.cpp 22:265 Ark::internal::IROptimizer::IROptimizer 1 224 14
src/arkreactor/Error/Diagnostics.cpp 194:209 Ark::Diagnostics::helper 6 15 2

CppCheck report

Filename Line Type Description
include/Ark/VM/VM.inl 258 style Variable 'maybe_value_ptr' can be declared as pointer to const
Report files about files you didn't modify in this PR
Filename Line Type Description
src/arkreactor/Builtins/Bytecode.cpp 22 style Parameter 'vm' can be declared as pointer to const
src/arkreactor/Builtins/IO.cpp 165 style Consider using std::transform algorithm instead of a raw loop.
src/arkreactor/Compiler/BytecodeReader.cpp 477 style struct member 'Arg::kind' is never used.
src/arkreactor/Compiler/IntermediateRepresentation/IROptimizer.cpp 254 style Consider using std::transform algorithm instead of a raw loop.
src/arkreactor/Compiler/IntermediateRepresentation/IROptimizer.cpp 261 style Consider using std::transform algorithm instead of a raw loop.
src/arkreactor/Compiler/NameResolution/ScopeResolver.cpp 134 style Consider using std::find_if algorithm instead of a raw loop.
include/Ark/VM/Future.hpp 50 style Unused private function: 'Future::deleteSelfViaVM'
src/arkreactor/VM/Future.cpp 23 performance Variable 'm_value' is assigned in constructor body. Consider performing initialization in initialization list.
src/arkreactor/VM/State.cpp 190 style Consider using std::any_of, std::all_of, std::none_of algorithm instead of a raw loop.
src/arkreactor/VM/VM.cpp 525 information Limiting ValueFlow analysis in function 'safeRun' since it is too complex. Please specify --check-level=exhaustive to perform full analysis.
src/arkreactor/VM/VM.cpp 403 error Iterators of different containers 'm_execution_contexts.emplace_back(std::make_unique())' and 'm_execution_contexts.front()' are used together.

@coveralls
Copy link

coveralls commented Jul 9, 2025

Coverage Status

coverage: 92.225% (-0.03%) from 92.251%
when pulling 83740a0 on fix/scope-bad-alloc
into d86cee8 on dev.

@github-actions
Copy link

github-actions bot commented Jul 9, 2025

Fuzzing report

/usr/local/bin/afl-whatsup status check tool for afl-fuzz by Michal Zalewski

Summary stats

    Fuzzers alive : 0
   Dead or remote : 1 (included in stats)
   Total run time : 5 minutes, 0 seconds
      Total execs : 39 thousands
 Cumulative speed : 132 execs/sec
    Pending items : 129 faves, 1237 total
 Coverage reached : 11.00%
    Crashes saved : 0
      Hangs saved : 0

Cycles without finds : 0
Time without finds : 0

[+] Captured 42487 tuples (map size 225639, highest value 255, total values 428003094) in '/dev/null'.
[+] A coverage of 42487 edges were achieved out of 225664 existing (18.83%) with 1245 input files.

@SuperFola SuperFola marked this pull request as draft July 9, 2025 18:49
@SuperFola SuperFola changed the title wip - bad performances Fix potential out of bounds scope allocation Jul 10, 2025
@codspeed-hq
Copy link

codspeed-hq bot commented Jul 23, 2025

CodSpeed Performance Report

Merging #559 will degrade performance by 3.48%

Comparing fix/scope-bad-alloc (83740a0) with dev (d86cee8)

Summary

❌ 4 regressions
✅ 14 untouched

⚠️ Please fix the performance issues or acknowledge them on CodSpeed.

Benchmarks breakdown

Benchmark BASE HEAD Efficiency
fibonacci 29 ms 29.6 ms -2.04%
binary_trees 4.8 s 5 s -3.01%
create_closure 4.7 ms 4.8 ms -1.07%
ackermann 331 ms 342.9 ms -3.48%

@SuperFola
Copy link
Member Author

Adding the check inside VM::store is deemed too expensive; perhaps changing the backing storage data structure might help?

It still needs to be contiguous though, so maybe we should allocate a few kernel pages via mmap, and mark them used as we go, like an arena allocator would do?

@SuperFola
Copy link
Member Author

Reproduction:

(let foo (fun (a b c d e f g h i j k l m n o p q) {
  (foo a b c d e f g h i j k l m n o p q)
  1 }))
(foo 1 2 3 4 5 6 7 9 1 2 3 4 5 6 7 8 9)

Fixing this bug heavily impacts performance, more than I'm willing to accept.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants