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

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 450:1903 Ark::VM::safeRun 3 1217 249
src/arkreactor/Compiler/Macros/Processor.cpp 239:617 Ark::internal::MacroProcessor::evaluate 3 355 125
src/arkreactor/Compiler/BytecodeReader.cpp 274:557 Ark::BytecodeReader::display 4 241 95
src/arkreactor/Exceptions.cpp 87:297 Ark::Diagnostics::makeContext 9 152 61
src/arkreactor/Compiler/Lowerer/ASTLowerer.cpp 539:710 Ark::internal::ASTLowerer::handleCalls 4 130 42
src/arkscript/JsonCompiler.cpp 27:260 JsonCompiler::_compile 1 200 36
include/Ark/Compiler/AST/Parser.hpp 107:196 Ark::internal::ARK_APIParser::string 0 87 32
src/arkreactor/Compiler/NameResolution/NameResolutionPass.cpp 162:273 Ark::internal::NameResolutionPass::visitKeyword 3 93 32
src/arkscript/main.cpp 23:352 main 2 284 31
src/arkreactor/Compiler/AST/Node.cpp 189:287 Ark::internal::Node::repr 0 84 28
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 289:362 Ark::internal::Node::debugPrint 1 64 24
src/arkreactor/TypeChecker.cpp 110:194 Ark::types::generateError 5 72 24
src/arkreactor/Compiler/NameResolution/NameResolutionPass.cpp 54:160 Ark::internal::NameResolutionPass::visit 2 87 23
src/arkreactor/Compiler/AST/Parser.cpp 292:427 Ark::internal::Parser::import_ 0 109 23
src/arkreactor/Compiler/Lowerer/ASTLowerer.cpp 267:326 Ark::internal::ASTLowerer::compileListInstruction 3 49 22
src/arkreactor/VM/VM.cpp 2022:2131 Ark::VM::backtrace 3 94 22
include/utf8.hpp 138:184 utf8::isValid 1 44 21
src/arkreactor/Compiler/AST/Optimizer.cpp 33:83 Ark::internal::Optimizer::countAndPruneDeadCode 1 42 20
src/arkscript/REPL/Utils.cpp 52:184 Ark::internal::getColorPerKeyword 0 110 19
src/arkreactor/TypeChecker.cpp 28:108 Ark::types::displayContract 4 70 19
src/arkreactor/Compiler/NameResolution/StaticScope.cpp 68:109 Ark::internal::NamespaceScope::get 2 32 18
src/arkreactor/VM/Value.cpp 72:132 Ark::Value::toString 1 48 18
src/arkreactor/Compiler/Macros/Executors/Function.cpp 16:88 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 168:224 Formatter::format 3 53 16
src/arkreactor/Compiler/Macros/Processor.cpp 699:738 Ark::internal::MacroProcessor::isConstEval 1 35 16
src/arkscript/Formatter.cpp 274:315 Formatter::formatFunction 2 35 15
src/arkreactor/Compiler/Lowerer/ASTLowerer.cpp 78:94 Ark::internal::ASTLowerer::nodeProducesOutput 1 13 15
src/arkreactor/Compiler/IntermediateRepresentation/IROptimizer.cpp 12:198 Ark::internal::IROptimizer::IROptimizer 1 163 4
src/arkreactor/Exceptions.cpp 299:310 Ark::Diagnostics::helper 9 11 2

CppCheck report

Filename Line Type Description
include/Ark/VM/VM.inl 250 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
include/Ark/VM/Value/Procedure.hpp 43 style Class 'Procedure' has a constructor with 1 argument that is not explicit.
include/Ark/VM/Value/Procedure.hpp 51 style Class 'Procedure' has a constructor with 1 argument that is not explicit.
include/Ark/Logger.hpp 77 performance Function parameter 'data' should be passed by const reference.
src/arkreactor/Builtins/IO.cpp 81 style Parameter 'n' can be declared as reference to const
src/arkreactor/Builtins/IO.cpp 105 style Parameter 'n' can be declared as reference to const
src/arkreactor/Compiler/BytecodeReader.cpp 440 style struct member 'Arg::kind' is never used.
src/arkreactor/Compiler/Lowerer/ASTLowerer.cpp 243 style Parameter 'x' can be declared as reference to const
src/arkreactor/Compiler/Lowerer/ASTLowerer.cpp 493 style Parameter 'x' can be declared as reference to const
src/arkreactor/Compiler/NameResolution/NameResolutionPass.cpp 142 style Consider using std::find_if algorithm instead of a raw loop.
src/arkreactor/VM/State.cpp 187 style Consider using std::any_of, std::all_of, std::none_of algorithm instead of a raw loop.
src/arkreactor/VM/Value/Procedure.cpp 15 performance Variable 'm_procedure' is assigned in constructor body. Consider performing initialization in initialization list.

@coveralls
Copy link

coveralls commented Jul 9, 2025

Coverage Status

coverage: 86.694% (-0.02%) from 86.717%
when pulling 36aea35 on fix/scope-bad-alloc
into 66d9c29 on dev.

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 : 41 thousands
 Cumulative speed : 137 execs/sec
    Pending items : 147 faves, 1131 total
 Coverage reached : 12.92%
    Crashes saved : 0
      Hangs saved : 0

Cycles without finds : 0
Time without finds : 0

[+] Captured 47650 tuples (map size 215762, highest value 255, total values 387050435) in '/dev/null'.
[+] A coverage of 47650 edges were achieved out of 215808 existing (22.08%) with 1141 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
Copy link

codspeed-hq bot commented Jul 23, 2025

CodSpeed Performance Report

Merging #559 will degrade performances by 3.71%

Comparing fix/scope-bad-alloc (36aea35) with dev (66d9c29)

Summary

❌ 4 regressions
✅ 13 untouched benchmarks

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

Benchmarks breakdown

Benchmark BASE HEAD Change
ackermann 309.9 ms 321.8 ms -3.71%
binary_trees 4.7 s 4.8 s -3.17%
create_closure 4.5 ms 4.6 ms -1.11%
fibonacci 27.8 ms 28.4 ms -2.13%

@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?

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.

2 participants