Skip to content

LibWeb: Accept an empty override span from C++ in the style FFI - #11233

Open
sideshowbarker wants to merge 1 commit into
LadybirdBrowser:masterfrom
sideshowbarker:rust-ffi-null-empty-span
Open

LibWeb: Accept an empty override span from C++ in the style FFI#11233
sideshowbarker wants to merge 1 commit into
LadybirdBrowser:masterfrom
sideshowbarker:rust-ffi-null-empty-span

Conversation

@sideshowbarker

@sideshowbarker sideshowbarker commented Aug 20, 2026

Copy link
Copy Markdown
Member

Problem: Rust panic in Debug builds: “unsafe precondition(s) violated: slice::from_raw_parts requires the pointer to be aligned and non-null”.

Cause: create_internal() collects the effective longhand overrides into plain AK::Vectors and passes their data() and size() to the table-group builder. collect_effective_longhand_overrides() returns early without touching either vector when there’s nothing to collect. So a vector that never allocated stays empty — and a Vector with no inline capacity returns its null outline buffer straight from data(). The Rust side then borrows the span with slice::from_raw_parts — which requires a non-null aligned pointer even for a zero-length slice. So the empty span on its own is UB, and trips a check compiled in only under debug_assertions.

Fix: Borrow the span through a slice_from_raw() helper that yields an empty slice for a zero length without reading the pointer — the way bytes_from_raw() already treats an empty byte span. C++ represents an empty range as a null pointer with length zero. So the FFI boundary is where that convention has to be accepted — rather than at every caller.

The problem was reported on Discord: https://discord.com/channels/1247070541085671459/1247070543136690270/1539926273294336061

Problem: Rust panic in Debug builds: “unsafe precondition(s) violated:
slice::from_raw_parts requires the pointer to be aligned and non-null”.

Cause: create_internal() collects the effective longhand overrides into
plain AK::Vectors and passes their data() and size() to the table-group
builder. collect_effective_longhand_overrides() returns early without
touching either vector when there’s nothing to collect. So a vector that
never allocated stays empty — and a Vector with no inline capacity
returns its null outline buffer straight from data(). The Rust side then
borrows the span with slice::from_raw_parts — which requires a non-null
aligned pointer even for a zero-length slice. So the empty span on its
own is UB, and trips a check compiled in only under debug_assertions.

Fix: Borrow the span through a slice_from_raw() helper that yields an
empty slice for a zero length without reading the pointer — the way
bytes_from_raw() already treats an empty byte span. C++ represents an
empty range as a null pointer with length zero. So the FFI boundary is
where that convention has to be accepted — rather than at every caller.
@coderabbitai

coderabbitai Bot commented Aug 20, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 2ac6db22-c136-4808-aa16-300b1dc614fa

📥 Commits

Reviewing files that changed from the base of the PR and between 6d0dccc and 247fcfa.

📒 Files selected for processing (2)
  • Libraries/LibWeb/Rust/src/css/table_group_builder.rs
  • Libraries/LibWeb/Rust/src/lib.rs

Included review availability: Your plan provides up to 10 included reviews per hour; 9 remain after this review.


📝 Walkthrough

Walkthrough

Added slice_from_raw, which returns an empty slice for zero-length spans, including null pointers, and borrows populated spans through from_raw_parts. Added unit tests for both cases. Updated table group payload construction to use the shared helper for override property and value spans. Group payload construction and fallback behavior remain unchanged.

Possibly related PRs

Suggested reviewers: awesomekling, kalenikaliaksandr, tcl3

Merge Risk: ⚪ Minimal · up to 247fc

This localized change allows empty style override spans to be handled safely without changing non-empty behavior; no actionable merge-blocking risk remains beyond normal checks and review.

🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Description check ✅ Passed The description clearly explains the Rust panic, its C++ FFI cause, and the slice_from_raw() fix described in the changeset.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

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.

1 participant