diff --git a/changelog.d/8685-fix-main-red-lint-cargo-test.md b/changelog.d/8685-fix-main-red-lint-cargo-test.md new file mode 100644 index 0000000000..44401e5144 --- /dev/null +++ b/changelog.d/8685-fix-main-red-lint-cargo-test.md @@ -0,0 +1,6 @@ +Fixed four pre-existing `main` gate failures, none caused by an in-flight PR: + +- `scripts/shape_descriptor_census.py` still asserted the old `select(I1, &is_stamp, I64, &id_token, "0")` fail-closed shape in the generic property-read PIC. PR #8665 legitimately replaced it with `icmp_ne(I32, &pcid, "0")` (a documented six-instruction perf win) but never updated this assertion. It also missed the two new, legitimate `object_header_size_bytes(...)` call sites `crates/perry-codegen/src/expr/index_get/inline_dyn_typed_array.rs` gained in PR #8680 — the checked-in baseline is refreshed to include them. +- `scripts/addr_class_inventory.py`'s `lone-valid-obj-ptr` rule reimplemented a narrower, ad hoc lookahead window (1 code line) instead of reusing the already-tested `band_predicate_near` helper (6 code lines, comment/blank-aware) used by the `handle-floor` rule. That gap flagged a legitimately paired `is_valid_obj_ptr` + `try_read_gc_header` guard PR #8680 added in `crates/perry-runtime/src/array/subclass.rs` as a false positive. Fixing the rule to call the shared helper also cleared several other long-standing false positives across the tree, so the ratchet baseline is refreshed down to the now-accurate counts. +- `commands::compile::build_cache::tests::codegen_env_vars_are_build_cache_inputs` failed because two new codegen env vars were never classified: `PERRY_CONST_ARRAY_DESCRIPTOR` (PR #8583's array-literal const-descriptor gate — changes emitted IR, added to `BUILD_CACHE_ENV_VARS`) and `PERRY_DIALECT_DUMP` (only read on an already-fatal dialect-construction failure, so it can't affect a successful build's bytes — added to `BUILD_CACHE_ENV_EXCLUSIONS`). +- `scripts/check_thread_locals.py` failed because PR #8640 added a raw `thread_local!` block in `crates/perry-runtime/src/node_submodules/test_runner.rs` instead of `crate::perry_thread_local!`. Swapped it to the hot-path macro (same syntax, same `.with()` call sites). diff --git a/crates/perry/src/commands/compile/build_cache.rs b/crates/perry/src/commands/compile/build_cache.rs index 725ecfae31..0cced3d6dd 100644 --- a/crates/perry/src/commands/compile/build_cache.rs +++ b/crates/perry/src/commands/compile/build_cache.rs @@ -115,6 +115,10 @@ const BUILD_CACHE_ENV_VARS: &[&str] = &[ // #8105 — number-by-construction locals (see the collector of the same // name); `=0` empties the fact and changes every affected function's IR. "PERRY_NUMBER_BY_CONSTRUCTION", + // #8583 follow-up gate: `=0/off/false` reverts every large constant array + // literal from the const-descriptor path to procedural construction — + // different emitted IR for the same source. + "PERRY_CONST_ARRAY_DESCRIPTOR", ]; /// #7183: codegen env vars that deliberately do NOT key the build cache. @@ -147,6 +151,10 @@ const BUILD_CACHE_ENV_EXCLUSIONS: &[&str] = &[ "PERRY_CODEGEN_UNIT_TIMINGS", // Entry outlining report output is observational only. "PERRY_OUTLINE_ENTRY_REPORT", + // Only read on an already-fatal dialect-construction failure (a unit that + // never parses); it writes a diagnostic IR dump to `/.ll` for + // triage and cannot affect the bytes of any build that actually succeeds. + "PERRY_DIALECT_DUMP", ]; #[cfg(test)] diff --git a/scripts/addr_class_inventory.py b/scripts/addr_class_inventory.py index d16f55e2b6..08dc7e3973 100644 --- a/scripts/addr_class_inventory.py +++ b/scripts/addr_class_inventory.py @@ -194,6 +194,76 @@ def band_predicate_near(lines: list[str], idx: int) -> bool: return bool(BAND_PREDICATE_RE.search("\n".join(context))) +# A new SIBLING conditional at the top of a code line — the boundary +# `lone_band_predicate_near` refuses to cross. `handle-floor`'s own #6321 fix +# shape deliberately spans two separate `if` statements (a coarse range +# pre-filter, then the real band-predicate gate a few lines below), so +# `band_predicate_near`'s generic forward scan must keep allowing that. But +# `lone-valid-obj-ptr`'s real safe shape is "guard early-returns, the very +# next statement is the band-predicate call" (see array/subclass.rs) — it +# never needs to reach into an unrelated LATER conditional to find its +# predicate. Stopping there closes the false negative a CodeRabbit review +# caught: an `is_valid_obj_ptr`-only guard that already dereferences inside +# its own block must not be cleared by some other, disconnected `if +# is_handle_band(...)` a few statements later. +# +# A conditional NESTED inside the guard's own block (brace depth still > 0 +# relative to the guard) is not that boundary — a second CodeRabbit pass +# caught this rule stopping on `if is_valid_obj_ptr(ptr) { if +# is_above_handle_band(...) { deref } }`, where the inner `if` is exactly the +# band predicate protecting the dereference, not a disconnected sibling. +SIBLING_CONDITIONAL_RE = re.compile(r"^\s*(?:\}\s*)?(?:else\s+)?(?:if|while|for|match)\b") + + +def apply_brace_deltas(depth: int, code: str) -> int: + """Advance `depth` by `code`'s braces, IN SOURCE ORDER, floored at zero. + + A net per-line count (`code.count("{") - code.count("}")`) gets an `} + else if ... {` line wrong: net zero reads as "still at the same depth", + but the leading `}` closes the ENCLOSING block first, so the following + `{` actually opens a brand-new nested one — depth should end at one + level deeper, not unchanged. Processing character by character (with + the floor so a leading close belonging to an outer block we don't track + can't push depth negative) gets this right. + """ + + for char in code: + if char == "{": + depth += 1 + elif char == "}": + depth = max(depth - 1, 0) + return depth + + +def lone_band_predicate_near(lines: list[str], idx: int) -> bool: + """`band_predicate_near`, scoped to the current guard's own statement. + + Same backward window; the forward scan stops (without including) at the + first later line that opens a new SIBLING conditional — one reached only + after the brace depth relative to `lines[idx]` has returned to zero or + below, i.e. the guard's own block (and any block nested inside it) has + already closed. A conditional still nested inside that block never + triggers the boundary, since it may be the very predicate guarding the + dereference. + """ + + start = max(0, idx - BAND_PREDICATE_LOOKBACK) + context = [strip_comment(line) for line in lines[start : idx + 1]] + depth = apply_brace_deltas(0, context[-1]) + taken = 0 + cursor = idx + 1 + while cursor < len(lines) and taken < BAND_PREDICATE_LOOKAHEAD_CODE: + code = strip_comment(lines[cursor]) + if code.strip(): + if depth <= 0 and SIBLING_CONDITIONAL_RE.search(code): + break + context.append(code) + taken += 1 + depth = apply_brace_deltas(depth, code) + cursor += 1 + return bool(BAND_PREDICATE_RE.search("\n".join(context))) + + def scan_text(rel_path: str, text: str) -> list[Finding]: findings: list[Finding] = [] if any(rel_path.startswith(prefix) for prefix in EXCLUDED_PREFIXES): @@ -214,11 +284,7 @@ def scan_text(rel_path: str, text: str) -> list[Finding]: findings.append(Finding(rel_path, line_no, "handle-floor", raw)) if VALID_OBJ_PTR_RE.search(code) and "fn is_valid_obj_ptr" not in code: # A band predicate anywhere in the enclosing guard clears it. - start = max(0, idx - BAND_PREDICATE_LOOKBACK) - context = "\n".join( - strip_comment(l) for l in lines[start : idx + 2] - ) - if not BAND_PREDICATE_RE.search(context): + if not lone_band_predicate_near(lines, idx): findings.append( Finding(rel_path, line_no, "lone-valid-obj-ptr", raw) ) @@ -414,6 +480,78 @@ def expect(cond: bool, message: str) -> None: ), "lone-valid-obj-ptr must not flag the definition", ) + # The real safe shape (array/subclass.rs): a guard that only early-returns, + # then the very next statement is the band predicate. + early_return_then_predicate = ( + " if obj.is_null() || !is_valid_obj_ptr(obj.cast::()) {\n" + " return None;\n" + " }\n" + " let header = unsafe { try_read_gc_header(obj as usize)? };\n" + ) + expect( + not any( + f.rule == "lone-valid-obj-ptr" + for f in scan_text(runtime, early_return_then_predicate) + ), + "lone-valid-obj-ptr must accept a guard that early-returns " + "then reads the band predicate in the very next statement", + ) + # CodeRabbit (PR #8685): a dereference inside an UNGUARDED is_valid_obj_ptr + # block must still be flagged even when a wholly unrelated, later sibling + # `if` happens to test a band predicate — that predicate does not guard + # the dereference above it. + separated_statement = ( + " if is_valid_obj_ptr(ptr) {\n" + " (*ptr).class_id\n" + " }\n" + " if is_handle_band(other) {}\n" + ) + expect( + any( + f.rule == "lone-valid-obj-ptr" + for f in scan_text(runtime, separated_statement) + ), + "lone-valid-obj-ptr must still flag a dereference cleared only by " + "a disconnected LATER sibling conditional's band predicate", + ) + # CodeRabbit (PR #8685, second pass): a band predicate NESTED inside the + # guard's own block — not a disconnected sibling — must still clear the + # finding, since it is exactly what protects the dereference below it. + nested_guard = ( + " if is_valid_obj_ptr(ptr) {\n" + " if is_above_handle_band(ptr as usize) {\n" + " (*ptr).class_id\n" + " }\n" + " }\n" + ) + expect( + not any( + f.rule == "lone-valid-obj-ptr" for f in scan_text(runtime, nested_guard) + ), + "lone-valid-obj-ptr must accept a band predicate NESTED inside the " + "guard's own block", + ) + # CodeRabbit (PR #8685, third pass): the guard itself following an `else + # if` chain (`} else if is_valid_obj_ptr(ptr) {`) must not miscompute its + # own starting depth as zero — a net per-line brace count treats the + # leading close and the trailing open as cancelling out, when the close + # belongs to the PRIOR branch and the open starts the guard's own block. + else_if_guard = ( + " if some_other_check() {\n" + " do_other_thing();\n" + " } else if is_valid_obj_ptr(ptr) {\n" + " if is_above_handle_band(ptr as usize) {\n" + " (*ptr).class_id\n" + " }\n" + " }\n" + ) + expect( + not any( + f.rule == "lone-valid-obj-ptr" for f in scan_text(runtime, else_if_guard) + ), + "lone-valid-obj-ptr must accept a nested band predicate when the " + "guard itself is an `else if` branch", + ) # Band literals in code are caught; comment-only mentions are not. hits = scan_text(runtime, "if addr < 0x100000 {\n") diff --git a/scripts/addr_class_ratchet_baseline.txt b/scripts/addr_class_ratchet_baseline.txt index 9b7bed7706..be65a3e608 100644 --- a/scripts/addr_class_ratchet_baseline.txt +++ b/scripts/addr_class_ratchet_baseline.txt @@ -218,7 +218,6 @@ handle-floor | crates/perry-stdlib/src/webcrypto/jwk.rs | 1 handle-floor | crates/perry-stdlib/src/webcrypto/supports.rs | 1 handle-floor | crates/perry-stdlib/src/webcrypto/util.rs | 5 handle-floor | crates/perry-stdlib/src/zlib.rs | 1 -lone-valid-obj-ptr | crates/perry-runtime/src/array/subclass.rs | 1 lone-valid-obj-ptr | crates/perry-runtime/src/buffer/access.rs | 1 lone-valid-obj-ptr | crates/perry-runtime/src/closure/dynamic_props.rs | 1 lone-valid-obj-ptr | crates/perry-runtime/src/collection_iter.rs | 1 @@ -226,23 +225,20 @@ lone-valid-obj-ptr | crates/perry-runtime/src/error.rs | 3 lone-valid-obj-ptr | crates/perry-runtime/src/intl.rs | 1 lone-valid-obj-ptr | crates/perry-runtime/src/intl/ctor_guard.rs | 2 lone-valid-obj-ptr | crates/perry-runtime/src/object/class_registry/class_meta.rs | 1 -lone-valid-obj-ptr | crates/perry-runtime/src/object/class_registry/construct.rs | 5 +lone-valid-obj-ptr | crates/perry-runtime/src/object/class_registry/construct.rs | 3 lone-valid-obj-ptr | crates/perry-runtime/src/object/class_registry/prototype_objects.rs | 1 lone-valid-obj-ptr | crates/perry-runtime/src/object/descriptors.rs | 1 lone-valid-obj-ptr | crates/perry-runtime/src/object/field_get_set/accessors.rs | 2 lone-valid-obj-ptr | crates/perry-runtime/src/object/field_get_set/enumeration.rs | 3 lone-valid-obj-ptr | crates/perry-runtime/src/object/field_get_set/field_ops.rs | 1 lone-valid-obj-ptr | crates/perry-runtime/src/object/field_get_set/get_field_by_name_tail.rs | 1 -lone-valid-obj-ptr | crates/perry-runtime/src/object/field_get_set/has_property.rs | 1 lone-valid-obj-ptr | crates/perry-runtime/src/object/field_get_set/ic_miss.rs | 1 lone-valid-obj-ptr | crates/perry-runtime/src/object/field_set_by_name/attr_variants.rs | 2 lone-valid-obj-ptr | crates/perry-runtime/src/object/global_this/array_error.rs | 1 lone-valid-obj-ptr | crates/perry-runtime/src/object/global_this/fetch_globals.rs | 1 -lone-valid-obj-ptr | crates/perry-runtime/src/object/global_this/typed_array.rs | 1 lone-valid-obj-ptr | crates/perry-runtime/src/object/iterator_prototypes.rs | 1 lone-valid-obj-ptr | crates/perry-runtime/src/object/native_call_method.rs | 3 lone-valid-obj-ptr | crates/perry-runtime/src/object/native_call_method/common_methods.rs | 2 -lone-valid-obj-ptr | crates/perry-runtime/src/object/native_call_method/primitive_methods.rs | 1 lone-valid-obj-ptr | crates/perry-runtime/src/object/native_call_method/proto_dispatch.rs | 1 lone-valid-obj-ptr | crates/perry-runtime/src/object/native_this_alias.rs | 1 lone-valid-obj-ptr | crates/perry-runtime/src/object/object_literal_ops.rs | 1 diff --git a/scripts/shape_descriptor_census.py b/scripts/shape_descriptor_census.py index c62a800629..9e0d96b1ca 100644 --- a/scripts/shape_descriptor_census.py +++ b/scripts/shape_descriptor_census.py @@ -582,7 +582,7 @@ def assert_authority_surfaces(sources: dict[str, str]) -> None: ) require_code( generic_body, - r"select\s*\(\s*I1\s*,\s*&is_stamp\s*,\s*I64\s*,\s*&id_token\s*,\s*\"0\"\s*\)", + r"icmp_ne\s*\(\s*I32\s*,\s*&pcid\s*,\s*\"0\"\s*\)", "generic read PIC invalid-id fail-closed token", ) for name in ("lower_put_value_static_write_ic", "lower_put_value_dyn_ic_inline"): @@ -815,6 +815,28 @@ def run_sabotage_selftests(sources: dict[str, str], baseline: dict[str, object]) lambda: assert_authority_surfaces(legacy_ir), ) + # #8665: the generic read PIC's invalid-id fail-closed token (pcid != 0) + # must not go quietly missing. Plant a regression that emits an + # always-nonzero comparand instead of the real ShapeId register, and + # prove the census still catches it -- this is what stands between the + # check above and a vacuous pass, per #6942/#6946/#7024's precedent that + # an unexercised assertion is a decision nobody actually made. + dropped_fail_closed = dict(sources) + path = "crates/perry-codegen/src/expr/property_get/generic_dispatch.rs" + sabotaged_body, substitutions = re.subn( + r'icmp_ne\(I32, &pcid, "0"\)', + 'icmp_ne(I32, &pcid, "-1")', + dropped_fail_closed[path], + count=1, + ) + if substitutions != 1: + raise CensusError("generic read PIC fail-closed sabotage fixture missing") + dropped_fail_closed[path] = sabotaged_body + expect_rejected( + "generic read PIC invalid-id fail-closed token silently changed", + lambda: assert_authority_surfaces(dropped_fail_closed), + ) + # #8113: the gep-spelled emitted guards. This arm was VACUOUS before — # it matched only `add(..., "N")` — so plant a keys-offset gep and prove # it is caught now. diff --git a/scripts/shape_descriptor_census_baseline.json b/scripts/shape_descriptor_census_baseline.json index 02529bfde0..4114a6348f 100644 --- a/scripts/shape_descriptor_census_baseline.json +++ b/scripts/shape_descriptor_census_baseline.json @@ -2,6 +2,8 @@ "codegen_object_header_size_callsite_multiset": { "crates/perry-codegen/src/codegen/artifacts.rs|crate::target_layout::object_header_size_bytes(target_triple),": 1, "crates/perry-codegen/src/expr/element_shape_guard.rs|let header_skip = crate::target_layout::object_header_size_bytes(ctx.target_triple).to_string();": 1, + "crates/perry-codegen/src/expr/index_get/inline_dyn_typed_array.rs|crate::target_layout::object_header_size_bytes(ctx.target_triple).to_string();": 1, + "crates/perry-codegen/src/expr/index_get/inline_dyn_typed_array.rs|let meta_offset = (crate::target_layout::object_header_size_bytes(ctx.target_triple)": 1, "crates/perry-codegen/src/expr/member_update.rs|let header_skip = crate::target_layout::object_header_size_bytes(": 1, "crates/perry-codegen/src/expr/property_get.rs|crate::target_layout::object_header_size_bytes(ctx.target_triple)": 3, "crates/perry-codegen/src/expr/property_get/generic_dispatch.rs|crate::target_layout::object_header_size_bytes(ctx.target_triple).to_string();": 1, @@ -45,7 +47,7 @@ "crates/perry-runtime/src/object/object_ops.rs|keys_array|declaration|pub(crate) use keys_array::{": 1 }, "summary": { - "codegen_object_header_size_sites": 34, + "codegen_object_header_size_sites": 36, "raw_member_files": 7, "raw_member_sites": { "keys_array": 24