From b828093c87f6d4180a61be87334eddfcbe131676 Mon Sep 17 00:00:00 2001 From: Jon Ross-Perkins Date: Fri, 21 Apr 2023 15:14:14 -0700 Subject: [PATCH] Start checking for a few possible resource exhaustion scenarios for explorer (#2793) I couldn't figure out a way to actually hit a reasonable out-of-memory case once I add the maximum interpreter step count. However, the step count limit seems more important. I've moved the todo stack limit out of function calls because there are plenty of ways to build up the todo stack without any function calls. Fixes #2791 --- explorer/common/arena.h | 5 ++++ explorer/interpreter/action_stack.cpp | 6 ++-- explorer/interpreter/action_stack.h | 4 +-- explorer/interpreter/interpreter.cpp | 29 +++++++++++++++---- explorer/interpreter/stack.h | 8 ++--- explorer/testdata/limits/README.md | 11 +++++++ explorer/testdata/limits/fail_allocate.carbon | 20 +++++++++++++ .../fail_function_recursion.carbon} | 8 ++--- explorer/testdata/limits/fail_loop.carbon | 14 +++++++++ .../limits/fail_type_check_loop.carbon | 18 ++++++++++++ 10 files changed, 104 insertions(+), 19 deletions(-) create mode 100644 explorer/testdata/limits/README.md create mode 100644 explorer/testdata/limits/fail_allocate.carbon rename explorer/testdata/{function/fail_recursion_stackoverflow.carbon => limits/fail_function_recursion.carbon} (59%) create mode 100644 explorer/testdata/limits/fail_loop.carbon create mode 100644 explorer/testdata/limits/fail_type_check_loop.carbon diff --git a/explorer/common/arena.h b/explorer/common/arena.h index ee7950c0893d..655d44931b79 100644 --- a/explorer/common/arena.h +++ b/explorer/common/arena.h @@ -35,6 +35,7 @@ class Arena { std::make_unique>(std::forward(args)...); Nonnull ptr = smart_ptr->Instance(); arena_.push_back(std::move(smart_ptr)); + allocated_ += sizeof(T); return ptr; } @@ -45,8 +46,11 @@ class Arena { void New(WriteAddressTo addr, Args&&... args) { arena_.push_back(std::make_unique>( addr, std::forward(args)...)); + allocated_ += sizeof(T); } + auto allocated() -> int64_t { return allocated_; } + private: // Virtualizes arena entries so that a single vector can contain many types, // avoiding templated statics. @@ -83,6 +87,7 @@ class Arena { // Manages allocations in an arena for destruction at shutdown. std::vector> arena_; + int64_t allocated_ = 0; }; } // namespace Carbon diff --git a/explorer/interpreter/action_stack.cpp b/explorer/interpreter/action_stack.cpp index 298f6dc91150..31cec81ea20b 100644 --- a/explorer/interpreter/action_stack.cpp +++ b/explorer/interpreter/action_stack.cpp @@ -21,7 +21,7 @@ void ActionStack::Print(llvm::raw_ostream& out) const { void ActionStack::Start(std::unique_ptr action) { result_ = std::nullopt; - CARBON_CHECK(todo_.IsEmpty()); + CARBON_CHECK(todo_.empty()); todo_.Push(std::move(action)); } @@ -253,7 +253,7 @@ auto ActionStack::UnwindPast(Nonnull ast_node, void ActionStack::PopScopes( std::stack>& cleanup_stack) { - while (!todo_.IsEmpty() && llvm::isa(*todo_.Top())) { + while (!todo_.empty() && llvm::isa(*todo_.Top())) { auto act = todo_.Pop(); if (act->scope()) { cleanup_stack.push(std::move(act)); @@ -262,7 +262,7 @@ void ActionStack::PopScopes( } void ActionStack::SetResult(Nonnull result) { - if (todo_.IsEmpty()) { + if (todo_.empty()) { result_ = result; } else { todo_.Top()->AddResult(result); diff --git a/explorer/interpreter/action_stack.h b/explorer/interpreter/action_stack.h index d2ad95d2c0bd..bb6d7aea8871 100644 --- a/explorer/interpreter/action_stack.h +++ b/explorer/interpreter/action_stack.h @@ -38,7 +38,7 @@ class ActionStack { void Start(std::unique_ptr action); // True if the stack is empty. - auto IsEmpty() const -> bool { return todo_.IsEmpty(); } + auto empty() const -> bool { return todo_.empty(); } // The Action currently at the top of the stack. This will never be a // ScopeAction. @@ -106,7 +106,7 @@ class ActionStack { void Pop() { todo_.Pop(); } - auto Count() const -> int { return todo_.Count(); } + auto size() const -> int { return todo_.size(); } private: // Pop any ScopeActions from the top of the stack, propagating results as diff --git a/explorer/interpreter/interpreter.cpp b/explorer/interpreter/interpreter.cpp index c0e6a7d92594..78ead1789c8c 100644 --- a/explorer/interpreter/interpreter.cpp +++ b/explorer/interpreter/interpreter.cpp @@ -40,6 +40,11 @@ using llvm::isa; namespace Carbon { +// Limits for various overflow conditions. +static constexpr int64_t MaxTodoSize = 1e3; +static constexpr int64_t MaxStepsTaken = 1e6; +static constexpr int64_t MaxArenaAllocated = 1e9; + // Constructs an ActionStack suitable for the specified phase. static auto MakeTodo(Phase phase, Nonnull heap) -> ActionStack { switch (phase) { @@ -185,6 +190,10 @@ class Interpreter { Nonnull print_stream_; Phase phase_; + + // The number of steps taken by the interpreter. Used for infinite loop + // detection. + int64_t steps_taken_ = 0; }; // @@ -951,10 +960,6 @@ auto Interpreter::CallFunction(const CallExpression& call, Nonnull fun, Nonnull arg, ImplWitnessMap&& witnesses) -> ErrorOr { - constexpr int StackSizeLimit = 1000; - if (todo_.Count() > StackSizeLimit) { - return ProgramError(call.source_loc()) << "stack overflow"; - } if (trace_stream_->is_enabled()) { *trace_stream_ << "calling function: " << *fun << "\n"; } @@ -2394,6 +2399,20 @@ auto Interpreter::StepCleanUp() -> ErrorOr { // State transition. auto Interpreter::Step() -> ErrorOr { + // Check for various overflow conditions before stepping. + if (todo_.size() > MaxTodoSize) { + return ProgramError(SourceLocation("overflow", 1)) + << "Stack overflow: too many interpreter actions on stack"; + } + if (++steps_taken_ > MaxStepsTaken) { + return ProgramError(SourceLocation("overflow", 1)) + << "Possible infinite loop: too many interpreter steps executed"; + } + if (arena_->allocated() > MaxArenaAllocated) { + return ProgramError(SourceLocation("overflow", 1)) + << "Out of memory: exceeded arena allocation limit"; + } + Action& act = todo_.CurrentAction(); switch (act.kind()) { case Action::Kind::LocationAction: @@ -2434,7 +2453,7 @@ auto Interpreter::RunAllSteps(std::unique_ptr action) TraceState(); } todo_.Start(std::move(action)); - while (!todo_.IsEmpty()) { + while (!todo_.empty()) { CARBON_RETURN_IF_ERROR(Step()); if (trace_stream_->is_enabled()) { TraceState(); diff --git a/explorer/interpreter/stack.h b/explorer/interpreter/stack.h index 99c4d2467ad4..79803a14bd63 100644 --- a/explorer/interpreter/stack.h +++ b/explorer/interpreter/stack.h @@ -32,7 +32,7 @@ struct Stack { // // - Requires: !this->IsEmpty() auto Pop() -> T { - CARBON_CHECK(!IsEmpty()) << "Can't pop from empty stack."; + CARBON_CHECK(!empty()) << "Can't pop from empty stack."; auto r = std::move(elements_.back()); elements_.pop_back(); return r; @@ -52,15 +52,15 @@ struct Stack { // // - Requires: !this->IsEmpty() auto Top() const -> const T& { - CARBON_CHECK(!IsEmpty()) << "Empty stack has no Top()."; + CARBON_CHECK(!empty()) << "Empty stack has no Top()."; return elements_.back(); } // Returns `true` iff `Count() > 0`. - auto IsEmpty() const -> bool { return elements_.empty(); } + auto empty() const -> bool { return elements_.empty(); } // Returns the number of elements in `*this`. - auto Count() const -> int { return elements_.size(); } + auto size() const -> int { return elements_.size(); } // Iterates over the Stack from top to bottom. auto begin() const -> const_iterator { return elements_.crbegin(); } diff --git a/explorer/testdata/limits/README.md b/explorer/testdata/limits/README.md new file mode 100644 index 000000000000..3a58c6df3386 --- /dev/null +++ b/explorer/testdata/limits/README.md @@ -0,0 +1,11 @@ +# Limit tests + + + +These tests check for various limit conditions (such as an infinite loop). The +tests collectively disable autoupdate so that tracing isn't enabled, because +tracing creates substantial additional overhead. diff --git a/explorer/testdata/limits/fail_allocate.carbon b/explorer/testdata/limits/fail_allocate.carbon new file mode 100644 index 000000000000..2e5fee0e9ff7 --- /dev/null +++ b/explorer/testdata/limits/fail_allocate.carbon @@ -0,0 +1,20 @@ +// Part of the Carbon Language project, under the Apache License v2.0 with LLVM +// Exceptions. See /LICENSE for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +// NOAUTOUPDATE +// RUN: %{not} %{explorer-run} +// CHECK:STDERR: RUNTIME ERROR: overflow:1: Possible infinite loop: too many interpreter steps executed + +package EmptyIdentifier impl; + +fn Main() -> i32 { + while (true) { + // Ideally we would hit an OOM here, but it's too difficult to OOM from heap + // allocations before hitting max steps. Maybe with string operations we + // could trigger actual excessive memory allocations by just doubling the + // size of the string each time. + heap.New((0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 0)); + } + return 0; +} diff --git a/explorer/testdata/function/fail_recursion_stackoverflow.carbon b/explorer/testdata/limits/fail_function_recursion.carbon similarity index 59% rename from explorer/testdata/function/fail_recursion_stackoverflow.carbon rename to explorer/testdata/limits/fail_function_recursion.carbon index 153b9718b8a3..e502cc7a0db1 100644 --- a/explorer/testdata/function/fail_recursion_stackoverflow.carbon +++ b/explorer/testdata/limits/fail_function_recursion.carbon @@ -2,19 +2,17 @@ // Exceptions. See /LICENSE for license information. // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception // -// AUTOUPDATE +// NOAUTOUPDATE // RUN: %{not} %{explorer-run} -// RUN: %{not} %{explorer-run-trace} +// CHECK:STDERR: RUNTIME ERROR: overflow:1: Stack overflow: too many interpreter actions on stack package EmptyIdentifier impl; fn A() { - // CHECK:STDERR: RUNTIME ERROR: {{.*}}/explorer/testdata/function/fail_recursion_stackoverflow.carbon:[[@LINE+1]]: stack overflow A(); } -fn Main() -> i32 -{ +fn Main() -> i32 { A(); return 0; } diff --git a/explorer/testdata/limits/fail_loop.carbon b/explorer/testdata/limits/fail_loop.carbon new file mode 100644 index 000000000000..76c544ff9275 --- /dev/null +++ b/explorer/testdata/limits/fail_loop.carbon @@ -0,0 +1,14 @@ +// Part of the Carbon Language project, under the Apache License v2.0 with LLVM +// Exceptions. See /LICENSE for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +// NOAUTOUPDATE +// RUN: %{not} %{explorer-run} +// CHECK:STDERR: RUNTIME ERROR: overflow:1: Possible infinite loop: too many interpreter steps executed + +package ExplorerTest impl; + +fn Main() -> i32 { + while (true) { } + return 0; +} diff --git a/explorer/testdata/limits/fail_type_check_loop.carbon b/explorer/testdata/limits/fail_type_check_loop.carbon new file mode 100644 index 000000000000..a6a98e51e766 --- /dev/null +++ b/explorer/testdata/limits/fail_type_check_loop.carbon @@ -0,0 +1,18 @@ +// Part of the Carbon Language project, under the Apache License v2.0 with LLVM +// Exceptions. See /LICENSE for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +// NOAUTOUPDATE +// RUN: %{not} %{explorer-run} +// CHECK:STDERR: COMPILATION ERROR: overflow:1: Possible infinite loop: too many interpreter steps executed + +package ExplorerTest impl; + +fn Loop() -> type { + while (true) {} + return i32; +} + +fn Main() -> Loop() { + return 0; +}