Skip to content

fix(validate): reject nullable borrow buffer parameters - #12

Merged
Sam Gammon (sgammon) merged 1 commit into
mainfrom
detail/bug-fix/fix-validate-reject-nullable-borrow-buffer-paramet-689c43
Oct 26, 2026
Merged

Sam Gammon (sgammon) merged 1 commit into
mainfrom
detail/bug-fix/fix-validate-reject-nullable-borrow-buffer-paramet-689c43

Conversation

@detail-app

@detail-app detail-app Bot commented Oct 23, 2026

Copy link
Copy Markdown
Contributor

Detail bug report: View on Detail

Bug

OwnershipValidator guards the borrow (call-buffer) feature of the .seam DSL, whose purpose is to reject any native declaration the generated ownership wrappers cannot faithfully honor — even in relaxed mode. It never rejected a nullable=true (or defaulted-nullable) buffer parameter on a borrow declaration, even though generated Java/Rust wrappers can never deliver NULL to the native callee:

  • Java substitutes a non-null stack dummy at address zero; byte[] overloads dereference .length and can't take null.
  • Rust uses &[u8] and passes bytes.as_ptr(), always non-null.

The DSL defaults pointer params to nullable=true, so simply omitting nullable=false on a borrow buffer silently advertised a NULLable ABI contract the bindings violate, in both relaxed and strict mode. The borrow loop validated type, ownership, access, length type, and nocapture/readonly facts, but omitted the !p.nullable() check its sibling resource-destroy rule already enforced for ownership-scoped pointers. Introduced in d62ce47.

Fix

Added require(!p.nullable(), "call buffer requires non-null ptr<u8>") to the borrow loop in OwnershipValidator, mirroring the existing resource-destroy nullability rule. This aligns with the validator's philosophy of rejecting unhonorable contracts loudly rather than emitting wrappers that silently misbehave.

Testing

  • Unit tests, typecheck/build, and the JaCoCo coverage gate all pass: mvn -B verify → 159/159 tests, "All coverage checks have been met".
  • Added a nullable-borrow regression guard to the existing rejectsUnsafeOrUnsupportedContractsEvenRelaxed enumeration (omitted nullable=false and explicit nullable=true), plus rejectsNullableBorrowBufferInBothModes, which asserts rejection in both relaxed and strict mode with the exact message and a non-nullable control that still generates.
  • End-to-end via the CLI: a fixture with a nullable borrow buffer is rejected (exit 2) in both --relaxed and strict mode; the unmodified examples/ownership.seam still validates and generates identical seam.rs borrow wrappers; examples/elide_lzma.seam (metadata-only ownership=borrowed, no resource/borrow declaration) is unaffected.
  • Negative control: temporarily reverted the one-line fix and confirmed the new tests fail ("Expected IllegalArgumentException ... but nothing was thrown"), then restored to green.

Closes #10


Automatic Fixes PRs can be configured here.

@codecov

codecov Bot commented Oct 23, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

@sgammon
Sam Gammon (sgammon) merged commit 2311058 into main Oct 26, 2026
7 of 9 checks passed
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.

[Detail Bug] Ownership DSL: borrow allows nullable buffer parameters even though generated bindings never pass NULL

1 participant