From 9ea2616f9bb901b4389059fbe74bf10eb54f2253 Mon Sep 17 00:00:00 2001 From: Federico Meini Date: Mon, 27 Jul 2026 10:50:36 +0200 Subject: [PATCH 1/2] fix: bound cyclic table walks at the eval boundary The standard Lua OOP idiom (T.__index = T) creates tables that contain themselves. Both eval-boundary walks recursed into them forever, growing memory without bound: Value.decode in decode: true mode and Display.peek_table in decode: false mode. The walk now terminates at the point of recurrence: decode leaves the table's {:tref, id} reference there, mirroring how functions already pass through as opaque references, and Display renders a :circular peek. Shared non-cyclic references decode in full. --- lib/lua/vm/display.ex | 40 +++++++++---- lib/lua/vm/display/table.ex | 15 ++++- lib/lua/vm/value.ex | 35 +++++++---- test/lua/vm/cyclic_table_test.exs | 97 +++++++++++++++++++++++++++++++ 4 files changed, 163 insertions(+), 24 deletions(-) create mode 100644 test/lua/vm/cyclic_table_test.exs diff --git a/lib/lua/vm/display.ex b/lib/lua/vm/display.ex index 69595e79..27876aed 100644 --- a/lib/lua/vm/display.ex +++ b/lib/lua/vm/display.ex @@ -87,41 +87,54 @@ defmodule Lua.VM.Display do Wraps a single eval-result value for display. See `wrap_results/3` for the decode-mode matrix. + + Tables that (transitively) contain themselves get a `:circular` + peek at the point of recurrence rather than recursing forever — + see `Lua.VM.Display.Table`. """ @spec wrap_value(term(), State.t(), boolean()) :: term() - def wrap_value(value, state, decode?) + def wrap_value(value, state, decode?), do: wrap_value(value, state, decode?, MapSet.new()) # decode: true — only wrap closures/native; tables and userdata # have already been decoded and are passed through unchanged. - def wrap_value({:lua_closure, _, _} = ref, _state, _decode?) do + defp wrap_value({:lua_closure, _, _} = ref, _state, _decode?, _ancestors) do wrap_closure(ref) end - def wrap_value({:compiled_closure, _, _} = ref, _state, _decode?) do + defp wrap_value({:compiled_closure, _, _} = ref, _state, _decode?, _ancestors) do wrap_closure(ref) end - def wrap_value({:native_func, fun} = ref, _state, _decode?) do + defp wrap_value({:native_func, fun} = ref, _state, _decode?, _ancestors) do %NativeFunc{fun: fun, ref: ref} end # decode: false — wrap tref/udref too, and recurse into table peek. - def wrap_value({:tref, id} = ref, state, false) do - peek = peek_table(state, id, false) + # `ancestors` holds the tref ids currently being peeked higher up + # this walk; revisiting one means the table contains itself. + defp wrap_value({:tref, id} = ref, state, false, ancestors) do + peek = + if MapSet.member?(ancestors, id) do + :circular + else + peek_table(state, id, false, MapSet.put(ancestors, id)) + end + %DTable{id: id, peek: peek, ref: ref} end - def wrap_value({:udref, id} = ref, state, false) do + defp wrap_value({:udref, id} = ref, state, false, _ancestors) do term = State.get_userdata(state, ref) %Userdata{id: id, term: term, ref: ref} end # decode: true catch-all (already-decoded values pass through) - def wrap_value(value, _state, _decode?), do: value + defp wrap_value(value, _state, _decode?, _ancestors), do: value # ---- internal helpers ---- - defp wrap_closure({tag, proto, _upvalues} = ref) when tag in [:lua_closure, :compiled_closure] do + defp wrap_closure({tag, proto, _upvalues} = ref) + when tag in [:lua_closure, :compiled_closure] do {first_line, _last_line} = proto.lines || {0, 0} %Closure{ @@ -138,15 +151,18 @@ defmodule Lua.VM.Display do # (1..N keys) render as a list; mixed-key tables render as a map. # Nested tables/closures are recursively wrapped so `Inspect` does # not have to know about live VM state. - defp peek_table(state, id, decode?) do + defp peek_table(state, id, decode?, ancestors) do case Map.fetch(state.tables, id) do {:ok, table} -> data = Lua.VM.Table.to_map(table) if sequence_like?(data) do - Enum.map(1..map_size(data), &wrap_value(Map.fetch!(data, &1), state, decode?)) + Enum.map( + 1..map_size(data), + &wrap_value(Map.fetch!(data, &1), state, decode?, ancestors) + ) else - Map.new(data, fn {k, v} -> {k, wrap_value(v, state, decode?)} end) + Map.new(data, fn {k, v} -> {k, wrap_value(v, state, decode?, ancestors)} end) end :error -> diff --git a/lib/lua/vm/display/table.ex b/lib/lua/vm/display/table.ex index 56732034..3ab6ecab 100644 --- a/lib/lua/vm/display/table.ex +++ b/lib/lua/vm/display/table.ex @@ -15,7 +15,10 @@ defmodule Lua.VM.Display.Table do - `:peek` — a snapshot of the table's data as it was at the time the eval boundary was crossed, suitable for human display. May be a list (sequence-like tables) or a map (mixed-key tables). - Truncated to `Inspect.Opts.limit` entries when rendered. + Truncated to `Inspect.Opts.limit` entries when rendered. When a + table (transitively) contains itself — e.g. the `T.__index = T` + OOP idiom — the recurring occurrence carries `:circular` instead + of a snapshot, bounding an otherwise infinite walk. - `:ref` — the original `{:tref, id}` tuple so callers can round-trip the value back into the VM (via `Lua.set!/3`, `Lua.encode!/2`, etc.). @@ -25,7 +28,7 @@ defmodule Lua.VM.Display.Table do @type t :: %__MODULE__{ id: non_neg_integer(), - peek: list() | map(), + peek: list() | map() | :circular, ref: tuple() } @@ -34,6 +37,14 @@ defmodule Lua.VM.Display.Table do defimpl Inspect do import Inspect.Algebra + def inspect(%Lua.VM.Display.Table{id: id, peek: :circular}, _opts) do + concat([ + "#Lua.Table" + ]) + end + def inspect(%Lua.VM.Display.Table{id: id, peek: peek}, opts) do concat([ "#Lua.Table {k, decode(v, state)} end) + Enum.map(Lua.VM.Table.to_map(table), fn {k, v} -> {k, decode(v, state, ancestors)} end) + end end - def decode(value, _state), do: value + defp decode(value, _state, _ancestors), do: value @doc """ Decodes a list of Lua VM values. diff --git a/test/lua/vm/cyclic_table_test.exs b/test/lua/vm/cyclic_table_test.exs new file mode 100644 index 00000000..37b192c6 --- /dev/null +++ b/test/lua/vm/cyclic_table_test.exs @@ -0,0 +1,97 @@ +defmodule Lua.VM.CyclicTableTest do + @moduledoc """ + Cyclic tables crossing the eval boundary must terminate. + + The common Lua OOP idiom `T.__index = T` creates a table that + contains itself. Both boundary walks — `decode: true` (Value.decode) + and `decode: false` (Display peek) — previously recursed forever on + such values, growing memory without bound until the VM's + `max_heap_size` (when set) killed the process. + """ + + use ExUnit.Case, async: true + + alias Lua.VM.Display.Table, as: DTable + + @self_cycle """ + local T = {} + T.__index = T + return T + """ + + @mutual_cycle """ + local a = {} + local b = {a = a} + a.b = b + return a + """ + + describe "decode: false (Display peek)" do + test "self-referential table peeks as :circular at the recurrence" do + {[t], _} = Lua.eval!(Lua.new(), @self_cycle, decode: false) + + assert %DTable{id: id, peek: %{"__index" => inner}} = t + assert %DTable{id: ^id, peek: :circular} = inner + assert inspect(inner) == "#Lua.Table" + end + + test "mutually recursive tables terminate and render" do + {[t], _} = Lua.eval!(Lua.new(), @mutual_cycle, decode: false) + + assert %DTable{id: a_id, peek: %{"b" => %DTable{peek: %{"a" => inner_a}}}} = t + assert %DTable{id: ^a_id, peek: :circular} = inner_a + assert inspect(t) =~ "circular" + end + + test "shared non-cyclic references still peek fully" do + code = """ + local shared = {x = 1} + return {a = shared, b = shared} + """ + + {[t], _} = Lua.eval!(Lua.new(), code, decode: false) + + assert %DTable{ + peek: %{"a" => %DTable{peek: %{"x" => 1}}, "b" => %DTable{peek: %{"x" => 1}}} + } = + t + end + end + + describe "decode: true (Value.decode)" do + test "self-referential table terminates with the table's reference at the recurrence" do + {[decoded], _} = Lua.eval!(Lua.new(), @self_cycle) + + assert [{"__index", {:tref, id}}] = decoded + assert is_integer(id) + end + + test "mutually recursive tables terminate with a reference" do + {[decoded], _} = Lua.eval!(Lua.new(), @mutual_cycle) + + assert [{"b", [{"a", {:tref, _}}]}] = decoded + end + + test "shared non-cyclic references decode normally" do + code = """ + local shared = {x = 1} + return {a = shared, b = shared} + """ + + {[decoded], _} = Lua.eval!(Lua.new(), code) + + assert Enum.sort(decoded) == [{"a", [{"x", 1}]}, {"b", [{"x", 1}]}] + end + + test "a table appearing under multiple sibling keys is not a false-positive cycle" do + code = """ + local leaf = {v = 1} + local mid = {l = leaf, r = leaf} + return {left = mid, right = mid} + """ + + {[decoded], _} = Lua.eval!(Lua.new(), code) + assert is_list(decoded) + end + end +end From 8c6f4323150afb43b602ba28530e71457ab9ff2d Mon Sep 17 00:00:00 2001 From: Federico Meini Date: Mon, 27 Jul 2026 14:34:49 +0200 Subject: [PATCH 2/2] dialyzer: track the ancestor set as a plain map OTP 28's opacity checker false-positives on the MapSet the walkers capture in their entry closures (call_without_opaque on every MapSet call), failing CI. Specs on the private clauses don't appease it. A plain map with the ids as keys is the same structure MapSet wraps, so behavior is identical and there is no opaque type left to police. --- lib/lua/vm/display.ex | 9 ++++----- lib/lua/vm/value.ex | 9 ++++----- 2 files changed, 8 insertions(+), 10 deletions(-) diff --git a/lib/lua/vm/display.ex b/lib/lua/vm/display.ex index 27876aed..f190a9af 100644 --- a/lib/lua/vm/display.ex +++ b/lib/lua/vm/display.ex @@ -93,7 +93,7 @@ defmodule Lua.VM.Display do see `Lua.VM.Display.Table`. """ @spec wrap_value(term(), State.t(), boolean()) :: term() - def wrap_value(value, state, decode?), do: wrap_value(value, state, decode?, MapSet.new()) + def wrap_value(value, state, decode?), do: wrap_value(value, state, decode?, %{}) # decode: true — only wrap closures/native; tables and userdata # have already been decoded and are passed through unchanged. @@ -114,10 +114,10 @@ defmodule Lua.VM.Display do # this walk; revisiting one means the table contains itself. defp wrap_value({:tref, id} = ref, state, false, ancestors) do peek = - if MapSet.member?(ancestors, id) do + if Map.has_key?(ancestors, id) do :circular else - peek_table(state, id, false, MapSet.put(ancestors, id)) + peek_table(state, id, false, Map.put(ancestors, id, true)) end %DTable{id: id, peek: peek, ref: ref} @@ -133,8 +133,7 @@ defmodule Lua.VM.Display do # ---- internal helpers ---- - defp wrap_closure({tag, proto, _upvalues} = ref) - when tag in [:lua_closure, :compiled_closure] do + defp wrap_closure({tag, proto, _upvalues} = ref) when tag in [:lua_closure, :compiled_closure] do {first_line, _last_line} = proto.lines || {0, 0} %Closure{ diff --git a/lib/lua/vm/value.ex b/lib/lua/vm/value.ex index 3cdfadb0..ef919cd4 100644 --- a/lib/lua/vm/value.ex +++ b/lib/lua/vm/value.ex @@ -235,8 +235,7 @@ defmodule Lua.VM.Value do def encode(value, state, _fun_wrapper) when is_binary(value), do: {value, state} def encode(value, state, _fun_wrapper) when is_atom(value), do: {Atom.to_string(value), state} - def encode(fun, state, fun_wrapper) when is_function(fun, 1) or is_function(fun, 2), - do: {fun_wrapper.(fun), state} + def encode(fun, state, fun_wrapper) when is_function(fun, 1) or is_function(fun, 2), do: {fun_wrapper.(fun), state} def encode({:userdata, value}, state, _fun_wrapper) do State.alloc_userdata(state, value) @@ -333,7 +332,7 @@ defmodule Lua.VM.Value do normally. """ @spec decode(term(), State.t()) :: term() - def decode(value, state), do: decode(value, state, MapSet.new()) + def decode(value, state), do: decode(value, state, %{}) defp decode(nil, _state, _ancestors), do: nil defp decode(value, _state, _ancestors) when is_boolean(value), do: value @@ -346,11 +345,11 @@ defmodule Lua.VM.Value do end defp decode({:tref, id} = ref, state, ancestors) do - if MapSet.member?(ancestors, id) do + if Map.has_key?(ancestors, id) do ref else table = Map.fetch!(state.tables, id) - ancestors = MapSet.put(ancestors, id) + ancestors = Map.put(ancestors, id, true) Enum.map(Lua.VM.Table.to_map(table), fn {k, v} -> {k, decode(v, state, ancestors)} end) end