From 0807f7455ed0329df6afe25655201cad3911e724 Mon Sep 17 00:00:00 2001 From: Kacy Fortner Date: Sun, 26 Jul 2026 18:49:36 +0000 Subject: [PATCH] build a result unwrap_or's fallback lazily, in the error arm MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit result `unwrap_or` evaluated its fallback before branching on ok/err. on the ok path the fallback was never used, so a heap default — `r.unwrap_or(bytes.empty())` is the everyday shape — was allocated and dropped every call, leaking one object per iteration. a loop over an ok result grew ~64 bytes an iteration without end. the optional `unwrap_or` already builds its default inside the none arm; the result path now does the same. on the ok path the fallback expression is never evaluated, which is both leak-free and the semantics you'd expect from a lazy-looking default. tests/cases/test_result_unwrap_or_fallback_leak drives 200k ok iterations through a heap-fallback unwrap_or and checks the footprint stays flat. it uses an inline (owned) operand on purpose: `unwrap_or` on a borrowed operand — a named result local, or a `T!` param — has a separate, pre-existing ownership bug (it shell-releases an operand its owner also releases) that is being fixed on its own; this test isolates the fallback. --- Makefile | 1 + self-host/bootstrap/ir_driver.ir | 168 +++++++++--------- self-host/ir_emitter_core.pith | 6 +- .../test_result_unwrap_or_fallback_leak.pith | 45 +++++ .../test_result_unwrap_or_fallback_leak.txt | 1 + 5 files changed, 136 insertions(+), 85 deletions(-) create mode 100644 tests/cases/test_result_unwrap_or_fallback_leak.pith create mode 100644 tests/expected/test_result_unwrap_or_fallback_leak.txt diff --git a/Makefile b/Makefile index fe66c6dc..6d0bc725 100644 --- a/Makefile +++ b/Makefile @@ -1032,6 +1032,7 @@ MEMCHECK_CASES := \ tests/cases/test_channel_fanout_ownership \ tests/cases/test_empty_list_field_ownership \ tests/cases/test_list_bytes_ownership \ + tests/cases/test_result_unwrap_or_fallback_leak \ tests/cases/test_struct_closure_field tests/cases/test_generic_struct_field_dtor \ tests/cases/test_iterator_drain tests/cases/test_method_fresh_string_return \ tests/cases/test_lazy_adapter_pipeline tests/cases/test_named_local_adapter_chain \ diff --git a/self-host/bootstrap/ir_driver.ir b/self-host/bootstrap/ir_driver.ir index 09b9ad0a..74fb4fb1 100644 --- a/self-host/bootstrap/ir_driver.ir +++ b/self-host/bootstrap/ir_driver.ir @@ -146854,81 +146854,81 @@ load 24 node iconst 25 0 call 26 ir_emitter_core_ir_method_call_arg_expr int 2 24 25 store arg_idx 26 -load 27 arg_idx -call 28 ir_emitter_core_ir_expr int 1 27 -store fallback_r 28 -load 29 ok_l -call 30 ir_builder_ir_label string 0 -call 31 pith_cstring_release void 1 29 -store ok_l 30 -load 32 fallback_l -call 33 ir_builder_ir_label string 0 -call 34 pith_cstring_release void 1 32 -store fallback_l 33 -load 35 merge_l -call 36 ir_builder_ir_label string 0 -call 37 pith_cstring_release void 1 35 -store merge_l 36 -strref 38 m46s829 -load 39 cond_r -call 40 int_to_string string 1 39 -concat 41 38 40 -call 42 pith_cstring_release void 1 38 -call 43 pith_cstring_release void 1 40 -strref 44 m46s333 -concat 45 41 44 -call 46 pith_cstring_release void 1 41 -call 47 pith_cstring_release void 1 44 -load 48 ok_l -concat 49 45 48 -call 50 pith_cstring_release void 1 45 -strref 51 m46s333 -concat 52 49 51 -call 53 pith_cstring_release void 1 49 -call 54 pith_cstring_release void 1 51 -load 55 fallback_l -concat 56 52 55 -call 57 pith_cstring_release void 1 52 -call 58 ir_builder_ir_emit void 1 56 -call 59 pith_cstring_release void 1 56 -load 60 ok_l -call 61 ir_call_emit_ir_emit_label void 1 60 -load 62 recv_r -load 63 recv_tid -strref 64 m46s182 -call 65 ir_result_abi_ir_emit_result_value_field int 3 62 63 64 -call 66 pith_cstring_release void 1 64 -store ok_value_r 65 -call 67 ir_builder_ir_reg int 0 -strref 68 m46s893 -strref 69 m46s823 -load 70 recv_r -call 71 ir_call_emit_ir_emit_call1 void 4 67 68 69 70 -call 72 pith_cstring_release void 1 68 -call 73 pith_cstring_release void 1 69 -strref 74 m46s825 -load 75 temp_name -concat 76 74 75 -call 77 pith_cstring_release void 1 74 -strref 78 m46s333 -concat 79 76 78 -call 80 pith_cstring_release void 1 76 -call 81 pith_cstring_release void 1 78 -load 82 ok_value_r -call 83 int_to_string string 1 82 -concat 84 79 83 -call 85 pith_cstring_release void 1 79 -call 86 pith_cstring_release void 1 83 -call 87 ir_builder_ir_emit void 1 84 -call 88 pith_cstring_release void 1 84 -strref 89 m46s828 -load 90 merge_l -concat 91 89 90 +load 27 ok_l +call 28 ir_builder_ir_label string 0 +call 29 pith_cstring_release void 1 27 +store ok_l 28 +load 30 fallback_l +call 31 ir_builder_ir_label string 0 +call 32 pith_cstring_release void 1 30 +store fallback_l 31 +load 33 merge_l +call 34 ir_builder_ir_label string 0 +call 35 pith_cstring_release void 1 33 +store merge_l 34 +strref 36 m46s829 +load 37 cond_r +call 38 int_to_string string 1 37 +concat 39 36 38 +call 40 pith_cstring_release void 1 36 +call 41 pith_cstring_release void 1 38 +strref 42 m46s333 +concat 43 39 42 +call 44 pith_cstring_release void 1 39 +call 45 pith_cstring_release void 1 42 +load 46 ok_l +concat 47 43 46 +call 48 pith_cstring_release void 1 43 +strref 49 m46s333 +concat 50 47 49 +call 51 pith_cstring_release void 1 47 +call 52 pith_cstring_release void 1 49 +load 53 fallback_l +concat 54 50 53 +call 55 pith_cstring_release void 1 50 +call 56 ir_builder_ir_emit void 1 54 +call 57 pith_cstring_release void 1 54 +load 58 ok_l +call 59 ir_call_emit_ir_emit_label void 1 58 +load 60 recv_r +load 61 recv_tid +strref 62 m46s182 +call 63 ir_result_abi_ir_emit_result_value_field int 3 60 61 62 +call 64 pith_cstring_release void 1 62 +store ok_value_r 63 +call 65 ir_builder_ir_reg int 0 +strref 66 m46s893 +strref 67 m46s823 +load 68 recv_r +call 69 ir_call_emit_ir_emit_call1 void 4 65 66 67 68 +call 70 pith_cstring_release void 1 66 +call 71 pith_cstring_release void 1 67 +strref 72 m46s825 +load 73 temp_name +concat 74 72 73 +call 75 pith_cstring_release void 1 72 +strref 76 m46s333 +concat 77 74 76 +call 78 pith_cstring_release void 1 74 +call 79 pith_cstring_release void 1 76 +load 80 ok_value_r +call 81 int_to_string string 1 80 +concat 82 77 81 +call 83 pith_cstring_release void 1 77 +call 84 pith_cstring_release void 1 81 +call 85 ir_builder_ir_emit void 1 82 +call 86 pith_cstring_release void 1 82 +strref 87 m46s828 +load 88 merge_l +concat 89 87 88 +call 90 pith_cstring_release void 1 87 +call 91 ir_builder_ir_emit void 1 89 call 92 pith_cstring_release void 1 89 -call 93 ir_builder_ir_emit void 1 91 -call 94 pith_cstring_release void 1 91 -load 95 fallback_l -call 96 ir_call_emit_ir_emit_label void 1 95 +load 93 fallback_l +call 94 ir_call_emit_ir_emit_label void 1 93 +load 95 arg_idx +call 96 ir_emitter_core_ir_expr int 1 95 +store fallback_r 96 load 97 recv_r load 98 recv_tid call 99 ir_emitter_core_ir_emit_release_result_error void 2 97 98 @@ -172849,18 +172849,18 @@ store __legacy_result_25_101 34 label L103 load 35 __legacy_result_25_101 field 36 35 0 bool is_ok -strref 37 m48s69 brif 36 L104 L105 label L104 -field 38 35 8 string ok -call 39 pith_struct_release void 1 35 -store __unwrap_or_37_104 38 +field 37 35 8 string ok +call 38 pith_struct_release void 1 35 +store __unwrap_or_37_104 37 jmp L106 label L105 +strref 39 m48s69 field 40 35 16 string err call 41 pith_cstring_release void 1 40 call 42 pith_struct_release void 1 35 -store __unwrap_or_37_104 37 +store __unwrap_or_37_104 39 label L106 load 43 __unwrap_or_37_104 call 44 pith_cstring_retain void 1 43 @@ -173276,18 +173276,18 @@ store __legacy_result_28_152 37 label L154 load 38 __legacy_result_28_152 field 39 38 0 bool is_ok -strref 40 m48s69 brif 39 L155 L156 label L155 -field 41 38 8 string ok -call 42 pith_struct_release void 1 38 -store __unwrap_or_40_155 41 +field 40 38 8 string ok +call 41 pith_struct_release void 1 38 +store __unwrap_or_40_155 40 jmp L157 label L156 +strref 42 m48s69 field 43 38 16 string err call 44 pith_cstring_release void 1 43 call 45 pith_struct_release void 1 38 -store __unwrap_or_40_155 40 +store __unwrap_or_40_155 42 label L157 load 46 __unwrap_or_40_155 call 47 pith_cstring_retain void 1 46 diff --git a/self-host/ir_emitter_core.pith b/self-host/ir_emitter_core.pith index 42978ea3..6945b834 100644 --- a/self-host/ir_emitter_core.pith +++ b/self-host/ir_emitter_core.pith @@ -3712,7 +3712,6 @@ fn ir_emit_result_unwrap_or(node: Node) -> Int: cond_r := ir_emit_result_flag_field(recv_r, "is_ok") temp_name := ir_new_temp_name("unwrap_or") arg_idx := ir_method_call_arg_expr(node, 0) - fallback_r := ir_expr(arg_idx) ok_l := ir_label() fallback_l := ir_label() merge_l := ir_label() @@ -3723,6 +3722,11 @@ fn ir_emit_result_unwrap_or(node: Node) -> Int: ir_emit("store " + temp_name + " " + ok_value_r.to_string()) ir_emit("jmp " + merge_l) ir_emit_label(fallback_l) + # build the fallback INSIDE the error arm: on the ok path it is never + # needed, and evaluating it eagerly before the branch leaked a heap default + # (`r.unwrap_or(bytes.empty())`) every ok iteration. this mirrors the + # optional unwrap_or, which already defers its default into the none arm. + fallback_r := ir_expr(arg_idx) ir_emit_release_result_error(recv_r, recv_tid) ir_emit_call1(ir_reg(), "pith_struct_release", "void", recv_r) ir_emit("store " + temp_name + " " + fallback_r.to_string()) diff --git a/tests/cases/test_result_unwrap_or_fallback_leak.pith b/tests/cases/test_result_unwrap_or_fallback_leak.pith new file mode 100644 index 00000000..bc2b7a0d --- /dev/null +++ b/tests/cases/test_result_unwrap_or_fallback_leak.pith @@ -0,0 +1,45 @@ +# a result's `unwrap_or` must not leak its fallback on the ok path. +# +# the fallback used to be built before the ok/err branch, so a heap default +# like `unwrap_or(bytes.empty())` was allocated every call and, whenever the +# result was ok, discarded without release. the optional `unwrap_or` already +# deferred its default into the none arm; the result path now matches. this +# runs the ok path many times over and checks the process footprint stays flat. + +import std.bytes as bytes +import std.fs as fs +import std.strings as strings + +fn make(i: Int) -> Bytes!: + if i < 0: + fail "negative" + return bytes.from_string_utf8("a payload wide enough to notice if it leaks") + +fn hwm_kb() -> Int: + status := fs.read_file("/proc/self/status") catch "" + for line in status.split(chr(10)): + if line.starts_with("VmHWM"): + for part in line.split(" "): + v := strings.parse_int(part).unwrap_or(0 - 1) + if v > 0: + return v + return 0 + +fn main(): + mut total := 0 + mut i := 0 + while i < 200000: + # inline (owned) operand: `unwrap_or` moves its ok payload out and frees + # the fresh result shell itself, so this exercises only the fallback, + # not the separate borrowed-operand ownership question. the fallback + # allocates every iteration and, on the ok path, must never be kept. + total = total + make(i).unwrap_or(bytes.empty()).len() + i = i + 1 + peak := hwm_kb() + # a per-iteration leak of the ~48-byte empty-bytes fallback would add + # ~9 mb over 200k iterations; flat sits near 3 mb. 20 mb is a wide margin + # that a real leak blows through and normal variance never reaches. + if peak < 20000: + print("flat total=" + total.to_string()) + else: + print("leaked peak_kb=" + peak.to_string()) diff --git a/tests/expected/test_result_unwrap_or_fallback_leak.txt b/tests/expected/test_result_unwrap_or_fallback_leak.txt new file mode 100644 index 00000000..3c840da9 --- /dev/null +++ b/tests/expected/test_result_unwrap_or_fallback_leak.txt @@ -0,0 +1 @@ +flat total=8600000